Merge "Consume TextureView matrix safely" into nyc-dev

libs/hwui/FrameBuilder.cpp

@@ -699,7 +699,17 @@ void FrameBuilder::deferTextOnPathOp(const TextOnPathOp& op) {
 
 void FrameBuilder::deferTextureLayerOp(const TextureLayerOp& op) {
     if (CC_UNLIKELY(!op.layer->isRenderable())) return;
-    BakedOpState* bakedState = tryBakeOpState(op);
+
+    const TextureLayerOp* textureLayerOp = &op;
+    // Now safe to access transform (which was potentially unready at record time)
+    if (!op.layer->getTransform().isIdentity()) {
+        // non-identity transform present, so 'inject it' into op by copying + replacing matrix
+        Matrix4 combinedMatrix(op.localMatrix);
+        combinedMatrix.multiply(op.layer->getTransform());
+        textureLayerOp = mAllocator.create<TextureLayerOp>(op, combinedMatrix);
+    }
+    BakedOpState* bakedState = tryBakeOpState(*textureLayerOp);
+
     if (!bakedState) return; // quick rejected
     currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::TextureLayer);
 }

libs/hwui/RecordedOp.h

@@ -419,6 +419,14 @@ struct TextureLayerOp : RecordedOp {
     TextureLayerOp(BASE_PARAMS_PAINTLESS, Layer* layer)
             : SUPER_PAINTLESS(TextureLayerOp)
             , layer(layer) {}
+
+    // Copy an existing TextureLayerOp, replacing the underlying matrix
+    TextureLayerOp(const TextureLayerOp& op, const Matrix4& replacementMatrix)
+            : RecordedOp(RecordedOpId::TextureLayerOp, op.unmappedBounds, replacementMatrix,
+                    op.localClip, op.paint)
+            , layer(op.layer) {
+
+    }
     Layer* layer;
 };
 

libs/hwui/RecordingCanvas.cpp

@@ -576,15 +576,9 @@ void RecordingCanvas::drawLayer(DeferredLayerUpdater* layerHandle) {
 
     // Note that the backing layer has *not* yet been updated, so don't trust
     // its width, height, transform, etc...!
-    Matrix4 totalTransform(*(mState.currentSnapshot()->transform));
-    if (layerHandle->getTransform()) {
-        Matrix4 layerTransform(*layerHandle->getTransform());
-        totalTransform.multiply(layerTransform);
-    }
-
     addOp(alloc().create_trivial<TextureLayerOp>(
             Rect(layerHandle->getWidth(), layerHandle->getHeight()),
-            totalTransform,
+            *(mState.currentSnapshot()->transform),
             getRecordedClip(),
             layerHandle->backingLayer()));
 }

libs/hwui/tests/common/TestUtils.cpp

