Make non-AA hairline stroke rects snap to pixels centers so they close.
BUG=skia:3717
Review URL: https://codereview.chromium.org/1101663007
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index 439733b..2a778bd 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -746,7 +746,7 @@
// if the target is MSAA then we want MSAA enabled when the clip is soft
if (rt->isMultisampled()) {
- pipelineBuilder.setState(GrPipelineBuilder::kHWAntialias_StateBit, element->isAA());
+ pipelineBuilder.setState(GrPipelineBuilder::kHWAntialias_Flag, element->isAA());
}
bool fillInverted = false;
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 743377d..4154005 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -798,6 +798,11 @@
SkScalar rad = SkScalarHalf(width);
bounds.outset(rad, rad);
viewMatrix.mapRect(&bounds);
+ // Depending on sub-pixel coordinates and the particular GPU, we may lose a corner of
+ // hairline rects. We jam all the vertices to pixel centers to avoid this, but not when MSAA
+ // is enabled because it can cause ugly artifacts.
+ pipelineBuilder.setState(GrPipelineBuilder::kSnapVerticesToPixelCenters_Flag,
+ 0 == width && !rt->isMultisampled());
target->drawBatch(&pipelineBuilder, batch, &bounds);
} else {
// filled BW rect
diff --git a/src/gpu/GrPipeline.cpp b/src/gpu/GrPipeline.cpp
index c77dfea..551882f 100644
--- a/src/gpu/GrPipeline.cpp
+++ b/src/gpu/GrPipeline.cpp
@@ -64,6 +64,9 @@
if (pipelineBuilder.isDither()) {
fFlags |= kDither_Flag;
}
+ if (pipelineBuilder.snapVerticesToPixelCenters()) {
+ fFlags |= kSnapVertices_Flag;
+ }
int firstColorStageIdx = colorPOI.firstEffectiveStageIndex();
diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h
index 2cb5aab..fdba0d7 100644
--- a/src/gpu/GrPipeline.h
+++ b/src/gpu/GrPipeline.h
@@ -80,6 +80,7 @@
bool isDitherState() const { return SkToBool(fFlags & kDither_Flag); }
bool isHWAntialiasState() const { return SkToBool(fFlags & kHWAA_Flag); }
+ bool snapVerticesToPixelCenters() const { return SkToBool(fFlags & kSnapVertices_Flag); }
// Skip any draws that refer to this pipeline (they should be a no-op).
bool mustSkip() const { return NULL == this->getRenderTarget(); }
@@ -119,6 +120,7 @@
enum Flags {
kDither_Flag = 0x1,
kHWAA_Flag = 0x2,
+ kSnapVertices_Flag = 0x4,
};
typedef GrPendingIOResource<GrRenderTarget, kWrite_GrIOType> RenderTarget;
diff --git a/src/gpu/GrPipelineBuilder.cpp b/src/gpu/GrPipelineBuilder.cpp
index 1d8aa4f..98deee9 100644
--- a/src/gpu/GrPipelineBuilder.cpp
+++ b/src/gpu/GrPipelineBuilder.cpp
@@ -16,7 +16,7 @@
#include "effects/GrPorterDuffXferProcessor.h"
GrPipelineBuilder::GrPipelineBuilder()
- : fFlagBits(0x0)
+ : fFlags(0x0)
, fDrawFace(kBoth_DrawFace)
, fColorProcInfoValid(false)
, fCoverageProcInfoValid(false)
@@ -27,7 +27,7 @@
GrPipelineBuilder& GrPipelineBuilder::operator=(const GrPipelineBuilder& that) {
fRenderTarget.reset(SkSafeRef(that.fRenderTarget.get()));
- fFlagBits = that.fFlagBits;
+ fFlags = that.fFlags;
fStencilSettings = that.fStencilSettings;
fDrawFace = that.fDrawFace;
fXPFactory.reset(SkRef(that.getXPFactory()));
@@ -69,12 +69,12 @@
// These have no equivalent in GrPaint, set them to defaults
fDrawFace = kBoth_DrawFace;
fStencilSettings.setDisabled();
- fFlagBits = 0;
+ fFlags = 0;
fClip = clip;
- this->setState(GrPipelineBuilder::kDither_StateBit, paint.isDither());
- this->setState(GrPipelineBuilder::kHWAntialias_StateBit,
+ this->setState(GrPipelineBuilder::kDither_Flag, paint.isDither());
+ this->setState(GrPipelineBuilder::kHWAntialias_Flag,
rt->isMultisampled() && paint.isAntiAlias());
fColorProcInfoValid = false;
diff --git a/src/gpu/GrPipelineBuilder.h b/src/gpu/GrPipelineBuilder.h
index ae9ca9b..24d72e8 100644
--- a/src/gpu/GrPipelineBuilder.h
+++ b/src/gpu/GrPipelineBuilder.h
@@ -275,49 +275,56 @@
* Flags that affect rendering. Controlled using enable/disableState(). All
* default to disabled.
*/
- enum StateBits {
+ enum Flags {
/**
* Perform dithering. TODO: Re-evaluate whether we need this bit
*/
- kDither_StateBit = 0x01,
+ kDither_Flag = 0x01,
/**
* Perform HW anti-aliasing. This means either HW FSAA, if supported by the render target,
* or smooth-line rendering if a line primitive is drawn and line smoothing is supported by
* the 3D API.
*/
- kHWAntialias_StateBit = 0x02,
+ kHWAntialias_Flag = 0x02,
- kLast_StateBit = kHWAntialias_StateBit,
+ /**
+ * Modifies the vertex shader so that vertices will be positioned at pixel centers.
+ */
+ kSnapVerticesToPixelCenters_Flag = 0x04,
+
+ kLast_Flag = kSnapVerticesToPixelCenters_Flag,
};
- bool isDither() const { return 0 != (fFlagBits & kDither_StateBit); }
- bool isHWAntialias() const { return 0 != (fFlagBits & kHWAntialias_StateBit); }
+ bool isDither() const { return SkToBool(fFlags & kDither_Flag); }
+ bool isHWAntialias() const { return SkToBool(fFlags & kHWAntialias_Flag); }
+ bool snapVerticesToPixelCenters() const {
+ return SkToBool(fFlags & kSnapVerticesToPixelCenters_Flag); }
/**
* Enable render state settings.
*
- * @param stateBits bitfield of StateBits specifying the states to enable
+ * @param flags bitfield of Flags specifying the states to enable
*/
- void enableState(uint32_t stateBits) { fFlagBits |= stateBits; }
-
+ void enableState(uint32_t flags) { fFlags |= flags; }
+
/**
* Disable render state settings.
*
- * @param stateBits bitfield of StateBits specifying the states to disable
+ * @param flags bitfield of Flags specifying the states to disable
*/
- void disableState(uint32_t stateBits) { fFlagBits &= ~(stateBits); }
+ void disableState(uint32_t flags) { fFlags &= ~(flags); }
/**
- * Enable or disable stateBits based on a boolean.
+ * Enable or disable flags based on a boolean.
*
- * @param stateBits bitfield of StateBits to enable or disable
+ * @param flags bitfield of Flags to enable or disable
* @param enable if true enable stateBits, otherwise disable
*/
- void setState(uint32_t stateBits, bool enable) {
+ void setState(uint32_t flags, bool enable) {
if (enable) {
- this->enableState(stateBits);
+ this->enableState(flags);
} else {
- this->disableState(stateBits);
+ this->disableState(flags);
}
}
@@ -422,7 +429,7 @@
typedef SkSTArray<4, GrFragmentStage> FragmentStageArray;
SkAutoTUnref<GrRenderTarget> fRenderTarget;
- uint32_t fFlagBits;
+ uint32_t fFlags;
GrStencilSettings fStencilSettings;
DrawFace fDrawFace;
mutable SkAutoTUnref<const GrXPFactory> fXPFactory;
diff --git a/src/gpu/GrProgramDesc.h b/src/gpu/GrProgramDesc.h
index 9f636a1..b306f02 100644
--- a/src/gpu/GrProgramDesc.h
+++ b/src/gpu/GrProgramDesc.h
@@ -56,10 +56,11 @@
uint8_t fFragPosKey; // set by GrGLShaderBuilder if there are
// effects that read the fragment position.
// Otherwise, 0.
-
+ uint8_t fSnapVerticesToPixelCenters;
int8_t fColorEffectCnt;
int8_t fCoverageEffectCnt;
};
+ GR_STATIC_ASSERT(sizeof(KeyHeader) == 4);
int numColorEffects() const {
return this->header().fColorEffectCnt;
diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp
index 96a8ada..87e7f2d 100644
--- a/src/gpu/gl/GrGLProgramDesc.cpp
+++ b/src/gpu/gl/GrGLProgramDesc.cpp
@@ -143,7 +143,7 @@
} else {
header->fFragPosKey = 0;
}
-
+ header->fSnapVerticesToPixelCenters = pipeline.snapVerticesToPixelCenters();
header->fColorEffectCnt = pipeline.numColorFragmentStages();
header->fCoverageEffectCnt = pipeline.numCoverageFragmentStages();
glDesc->finalize();
diff --git a/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp b/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp
index 00e96a5..20e9f08 100644
--- a/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp
+++ b/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp
@@ -42,11 +42,18 @@
kVec4f_GrSLType, kDefault_GrSLPrecision,
fProgramBuilder->rtAdjustment(),
&fRtAdjustName);
-
- // Transform from Skia's device coords to GL's normalized device coords. Note that
- // because we want to "nudge" the device space positions we are converting to
- // non-homogeneous NDC.
- if (kVec3f_GrSLType == posVar.getType()) {
+ if (this->getProgramBuilder()->desc().header().fSnapVerticesToPixelCenters) {
+ if (kVec3f_GrSLType == posVar.getType()) {
+ const char* p = posVar.c_str();
+ this->codeAppendf("{vec2 _posTmp = vec2(%s.x/%s.z, %s.y/%s.z);", p, p, p, p);
+ } else {
+ SkASSERT(kVec2f_GrSLType == posVar.getType());
+ this->codeAppendf("{vec2 _posTmp = %s;", posVar.c_str());
+ }
+ this->codeAppendf("_posTmp = floor(_posTmp) + vec2(0.5, 0.5);"
+ "gl_Position = vec4(_posTmp.x * %s.x + %s.y, _posTmp.y * %s.z + %s.w, 0, 1);}",
+ fRtAdjustName, fRtAdjustName, fRtAdjustName, fRtAdjustName);
+ } else if (kVec3f_GrSLType == posVar.getType()) {
this->codeAppendf("gl_Position = vec4(dot(%s.xz, %s.xy)/%s.z, dot(%s.yz, %s.zw)/%s.z, 0, 1);",
posVar.c_str(), fRtAdjustName, posVar.c_str(),
posVar.c_str(), fRtAdjustName, posVar.c_str());
@@ -56,9 +63,8 @@
posVar.c_str(), fRtAdjustName, fRtAdjustName,
posVar.c_str(), fRtAdjustName, fRtAdjustName);
}
-
- // We could have the GrGeometryProcessor do this, but its just easier to have it performed here.
- // If we ever need to set variable pointsize, then we can reinvestigate
+ // We could have the GrGeometryProcessor do this, but its just easier to have it performed
+ // here. If we ever need to set variable pointsize, then we can reinvestigate
this->codeAppend("gl_PointSize = 1.0;");
}
diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp
index ed74ae8..41c6679 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -186,7 +186,7 @@
static void set_random_state(GrPipelineBuilder* pipelineBuilder, SkRandom* random) {
int state = 0;
- for (int i = 1; i <= GrPipelineBuilder::kLast_StateBit; i <<= 1) {
+ for (int i = 1; i <= GrPipelineBuilder::kLast_Flag; i <<= 1) {
state |= random->nextBool() * i;
}
pipelineBuilder->enableState(state);