Fix assert caused by floating point error in tessellating path renderer.
authorsenorblanco <senorblanco@chromium.org>
Wed, 17 Aug 2016 21:56:22 +0000 (14:56 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 17 Aug 2016 21:56:22 +0000 (14:56 -0700)
In rare cases, floating point error causes the tesselator to add the
same Vertex to more than one Poly on the same side. This was not a big
problem when we were allocating new vertices when constructing Polys,
but after https://codereview.chromium.org/2029243002 it causes more serious
issues, since each Edge can only belong to two Polys, and violating this
condition messes up the linked list of Edges used for left & right Polys
and the associated estimated vertex count.

The fix is to simply let the first Poly win, and skip that vertex for
subsequent Polys. Since this only occurs in cases where vertices are very
close to each other, it should have little visual effect.

This is also exercised by Nebraska-StateSeal.svg.

BUG=skia:5636
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2259493002

Review-Url: https://codereview.chromium.org/2259493002

src/gpu/GrTessellator.cpp
tests/TessellatingPathRendererTests.cpp

index cefe4cd..50d4af6 100644 (file)
@@ -253,7 +253,9 @@ struct Edge {
         , fLeftPolyPrev(nullptr)
         , fLeftPolyNext(nullptr)
         , fRightPolyPrev(nullptr)
-        , fRightPolyNext(nullptr) {
+        , fRightPolyNext(nullptr)
+        , fUsedInLeftPoly(false)
+        , fUsedInRightPoly(false) {
             recompute();
         }
     int      fWinding;          // 1 == edge goes downward; -1 = edge goes upward.
@@ -271,6 +273,8 @@ struct Edge {
     Edge*    fLeftPolyNext;
     Edge*    fRightPolyPrev;
     Edge*    fRightPolyNext;
+    bool     fUsedInLeftPoly;
+    bool     fUsedInRightPoly;
     double   fDX;               // The line equation for this edge, in implicit form.
     double   fDY;               // fDY * x + fDX * y + fC = 0, for point (x, y) on the line.
     double   fC;
@@ -356,11 +360,19 @@ struct Poly {
         MonotonePoly* fNext;
         void addEdge(Edge* edge) {
             if (fSide == kRight_Side) {
+                if (edge->fUsedInRightPoly) {
+                    return;
+                }
                 list_insert<Edge, &Edge::fRightPolyPrev, &Edge::fRightPolyNext>(
                     edge, fLastEdge, nullptr, &fFirstEdge, &fLastEdge);
+                edge->fUsedInRightPoly = true;
             } else {
+                if (edge->fUsedInLeftPoly) {
+                    return;
+                }
                 list_insert<Edge, &Edge::fLeftPolyPrev, &Edge::fLeftPolyNext>(
                     edge, fLastEdge, nullptr, &fFirstEdge, &fLastEdge);
+                edge->fUsedInLeftPoly = true;
             }
         }
 
@@ -407,9 +419,8 @@ struct Poly {
         }
     };
     Poly* addEdge(Edge* e, Side side, SkChunkAlloc& alloc) {
-        LOG("addEdge (%g,%g)-(%g,%g) to poly %d, %s side\n",
-               e->fTop->fPoint.fX, e->fTop->fPoint.fY, e->fBottom->fPoint.fX, e->fBottom->fPoint.fY,
-               fID, side == kLeft_Side ? "left" : "right");
+        LOG("addEdge (%g -> %g) to poly %d, %s side\n",
+               e->fTop->fID, e->fBottom->fID, fID, side == kLeft_Side ? "left" : "right");
         Poly* partner = fPartner;
         Poly* poly = this;
         if (partner) {
index d9afcc4..232ea36 100644 (file)
@@ -232,6 +232,23 @@ static SkPath create_path_15() {
     return path;
 }
 
+// Reduction of Nebraska-StateSeal.svg. Floating point error causes the
+// same edge to be added to more than one poly on the same side.
+static SkPath create_path_16() {
+    SkPath path;
+    path.moveTo(170.8199920654296875,   491.86700439453125);
+    path.lineTo(173.7649993896484375,    489.7340087890625);
+    path.lineTo(174.1450958251953125,  498.545989990234375);
+    path.lineTo( 171.998992919921875,   500.88201904296875);
+    path.moveTo(168.2922515869140625,   498.66265869140625);
+    path.lineTo(169.8589935302734375,   497.94500732421875);
+    path.lineTo(                 172,   500.88299560546875);
+    path.moveTo( 169.555267333984375,   490.70111083984375);
+    path.lineTo(173.7649993896484375,    489.7340087890625);
+    path.lineTo(  170.82000732421875,   491.86700439453125);
+    return path;
+}
+
 static void test_path(GrDrawContext* drawContext, GrResourceProvider* rp, const SkPath& path) {
     GrTessellatingPathRenderer tess;
 
@@ -284,5 +301,6 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(TessellatingPathRendererTests, reporter, ctxInfo) {
     test_path(dc.get(), rp, create_path_13());
     test_path(dc.get(), rp, create_path_14());
     test_path(dc.get(), rp, create_path_15());
+    test_path(dc.get(), rp, create_path_16());
 }
 #endif