Restore matrix transform for out-of-order render nodes
Restore matrix for render nodes, which are drawn out of order.
Test: DrawChildBug-debug.apk draws correctly, new test ag/4355529
Bug: 80173852
Change-Id: I3f789a7cf0ee5816da84255199b265643f95af1c
diff --git a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp
index 1b816fe..c195a8e 100644
--- a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp
+++ b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp
@@ -142,7 +142,7 @@
LOG_ALWAYS_FATAL_IF(!mProjectedDisplayList->mProjectedOutline);
const bool shouldClip = mProjectedDisplayList->mProjectedOutline->getPath();
SkAutoCanvasRestore acr2(canvas, shouldClip);
- canvas->setMatrix(mProjectedDisplayList->mProjectedReceiverParentMatrix);
+ canvas->setMatrix(mProjectedDisplayList->mParentMatrix);
if (shouldClip) {
clipOutline(*mProjectedDisplayList->mProjectedOutline, canvas, nullptr);
}
@@ -200,9 +200,7 @@
setViewProperties(properties, canvas, &alphaMultiplier);
}
SkiaDisplayList* displayList = (SkiaDisplayList*)mRenderNode->getDisplayList();
- if (displayList->containsProjectionReceiver()) {
- displayList->mProjectedReceiverParentMatrix = canvas->getTotalMatrix();
- }
+ displayList->mParentMatrix = canvas->getTotalMatrix();
// TODO should we let the bound of the drawable do this for us?
const SkRect bounds = SkRect::MakeWH(properties.getWidth(), properties.getHeight());
diff --git a/libs/hwui/pipeline/skia/ReorderBarrierDrawables.cpp b/libs/hwui/pipeline/skia/ReorderBarrierDrawables.cpp
index 6292a6c..dba97fe 100644
--- a/libs/hwui/pipeline/skia/ReorderBarrierDrawables.cpp
+++ b/libs/hwui/pipeline/skia/ReorderBarrierDrawables.cpp
@@ -55,6 +55,11 @@
if (casterZ >= -NON_ZERO_EPSILON) { // draw only children with negative Z
return;
}
+ SkAutoCanvasRestore acr(canvas, true);
+ // Since we're drawing out of recording order, the child's matrix needs to be applied to the
+ // canvas. In in-order drawing, the canvas already has the child's matrix applied.
+ canvas->setMatrix(mDisplayList->mParentMatrix);
+ canvas->concat(childNode->getRecordedMatrix());
childNode->forceDraw(canvas);
drawIndex++;
}
@@ -102,6 +107,11 @@
RenderNodeDrawable* childNode = zChildren[drawIndex];
SkASSERT(childNode);
+ SkAutoCanvasRestore acr(canvas, true);
+ // Since we're drawing out of recording order, the child's matrix needs to be applied to the
+ // canvas. In in-order drawing, the canvas already has the child's matrix applied.
+ canvas->setMatrix(mStartBarrier->mDisplayList->mParentMatrix);
+ canvas->concat(childNode->getRecordedMatrix());
childNode->forceDraw(canvas);
drawIndex++;
@@ -153,10 +163,15 @@
}
SkAutoCanvasRestore acr(canvas, true);
+ // Since we're drawing out of recording order, the child's matrix needs to be applied to the
+ // canvas. In in-order drawing, the canvas already has the child's matrix applied.
+ canvas->setMatrix(mStartBarrier->mDisplayList->mParentMatrix);
SkMatrix shadowMatrix;
- mat4 hwuiMatrix;
+ mat4 hwuiMatrix(caster->getRecordedMatrix());
// TODO we don't pass the optional boolean to treat it as a 4x4 matrix
+ // applyViewPropertyTransforms gets the same matrix, which render nodes apply with
+ // RenderNodeDrawable::setViewProperties as a part if their draw.
caster->getRenderNode()->applyViewPropertyTransforms(hwuiMatrix);
hwuiMatrix.copyTo(shadowMatrix);
canvas->concat(shadowMatrix);
diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.h b/libs/hwui/pipeline/skia/SkiaDisplayList.h
index 58b9242..6eff589 100644
--- a/libs/hwui/pipeline/skia/SkiaDisplayList.h
+++ b/libs/hwui/pipeline/skia/SkiaDisplayList.h
@@ -173,12 +173,12 @@
// node is drawn.
const Outline* mProjectedOutline = nullptr;
- // mProjectedReceiverParentMatrix is valid when render node tree is traversed during the draw
- // pass. Render nodes that have a child receiver node, will store their matrix in
- // mProjectedReceiverParentMatrix. Child receiver node will set the matrix and then clip with
- // the
- // outline of their parent.
- SkMatrix mProjectedReceiverParentMatrix;
+ // mParentMatrix is set and valid when render node tree is traversed during the draw
+ // pass. Render nodes, which draw in a order different than recording order (e.g. nodes with a
+ // child receiver node or Z elevation), can use mParentMatrix to calculate the final transform
+ // without replaying the matrix transform OPs from the display list.
+ // Child receiver node will set the matrix and then clip with the outline of their parent.
+ SkMatrix mParentMatrix;
};
}; // namespace skiapipeline
diff --git a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
index 15c0ab1..eb67e6c 100644
--- a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
@@ -1094,7 +1094,7 @@
class ShadowTestCanvas : public SkCanvas {
public:
ShadowTestCanvas(int width, int height) : SkCanvas(width, height) {}
- int getIndex() { return mDrawCounter; }
+ int getDrawCounter() { return mDrawCounter; }
virtual void onDrawDrawable(SkDrawable* drawable, const SkMatrix* matrix) override {
// expect to draw 2 RenderNodeDrawable, 1 StartReorderBarrierDrawable,
@@ -1109,17 +1109,36 @@
EXPECT_EQ(dy, TRANSLATE_Y);
}
- virtual void didConcat(const SkMatrix& matrix) override {
- // This function is invoked by EndReorderBarrierDrawable::drawShadow to apply shadow
- // matrix.
+ virtual void didSetMatrix(const SkMatrix& matrix) override {
mDrawCounter++;
- EXPECT_EQ(SkMatrix::MakeTrans(CASTER_X, CASTER_Y), matrix);
- EXPECT_EQ(SkMatrix::MakeTrans(CASTER_X + TRANSLATE_X, CASTER_Y + TRANSLATE_Y),
- getTotalMatrix());
+ // First invocation is EndReorderBarrierDrawable::drawShadow to apply shadow matrix.
+ // Second invocation is preparing the matrix for an elevated RenderNodeDrawable.
+ EXPECT_TRUE(matrix.isIdentity());
+ EXPECT_TRUE(getTotalMatrix().isIdentity());
+ }
+
+ virtual void didConcat(const SkMatrix& matrix) override {
+ mDrawCounter++;
+ if (mFirstDidConcat) {
+ // First invocation is EndReorderBarrierDrawable::drawShadow to apply shadow matrix.
+ mFirstDidConcat = false;
+ EXPECT_EQ(SkMatrix::MakeTrans(CASTER_X + TRANSLATE_X, CASTER_Y + TRANSLATE_Y),
+ matrix);
+ EXPECT_EQ(SkMatrix::MakeTrans(CASTER_X + TRANSLATE_X, CASTER_Y + TRANSLATE_Y),
+ getTotalMatrix());
+ } else {
+ // Second invocation is preparing the matrix for an elevated RenderNodeDrawable.
+ EXPECT_EQ(SkMatrix::MakeTrans(TRANSLATE_X, TRANSLATE_Y),
+ matrix);
+ EXPECT_EQ(SkMatrix::MakeTrans(TRANSLATE_X, TRANSLATE_Y),
+ getTotalMatrix());
+ }
}
protected:
int mDrawCounter = 0;
+ private:
+ bool mFirstDidConcat = true;
};
auto parent = TestUtils::createSkiaNode(
@@ -1143,7 +1162,7 @@
ShadowTestCanvas canvas(CANVAS_WIDTH, CANVAS_HEIGHT);
RenderNodeDrawable drawable(parent.get(), &canvas, false);
canvas.drawDrawable(&drawable);
- EXPECT_EQ(6, canvas.getIndex());
+ EXPECT_EQ(9, canvas.getDrawCounter());
}
// Draw a vector drawable twice but with different bounds and verify correct bounds are used.