From ee43091c1b8ebf2f62918688494b0eb82ceedb38 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Wed, 23 May 2018 12:12:21 -0400 Subject: fix 0.499999f rounding case for triangles Bug: skia:7994 Change-Id: I83bb309a2c8fb0bddaf78ba32c0a07537e483900 Reviewed-on: https://skia-review.googlesource.com/129648 Commit-Queue: Mike Reed Reviewed-by: Mike Klein --- include/private/SkFloatingPoint.h | 2 +- src/core/SkScan_Path.cpp | 33 ++++++++++++++++++++++++++++++++- tests/PathTest.cpp | 15 +++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/include/private/SkFloatingPoint.h b/include/private/SkFloatingPoint.h index adf7b279b9..6cb4cf1542 100644 --- a/include/private/SkFloatingPoint.h +++ b/include/private/SkFloatingPoint.h @@ -124,7 +124,7 @@ static inline int64_t sk_float_saturate2int64(float x) { #define sk_double_round(x) floor((x) + 0.5) #define sk_double_ceil(x) ceil(x) #define sk_double_floor2int(x) (int)floor(x) -#define sk_double_round2int(x) (int)floor((x) + 0.5f) +#define sk_double_round2int(x) (int)floor((x) + 0.5) #define sk_double_ceil2int(x) (int)ceil(x) // Cast double to float, ignoring any warning about too-large finite values being cast to float. diff --git a/src/core/SkScan_Path.cpp b/src/core/SkScan_Path.cpp index 4110b832a9..43240eae37 100644 --- a/src/core/SkScan_Path.cpp +++ b/src/core/SkScan_Path.cpp @@ -738,6 +738,37 @@ 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()) { @@ -757,7 +788,7 @@ void SkScan::FillTriangle(const SkPoint pts[], const SkRasterClip& clip, return; } - SkIRect ir = r.round(); + SkIRect ir = double_round(r); if (ir.isEmpty() || !SkIRect::Intersects(ir, clip.getBounds())) { return; } diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 9d8c18f997..45ebd45e31 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -5051,3 +5051,18 @@ DEF_TEST(Path_isRect, reporter) { compare.set(&points53[1], 4); REPORTER_ASSERT(reporter, rect == compare); } + +#include "SkVertices.h" +DEF_TEST(triangle_onehalf, reporter) { + auto surface(SkSurface::MakeRasterN32Premul(100, 100)); + + const SkPoint pts[] = { + { 0.499069244f, 9.63295173f }, + { 0.499402374f, 7.88207579f }, + { 10.2363272f, 0.49999997f } + }; + const SkColor colors[] = { SK_ColorBLACK, SK_ColorBLACK, SK_ColorBLACK }; + + auto v = SkVertices::MakeCopy(SkVertices::kTriangles_VertexMode, 3, pts, nullptr, colors); + surface->getCanvas()->drawVertices(v, SkBlendMode::kSrcOver, SkPaint()); +} -- cgit v1.2.3