keep integral rectangle intersections integral

A pair of coincident lines can generate multiple intersection
points. Path ops is more stable when the intersection T value
is used to recompute the intersection point, but this has
the side-effect of making integral edges intersect at non-integral
values.

While it's worthwhile to fix this, for the moment it is less
disruptive to only worry about keeping intersection values
integral if the original intersection point is integral in
both axes.

Also, fix some debugging code that bit-rotted.

R=msarett@google.com

Change-Id: Iefd27b25d1d21c22b224c174bd59bc6c105033c4
Reviewed-on: https://skia-review.googlesource.com/13721
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>

src/pathops/SkAddIntersections.cpp

@@ -509,9 +509,17 @@ bool AddIntersectTs(SkOpContour* test, SkOpContour* next, SkOpCoincidence* coinc
                 SkASSERT(ts[0][pt] >= 0 && ts[0][pt] <= 1);
                 SkASSERT(ts[1][pt] >= 0 && ts[1][pt] <= 1);
                 wt.segment()->debugValidate();
-                SkOpPtT* testTAt = wt.segment()->addT(ts[swap][pt]);
+                // if t value is used to compute pt in addT, error may creep in and
+                // rect intersections may result in non-rects. if pt value from intersection
+                // is passed in, current tests break. As a workaround, pass in pt
+                // value from intersection only if pt.x and pt.y is integral
+                SkPoint iPt = ts.pt(pt).asSkPoint();
+                bool iPtIsIntegral = iPt.fX == floor(iPt.fX) && iPt.fY == floor(iPt.fY);
+                SkOpPtT* testTAt = iPtIsIntegral ? wt.segment()->addT(ts[swap][pt], iPt)
+                        : wt.segment()->addT(ts[swap][pt]);
                 wn.segment()->debugValidate();
-                SkOpPtT* nextTAt = wn.segment()->addT(ts[!swap][pt]);
+                SkOpPtT* nextTAt = iPtIsIntegral ? wn.segment()->addT(ts[!swap][pt], iPt)
+                        : wn.segment()->addT(ts[!swap][pt]);
                 if (!testTAt->contains(nextTAt)) {
                     SkOpPtT* oppPrev = testTAt->oppPrev(nextTAt);  //  Returns nullptr if pair 
                     if (oppPrev) {                                 //  already share a pt-t loop.

src/pathops/SkOpSegment.cpp

@@ -244,9 +244,8 @@ bool SkOpSegment::addExpanded(double newT, const SkOpSpanBase* test, bool* start
 }
 
 // Please keep this in sync with debugAddT()
-SkOpPtT* SkOpSegment::addT(double t) {
+SkOpPtT* SkOpSegment::addT(double t, const SkPoint& pt) {
     debugValidate();
-    SkPoint pt = this->ptAtT(t);
     SkOpSpanBase* spanBase = &fHead;
     do {
         SkOpPtT* result = spanBase->ptT();
@@ -274,6 +273,10 @@ SkOpPtT* SkOpSegment::addT(double t) {
     return nullptr;  // we never get here, but need this to satisfy compiler
 }
 
+SkOpPtT* SkOpSegment::addT(double t) {
+    return addT(t, this->ptAtT(t));
+}
+
 void SkOpSegment::calcAngles() {
     bool activePrior = !fHead.isCanceled();
     if (activePrior && !fHead.simple()) {

src/pathops/SkOpSegment.h

@@ -93,6 +93,7 @@ public:
     }
 
     SkOpPtT* addT(double t);
+    SkOpPtT* addT(double t, const SkPoint& pt);
 
     template<typename T> T* allocateArray(int count) {
         return SkOpTAllocator<T>::AllocateArray(this->globalState()->allocator(), count);

src/pathops/SkPathOpsDebug.cpp

@@ -1089,7 +1089,11 @@ void SkOpSegment::debugMoveNearby(SkPathOpsDebug::GlitchLog* glitches) const {
     spanBase = &fHead;
     do {  // iterate through all spans associated with start
         const SkOpSpanBase* test = spanBase->upCast()->next();
-        if (this->spansNearby(spanBase, test)) {
+        bool found;
+        if (!this->spansNearby(spanBase, test, &found)) {
+            glitches->record(SkPathOpsDebug::kMoveNearbyMergeFinal_Glitch, test);
+        }
+        if (found) {
             if (test->final()) {
                 if (spanBase->prev()) {
                     glitches->record(SkPathOpsDebug::kMoveNearbyMergeFinal_Glitch, test);

tests/PathOpsOpTest.cpp

@@ -5409,6 +5409,24 @@ path.close();
     testPathOp(reporter, path, path, kUnion_SkPathOp, filename);
 }
 
+static void android1(skiatest::Reporter* reporter, const char* filename) {
+ SkPath path, pathB;
+path.moveTo(SkBits2Float(0xc0a00000), SkBits2Float(0x00000000));  // -5, 0
+path.lineTo(SkBits2Float(0x44866000), SkBits2Float(0x00000000));  // 1075, 0
+path.lineTo(SkBits2Float(0x44866000), SkBits2Float(0x43720000));  // 1075, 242
+path.lineTo(SkBits2Float(0xc0a00000), SkBits2Float(0x43720000));  // -5, 242
+path.lineTo(SkBits2Float(0xc0a00000), SkBits2Float(0x00000000));  // -5, 0
+path.close();
+pathB.moveTo(SkBits2Float(0x00000000), SkBits2Float(0x00000000));  // 0, 0
+pathB.lineTo(SkBits2Float(0x44870000), SkBits2Float(0x00000000));  // 1080, 0
+pathB.lineTo(SkBits2Float(0x44870000), SkBits2Float(0x43720000));  // 1080, 242
+pathB.lineTo(SkBits2Float(0x00000000), SkBits2Float(0x43720000));  // 0, 242
+pathB.lineTo(SkBits2Float(0x00000000), SkBits2Float(0x00000000));  // 0, 0
+pathB.close();
+    testPathOp(reporter, path, pathB, kIntersect_SkPathOp, filename);
+}
+
+
 static void (*skipTest)(skiatest::Reporter* , const char* filename) = 0;
 static void (*firstTest)(skiatest::Reporter* , const char* filename) = 0;
 static void (*stopTest)(skiatest::Reporter* , const char* filename) = 0;
@@ -5416,6 +5434,7 @@ static void (*stopTest)(skiatest::Reporter* , const char* filename) = 0;
 #define TEST(name) { name, #name }
 
 static struct TestDesc tests[] = {
+    TEST(android1),
     TEST(bug5240),
     TEST(circlesOp4),
     TEST(loop17),