@@ -44,6 +44,7 @@ sp<DeferredLayerUpdater> TestUtils::createTextureLayerUpdater(
         renderthread::RenderThread& renderThread, uint32_t width, uint32_t height,
         const SkMatrix& transform) {
     Layer* layer = LayerRenderer::createTextureLayer(renderThread.renderState());
+    layer->getTransform().load(transform);
 
     sp<DeferredLayerUpdater> layerUpdater = new DeferredLayerUpdater(layer);
     layerUpdater->setSize(width, height);

libs/hwui/tests/unit/FrameBuilderTests.cpp

@@ -372,8 +372,8 @@ RENDERTHREAD_TEST(FrameBuilder, textStyle) {
     EXPECT_EQ(3, renderer.getIndex()) << "Expect 3 ops";
 }
 
-RENDERTHREAD_TEST(FrameBuilder, textureLayer) {
-    class TextureLayerTestRenderer : public TestRendererBase {
+RENDERTHREAD_TEST(FrameBuilder, textureLayer_clipLocalMatrix) {
+    class TextureLayerClipLocalMatrixTestRenderer : public TestRendererBase {
     public:
         void onTextureLayerOp(const TextureLayerOp& op, const BakedOpState& state) override {
             EXPECT_EQ(0, mIndex++);
@@ -398,11 +398,56 @@ RENDERTHREAD_TEST(FrameBuilder, textureLayer) {
     });
     FrameBuilder frameBuilder(sEmptyLayerUpdateQueue, SkRect::MakeWH(200, 200), 200, 200,
             TestUtils::createSyncedNodeList(node), sLightGeometry, Caches::getInstance());
-    TextureLayerTestRenderer renderer;
+    TextureLayerClipLocalMatrixTestRenderer renderer;
     frameBuilder.replayBakedOps<TestDispatcher>(renderer);
     EXPECT_EQ(1, renderer.getIndex());
 }
 
+RENDERTHREAD_TEST(FrameBuilder, textureLayer_combineMatrices) {
+    class TextureLayerCombineMatricesTestRenderer : public TestRendererBase {
+    public:
+        void onTextureLayerOp(const TextureLayerOp& op, const BakedOpState& state) override {
+            EXPECT_EQ(0, mIndex++);
+
+            Matrix4 expected;
+            expected.loadTranslate(35, 45, 0);
+            EXPECT_MATRIX_APPROX_EQ(expected, state.computedState.transform);
+        }
+    };
+
+    auto layerUpdater = TestUtils::createTextureLayerUpdater(renderThread, 100, 100,
+            SkMatrix::MakeTrans(5, 5));
+
+    auto node = TestUtils::createNode(0, 0, 200, 200,
+            [&layerUpdater](RenderProperties& props, RecordingCanvas& canvas) {
+        canvas.save(SaveFlags::MatrixClip);
+        canvas.translate(30, 40);
+        canvas.drawLayer(layerUpdater.get());
+        canvas.restore();
+    });
+
+    FrameBuilder frameBuilder(sEmptyLayerUpdateQueue, SkRect::MakeWH(200, 200), 200, 200,
+            TestUtils::createSyncedNodeList(node), sLightGeometry, Caches::getInstance());
+    TextureLayerCombineMatricesTestRenderer renderer;
+    frameBuilder.replayBakedOps<TestDispatcher>(renderer);
+    EXPECT_EQ(1, renderer.getIndex());
+}
+
+RENDERTHREAD_TEST(FrameBuilder, textureLayer_reject) {
+    auto layerUpdater = TestUtils::createTextureLayerUpdater(renderThread, 100, 100,
+            SkMatrix::MakeTrans(5, 5));
+    layerUpdater->backingLayer()->setRenderTarget(GL_NONE); // Should be rejected
+
+    auto node = TestUtils::createNode(0, 0, 200, 200,
+            [&layerUpdater](RenderProperties& props, RecordingCanvas& canvas) {
+        canvas.drawLayer(layerUpdater.get());
+    });
+    FrameBuilder frameBuilder(sEmptyLayerUpdateQueue, SkRect::MakeWH(200, 200), 200, 200,
+            TestUtils::createSyncedNodeList(node), sLightGeometry, Caches::getInstance());
+    FailRenderer renderer;
+    frameBuilder.replayBakedOps<TestDispatcher>(renderer);
+}
+
 RENDERTHREAD_TEST(FrameBuilder, functor_reject) {
     class FunctorTestRenderer : public TestRendererBase {
     public:

libs/hwui/tests/unit/RecordingCanvasTests.cpp

@@ -16,6 +16,7 @@
 
 #include <gtest/gtest.h>
 
+#include <DeferredLayerUpdater.h>
 #include <RecordedOp.h>
 #include <RecordingCanvas.h>
 #include <hwui/Paint.h>
@@ -39,6 +40,12 @@ static void playbackOps(const DisplayList& displayList,
     }
 }
 
+static void validateSingleOp(std::unique_ptr<DisplayList>& dl,
+        std::function<void(const RecordedOp& op)> opValidator) {
+    ASSERT_EQ(1u, dl->getOps().size()) << "Must be exactly one op";
+    opValidator(*(dl->getOps()[0]));
+}
+
 TEST(RecordingCanvas, emptyPlayback) {
     auto dl = TestUtils::createDisplayList<RecordingCanvas>(100, 200, [](RecordingCanvas& canvas) {
         canvas.save(SaveFlags::MatrixClip);
@@ -284,6 +291,21 @@ TEST(RecordingCanvas, backgroundAndImage) {
     ASSERT_EQ(2, count);
 }
 
+RENDERTHREAD_TEST(RecordingCanvas, textureLayer) {
+    auto layerUpdater = TestUtils::createTextureLayerUpdater(renderThread, 100, 100,
+            SkMatrix::MakeTrans(5, 5));
+
+    auto dl = TestUtils::createDisplayList<RecordingCanvas>(200, 200,
+            [&layerUpdater](RecordingCanvas& canvas) {
+        canvas.drawLayer(layerUpdater.get());
+    });
+
+    validateSingleOp(dl, [] (const RecordedOp& op) {
+        ASSERT_EQ(RecordedOpId::TextureLayerOp, op.opId);
+        ASSERT_TRUE(op.localMatrix.isIdentity()) << "Op must not apply matrix at record time.";
+    });
+}
+
 TEST(RecordingCanvas, saveLayer_simple) {
     auto dl = TestUtils::createDisplayList<RecordingCanvas>(200, 200, [](RecordingCanvas& canvas) {
         canvas.saveLayerAlpha(10, 20, 190, 180, 128, SaveFlags::ClipToLayer);