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

4 files changed, 50 insertions, 18 deletions

diff --git a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp
index 1b816febf846..c195a8eee870 100644
--- a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp
+++ b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp
@@ -142,7 +142,7 @@ void RenderNodeDrawable::forceDraw(SkCanvas* canvas) {
             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 @@ void RenderNodeDrawable::drawContent(SkCanvas* canvas) const {
         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 6292a6c5c591..dba97fe5ef9f 100644
--- a/libs/hwui/pipeline/skia/ReorderBarrierDrawables.cpp
+++ b/libs/hwui/pipeline/skia/ReorderBarrierDrawables.cpp
@@ -55,6 +55,11 @@ void StartReorderBarrierDrawable::onDraw(SkCanvas* canvas) {
         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 @@ void EndReorderBarrierDrawable::onDraw(SkCanvas* canvas) {
 
         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 @@ void EndReorderBarrierDrawable::drawShadow(SkCanvas* canvas, RenderNodeDrawable*
     }
 
     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 58b9242e087f..6eff5895a620 100644
--- a/libs/hwui/pipeline/skia/SkiaDisplayList.h
+++ b/libs/hwui/pipeline/skia/SkiaDisplayList.h
@@ -173,12 +173,12 @@ public:
     // 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 15c0ab1ad8d2..eb67e6cf33b6 100644
--- a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
@@ -1094,7 +1094,7 @@ TEST(ReorderBarrierDrawable, testShadowMatrix) {
     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 @@ TEST(ReorderBarrierDrawable, testShadowMatrix) {
             EXPECT_EQ(dy, TRANSLATE_Y);
         }
 
+        virtual void didSetMatrix(const SkMatrix& matrix) override {
+            mDrawCounter++;
+            // 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 {
-            // This function is invoked by EndReorderBarrierDrawable::drawShadow to apply shadow
-            // matrix.
             mDrawCounter++;
-            EXPECT_EQ(SkMatrix::MakeTrans(CASTER_X, CASTER_Y), matrix);
-            EXPECT_EQ(SkMatrix::MakeTrans(CASTER_X + TRANSLATE_X, CASTER_Y + TRANSLATE_Y),
-                      getTotalMatrix());
+            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 @@ TEST(ReorderBarrierDrawable, testShadowMatrix) {
     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.