From ea1fe9b9d6ff9f0a543489979a0a909acc9ea564 Mon Sep 17 00:00:00 2001 From: Derek Sollenberger Date: Wed, 1 Mar 2017 13:02:43 -0500 Subject: [PATCH] Update SkiaDisplayList to use the modified SkLiteDL SkLiteDL is no longer refcounted or a subclass of SkDrawable. Test: on device testing in SkiaGL mode Change-Id: I9ad53d764a26ab382d80d54908325962f2075802 --- .../hwui/pipeline/skia/RenderNodeDrawable.cpp | 4 +-- libs/hwui/pipeline/skia/SkiaDisplayList.cpp | 12 +++------ libs/hwui/pipeline/skia/SkiaDisplayList.h | 26 ++++++++++++------- .../pipeline/skia/SkiaRecordingCanvas.cpp | 9 +++---- .../tests/unit/RenderNodeDrawableTests.cpp | 4 +-- libs/hwui/tests/unit/SkiaDisplayListTests.cpp | 25 +++++++----------- 6 files changed, 36 insertions(+), 44 deletions(-) diff --git a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp index 117395bfc75c7..39f11b8e6bfe9 100644 --- a/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp +++ b/libs/hwui/pipeline/skia/RenderNodeDrawable.cpp @@ -197,12 +197,12 @@ void RenderNodeDrawable::drawContent(SkCanvas* canvas) const { if (needsLayer) { canvas->saveLayer(bounds, &paint); } - canvas->drawDrawable(displayList->mDrawable.get()); + displayList->draw(canvas); if (needsLayer) { canvas->restore(); } } else { - canvas->drawDrawable(displayList->mDrawable.get()); + displayList->draw(canvas); } } } diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.cpp b/libs/hwui/pipeline/skia/SkiaDisplayList.cpp index b4babcb9564e4..496f7babd3cc7 100644 --- a/libs/hwui/pipeline/skia/SkiaDisplayList.cpp +++ b/libs/hwui/pipeline/skia/SkiaDisplayList.cpp @@ -27,10 +27,6 @@ namespace android { namespace uirenderer { namespace skiapipeline { -SkiaDisplayList::SkiaDisplayList(SkRect bounds) : mDrawable(SkLiteDL::New(bounds)) { - SkASSERT(projectionReceiveIndex == -1); -} - void SkiaDisplayList::syncContents() { for (auto& functor : mChildFunctors) { functor.syncFunctor(); @@ -41,7 +37,7 @@ void SkiaDisplayList::syncContents() { } bool SkiaDisplayList::reuseDisplayList(RenderNode* node, renderthread::CanvasContext* context) { - reset(SkRect::MakeEmpty()); + reset(); node->attachAvailableList(this); return true; } @@ -102,10 +98,10 @@ bool SkiaDisplayList::prepareListAndChildren(TreeObserver& observer, TreeInfo& i return isDirty; } -void SkiaDisplayList::reset(SkRect bounds) { +void SkiaDisplayList::reset() { mProjectionReceiver = nullptr; - mDrawable->reset(bounds); + mDisplayList.reset(); mMutableImages.clear(); mVectorDrawables.clear(); @@ -119,7 +115,7 @@ void SkiaDisplayList::reset(SkRect bounds) { void SkiaDisplayList::output(std::ostream& output, uint32_t level) { DumpOpsCanvas canvas(output, level, *this); - mDrawable->draw(&canvas, nullptr); + mDisplayList.draw(&canvas); } }; // namespace skiapipeline diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.h b/libs/hwui/pipeline/skia/SkiaDisplayList.h index 439b999380ac4..6ee5922f9cd65 100644 --- a/libs/hwui/pipeline/skia/SkiaDisplayList.h +++ b/libs/hwui/pipeline/skia/SkiaDisplayList.h @@ -22,7 +22,7 @@ #include #include -#include +#include namespace android { namespace uirenderer { @@ -39,22 +39,22 @@ namespace skiapipeline { */ class SkiaDisplayList : public DisplayList { public: - SkiaDisplayList(SkRect bounds); + SkiaDisplayList() { SkASSERT(projectionReceiveIndex == -1); } virtual ~SkiaDisplayList() { /* Given that we are using a LinearStdAllocator to store some of the * SkDrawable contents we must ensure that any other object that is * holding a reference to those drawables is destroyed prior to their * deletion. */ - mDrawable.reset(); + mDisplayList.reset(); } /** * This resets the DisplayList so that it behaves as if the object were newly - * constructed with the provided bounds. The reuse avoids any overhead - * associated with destroying the SkLiteDL as well as the deques and vectors. + * constructed. The reuse avoids any overhead associated with destroying + * the SkLiteDL as well as the deques and vectors. */ - void reset(SkRect bounds); + void reset(); /** * Use the linear allocator to create any SkDrawables needed by the display @@ -72,7 +72,7 @@ public: /** * Returns true if the DisplayList does not have any recorded content */ - bool isEmpty() const override { return mDrawable->empty(); } + bool isEmpty() const override { return mDisplayList.empty(); } /** * Returns true if this list directly contains a GLFunctor drawing command. @@ -126,18 +126,24 @@ public: */ inline bool containsProjectionReceiver() const { return mProjectionReceiver; } + void attachRecorder(SkLiteRecorder* recorder, const SkIRect& bounds) { + recorder->reset(&mDisplayList, bounds); + } + + void draw(SkCanvas* canvas) { mDisplayList.draw(canvas); } + void output(std::ostream& output, uint32_t level) override; /** * We use std::deque here because (1) we need to iterate through these - * elements and (2) mDrawable holds pointers to the elements, so they cannot - * relocate. + * elements and (2) mDisplayList holds pointers to the elements, so they + * cannot relocate. */ std::deque mChildNodes; std::deque mChildFunctors; std::vector mMutableImages; std::vector mVectorDrawables; - sk_sp mDrawable; + SkLiteDL mDisplayList; //mProjectionReceiver points to a child node (stored in mChildNodes) that is as a projection //receiver. It is set at record time and used at both prepare and draw tree traversals to diff --git a/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp b/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp index dbe0296eae24a..559d268b71f75 100644 --- a/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp +++ b/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp @@ -39,14 +39,11 @@ void SkiaRecordingCanvas::initDisplayList(uirenderer::RenderNode* renderNode, in if (renderNode) { mDisplayList = renderNode->detachAvailableList(); } - SkRect bounds = SkRect::MakeWH(width, height); - if (mDisplayList) { - mDisplayList->reset(bounds); - } else { - mDisplayList.reset(new SkiaDisplayList(bounds)); + if (!mDisplayList) { + mDisplayList.reset(new SkiaDisplayList()); } - mRecorder.reset(mDisplayList->mDrawable.get()); + mDisplayList->attachRecorder(&mRecorder, SkIRect::MakeWH(width, height)); SkiaCanvas::reset(&mRecorder); } diff --git a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp index f21b3f7ec334f..cbea501e3df35 100644 --- a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp +++ b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp @@ -44,9 +44,9 @@ TEST(RenderNodeDrawable, create) { canvas.drawColor(Color::Red_500, SkBlendMode::kSrcOver); }); - auto skLiteDL = SkLiteDL::New(SkRect::MakeWH(1, 1)); + SkLiteDL skLiteDL; SkLiteRecorder canvas; - canvas.reset(skLiteDL.get()); + canvas.reset(&skLiteDL, SkIRect::MakeWH(1, 1)); canvas.translate(100, 100); RenderNodeDrawable drawable(rootNode.get(), &canvas); diff --git a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp index be460bf6bdfe0..dd8f4b4ec8ccc 100644 --- a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp +++ b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp @@ -30,16 +30,13 @@ using namespace android::uirenderer::renderthread; using namespace android::uirenderer::skiapipeline; TEST(SkiaDisplayList, create) { - SkRect bounds = SkRect::MakeWH(200, 200); - SkiaDisplayList skiaDL(bounds); + SkiaDisplayList skiaDL; ASSERT_TRUE(skiaDL.isEmpty()); ASSERT_FALSE(skiaDL.mProjectionReceiver); - ASSERT_EQ(skiaDL.mDrawable->getBounds(), bounds); } TEST(SkiaDisplayList, reset) { - SkRect bounds = SkRect::MakeWH(200, 200); - SkiaDisplayList skiaDL(bounds); + SkiaDisplayList skiaDL; SkCanvas dummyCanvas; RenderNodeDrawable drawable(nullptr, &dummyCanvas); @@ -47,10 +44,9 @@ TEST(SkiaDisplayList, reset) { skiaDL.mChildFunctors.emplace_back(nullptr, nullptr, &dummyCanvas); skiaDL.mMutableImages.push_back(nullptr); skiaDL.mVectorDrawables.push_back(nullptr); - skiaDL.mDrawable->drawAnnotation(bounds, "testAnnotation", nullptr); + skiaDL.mDisplayList.drawAnnotation(SkRect::MakeWH(200, 200), "testAnnotation", nullptr); skiaDL.mProjectionReceiver = &drawable; - ASSERT_EQ(skiaDL.mDrawable->getBounds(), bounds); ASSERT_FALSE(skiaDL.mChildNodes.empty()); ASSERT_FALSE(skiaDL.mChildFunctors.empty()); ASSERT_FALSE(skiaDL.mMutableImages.empty()); @@ -58,10 +54,8 @@ TEST(SkiaDisplayList, reset) { ASSERT_FALSE(skiaDL.isEmpty()); ASSERT_TRUE(skiaDL.mProjectionReceiver); - bounds = SkRect::MakeWH(100, 100); - skiaDL.reset(bounds); + skiaDL.reset(); - ASSERT_EQ(skiaDL.mDrawable->getBounds(), bounds); ASSERT_TRUE(skiaDL.mChildNodes.empty()); ASSERT_TRUE(skiaDL.mChildFunctors.empty()); ASSERT_TRUE(skiaDL.mMutableImages.empty()); @@ -79,7 +73,7 @@ TEST(SkiaDisplayList, reuseDisplayList) { ASSERT_EQ(availableList.get(), nullptr); // attach a displayList for reuse - SkiaDisplayList skiaDL(SkRect::MakeWH(200, 200)); + SkiaDisplayList skiaDL; ASSERT_TRUE(skiaDL.reuseDisplayList(renderNode.get(), nullptr)); // detach the list that you just attempted to reuse @@ -93,13 +87,13 @@ TEST(SkiaDisplayList, reuseDisplayList) { } TEST(SkiaDisplayList, syncContexts) { - SkRect bounds = SkRect::MakeWH(200, 200); - SkiaDisplayList skiaDL(bounds); + SkiaDisplayList skiaDL; SkCanvas dummyCanvas; TestUtils::MockFunctor functor; skiaDL.mChildFunctors.emplace_back(&functor, nullptr, &dummyCanvas); + SkRect bounds = SkRect::MakeWH(200, 200); VectorDrawableRoot vectorDrawable(new VectorDrawable::Group()); vectorDrawable.mutateStagingProperties()->setBounds(bounds); skiaDL.mVectorDrawables.push_back(&vectorDrawable); @@ -127,7 +121,7 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaDisplayList, prepareListAndChildren) { DamageAccumulator damageAccumulator; info.damageAccumulator = &damageAccumulator; - SkiaDisplayList skiaDL(SkRect::MakeWH(200, 200)); + SkiaDisplayList skiaDL; // prepare with a clean VD VectorDrawableRoot cleanVD(new VectorDrawable::Group()); @@ -170,8 +164,7 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaDisplayList, prepareListAndChildren) { } TEST(SkiaDisplayList, updateChildren) { - SkRect bounds = SkRect::MakeWH(200, 200); - SkiaDisplayList skiaDL(bounds); + SkiaDisplayList skiaDL; sp renderNode = new RenderNode(); SkCanvas dummyCanvas;