Fix clip rect snapping at non-integer scale values

bug:26923968

Also removes several reinterprets to ClipRect, since Rect member is in
base class.

Change-Id: If46dbdcea05b1257af185ccb38058735ebe81f79

3 files changed, 38 insertions, 18 deletions

diff --git a/libs/hwui/ClipArea.cpp b/libs/hwui/ClipArea.cpp
index 501cbe50ee05..afe980712d81 100644
--- a/libs/hwui/ClipArea.cpp
+++ b/libs/hwui/ClipArea.cpp
@@ -375,15 +375,13 @@ const ClipBase* ClipArea::serializeClip(LinearAllocator& allocator) {
             serialization->rect.set(mClipRegion.getBounds());
             break;
         }
+        // TODO: this is only done for draw time, should eventually avoid for record time
+        serialization->rect.snapToPixelBoundaries();
         mLastSerialization = serialization;
     }
     return mLastSerialization;
 }
 
-inline static const Rect& getRect(const ClipBase* scb) {
-    return reinterpret_cast<const ClipRect*>(scb)->rect;
-}
-
 inline static const RectangleList& getRectList(const ClipBase* scb) {
     return reinterpret_cast<const ClipRectList*>(scb)->rectList;
 }
@@ -425,9 +423,10 @@ const ClipBase* ClipArea::serializeIntersectedClip(LinearAllocator& allocator,
                 && recordedClip->mode == ClipMode::Rectangle
                 && recordedClipTransform.rectToRect())) {
             // common case - result is a single rectangle
-            auto rectClip = allocator.create<ClipRect>(getRect(recordedClip));
+            auto rectClip = allocator.create<ClipRect>(recordedClip->rect);
             recordedClipTransform.mapRect(rectClip->rect);
             rectClip->rect.doIntersect(mClipRect);
+            rectClip->rect.snapToPixelBoundaries();
             mLastResolutionResult = rectClip;
         } else if (CC_UNLIKELY(mMode == ClipMode::Region
                 || recordedClip->mode == ClipMode::Region
@@ -438,11 +437,11 @@ const ClipBase* ClipArea::serializeIntersectedClip(LinearAllocator& allocator,
             case ClipMode::Rectangle:
                 if (CC_LIKELY(recordedClipTransform.rectToRect())) {
                     // simple transform, skip creating SkPath
-                    Rect resultClip(getRect(recordedClip));
+                    Rect resultClip(recordedClip->rect);
                     recordedClipTransform.mapRect(resultClip);
                     other.setRect(resultClip.toSkIRect());
                 } else {
-                    SkPath transformedRect = pathFromTransformedRectangle(getRect(recordedClip),
+                    SkPath transformedRect = pathFromTransformedRectangle(recordedClip->rect,
                             recordedClipTransform);
                     other.setPath(transformedRect, createViewportRegion());
                 }
@@ -474,6 +473,7 @@ const ClipBase* ClipArea::serializeIntersectedClip(LinearAllocator& allocator,
                 regionClip->region.op(mClipRegion, other, SkRegion::kIntersect_Op);
                 break;
             }
+            // Don't need to snap, since region's in int bounds
             regionClip->rect.set(regionClip->region.getBounds());
             mLastResolutionResult = regionClip;
         } else {
@@ -484,7 +484,7 @@ const ClipBase* ClipArea::serializeIntersectedClip(LinearAllocator& allocator,
             }
 
             if (recordedClip->mode == ClipMode::Rectangle) {
-                rectList.intersectWith(getRect(recordedClip), recordedClipTransform);
+                rectList.intersectWith(recordedClip->rect, recordedClipTransform);
             } else {
                 const RectangleList& other = getRectList(recordedClip);
                 for (int i = 0; i < other.getTransformedRectanglesCount(); i++) {
@@ -495,6 +495,7 @@ const ClipBase* ClipArea::serializeIntersectedClip(LinearAllocator& allocator,
                 }
             }
             rectListClip->rect = rectList.calculateBounds();
+            rectListClip->rect.snapToPixelBoundaries();
             mLastResolutionResult = rectListClip;
         }
     }
@@ -505,7 +506,7 @@ void ClipArea::applyClip(const ClipBase* clip, const Matrix4& transform) {
     if (!clip) return; // nothing to do
 
     if (CC_LIKELY(clip->mode == ClipMode::Rectangle)) {
-        clipRectWithTransform(getRect(clip), &transform, SkRegion::kIntersect_Op);
+        clipRectWithTransform(clip->rect, &transform, SkRegion::kIntersect_Op);
     } else if (CC_LIKELY(clip->mode == ClipMode::RectangleList)) {
         auto&& rectList = getRectList(clip);
         for (int i = 0; i < rectList.getTransformedRectanglesCount(); i++) {
diff --git a/libs/hwui/tests/unit/ClipAreaTests.cpp b/libs/hwui/tests/unit/ClipAreaTests.cpp
index dc2ea0784b7e..822d04f61eb5 100644
--- a/libs/hwui/tests/unit/ClipAreaTests.cpp
+++ b/libs/hwui/tests/unit/ClipAreaTests.cpp
@@ -27,7 +27,7 @@
 namespace android {
 namespace uirenderer {
 
-static Rect kViewportBounds(0, 0, 2048, 2048);
+static Rect kViewportBounds(2048, 2048);
 
 static ClipArea createClipArea() {
     ClipArea area;
@@ -140,17 +140,15 @@ TEST(ClipArea, serializeClip) {
 
     // rect list
     Matrix4 rotate;
-    rotate.loadRotate(2.0f);
-    area.clipRectWithTransform(Rect(200, 200), &rotate, SkRegion::kIntersect_Op);
+    rotate.loadRotate(5.0f);
+    area.clipRectWithTransform(Rect(50, 50, 150, 150), &rotate, SkRegion::kIntersect_Op);
     {
         auto serializedClip = area.serializeClip(allocator);
         ASSERT_NE(nullptr, serializedClip);
         ASSERT_EQ(ClipMode::RectangleList, serializedClip->mode);
         auto clipRectList = reinterpret_cast<const ClipRectList*>(serializedClip);
         EXPECT_EQ(2, clipRectList->rectList.getTransformedRectanglesCount());
-        EXPECT_FALSE(clipRectList->rect.isEmpty());
-        EXPECT_FLOAT_EQ(199.87817f, clipRectList->rect.right)
-            << "Right side should be clipped by rotated rect";
+        EXPECT_EQ(Rect(37, 54, 145, 163), clipRectList->rect);
         EXPECT_EQ(serializedClip, area.serializeClip(allocator))
                 << "Requery of clip on unmodified ClipArea must return same pointer.";
     }
@@ -241,5 +239,28 @@ TEST(ClipArea, serializeIntersectedClip) {
     }
 }
 
+TEST(ClipArea, serializeIntersectedClip_snap) {
+    ClipArea area(createClipArea());
+    area.setClip(100.2, 100.4, 500.6, 500.8);
+    LinearAllocator allocator;
+
+    {
+        // no recorded clip case
+        auto resolvedClip = area.serializeIntersectedClip(allocator, nullptr, Matrix4::identity());
+        EXPECT_EQ(Rect(100, 100, 501, 501), resolvedClip->rect);
+    }
+    {
+        // recorded clip case
+        ClipRect recordedClip(Rect(100.12, 100.74));
+        Matrix4 translateScale;
+        translateScale.loadTranslate(100, 100, 0);
+        translateScale.scale(2, 3, 1); // recorded clip will have non-int coords, even after transform
+        auto resolvedClip = area.serializeIntersectedClip(allocator, &recordedClip, translateScale);
+        ASSERT_NE(nullptr, resolvedClip);
+        EXPECT_EQ(ClipMode::Rectangle, resolvedClip->mode);
+        EXPECT_EQ(Rect(100, 100, 300, 402), resolvedClip->rect);
+    }
+}
+
 } // namespace uirenderer
 } // namespace android
diff --git a/libs/hwui/tests/unit/RecordingCanvasTests.cpp b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
index c39047ca1d89..d35b1f94583d 100644
--- a/libs/hwui/tests/unit/RecordingCanvasTests.cpp
+++ b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
@@ -442,9 +442,7 @@ TEST(RecordingCanvas, saveLayer_rotateClipped) {
             // since the same clip will be computed at draw time. If such a change is made, this
             // check could be done at record time by querying the clip, or the clip could be altered
             // slightly so that it is serialized.
-            EXPECT_RECT_APPROX_EQ(Rect(58.57864, 58.57864, 341.42136, 341.42136),
-                    (reinterpret_cast<const ClipRect*>(op.localClip))->rect);
-
+            EXPECT_EQ(Rect(59, 59, 341, 341), op.localClip->rect);
             EXPECT_EQ(Rect(400, 400), op.unmappedBounds);
             expectedMatrix.loadIdentity();
             EXPECT_MATRIX_APPROX_EQ(expectedMatrix, op.localMatrix);