diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp index 5e21dfc6db8af..6786a69c8dd1c 100644 --- a/libs/hwui/SkiaCanvas.cpp +++ b/libs/hwui/SkiaCanvas.cpp @@ -52,16 +52,20 @@ Canvas* Canvas::create_canvas(SkCanvas* skiaCanvas) { SkiaCanvas::SkiaCanvas() {} SkiaCanvas::SkiaCanvas(SkCanvas* canvas) - : mCanvas(SkRef(canvas)) {} + : mCanvas(canvas) {} SkiaCanvas::SkiaCanvas(const SkBitmap& bitmap) { - mCanvas.reset(new SkCanvas(bitmap)); + mCanvasOwned = std::unique_ptr(new SkCanvas(bitmap)); + mCanvas = mCanvasOwned.get(); } SkiaCanvas::~SkiaCanvas() {} void SkiaCanvas::reset(SkCanvas* skiaCanvas) { - mCanvas.reset(SkRef(skiaCanvas)); + if (mCanvas != skiaCanvas) { + mCanvas = skiaCanvas; + mCanvasOwned.reset(); + } mSaveStack.reset(nullptr); mHighContrastText = false; } @@ -99,8 +103,9 @@ void SkiaCanvas::setBitmap(const SkBitmap& bitmap) { mCanvas->replayClips(&copier); } - // unrefs the existing canvas - mCanvas.reset(newCanvas); + // deletes the previously owned canvas (if any) + mCanvasOwned = std::unique_ptr(newCanvas); + mCanvas = newCanvas; // clean up the old save stack mSaveStack.reset(nullptr); @@ -307,7 +312,7 @@ void SkiaCanvas::applyPersistentClips(size_t clipStartIndex) { const SkMatrix saveMatrix = mCanvas->getTotalMatrix(); for (auto clip = begin; clip != end; ++clip) { - clip->apply(mCanvas.get()); + clip->apply(mCanvas); } mCanvas->setMatrix(saveMatrix); @@ -562,7 +567,7 @@ void SkiaCanvas::drawBitmap(Bitmap& bitmap, float left, float top, const SkPaint void SkiaCanvas::drawBitmap(Bitmap& hwuiBitmap, const SkMatrix& matrix, const SkPaint* paint) { SkBitmap bitmap; hwuiBitmap.getSkBitmap(&bitmap); - SkAutoCanvasRestore acr(mCanvas.get(), true); + SkAutoCanvasRestore acr(mCanvas, true); mCanvas->concat(matrix); mCanvas->drawBitmap(bitmap, 0, 0, paint); } diff --git a/libs/hwui/SkiaCanvas.h b/libs/hwui/SkiaCanvas.h index a0cdfcbfeab7e..4f1d8572eb2d5 100644 --- a/libs/hwui/SkiaCanvas.h +++ b/libs/hwui/SkiaCanvas.h @@ -35,15 +35,15 @@ public: * Create a new SkiaCanvas. * * @param canvas SkCanvas to handle calls made to this SkiaCanvas. Must - * not be NULL. This constructor will ref() the SkCanvas, and unref() - * it in its destructor. + * not be NULL. This constructor does not take ownership, so the caller + * must guarantee that it remains valid while the SkiaCanvas is valid. */ explicit SkiaCanvas(SkCanvas* canvas); virtual ~SkiaCanvas(); virtual SkCanvas* asSkCanvas() override { - return mCanvas.get(); + return mCanvas; } virtual void resetRecording(int width, int height, @@ -182,7 +182,9 @@ private: class Clip; - sk_sp mCanvas; + std::unique_ptr mCanvasOwned; // might own a canvas we allocated + SkCanvas* mCanvas; // we do NOT own this canvas, it must survive us + // unless it is the same as mCanvasOwned.get() std::unique_ptr mSaveStack; // lazily allocated, tracks partial saves. std::vector mClipStack; // tracks persistent clips. }; diff --git a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp index fafce86740ead..c2df9ecfe7032 100644 --- a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp +++ b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp @@ -311,40 +311,42 @@ RENDERTHREAD_TEST(RenderNodeDrawable, projectionHwLayer) { static const int LAYER_HEIGHT = 200; class ProjectionTestCanvas : public SkCanvas { public: - ProjectionTestCanvas() : SkCanvas(CANVAS_WIDTH, CANVAS_HEIGHT) {} + ProjectionTestCanvas(int* drawCounter) + : SkCanvas(CANVAS_WIDTH, CANVAS_HEIGHT) + , mDrawCounter(drawCounter) + {} void onDrawArc(const SkRect&, SkScalar startAngle, SkScalar sweepAngle, bool useCenter, const SkPaint&) override { - EXPECT_EQ(0, mIndex++); //part of painting the layer + EXPECT_EQ(0, (*mDrawCounter)++); //part of painting the layer EXPECT_EQ(SkRect::MakeLTRB(0, 0, LAYER_WIDTH, LAYER_HEIGHT), getBounds(this)); } void onDrawRect(const SkRect& rect, const SkPaint& paint) override { - EXPECT_EQ(1, mIndex++); + EXPECT_EQ(1, (*mDrawCounter)++); EXPECT_EQ(SkRect::MakeLTRB(0, 0, CANVAS_WIDTH, CANVAS_HEIGHT), getBounds(this)); } void onDrawOval(const SkRect&, const SkPaint&) override { - EXPECT_EQ(2, mIndex++); + EXPECT_EQ(2, (*mDrawCounter)++); SkMatrix expectedMatrix; expectedMatrix.setTranslate(100 - SCROLL_X, 100 - SCROLL_Y); EXPECT_EQ(expectedMatrix, getTotalMatrix()); EXPECT_EQ(SkRect::MakeLTRB(-85, -80, 295, 300), getLocalBounds(this)); } - int mIndex = 0; + int* mDrawCounter; }; class ProjectionLayer : public SkSurface_Base { public: - ProjectionLayer(ProjectionTestCanvas *canvas) + ProjectionLayer(int* drawCounter) : SkSurface_Base(SkImageInfo::MakeN32Premul(LAYER_WIDTH, LAYER_HEIGHT), nullptr) - , mCanvas(canvas) { + , mDrawCounter(drawCounter) { } void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) override { - EXPECT_EQ(3, mCanvas->mIndex++); + EXPECT_EQ(3, (*mDrawCounter)++); EXPECT_EQ(SkRect::MakeLTRB(100 - SCROLL_X, 100 - SCROLL_Y, 300 - SCROLL_X, - 300 - SCROLL_Y), getBounds(mCanvas)); + 300 - SCROLL_Y), getBounds(this->getCanvas())); } SkCanvas* onNewCanvas() override { - mCanvas->ref(); - return mCanvas; + return new ProjectionTestCanvas(mDrawCounter); } sk_sp onNewSurface(const SkImageInfo&) override { return sk_sp(); @@ -353,7 +355,7 @@ RENDERTHREAD_TEST(RenderNodeDrawable, projectionHwLayer) { return sk_sp(); } void onCopyOnWrite(ContentChangeMode) override {} - ProjectionTestCanvas* mCanvas; + int* mDrawCounter; }; auto receiverBackground = TestUtils::createSkiaNode(0, 0, CANVAS_WIDTH, CANVAS_HEIGHT, @@ -397,10 +399,10 @@ RENDERTHREAD_TEST(RenderNodeDrawable, projectionHwLayer) { info.observer = nullptr; parent->prepareTree(info); - sk_sp canvas(new ProjectionTestCanvas()); + int drawCounter = 0; //set a layer after prepareTree to avoid layer logic there child->animatorProperties().mutateLayerProperties().setType(LayerType::RenderLayer); - sk_sp surfaceLayer1(new ProjectionLayer(canvas.get())); + sk_sp surfaceLayer1(new ProjectionLayer(&drawCounter)); child->setLayerSurface(surfaceLayer1); Matrix4 windowTransform; windowTransform.loadTranslate(100, 100, 0); @@ -410,11 +412,11 @@ RENDERTHREAD_TEST(RenderNodeDrawable, projectionHwLayer) { layerUpdateQueue.enqueueLayerWithDamage(child.get(), android::uirenderer::Rect(LAYER_WIDTH, LAYER_HEIGHT)); SkiaPipeline::renderLayersImpl(layerUpdateQueue, true); - EXPECT_EQ(1, canvas->mIndex); //assert index 0 is drawn on the layer + EXPECT_EQ(1, drawCounter); //assert index 0 is drawn on the layer - RenderNodeDrawable drawable(parent.get(), canvas.get(), true); - canvas->drawDrawable(&drawable); - EXPECT_EQ(4, canvas->mIndex); + RenderNodeDrawable drawable(parent.get(), surfaceLayer1->getCanvas(), true); + surfaceLayer1->getCanvas()->drawDrawable(&drawable); + EXPECT_EQ(4, drawCounter); // clean up layer pointer, so we can safely destruct RenderNode child->setLayerSurface(nullptr); @@ -487,7 +489,7 @@ RENDERTHREAD_TEST(RenderNodeDrawable, projectionChildScroll) { info.observer = nullptr; parent->prepareTree(info); - sk_sp canvas(new ProjectionChildScrollTestCanvas()); + std::unique_ptr canvas(new ProjectionChildScrollTestCanvas()); RenderNodeDrawable drawable(parent.get(), canvas.get(), true); canvas->drawDrawable(&drawable); EXPECT_EQ(2, canvas->mIndex);