From 6df611574a3cf8abf2617af0d03a5553bb17360d Mon Sep 17 00:00:00 2001 From: caryclark Date: Mon, 4 Jan 2016 14:17:47 -0800 Subject: [PATCH] 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 --- gm/composeshader.cpp | 10 ++++++++- include/core/SkRect.h | 18 ---------------- src/core/SkScan_Path.cpp | 46 +++++++++++++++++++++++++++++++++++----- tests/DrawPathTest.cpp | 40 ++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 24 deletions(-) diff --git a/gm/composeshader.cpp b/gm/composeshader.cpp index e012bbb285..b2be9ba28e 100644 --- a/gm/composeshader.cpp +++ b/gm/composeshader.cpp @@ -162,7 +162,14 @@ static SkShader* make_linear_gradient_shader(int length) { 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 @@ public: 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 fe276e6710..4f649b7c73 100644 --- a/include/core/SkRect.h +++ b/include/core/SkRect.h @@ -824,24 +824,6 @@ public: SkScalarRoundToInt(fRight), SkScalarRoundToInt(fBottom)); } - /** - * 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 30a7c57659..804148db36 100644 --- a/src/core/SkScan_Path.cpp +++ b/src/core/SkScan_Path.cpp @@ -559,6 +559,42 @@ static bool clip_to_limit(const SkRegion& orig, SkRegion* reduced) { 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 @@ void SkScan::FillPath(const SkPath& path, const SkRegion& origClip, // 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 721b030477..364a297123 100644 --- a/tests/DrawPathTest.cpp +++ b/tests/DrawPathTest.cpp @@ -217,6 +217,45 @@ static void test_bigcubic() { 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 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 @@ DEF_TEST(DrawPath, reporter) { test_infinite_dash(reporter); test_crbug_165432(reporter); test_big_aa_rect(reporter); + test_halfway(); }