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
diff --git a/libs/hwui/ClipArea.cpp b/libs/hwui/ClipArea.cpp
index 501cbe5..afe9807 100644
--- a/libs/hwui/ClipArea.cpp
+++ b/libs/hwui/ClipArea.cpp
@@ -375,15 +375,13 @@
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 @@
&& 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 @@
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 @@
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 @@
}
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 @@
}
}
rectListClip->rect = rectList.calculateBounds();
+ rectListClip->rect.snapToPixelBoundaries();
mLastResolutionResult = rectListClip;
}
}
@@ -505,7 +506,7 @@
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 dc2ea07..822d04f 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 @@
// 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_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 c39047c..d35b1f9 100644
--- a/libs/hwui/tests/unit/RecordingCanvasTests.cpp
+++ b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
@@ -442,9 +442,7 @@
// 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);