diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp index e58fbbe8e6673..c8eb1ca559107 100644 --- a/libs/hwui/RecordingCanvas.cpp +++ b/libs/hwui/RecordingCanvas.cpp @@ -274,7 +274,12 @@ struct DrawDrawable final : Op { } sk_sp drawable; SkMatrix matrix = SkMatrix::I(); - void draw(SkCanvas* c, const SkMatrix&) const { c->drawDrawable(drawable.get(), &matrix); } + // It is important that we call drawable->draw(c) here instead of c->drawDrawable(drawable). + // Drawables are mutable and in cases, like RenderNodeDrawable, are not expected to produce the + // same content if retained outside the duration of the frame. Therefore we resolve + // them now and do not allow the canvas to take a reference to the drawable and potentially + // keep it alive for longer than the frames duration (e.g. SKP serialization). + void draw(SkCanvas* c, const SkMatrix&) const { drawable->draw(c, &matrix); } }; struct DrawPicture final : Op { static const auto kType = Type::DrawPicture; diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp index 84c0d1369e835..530926bf8dd01 100644 --- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp +++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp @@ -313,8 +313,7 @@ SkCanvas* SkiaPipeline::tryCapture(SkSurface* surface) { case CaptureMode::CallbackAPI: case CaptureMode::SingleFrameSKP: mRecorder.reset(new SkPictureRecorder()); - pictureCanvas = mRecorder->beginRecording(surface->width(), surface->height(), - nullptr, SkPictureRecorder::kPlaybackDrawPicture_RecordFlag); + pictureCanvas = mRecorder->beginRecording(surface->width(), surface->height()); break; case CaptureMode::MultiFrameSKP: // If a multi frame recording is active, initialize recording for a single frame of a diff --git a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp index c813cd945905b..e70378bd15a59 100644 --- a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp +++ b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp @@ -1096,10 +1096,8 @@ TEST(ReorderBarrierDrawable, testShadowMatrix) { int getDrawCounter() { return mDrawCounter; } virtual void onDrawDrawable(SkDrawable* drawable, const SkMatrix* matrix) override { - // expect to draw 2 RenderNodeDrawable, 1 StartReorderBarrierDrawable, - // 1 EndReorderBarrierDrawable - mDrawCounter++; - SkCanvas::onDrawDrawable(drawable, matrix); + // Do not expect this to be called. See RecordingCanvas.cpp DrawDrawable for context. + EXPECT_TRUE(false); } virtual void didTranslate(SkScalar dx, SkScalar dy) override { @@ -1159,8 +1157,8 @@ TEST(ReorderBarrierDrawable, testShadowMatrix) { // create a canvas not backed by any device/pixels, but with dimensions to avoid quick rejection ShadowTestCanvas canvas(CANVAS_WIDTH, CANVAS_HEIGHT); RenderNodeDrawable drawable(parent.get(), &canvas, false); - canvas.drawDrawable(&drawable); - EXPECT_EQ(9, canvas.getDrawCounter()); + drawable.draw(&canvas); + EXPECT_EQ(5, canvas.getDrawCounter()); } // Draw a vector drawable twice but with different bounds and verify correct bounds are used.