diff --git a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp index 1b816febf846e..c195a8eee8701 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 6292a6c5c591d..dba97fe5ef9f5 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 58b9242e087f9..6eff5895a620e 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 15c0ab1ad8d26..eb67e6cf33b6f 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 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 @@ 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.