diff options
author | Mike Reed <reed@google.com> | 2018-05-30 16:32:33 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2018-06-04 15:47:19 +0000 |
commit | 5059541dcc1950107ffeed930f85d986f4b5d613 (patch) | |
tree | 2c8cfe04a155f3163c7cbc37710d8b599dd65023 | |
parent | ad445ce8416474056fded4fa912dae7f53e462ce (diff) |
need conservative bounds for triangles due to SkFixed drift in SkEdge
Bug: oss-fuzz:8018
Change-Id: I09456f906b7eb89f74ffd2c484bc6e30e029bfbb
Reviewed-on: https://skia-review.googlesource.com/131021
Reviewed-by: Cary Clark <caryclark@google.com>
Auto-Submit: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
-rw-r--r-- | src/core/SkScan_Path.cpp | 33 | ||||
-rw-r--r-- | tests/PathTest.cpp | 34 |
2 files changed, 32 insertions, 35 deletions
diff --git a/src/core/SkScan_Path.cpp b/src/core/SkScan_Path.cpp index 43240eae37..89ef4ca32d 100644 --- a/src/core/SkScan_Path.cpp +++ b/src/core/SkScan_Path.cpp @@ -738,37 +738,6 @@ static void sk_fill_triangle(const SkPoint pts[], const SkIRect* clipRect, // walk_edges(&headEdge, SkPath::kEvenOdd_FillType, blitter, start_y, stop_y, nullptr); } -/** - * We need to match the rounding behavior of the line edge, which does this: - * 1. scale by 64 (to get into FDot6) - * 2. cast to an int - * 3. round that to an int (undoing the FDot6) - * This should (in theory) be the same as sk_float_round2int, except for float values very very - * close to 0.5 (like 0.49999997f). For those values, x + 0.5 gives 1.0 instead of 0.9999999, - * and therefore they round2int differently as floats than as FDot6 values in the edge code. - * - * A fix is to go into double temporarily, so that 0.49999997f + 0.5 stays < 1.0. - * - * This sample triangle triggers the problem (if we just use SkRect::round() instead of - * this double_round version. - * - * { 0.499069244f, 9.63295173f }, - * { 0.499402374f, 7.88207579f }, - * { 10.2363272f, 0.49999997f }, - * - * Note: this version is basically just more correct than SkRect::round(). If we thought we could - * afford the perf hit (assuming going to doubles cost more), then we might replace round()'s - * impl with this. - */ -static SkIRect double_round(const SkRect& r) { - return { - sk_double_round2int(r.fLeft), - sk_double_round2int(r.fTop), - sk_double_round2int(r.fRight), - sk_double_round2int(r.fBottom), - }; -} - void SkScan::FillTriangle(const SkPoint pts[], const SkRasterClip& clip, SkBlitter* blitter) { if (clip.isEmpty()) { @@ -788,7 +757,7 @@ void SkScan::FillTriangle(const SkPoint pts[], const SkRasterClip& clip, return; } - SkIRect ir = double_round(r); + SkIRect ir = conservative_round_to_int(r); if (ir.isEmpty() || !SkIRect::Intersects(ir, clip.getBounds())) { return; } diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 45ebd45e31..6cf5dfba13 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -5053,6 +5053,20 @@ DEF_TEST(Path_isRect, reporter) { } #include "SkVertices.h" +static void draw_triangle(SkCanvas* canvas, const SkPoint pts[]) { + // draw in different ways, looking for an assert + + { + SkPath path; + path.addPoly(pts, 3, false); + canvas->drawPath(path, SkPaint()); + } + + const SkColor colors[] = { SK_ColorBLACK, SK_ColorBLACK, SK_ColorBLACK }; + auto v = SkVertices::MakeCopy(SkVertices::kTriangles_VertexMode, 3, pts, nullptr, colors); + canvas->drawVertices(v, SkBlendMode::kSrcOver, SkPaint()); +} + DEF_TEST(triangle_onehalf, reporter) { auto surface(SkSurface::MakeRasterN32Premul(100, 100)); @@ -5061,8 +5075,22 @@ DEF_TEST(triangle_onehalf, reporter) { { 0.499402374f, 7.88207579f }, { 10.2363272f, 0.49999997f } }; - const SkColor colors[] = { SK_ColorBLACK, SK_ColorBLACK, SK_ColorBLACK }; + draw_triangle(surface->getCanvas(), pts); +} - auto v = SkVertices::MakeCopy(SkVertices::kTriangles_VertexMode, 3, pts, nullptr, colors); - surface->getCanvas()->drawVertices(v, SkBlendMode::kSrcOver, SkPaint()); +DEF_TEST(triangle_big, reporter) { + auto surface(SkSurface::MakeRasterN32Premul(4, 4304)); + + // The first two points, when sent through our fixed-point SkEdge, can walk negative beyond + // -0.5 due to accumulated += error of the slope. We have since make the bounds calculation + // be conservative, so we invoke clipping if we get in this situation. + // This test was added to demonstrate the need for this conservative bounds calc. + // (found by a fuzzer) + const SkPoint pts[] = { + { 0.327190518f, -114.945152f }, + { -0.5f, 1.00003874f }, + { 0.666425824f, 4304.26172f }, + }; + draw_triangle(surface->getCanvas(), pts); } + |