Fix bug in calculating perspective damage
Change-Id: Iacab98cf3525f891012087acf85e4205b5e8f0d0
diff --git a/libs/hwui/DamageAccumulator.cpp b/libs/hwui/DamageAccumulator.cpp
index 9bd3bdc..c2e14a2 100644
--- a/libs/hwui/DamageAccumulator.cpp
+++ b/libs/hwui/DamageAccumulator.cpp
@@ -121,7 +121,14 @@
static inline void mapRect(const Matrix4* matrix, const SkRect& in, SkRect* out) {
if (in.isEmpty()) return;
Rect temp(in);
- matrix->mapRect(temp);
+ if (CC_LIKELY(!matrix->isPerspective())) {
+ matrix->mapRect(temp);
+ } else {
+ // Don't attempt to calculate damage for a perspective transform
+ // as the numbers this works with can break the perspective
+ // calculations. Just give up and expand to DIRTY_MIN/DIRTY_MAX
+ temp.set(DIRTY_MIN, DIRTY_MIN, DIRTY_MAX, DIRTY_MAX);
+ }
out->join(RECT_ARGS(temp));
}
@@ -134,7 +141,14 @@
const SkMatrix* transform = props.getTransformMatrix();
SkRect temp(in);
if (transform && !transform->isIdentity()) {
- transform->mapRect(&temp);
+ if (CC_LIKELY(!transform->hasPerspective())) {
+ transform->mapRect(&temp);
+ } else {
+ // Don't attempt to calculate damage for a perspective transform
+ // as the numbers this works with can break the perspective
+ // calculations. Just give up and expand to DIRTY_MIN/DIRTY_MAX
+ temp.set(DIRTY_MIN, DIRTY_MIN, DIRTY_MAX, DIRTY_MAX);
+ }
}
temp.offset(props.getLeft(), props.getTop());
out->join(temp);
diff --git a/libs/hwui/DamageAccumulator.h b/libs/hwui/DamageAccumulator.h
index dd3365a..e44fc20 100644
--- a/libs/hwui/DamageAccumulator.h
+++ b/libs/hwui/DamageAccumulator.h
@@ -24,6 +24,11 @@
#include "utils/Macros.h"
+// Smaller than INT_MIN/INT_MAX because we offset these values
+// and thus don't want to be adding offsets to INT_MAX, that's bad
+#define DIRTY_MIN (-0x7ffffff-1)
+#define DIRTY_MAX (0x7ffffff)
+
namespace android {
namespace uirenderer {
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index af8aaa20..6cf894e 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -134,7 +134,7 @@
} else {
// Hope this is big enough?
// TODO: Get this from the display list ops or something
- info.damageAccumulator->dirty(INT_MIN, INT_MIN, INT_MAX, INT_MAX);
+ info.damageAccumulator->dirty(DIRTY_MIN, DIRTY_MIN, DIRTY_MAX, DIRTY_MAX);
}
}
}
diff --git a/libs/hwui/unit_tests/DamageAccumulatorTests.cpp b/libs/hwui/unit_tests/DamageAccumulatorTests.cpp
index c8d6004..29354a7 100644
--- a/libs/hwui/unit_tests/DamageAccumulatorTests.cpp
+++ b/libs/hwui/unit_tests/DamageAccumulatorTests.cpp
@@ -18,6 +18,7 @@
#include <DamageAccumulator.h>
#include <Matrix.h>
+#include <RenderNode.h>
#include <utils/LinearAllocator.h>
#include <SkRect.h>
@@ -35,10 +36,12 @@
identity.loadIdentity();
da.pushTransform(&identity);
da.dirty(50, 50, 100, 100);
- da.pushTransform(&identity);
- da.peekAtDirty(&curDirty);
- ASSERT_EQ(SkRect(), curDirty);
- da.popTransform();
+ {
+ da.pushTransform(&identity);
+ da.peekAtDirty(&curDirty);
+ ASSERT_EQ(SkRect(), curDirty);
+ da.popTransform();
+ }
da.peekAtDirty(&curDirty);
ASSERT_EQ(SkRect::MakeLTRB(50, 50, 100, 100), curDirty);
da.popTransform();
@@ -69,13 +72,62 @@
SkRect curDirty;
identity.loadIdentity();
da.pushTransform(&identity);
- da.pushTransform(&identity);
- da.dirty(50, 50, 100, 100);
- da.popTransform();
- da.pushTransform(&identity);
- da.dirty(150, 50, 200, 125);
- da.popTransform();
+ {
+ da.pushTransform(&identity);
+ da.dirty(50, 50, 100, 100);
+ da.popTransform();
+ da.pushTransform(&identity);
+ da.dirty(150, 50, 200, 125);
+ da.popTransform();
+ }
da.popTransform();
da.finish(&curDirty);
ASSERT_EQ(SkRect::MakeLTRB(50, 50, 200, 125), curDirty);
}
+
+TEST(DamageAccumulator, basicRenderNode) {
+ DamageAccumulator da;
+ RenderNode node1;
+ node1.animatorProperties().setLeftTopRightBottom(50, 50, 500, 500);
+ node1.animatorProperties().updateMatrix();
+ da.pushTransform(&node1);
+ {
+ RenderNode node2;
+ node2.animatorProperties().setLeftTopRightBottom(50, 50, 100, 100);
+ node2.animatorProperties().updateMatrix();
+ da.pushTransform(&node2);
+ da.dirty(0, 0, 25, 25);
+ da.popTransform();
+ }
+ da.popTransform();
+ SkRect dirty;
+ da.finish(&dirty);
+ ASSERT_EQ(SkRect::MakeLTRB(100, 100, 125, 125), dirty);
+}
+
+TEST(DamageAccumulator, perspectiveTransform) {
+ DamageAccumulator da;
+ RenderNode node1;
+ node1.animatorProperties().setLeftTopRightBottom(50, 50, 500, 500);
+ node1.animatorProperties().setClipToBounds(true);
+ node1.animatorProperties().updateMatrix();
+ da.pushTransform(&node1);
+ {
+ RenderNode node2;
+ node2.animatorProperties().setLeftTopRightBottom(50, 50, 100, 100);
+ node2.animatorProperties().setClipToBounds(false);
+ node2.animatorProperties().setRotationX(1.0f);
+ node2.animatorProperties().setRotationY(1.0f);
+ node2.animatorProperties().setRotation(20.0f);
+ node2.animatorProperties().setCameraDistance(500.0f);
+ node2.animatorProperties().setTranslationZ(30.0f);
+ node2.animatorProperties().updateMatrix();
+ da.pushTransform(&node2);
+ da.dirty(DIRTY_MIN, DIRTY_MIN, DIRTY_MAX, DIRTY_MAX);
+ da.popTransform();
+ }
+ da.popTransform();
+ SkRect dirty;
+ da.finish(&dirty);
+ ASSERT_EQ(SkRect::MakeLTRB(50, 50, 500, 500), dirty);
+}