handle halfway case in scan converter
Scan edges that start at exactly -0.5 aren't trimmed by
clipping or by rounding, triggering a debug assert.
One way to fix this is to round the top and left
down instead of up.
Also, move the path initialization of gm/composeshader.cpp
to make debugging other path problems easier.
R=reed@google.com
BUG=skia:2715
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1544873002
Review URL: https://codereview.chromium.org/1544873002
diff --git a/gm/composeshader.cpp b/gm/composeshader.cpp
index e012bbb..b2be9ba 100644
--- a/gm/composeshader.cpp
+++ b/gm/composeshader.cpp
@@ -162,7 +162,14 @@
class ComposeShaderBitmapGM : public skiagm::GM {
public:
- ComposeShaderBitmapGM() {
+ ComposeShaderBitmapGM()
+ : fColorBitmapShader(nullptr)
+ , fAlpha8BitmapShader(nullptr)
+ , fLinearGradientShader(nullptr)
+ {
+ }
+
+ void onOnceBeforeDraw() override {
draw_color_bm(&fColorBitmap, squareLength);
draw_alpha8_bm(&fAlpha8Bitmap, squareLength);
SkMatrix s;
@@ -173,6 +180,7 @@
SkShader::kRepeat_TileMode, &s);
fLinearGradientShader = make_linear_gradient_shader(squareLength);
}
+
~ComposeShaderBitmapGM() {
SkSafeUnref(fColorBitmapShader);
SkSafeUnref(fAlpha8BitmapShader);
diff --git a/include/core/SkRect.h b/include/core/SkRect.h
index fe276e6..4f649b7 100644
--- a/include/core/SkRect.h
+++ b/include/core/SkRect.h
@@ -825,24 +825,6 @@
}
/**
- * Variant of round() that explicitly performs the rounding step (i.e. floor(x + 0.5)) using
- * double instead of SkScalar (float). It does this by calling SkDScalarRoundToInt(), which
- * may be slower than calling SkScalarRountToInt(), but gives slightly more accurate results.
- *
- * e.g.
- * SkScalar x = 0.49999997f;
- * int ix = SkScalarRoundToInt(x);
- * SkASSERT(0 == ix); // <--- fails
- * ix = SkDScalarRoundToInt(x);
- * SkASSERT(0 == ix); // <--- succeeds
- */
- void dround(SkIRect* dst) const {
- SkASSERT(dst);
- dst->set(SkDScalarRoundToInt(fLeft), SkDScalarRoundToInt(fTop),
- SkDScalarRoundToInt(fRight), SkDScalarRoundToInt(fBottom));
- }
-
- /**
* Set the dst rectangle by rounding "out" this rectangle, choosing the
* SkScalarFloor of top and left, and the SkScalarCeil of right and bottom.
*/
diff --git a/src/core/SkScan_Path.cpp b/src/core/SkScan_Path.cpp
index 30a7c57..804148d 100644
--- a/src/core/SkScan_Path.cpp
+++ b/src/core/SkScan_Path.cpp
@@ -559,6 +559,42 @@
return true;
}
+/**
+ * Variant of SkScalarRoundToInt, identical to SkDScalarRoundToInt except when the input fraction
+ * is 0.5. In this case only, round the value down. This is used to round the top and left
+ * of a rectangle, and corresponds to the way the scan converter treats the top and left edges.
+ */
+static inline int round_down_to_int(SkScalar x) {
+ double xx = x;
+ xx += 0.5;
+ double floorXX = floor(xx);
+ return (int)floorXX - (xx == floorXX);
+}
+
+/**
+ * Variant of SkRect::round() that explicitly performs the rounding step (i.e. floor(x + 0.5))
+ * using double instead of SkScalar (float). It does this by calling SkDScalarRoundToInt(),
+ * which may be slower than calling SkScalarRountToInt(), but gives slightly more accurate
+ * results. Also rounds top and left using double, flooring when the fraction is exactly 0.5f.
+ *
+ * e.g.
+ * SkScalar left = 0.5f;
+ * int ileft = SkScalarRoundToInt(left);
+ * SkASSERT(0 == ileft); // <--- fails
+ * int ileft = round_down_to_int(left);
+ * SkASSERT(0 == ileft); // <--- succeeds
+ * SkScalar right = 0.49999997f;
+ * int iright = SkScalarRoundToInt(right);
+ * SkASSERT(0 == iright); // <--- fails
+ * iright = SkDScalarRoundToInt(right);
+ * SkASSERT(0 == iright); // <--- succeeds
+ */
+static void round_asymmetric_to_int(const SkRect& src, SkIRect* dst) {
+ SkASSERT(dst);
+ dst->set(round_down_to_int(src.fLeft), round_down_to_int(src.fTop),
+ SkDScalarRoundToInt(src.fRight), SkDScalarRoundToInt(src.fBottom));
+}
+
void SkScan::FillPath(const SkPath& path, const SkRegion& origClip,
SkBlitter* blitter) {
if (origClip.isEmpty()) {
@@ -578,11 +614,11 @@
// don't reference "origClip" any more, just use clipPtr
SkIRect ir;
- // We deliberately call dround() instead of round(), since we can't afford to generate a
- // bounds that is tighter than the corresponding SkEdges. The edge code basically converts
- // the floats to fixed, and then "rounds". If we called round() instead of dround() here,
- // we could generate the wrong ir for values like 0.4999997.
- path.getBounds().dround(&ir);
+ // We deliberately call round_asymmetric_to_int() instead of round(), since we can't afford
+ // to generate a bounds that is tighter than the corresponding SkEdges. The edge code basically
+ // converts the floats to fixed, and then "rounds". If we called round() instead of
+ // round_asymmetric_to_int() here, we could generate the wrong ir for values like 0.4999997.
+ round_asymmetric_to_int(path.getBounds(), &ir);
if (ir.isEmpty()) {
if (path.isInverseFillType()) {
blitter->blitRegion(*clipPtr);
diff --git a/tests/DrawPathTest.cpp b/tests/DrawPathTest.cpp
index 721b030..364a297 100644
--- a/tests/DrawPathTest.cpp
+++ b/tests/DrawPathTest.cpp
@@ -217,6 +217,45 @@
surface->getCanvas()->drawPath(path, paint);
}
+// asserts if halfway case is not handled
+static void test_halfway() {
+ SkPaint paint;
+ SkPath path;
+ path.moveTo(16365.5f, 1394);
+ path.lineTo(16365.5f, 1387.5f);
+ path.quadTo(16365.5f, 1385.43f, 16367, 1383.96f);
+ path.quadTo(16368.4f, 1382.5f, 16370.5f, 1382.5f);
+ path.lineTo(16465.5f, 1382.5f);
+ path.quadTo(16467.6f, 1382.5f, 16469, 1383.96f);
+ path.quadTo(16470.5f, 1385.43f, 16470.5f, 1387.5f);
+ path.lineTo(16470.5f, 1394);
+ path.quadTo(16470.5f, 1396.07f, 16469, 1397.54f);
+ path.quadTo(16467.6f, 1399, 16465.5f, 1399);
+ path.lineTo(16370.5f, 1399);
+ path.quadTo(16368.4f, 1399, 16367, 1397.54f);
+ path.quadTo(16365.5f, 1396.07f, 16365.5f, 1394);
+ path.close();
+ SkPath p2;
+ SkMatrix m;
+ m.reset();
+ m.postTranslate(0.001f, 0.001f);
+ path.transform(m, &p2);
+
+ SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(640, 480));
+ SkCanvas* canvas = surface->getCanvas();
+ canvas->translate(-16366, -1383);
+ canvas->drawPath(p2, paint);
+
+ m.reset();
+ m.postTranslate(-0.001f, -0.001f);
+ path.transform(m, &p2);
+ canvas->drawPath(p2, paint);
+
+ m.reset();
+ path.transform(m, &p2);
+ canvas->drawPath(p2, paint);
+}
+
// we used to assert if the bounds of the device (clip) was larger than 32K
// even when the path itself was smaller. We just draw and hope in the debug
// version to not assert.
@@ -287,4 +326,5 @@
test_infinite_dash(reporter);
test_crbug_165432(reporter);
test_big_aa_rect(reporter);
+ test_halfway();
}