commit e30b53cec5eb6eaa5d24231feb3207b917c1baab
author Tom Hudson <tomhudson@google.com> Mon Mar 30 15:59:02 2015 -0400
committer Tom Hudson <tomhudson@google.com> Mon Mar 30 16:57:43 2015 -0400
tree 386c65cec91fa48e12cec5b46bf0f6795bc094a2
parent b13f44bc6ee65e155c54f645ab4aa6786feeb162

Enter RectangleMode when the clip region is a rectangle

ClipArea's region mode implementation store the region's boundary in
a rectangle, and if the region is that rectangle, set its region to
empty.  This is a bug, since as long as the ClipArea is in region
mode it treats its region as determinitive. When the next clipping
operation comes in, it is tested against the empty region instead of
against the valid rect.

Discovered via Skia GM testing.

Change-Id: I3bbc4e81c8d68636061f830156e017e9b5c7e587

libs/hwui/ClipArea.cpp

diff --git a/libs/hwui/ClipArea.cpp b/libs/hwui/ClipArea.cpp
index 852204a..eb520b4 100644
--- a/libs/hwui/ClipArea.cpp
+++ b/libs/hwui/ClipArea.cpp
@@ -234,7 +234,7 @@
 bool ClipArea::clipRegion(const SkRegion& region, SkRegion::Op op) {
     enterRegionMode();
     mClipRegion.op(region, op);
-    setClipRectToRegionBounds();
+    onClipRegionUpdated();
     return true;
 }
 
@@ -263,6 +263,9 @@
 bool ClipArea::rectangleModeClipRectWithTransform(const Rect& r,
         const mat4* transform, SkRegion::Op op) {
 
+    // TODO: we should be able to handle kReplace_Op efficiently without
+    // going through RegionMode and later falling back into RectangleMode.
+
     if (op != SkRegion::kIntersect_Op) {
         enterRegionMode();
         return regionModeClipRectWithTransform(r, transform, op);
@@ -324,15 +327,16 @@
  */
 
 void ClipArea::enterRegionMode() {
-    if (mMode != kModeRegion) {
-        if (mMode == kModeRectangle) {
+    Mode oldMode = mMode;
+    mMode = kModeRegion;
+    if (oldMode != kModeRegion) {
+        if (oldMode == kModeRectangle) {
             mClipRegion.setRect(mClipRect.left, mClipRect.top,
                     mClipRect.right, mClipRect.bottom);
         } else {
             mClipRegion = mRectangleList.convertToRegion(createViewportRegion());
-            setClipRectToRegionBounds();
+            onClipRegionUpdated();
         }
-        mMode = kModeRegion;
     }
 }
 
@@ -342,7 +346,7 @@
     SkRegion transformedRectRegion;
     regionFromPath(transformedRect, transformedRectRegion);
     mClipRegion.op(transformedRectRegion, op);
-    setClipRectToRegionBounds();
+    onClipRegionUpdated();
     return true;
 }
 
@@ -352,12 +356,13 @@
             transform, op);
 }
 
-void ClipArea::setClipRectToRegionBounds() {
+void ClipArea::onClipRegionUpdated() {
     if (!mClipRegion.isEmpty()) {
         mClipRect.set(mClipRegion.getBounds());
 
         if (mClipRegion.isRect()) {
             mClipRegion.setEmpty();
+            enterRectangleMode();
         }
     } else {
         mClipRect.setEmpty();