aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar robertphillips <robertphillips@google.com>2015-01-30 14:44:22 -0800
committerGravatar Commit bot <commit-bot@chromium.org>2015-01-30 14:44:22 -0800
commit7f14c9bbf1954a88226a0b56f1abb1a17f31534c (patch)
treeed293be50c7e7130b96a2ea0bd2fe121b7b1af14
parentedc93bc74003e2c19219ba0b30383868b5371a96 (diff)
Revert of Add device space "nudge" to gpu draws (patchset #6 id:90001 of https://codereview.chromium.org/877473005/)
Reason for revert: Blink layout tests this time :( Original issue's description: > Add device space "nudge" to gpu draws > > This CL nudges all the GPU draws and clips slightly to match raster's round behavior for BW draws. We assume the effect will be negligible and do it for AA draws too. > > BUG=423834 > > Committed: https://skia.googlesource.com/skia/+/2d55d07501c56310f97d2092d789a2bc9fa01b78 > > Committed: https://skia.googlesource.com/skia/+/b9329991426d0b77ea194a380d72d73fb855308a TBR=bsalomon@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=423834 Review URL: https://codereview.chromium.org/886153002
-rw-r--r--samplecode/SampleClipDrawMatch.cpp125
-rw-r--r--src/core/SkClipStack.cpp13
-rw-r--r--src/gpu/effects/GrConvexPolyEffect.cpp23
-rw-r--r--src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp10
4 files changed, 83 insertions, 88 deletions
diff --git a/samplecode/SampleClipDrawMatch.cpp b/samplecode/SampleClipDrawMatch.cpp
index f897b7185e..8e2fb46dc9 100644
--- a/samplecode/SampleClipDrawMatch.cpp
+++ b/samplecode/SampleClipDrawMatch.cpp
@@ -16,7 +16,6 @@
// fractionally (via an animator) to expose snapping bugs. The key bindings are:
// 1-9: the different geometries
// t: toggle which is drawn first the clip or the normal geometry
-// f: flip-flops which corner the bottom AA clip rect occupies in the complex clip cases
// The possible geometric combinations to test
enum Geometry {
@@ -36,10 +35,6 @@ static const float kMin = 100.5f;
static const float kMid = 200.0f;
static const float kMax = 299.5f;
-// The translation applied to the base AA rect in the combination cases
-// (i.e., kRectAndRect through kRectAndConcave)
-static const float kXlate = 100.0f;
-
SkRect create_rect(const SkPoint& offset) {
SkRect r = SkRect::MakeLTRB(kMin, kMin, kMax, kMax);
r.offset(offset);
@@ -84,6 +79,62 @@ SkPath create_concave_path(const SkPoint& offset) {
return concavePath;
}
+static void draw_clipped_geom(SkCanvas* canvas, const SkPoint& offset, int geom, bool useAA) {
+
+ int count = canvas->save();
+
+ switch (geom) {
+ case kRect_Geometry:
+ canvas->clipRect(create_rect(offset), SkRegion::kReplace_Op, useAA);
+ break;
+ case kRRect_Geometry:
+ canvas->clipRRect(create_rrect(offset), SkRegion::kReplace_Op, useAA);
+ break;
+ case kCircle_Geometry:
+ canvas->clipRRect(create_circle(offset), SkRegion::kReplace_Op, useAA);
+ break;
+ case kConvexPath_Geometry:
+ canvas->clipPath(create_convex_path(offset), SkRegion::kReplace_Op, useAA);
+ break;
+ case kConcavePath_Geometry:
+ canvas->clipPath(create_concave_path(offset), SkRegion::kReplace_Op, useAA);
+ break;
+ case kRectAndRect_Geometry: {
+ SkRect r = create_rect(offset);
+ r.offset(-100.0f, -100.0f);
+ canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
+ canvas->clipRect(create_rect(offset), SkRegion::kIntersect_Op, useAA);
+ } break;
+ case kRectAndRRect_Geometry: {
+ SkRect r = create_rect(offset);
+ r.offset(-100.0f, -100.0f);
+ canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
+ canvas->clipRRect(create_rrect(offset), SkRegion::kIntersect_Op, useAA);
+ } break;
+ case kRectAndConvex_Geometry: {
+ SkRect r = create_rect(offset);
+ r.offset(-100.0f, -100.0f);
+ canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
+ canvas->clipPath(create_convex_path(offset), SkRegion::kIntersect_Op, useAA);
+ } break;
+ case kRectAndConcave_Geometry: {
+ SkRect r = create_rect(offset);
+ r.offset(-100.0f, -100.0f);
+ canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
+ canvas->clipPath(create_concave_path(offset), SkRegion::kIntersect_Op, useAA);
+ } break;
+ }
+
+ SkISize size = canvas->getDeviceSize();
+ SkRect bigR = SkRect::MakeWH(SkIntToScalar(size.width()), SkIntToScalar(size.height()));
+
+ SkPaint p;
+ p.setColor(SK_ColorRED);
+
+ canvas->drawRect(bigR, p);
+ canvas->restoreToCount(count);
+}
+
static void draw_normal_geom(SkCanvas* canvas, const SkPoint& offset, int geom, bool useAA) {
SkPaint p;
p.setAntiAlias(useAA);
@@ -114,7 +165,7 @@ static void draw_normal_geom(SkCanvas* canvas, const SkPoint& offset, int geom,
class ClipDrawMatchView : public SampleView {
public:
- ClipDrawMatchView() : fTrans(2, 5), fGeom(kRect_Geometry), fClipFirst(true), fSign(1) {
+ ClipDrawMatchView() : fTrans(2, 5), fGeom(kRect_Geometry), fClipFirst(true) {
SkScalar values[2];
fTrans.setRepeatCount(999);
@@ -148,7 +199,6 @@ protected:
case '7': fGeom = kRectAndRRect_Geometry; this->inval(NULL); return true;
case '8': fGeom = kRectAndConvex_Geometry; this->inval(NULL); return true;
case '9': fGeom = kRectAndConcave_Geometry; this->inval(NULL); return true;
- case 'f': fSign = -fSign; this->inval(NULL); return true;
case 't': fClipFirst = !fClipFirst; this->inval(NULL); return true;
default: break;
}
@@ -156,74 +206,18 @@ protected:
return this->INHERITED::onQuery(evt);
}
- void drawClippedGeom(SkCanvas* canvas, const SkPoint& offset, bool useAA) {
-
- int count = canvas->save();
-
- switch (fGeom) {
- case kRect_Geometry:
- canvas->clipRect(create_rect(offset), SkRegion::kReplace_Op, useAA);
- break;
- case kRRect_Geometry:
- canvas->clipRRect(create_rrect(offset), SkRegion::kReplace_Op, useAA);
- break;
- case kCircle_Geometry:
- canvas->clipRRect(create_circle(offset), SkRegion::kReplace_Op, useAA);
- break;
- case kConvexPath_Geometry:
- canvas->clipPath(create_convex_path(offset), SkRegion::kReplace_Op, useAA);
- break;
- case kConcavePath_Geometry:
- canvas->clipPath(create_concave_path(offset), SkRegion::kReplace_Op, useAA);
- break;
- case kRectAndRect_Geometry: {
- SkRect r = create_rect(offset);
- r.offset(fSign * kXlate, fSign * kXlate);
- canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
- canvas->clipRect(create_rect(offset), SkRegion::kIntersect_Op, useAA);
- } break;
- case kRectAndRRect_Geometry: {
- SkRect r = create_rect(offset);
- r.offset(fSign * kXlate, fSign * kXlate);
- canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
- canvas->clipRRect(create_rrect(offset), SkRegion::kIntersect_Op, useAA);
- } break;
- case kRectAndConvex_Geometry: {
- SkRect r = create_rect(offset);
- r.offset(fSign * kXlate, fSign * kXlate);
- canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
- canvas->clipPath(create_convex_path(offset), SkRegion::kIntersect_Op, useAA);
- } break;
- case kRectAndConcave_Geometry: {
- SkRect r = create_rect(offset);
- r.offset(fSign * kXlate, fSign * kXlate);
- canvas->clipRect(r, SkRegion::kReplace_Op, true); // AA here forces shader clips
- canvas->clipPath(create_concave_path(offset), SkRegion::kIntersect_Op, useAA);
- } break;
- }
-
- SkISize size = canvas->getDeviceSize();
- SkRect bigR = SkRect::MakeWH(SkIntToScalar(size.width()), SkIntToScalar(size.height()));
-
- SkPaint p;
- p.setColor(SK_ColorRED);
-
- canvas->drawRect(bigR, p);
- canvas->restoreToCount(count);
- }
-
// Draw a big red rect through some clip geometry and also draw that same
// geometry in black. The order in which they are drawn can be swapped.
// This tests whether the clip and normally drawn geometry match up.
void drawGeometry(SkCanvas* canvas, const SkPoint& offset, bool useAA) {
if (fClipFirst) {
- this->drawClippedGeom(canvas, offset, useAA);
+ draw_clipped_geom(canvas, offset, fGeom, useAA);
}
draw_normal_geom(canvas, offset, fGeom, useAA);
if (!fClipFirst) {
- this->drawClippedGeom(canvas, offset, useAA);
+ draw_clipped_geom(canvas, offset, fGeom, useAA);
}
}
@@ -245,7 +239,6 @@ private:
SkInterpolator fTrans;
Geometry fGeom;
bool fClipFirst;
- int fSign;
typedef SampleView INHERITED;
};
diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp
index 863dc9a387..515596a969 100644
--- a/src/core/SkClipStack.cpp
+++ b/src/core/SkClipStack.cpp
@@ -418,6 +418,19 @@ void SkClipStack::Element::updateBoundAndGenID(const Element* prior) {
break;
}
+ if (!fDoAA) {
+ // Here we mimic a non-anti-aliased scanline system. If there is
+ // no anti-aliasing we can integerize the bounding box to exclude
+ // fractional parts that won't be rendered.
+ // Note: the left edge is handled slightly differently below. We
+ // are a bit more generous in the rounding since we don't want to
+ // risk missing the left pixels when fLeft is very close to .5
+ fFiniteBound.set(SkScalarFloorToScalar(fFiniteBound.fLeft+0.45f),
+ SkScalarRoundToScalar(fFiniteBound.fTop),
+ SkScalarRoundToScalar(fFiniteBound.fRight),
+ SkScalarRoundToScalar(fFiniteBound.fBottom));
+ }
+
// Now determine the previous Element's bound information taking into
// account that there may be no previous clip
SkRect prevFinite;
diff --git a/src/gpu/effects/GrConvexPolyEffect.cpp b/src/gpu/effects/GrConvexPolyEffect.cpp
index 615c341ef2..7035094fa6 100644
--- a/src/gpu/effects/GrConvexPolyEffect.cpp
+++ b/src/gpu/effects/GrConvexPolyEffect.cpp
@@ -137,14 +137,10 @@ void GLAARectEffect::emitCode(GrGLFPBuilder* builder,
fsBuilder->codeAppendf("\t\tfloat alpha = (1.0 + max(xSub, -1.0)) * (1.0 + max(ySub, -1.0));\n");
} else {
fsBuilder->codeAppendf("\t\tfloat alpha = 1.0;\n");
- fsBuilder->codeAppendf("\t\talpha *= (%s.x - %s.x) > -0.5 ? 1.0 : 0.0;\n",
- fragmentPos, rectName);
- fsBuilder->codeAppendf("\t\talpha *= (%s.z - %s.x) > -0.5 ? 1.0 : 0.0;\n",
- rectName, fragmentPos);
- fsBuilder->codeAppendf("\t\talpha *= (%s.y - %s.y) > -0.5 ? 1.0 : 0.0;\n",
- fragmentPos, rectName);
- fsBuilder->codeAppendf("\t\talpha *= (%s.w - %s.y) > -0.5 ? 1.0 : 0.0;\n",
- rectName, fragmentPos);
+ fsBuilder->codeAppendf("\t\talpha *= (%s.x - %s.x) > -0.5 ? 1.0 : 0.0;\n", fragmentPos, rectName);
+ fsBuilder->codeAppendf("\t\talpha *= (%s.z - %s.x) > -0.5 ? 1.0 : 0.0;\n", rectName, fragmentPos);
+ fsBuilder->codeAppendf("\t\talpha *= (%s.y - %s.y) > -0.5 ? 1.0 : 0.0;\n", fragmentPos, rectName);
+ fsBuilder->codeAppendf("\t\talpha *= (%s.w - %s.y) > -0.5 ? 1.0 : 0.0;\n", rectName, fragmentPos);
}
if (GrProcessorEdgeTypeIsInverseFill(aare.getEdgeType())) {
@@ -158,10 +154,8 @@ void GLAARectEffect::setData(const GrGLProgramDataManager& pdman, const GrProces
const AARectEffect& aare = processor.cast<AARectEffect>();
const SkRect& rect = aare.getRect();
if (rect != fPrevRect) {
- // Add a device space "nudge" of 0.05f, 0.05f to match raster's rounding behavior for
- // BW clipping/drawing
- pdman.set4f(fRectUniform, rect.fLeft + 0.55f, rect.fTop + 0.55f,
- rect.fRight - 0.45f, rect.fBottom - 0.45f);
+ pdman.set4f(fRectUniform, rect.fLeft + 0.5f, rect.fTop + 0.5f,
+ rect.fRight - 0.5f, rect.fBottom - 0.5f);
fPrevRect = rect;
}
}
@@ -227,10 +221,7 @@ void GrGLConvexPolyEffect::emitCode(GrGLFPBuilder* builder,
fsBuilder->codeAppend("\t\tfloat edge;\n");
const char* fragmentPos = fsBuilder->fragmentPosition();
for (int i = 0; i < cpe.getEdgeCount(); ++i) {
- // Add a device space "nudge" of 0.05f, 0.05f to match raster's rounding behavior for
- // BW clipping/drawing. Since we are "nudging" fragment positions we have to go in
- // the opposite direction.
- fsBuilder->codeAppendf("\t\tedge = dot(%s[%d], vec3(%s.x - 0.05, %s.y - 0.05, 1));\n",
+ fsBuilder->codeAppendf("\t\tedge = dot(%s[%d], vec3(%s.x, %s.y, 1));\n",
edgeArrayName, i, fragmentPos, fragmentPos);
if (GrProcessorEdgeTypeIsAA(cpe.getEdgeType())) {
fsBuilder->codeAppend("\t\tedge = clamp(edge, 0.0, 1.0);\n");
diff --git a/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp b/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp
index f0b645ee42..f17e741169 100644
--- a/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp
+++ b/src/gpu/gl/builders/GrGLVertexShaderBuilder.cpp
@@ -47,15 +47,13 @@ void GrGLVertexBuilder::transformToNormalizedDeviceSpace(const GrShaderVar& posV
// 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.
- // The "0.05" nudge serves to match the raster path's rounding for bw draws.
- // For aa draws we just assume the impact will be minimal - so we always perform the nudge.
if (kVec3f_GrSLType == posVar.getType()) {
- this->codeAppendf("gl_Position = vec4((dot(%s.xz, %s.xy)/%s.z) + 0.05 * %s.x, (dot(%s.yz, %s.zw)/%s.z) + 0.05 * %s.z, 0, 1);",
- posVar.c_str(), fRtAdjustName, posVar.c_str(), fRtAdjustName,
- posVar.c_str(), fRtAdjustName, posVar.c_str(), fRtAdjustName);
+ 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());
} else {
SkASSERT(kVec2f_GrSLType == posVar.getType());
- this->codeAppendf("gl_Position = vec4((%s.x + 0.05) * %s.x + %s.y, (%s.y + 0.05) * %s.z + %s.w, 0, 1);",
+ this->codeAppendf("gl_Position = vec4(%s.x * %s.x + %s.y, %s.y * %s.z + %s.w, 0, 1);",
posVar.c_str(), fRtAdjustName, fRtAdjustName,
posVar.c_str(), fRtAdjustName, fRtAdjustName);
}