commit 14ac42b280448a0742d5dcbf1cbad180d4e1f88f
author Brian Osman <brianosman@google.com> Thu Apr 18 12:17:04 2019 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Apr 18 17:50:37 2019 +0000
tree 9946602c2a1418d410bbf63da1c55e034cd70734
parent 1a668d26f80af893fd8fb94dcab83bfc3bb77dbe

Remove the axis-aligned vector special case to convexity checker

There are bugs in the other code path that will require more state,
and maintaining this special case in parallel is going to be tricky.

Change-Id: I0083b0aa2af16391504f0993e765faa0f5522502
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209163
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

src/core/SkPath.cpp

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 3593967..6655ff1 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2584,7 +2584,6 @@
 
 #else
 
-static int sign(SkScalar x1, SkScalar x2) { SkASSERT(x1 != x2); return x2 < x1; }
 static int sign(SkScalar x) { return x < 0; }
 #define kValueNeverReturnedBySign   2
 
@@ -2613,10 +2612,6 @@
     return aBits < bBits + epsilon && bBits < aBits + epsilon;
 }
 
-static DirChange same_sign(SkScalar curr, SkScalar last, SkScalar prior) {
-    return sign(curr, last) == sign(last, prior) ? kStraight_DirChange : kBackwards_DirChange;
-}
-
 // only valid for a single contour
 struct Convexicator {
 
@@ -2634,13 +2629,11 @@
         fCurrPt = pt;
         if (fPriorPt == fLastPt) {  // should only be true for first non-zero vector
             fFirstPt = pt;
-            fCurrAligned = pt.fX == fLastPt.fX || pt.fY == fLastPt.fY;
         } else if (!this->addVec()) {
             return false;
         }
         fPriorPt = fLastPt;
         fLastPt = fCurrPt;
-        fLastAligned = fCurrAligned;
         return true;
     }
 
@@ -2694,35 +2687,6 @@
 
 private:
     DirChange directionChange() {
-        // if both vectors are axis-aligned, don't do cross product
-        fCurrAligned = fCurrPt.fX == fLastPt.fX || fCurrPt.fY == fLastPt.fY;
-        if (fLastAligned && fCurrAligned) {
-            bool noYChange = fCurrPt.fY == fLastPt.fY && fLastPt.fY == fPriorPt.fY;
-            if (fCurrPt.fX == fLastPt.fX && fLastPt.fX == fPriorPt.fX) {
-                if (noYChange) {
-                    return kStraight_DirChange;
-                }
-                return same_sign(fCurrPt.fY, fLastPt.fY, fPriorPt.fY);
-            }
-            if (!noYChange) { // must be turn to left or right
-                bool flip = fCurrPt.fX != fLastPt.fX;
-                SkASSERT(flip ? fCurrPt.fY == fLastPt.fY &&
-                        fLastPt.fY != fPriorPt.fY && fLastPt.fX == fPriorPt.fX :
-                        fCurrPt.fY != fLastPt.fY &&
-                        fLastPt.fY == fPriorPt.fY && fLastPt.fX != fPriorPt.fX);
-                bool product = flip ? (fCurrPt.fX > fLastPt.fX) != (fLastPt.fY > fPriorPt.fY) :
-                        (fCurrPt.fY > fLastPt.fY) == (fLastPt.fX > fPriorPt.fX);
-                SkDEBUGCODE(SkVector lastV = fLastPt - fPriorPt);
-                SkDEBUGCODE(SkVector curV = fCurrPt - fLastPt);
-                SkDEBUGCODE(SkScalar crossV = SkPoint::CrossProduct(lastV, curV));
-                SkDEBUGCODE(int signV = SkScalarSignAsInt(crossV));
-                SkASSERT(!signV || signV == (product ? 1 : -1));
-                return product ? kRight_DirChange : kLeft_DirChange;
-            }
-            return same_sign(fCurrPt.fX, fLastPt.fX, fPriorPt.fX);
-        }
-        // there are no subtractions above this line; axis aligned paths
-        // are robust and can handle arbitrary values
         SkVector lastVec = fLastPt - fPriorPt;
         SkVector curVec = fCurrPt - fLastPt;
         SkScalar cross = SkPoint::CrossProduct(lastVec, curVec);
@@ -2790,8 +2754,6 @@
     SkPathPriv::FirstDirection   fFirstDirection { SkPathPriv::kUnknown_FirstDirection };
     int                 fReversals { 0 };
     bool                fIsFinite { true };
-    bool                fLastAligned { true };
-    bool                fCurrAligned { true };
 };
 
 SkPath::Convexity SkPath::internalGetConvexity() const {

tests/PathTest.cpp

diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 8c9148d..7ce1ee3 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -3628,7 +3628,7 @@
     REPORTER_ASSERT(reporter, path->isConvex());
     REPORTER_ASSERT(reporter, SkPathPriv::CheapIsFirstDirection(*path, SkPathPriv::AsFirstDirection(dir)));
     path->setConvexity(SkPath::kUnknown_Convexity);
-    REPORTER_ASSERT(reporter, path->getConvexity() == SkPath::kConvex_Convexity);
+    REPORTER_ASSERT(reporter, path->getConvexity() == SkPath::kConcave_Convexity);
     path->reset();
 }