Filter out degenerate contours in GrConvexPolyEffect
authorlsalzman <lsalzman@mozilla.com>
Tue, 31 May 2016 16:46:00 +0000 (09:46 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 31 May 2016 16:46:00 +0000 (09:46 -0700)
As noticed in a downstream Firefox bug report https://bugzilla.mozilla.org/show_bug.cgi?id=1255062

If a path such as (moveTo, moveTo, lineTo, lineTo, close) is supplied,
and if the non-degenerate contour is convex, the convexity test will
pass, and GrConvexPolyEffect will be used.

However, the path's raw points are used to build the edge list, which
does not filter out degenerate contours. This may cause the polygon to
fail to draw.

This patch ensures that the degenerate contours are filtered out by
using an iterator as the path convexity test does.

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

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

src/gpu/effects/GrConvexPolyEffect.cpp

index 7b78305..caf86c2 100644 (file)
@@ -247,13 +247,6 @@ GrFragmentProcessor* GrConvexPolyEffect::Create(GrPrimitiveEdgeType type, const
         return nullptr;
     }
 
-    if (path.countPoints() > kMaxEdges) {
-        return nullptr;
-    }
-
-    SkPoint pts[kMaxEdges];
-    SkScalar edges[3 * kMaxEdges];
-
     SkPathPriv::FirstDirection dir;
     // The only way this should fail is if the clip is effectively a infinitely thin line. In that
     // case nothing is inside the clip. It'd be nice to detect this at a higher level and either
@@ -273,24 +266,45 @@ GrFragmentProcessor* GrConvexPolyEffect::Create(GrPrimitiveEdgeType type, const
         t = *offset;
     }
 
-    int count = path.getPoints(pts, kMaxEdges);
+    SkScalar        edges[3 * kMaxEdges];
+    SkPoint         pts[4];
+    SkPath::Verb    verb;
+    SkPath::Iter    iter(path, true);
+
+    // SkPath considers itself convex so long as there is a convex contour within it,
+    // regardless of any degenerate contours such as a string of moveTos before it.
+    // Iterate here to consume any degenerate contours and only process the points
+    // on the actual convex contour.
     int n = 0;
-    for (int lastPt = count - 1, i = 0; i < count; lastPt = i++) {
-        if (pts[lastPt] != pts[i]) {
-            SkVector v = pts[i] - pts[lastPt];
-            v.normalize();
-            if (SkPathPriv::kCCW_FirstDirection == dir) {
-                edges[3 * n] = v.fY;
-                edges[3 * n + 1] = -v.fX;
-            } else {
-                edges[3 * n] = -v.fY;
-                edges[3 * n + 1] = v.fX;
+    while ((verb = iter.next(pts, true, true)) != SkPath::kDone_Verb) {
+        switch (verb) {
+            case SkPath::kMove_Verb:
+                SkASSERT(n == 0);
+            case SkPath::kClose_Verb:
+                break;
+            case SkPath::kLine_Verb: {
+                if (n >= kMaxEdges) {
+                    return nullptr;
+                }
+                SkVector v = pts[1] - pts[0];
+                v.normalize();
+                if (SkPathPriv::kCCW_FirstDirection == dir) {
+                    edges[3 * n] = v.fY;
+                    edges[3 * n + 1] = -v.fX;
+                } else {
+                    edges[3 * n] = -v.fY;
+                    edges[3 * n + 1] = v.fX;
+                }
+                SkPoint p = pts[1] + t;
+                edges[3 * n + 2] = -(edges[3 * n] * p.fX + edges[3 * n + 1] * p.fY);
+                ++n;
+                break;
             }
-            SkPoint p = pts[i] + t;
-            edges[3 * n + 2] = -(edges[3 * n] * p.fX + edges[3 * n + 1] * p.fY);
-            ++n;
+            default:
+                return nullptr;
         }
     }
+
     if (path.isInverseFillType()) {
         type = GrInvertProcessorEdgeType(type);
     }