Don't over simplify near-colinear vertices

This adds an error variable that keeps track of the total distance from
the simplified line, and includes it when determining if we should
keep the next point. Using a sum of line distance is just a heuristic
but seems to address the current case of over simplification while
allowing us to keep a greedy strategy.

Bug: chromium:1086705
Change-Id: I29e21724db6b30495c2934e376a5e4d787c846a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309328
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>

gm/crbug_1086705.cpp

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm/gm.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkPaint.h"
+#include "include/core/SkPath.h"
+
+// See crbug.com/1086705. The convex linearizing path renderer would collapse too many of the
+// very-near duplicate vertices and turn the path into a triangle. Since the stroke width is larger
+// than the radius of the circle, there's the separate issue of what to do when stroke
+// self-intersects
+DEF_SIMPLE_GM(crbug_1086705, canvas, 200, 200) {
+    SkPaint paint;
+    paint.setStyle(SkPaint::kStroke_Style);
+    paint.setStrokeWidth(5.f);
+    paint.setAntiAlias(true);
+
+    SkPoint circleVertices[700];
+    for (int i = 0; i < 700; ++i) {
+        SkScalar angleRads = 2 * SK_ScalarPI * i / 700.f;
+        circleVertices[i] = {100.f + 2.f * SkScalarCos(angleRads),
+                             100.f + 2.f * SkScalarSin(angleRads)};
+    }
+
+    SkPath circle;
+    circle.moveTo(circleVertices[0]);
+    for (int i = 1; i < 700; ++i) {
+        circle.lineTo(circleVertices[i]);
+    }
+    circle.close();
+
+    canvas->drawPath(circle, paint);
+}

gn/gm.gni

@@ -107,6 +107,7 @@ gm_sources = [
   "$_gm/copy_to_4444.cpp",
   "$_gm/crbug_1041204.cpp",
   "$_gm/crbug_1073670.cpp",
+  "$_gm/crbug_1086705.cpp",
   "$_gm/crbug_224618.cpp",
   "$_gm/crbug_691386.cpp",
   "$_gm/crbug_788500.cpp",

src/gpu/ops/GrAAConvexTessellator.cpp

@@ -60,21 +60,22 @@ static bool duplicate_pt(const SkPoint& p0, const SkPoint& p1) {
 }
 
 static bool points_are_colinear_and_b_is_middle(const SkPoint& a, const SkPoint& b,
-                                                const SkPoint& c) {
+                                                const SkPoint& c, float* accumError) {
     // First check distance from b to the infinite line through a, c
     SkVector aToC = c - a;
     SkVector n = {aToC.fY, -aToC.fX};
     n.normalize();
 
-    SkScalar distBToLineAC = n.dot(b) - n.dot(a);
-    if (SkScalarAbs(distBToLineAC) >= kClose) {
-        // Too far from the line, cannot be colinear
+    SkScalar distBToLineAC = SkScalarAbs(n.dot(b) - n.dot(a));
+    if (*accumError + distBToLineAC >= kClose || aToC.dot(b - a) <= 0.f || aToC.dot(c - b) <= 0.f) {
+        // Too far from the line or not between the line segment from a to c
         return false;
+    } else {
+        // Accumulate the distance from b to |ac| that goes "away" when this near-colinear point
+        // is removed to simplify the path.
+        *accumError += distBToLineAC;
+        return true;
     }
-
-    // b is colinear, but it may not be in the line segment between a and c. It's in the middle if
-    // both the angle at a and the angle at c are acute.
-    return aToC.dot(b - a) > 0 && aToC.dot(c - b) > 0;
 }
 
 int GrAAConvexTessellator::addPt(const SkPoint& pt,
@@ -396,6 +397,8 @@ bool GrAAConvexTessellator::extractFromPath(const SkMatrix& m, const SkPath& pat
     // Presumptive inner ring: 6*numPts + 6
     fIndices.setReserve(18*path.countPoints() + 6);
 
+    // Reset the accumulated error for all the future lineTo() calls when iterating over the path.
+    fAccumLinearError = 0.f;
     // TODO: is there a faster way to extract the points from the path? Perhaps
     // get all the points via a new entry point, transform them all in bulk
     // and then walk them to find duplicates?
@@ -435,11 +438,14 @@ bool GrAAConvexTessellator::extractFromPath(const SkMatrix& m, const SkPath& pat
     }
 
     // Remove any lingering colinear points where the path wraps around
+    fAccumLinearError = 0.f;
     bool noRemovalsToDo = false;
     while (!noRemovalsToDo && this->numPts() >= 3) {
-        if (points_are_colinear_and_b_is_middle(fPts[fPts.count() - 2], fPts.top(), fPts[0])) {
+        if (points_are_colinear_and_b_is_middle(fPts[fPts.count() - 2], fPts.top(), fPts[0],
+                                                &fAccumLinearError)) {
             this->popLastPt();
-        } else if (points_are_colinear_and_b_is_middle(fPts.top(), fPts[0], fPts[1])) {
+        } else if (points_are_colinear_and_b_is_middle(fPts.top(), fPts[0], fPts[1],
+                                                       &fAccumLinearError)) {
             this->popFirstPtShuffle();
         } else {
             noRemovalsToDo = true;
@@ -922,7 +928,8 @@ void GrAAConvexTessellator::lineTo(const SkPoint& p, CurveState curve) {
     }
 
     if (this->numPts() >= 2 &&
-        points_are_colinear_and_b_is_middle(fPts[fPts.count() - 2], fPts.top(), p)) {
+        points_are_colinear_and_b_is_middle(fPts[fPts.count() - 2], fPts.top(), p,
+                                            &fAccumLinearError)) {
         // The old last point is on the line from the second to last to the new point
         this->popLastPt();
         // double-check that the new last point is not a duplicate of the new point. In an ideal
@@ -932,6 +939,8 @@ void GrAAConvexTessellator::lineTo(const SkPoint& p, CurveState curve) {
         if (duplicate_pt(p, this->lastPoint())) {
             return;
         }
+    } else {
+        fAccumLinearError = 0.f;
     }
     SkScalar initialRingCoverage = (SkStrokeRec::kFill_Style == fStyle) ? 0.5f : 1.0f;
     this->addPt(p, 0.0f, initialRingCoverage, false, curve);

src/gpu/ops/GrAAConvexTessellator.h

@@ -285,6 +285,10 @@ private:
 
     SkScalar              fMiterLimit;
 
+    // accumulated error when removing near colinear points to prevent an
+    // overly greedy simplification
+    SkScalar              fAccumLinearError;
+
     SkTDArray<SkPoint>    fPointBuffer;
 };