Bug 1130978 - Fix VisitEdges. r=kats, a=lmandel
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Wed, 11 Mar 2015 01:18:30 -0400
changeset 250455 fb9ae74a783a
parent 250454 5bd29483f85e
child 250456 52b55d9c1d61
push id4592
push userryanvm@gmail.com
push date2015-03-19 20:47 +0000
treeherdermozilla-beta@52b55d9c1d61 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, lmandel
bugs1130978
milestone37.0
Bug 1130978 - Fix VisitEdges. r=kats, a=lmandel The code is broken because in the else case of VisitNextEdgeBetweenRect we assume that r2->x1 < r1->x1. This is not always the case. The fix is to have VisitNextEdgeBetweenRect return whether there's an overlap. The calling code can than adjust x1 appropriately if r1 != r1_end && r2 != r2_end.
gfx/src/nsRegion.cpp
gfx/tests/gtest/TestRegion.cpp
--- a/gfx/src/nsRegion.cpp
+++ b/gfx/src/nsRegion.cpp
@@ -367,38 +367,43 @@ void nsRegion::SimplifyOutwardByArea(uin
   } else {
     *this = GetBounds();
   }
 }
 
 
 typedef void (*visit_fn)(void *closure, VisitSide side, int x1, int y1, int x2, int y2);
 
-static void VisitNextEdgeBetweenRect(visit_fn visit, void *closure, VisitSide side,
+static bool VisitNextEdgeBetweenRect(visit_fn visit, void *closure, VisitSide side,
 				     pixman_box32_t *&r1, pixman_box32_t *&r2, const int y, int &x1)
 {
   // check for overlap
   if (r1->x2 >= r2->x1) {
     MOZ_ASSERT(r2->x1 >= x1);
     visit(closure, side, x1, y, r2->x1, y);
 
     // find the rect that ends first or always drop the one that comes first?
     if (r1->x2 < r2->x2) {
       x1 = r1->x2;
       r1++;
     } else {
       x1 = r2->x2;
       r2++;
     }
+    return true;
   } else {
     MOZ_ASSERT(r1->x2 < r2->x2);
     // we handle the corners by just extending the top and bottom edges
     visit(closure, side, x1, y, r1->x2+1, y);
     r1++;
+    // we assign x1 because we can assume that x1 <= r2->x1 - 1
+    // However the caller may know better and if so, may update
+    // x1 to r1->x1
     x1 = r2->x1 - 1;
+    return false;
   }
 }
 
 //XXX: if we need to this can compute the end of the row
 static void
 VisitSides(visit_fn visit, void *closure, pixman_box32_t *r, pixman_box32_t *r_end)
 {
   // XXX: we can drop LEFT/RIGHT and just use the orientation
@@ -432,29 +437,32 @@ static pixman_box32_t *
 VisitInbetween(visit_fn visit, void *closure, pixman_box32_t *r1,
                pixman_box32_t *r1_end,
                pixman_box32_t *r2,
                pixman_box32_t *r2_end)
 {
   const int y = r1->y2;
   int x1;
 
-  /* Find the left-most edge */
-  if (r1->x1 < r2->x1) {
-    x1 = r1->x1 - 1;
-  } else {
-    x1 = r2->x1 - 1;
-  }
+  bool overlap = false;
+  while (r1 != r1_end && r2 != r2_end) {
+    if (!overlap) {
+      /* Find the left-most edge */
+      if (r1->x1 < r2->x1) {
+	x1 = r1->x1 - 1;
+      } else {
+	x1 = r2->x1 - 1;
+      }
+    }
 
-  while (r1 != r1_end && r2 != r2_end) {
     MOZ_ASSERT((x1 >= (r1->x1 - 1)) || (x1 >= (r2->x1 - 1)));
     if (r1->x1 < r2->x1) {
-      VisitNextEdgeBetweenRect(visit, closure, VisitSide::BOTTOM, r1, r2, y, x1);
+      overlap = VisitNextEdgeBetweenRect(visit, closure, VisitSide::BOTTOM, r1, r2, y, x1);
     } else {
-      VisitNextEdgeBetweenRect(visit, closure, VisitSide::TOP, r2, r1, y, x1);
+      overlap = VisitNextEdgeBetweenRect(visit, closure, VisitSide::TOP, r2, r1, y, x1);
     }
   }
 
   /* Finish up which ever row has remaining rects*/
   if (r1 != r1_end) {
     // top row
     do {
       visit(closure, VisitSide::BOTTOM, x1, y, r1->x2 + 1, y);
--- a/gfx/tests/gtest/TestRegion.cpp
+++ b/gfx/tests/gtest/TestRegion.cpp
@@ -411,16 +411,17 @@ struct RegionBitmap {
 
   unsigned char *bitmap;
   int width;
   int height;
 };
 
 void VisitEdge(void *closure, VisitSide side, int x1, int y1, int x2, int y2)
 {
+  EXPECT_GE(x2, x1);
   RegionBitmap *visitor = static_cast<RegionBitmap*>(closure);
   unsigned char *bitmap = visitor->bitmap;
   const int width = visitor->width;
 
   if (side == VisitSide::TOP) {
     while (x1 != x2) {
       bitmap[x1 + (y1 - 1) * width] = DILATE_VALUE;
       x1++;
@@ -532,9 +533,32 @@ TEST(Gfx, RegionVisitEdges) {
   {
     // vertically separated
     nsRegion r(nsRect(20, 20, 100, 100));
     r.Or(r, nsRect(120, 125, 100, 100));
 
     TestVisit(r);
   }
 
+  {
+    // two upper rects followed by a lower one
+    // on the same line
+    nsRegion r(nsRect(5, 5, 50, 50));
+    r.Or(r, nsRect(100, 5, 50, 50));
+    r.Or(r, nsRect(200, 50, 50, 50));
+
+    TestVisit(r);
+  }
+
+  {
+    // bug 1130978.
+    nsRegion r(nsRect(4, 1, 61, 49));
+    r.Or(r, nsRect(115, 1, 99, 49));
+    r.Or(r, nsRect(115, 49, 99, 1));
+    r.Or(r, nsRect(12, 50, 11, 5));
+    r.Or(r, nsRect(25, 50, 28, 5));
+    r.Or(r, nsRect(115, 50, 99, 5));
+    r.Or(r, nsRect(115, 55, 99, 12));
+
+    TestVisit(r);
+  }
+
 }