Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3afd6377 authored by John Reck's avatar John Reck
Browse files

Switch how destroyHardwareResources works 

destroyHardwareResources will now only force-destroy
the specific node it was called on, which are only
ever the root nodes. Rely on onRemovedFromTree()
to clean up resources for all other nodes.

Bug: 34736819

Test: RenderNode.multiTreeValidity passes, manually
verified fixes b/34736819

Change-Id: I1c275ad6a98b63bf50f265602f09bffe3e1f169b
parent fda076a1

libs/hwui/RenderNode.cpp

+4 −12
Original line number Original line Diff line number Diff line
@@ -406,22 +406,14 @@ void RenderNode::deleteDisplayList(TreeObserver& observer, TreeInfo* info) { 
}

}





void RenderNode::destroyHardwareResources(TreeInfo* info) {

void RenderNode::destroyHardwareResources(TreeInfo* info) {

    ImmediateRemoved observer(info);

    destroyHardwareResourcesImpl(observer, info);

}



void RenderNode::destroyHardwareResourcesImpl(TreeObserver& observer, TreeInfo* info) {

    if (hasLayer()) {

    if (hasLayer()) {

        renderthread::CanvasContext::destroyLayer(this);

        renderthread::CanvasContext::destroyLayer(this);

    }

    }

    if (mDisplayList) {

        mDisplayList->updateChildren([&observer, info](RenderNode* child) {

            child->destroyHardwareResourcesImpl(observer, info);

        });

    setStagingDisplayList(nullptr);

    setStagingDisplayList(nullptr);



    ImmediateRemoved observer(info);

    deleteDisplayList(observer, info);

    deleteDisplayList(observer, info);

}

}

}





void RenderNode::destroyLayers() {

void RenderNode::destroyLayers() {

    if (hasLayer()) {

    if (hasLayer()) {

libs/hwui/RenderNode.h

+0 −1
Original line number Original line Diff line number Diff line
@@ -262,7 +262,6 @@ private: 
    void prepareLayer(TreeInfo& info, uint32_t dirtyMask);

    void prepareLayer(TreeInfo& info, uint32_t dirtyMask);

    void pushLayerUpdate(TreeInfo& info);

    void pushLayerUpdate(TreeInfo& info);

    void deleteDisplayList(TreeObserver& observer, TreeInfo* info = nullptr);

    void deleteDisplayList(TreeObserver& observer, TreeInfo* info = nullptr);

    void destroyHardwareResourcesImpl(TreeObserver& observer, TreeInfo* info = nullptr);

    void damageSelf(TreeInfo& info);

    void damageSelf(TreeInfo& info);





    void incParentRefCount() { mParentCount++; }

    void incParentRefCount() { mParentCount++; }

libs/hwui/tests/common/TestUtils.h

+4 −1
Original line number Original line Diff line number Diff line
@@ -357,7 +357,10 @@ private: 
    static void syncHierarchyPropertiesAndDisplayListImpl(RenderNode* node) {

    static void syncHierarchyPropertiesAndDisplayListImpl(RenderNode* node) {

        MarkAndSweepRemoved observer(nullptr);

        MarkAndSweepRemoved observer(nullptr);

        node->syncProperties();

        node->syncProperties();

        if (node->mNeedsDisplayListSync) {

            node->mNeedsDisplayListSync = false;

            node->syncDisplayList(observer, nullptr);

            node->syncDisplayList(observer, nullptr);

        }

        auto displayList = node->getDisplayList();

        auto displayList = node->getDisplayList();

        if (displayList) {

        if (displayList) {

            for (auto&& childOp : displayList->getChildren()) {

            for (auto&& childOp : displayList->getChildren()) {

libs/hwui/tests/unit/RenderNodeTests.cpp

+106 −0
Original line number Original line Diff line number Diff line
@@ -130,6 +130,112 @@ TEST(RenderNode, validity) { 
    EXPECT_TRUE(parent->nothingToDraw());

    EXPECT_TRUE(parent->nothingToDraw());

}

}





TEST(RenderNode, multiTreeValidity) {

    auto child = TestUtils::createNode(0, 0, 200, 400,

            [](RenderProperties& props, Canvas& canvas) {

        canvas.drawColor(Color::Red_500, SkBlendMode::kSrcOver);

    });

    auto parent1 = TestUtils::createNode(0, 0, 200, 400,

            [&child](RenderProperties& props, Canvas& canvas) {

        canvas.drawRenderNode(child.get());

    });

    auto parent2 = TestUtils::createNode(0, 0, 200, 400,

            [&child](RenderProperties& props, Canvas& canvas) {

        canvas.drawRenderNode(child.get());

    });



    EXPECT_TRUE(child->isValid());

    EXPECT_TRUE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_TRUE(child->nothingToDraw());

    EXPECT_TRUE(parent1->nothingToDraw());

    EXPECT_TRUE(parent2->nothingToDraw());



    TestUtils::syncHierarchyPropertiesAndDisplayList(parent1);



    EXPECT_TRUE(child->isValid());

    EXPECT_TRUE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_FALSE(child->nothingToDraw());

    EXPECT_FALSE(parent1->nothingToDraw());

    EXPECT_TRUE(parent2->nothingToDraw());



    TestUtils::syncHierarchyPropertiesAndDisplayList(parent2);



    EXPECT_TRUE(child->isValid());

    EXPECT_TRUE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_FALSE(child->nothingToDraw());

    EXPECT_FALSE(parent1->nothingToDraw());

    EXPECT_FALSE(parent2->nothingToDraw());



    TestUtils::recordNode(*parent1, [](Canvas& canvas) {

        canvas.drawColor(Color::Amber_500, SkBlendMode::kSrcOver);

    });



    TestUtils::syncHierarchyPropertiesAndDisplayList(parent1);



    EXPECT_TRUE(child->isValid());

    EXPECT_TRUE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_FALSE(child->nothingToDraw());

    EXPECT_FALSE(parent1->nothingToDraw());

    EXPECT_FALSE(parent2->nothingToDraw());



    TestUtils::recordNode(*parent2, [](Canvas& canvas) {

        canvas.drawColor(Color::Amber_500, SkBlendMode::kSrcOver);

    });



    TestUtils::syncHierarchyPropertiesAndDisplayList(parent2);



    EXPECT_FALSE(child->isValid());

    EXPECT_TRUE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_TRUE(child->nothingToDraw());

    EXPECT_FALSE(parent1->nothingToDraw());

    EXPECT_FALSE(parent2->nothingToDraw());



    TestUtils::recordNode(*child, [](Canvas& canvas) {

        canvas.drawColor(Color::Red_500, SkBlendMode::kSrcOver);

    });

    TestUtils::syncHierarchyPropertiesAndDisplayList(child);



    TestUtils::recordNode(*parent1, [&child](Canvas& canvas) {

        canvas.drawRenderNode(child.get());

    });

    TestUtils::syncHierarchyPropertiesAndDisplayList(parent1);



    TestUtils::recordNode(*parent2, [&child](Canvas& canvas) {

        canvas.drawRenderNode(child.get());

    });

    TestUtils::syncHierarchyPropertiesAndDisplayList(parent2);



    EXPECT_TRUE(child->isValid());

    EXPECT_TRUE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_FALSE(child->nothingToDraw());

    EXPECT_FALSE(parent1->nothingToDraw());

    EXPECT_FALSE(parent2->nothingToDraw());



    parent1->destroyHardwareResources();



    EXPECT_TRUE(child->isValid());

    EXPECT_FALSE(parent1->isValid());

    EXPECT_TRUE(parent2->isValid());

    EXPECT_FALSE(child->nothingToDraw());

    EXPECT_TRUE(parent1->nothingToDraw());

    EXPECT_FALSE(parent2->nothingToDraw());



    parent2->destroyHardwareResources();



    EXPECT_FALSE(child->isValid());

    EXPECT_FALSE(parent1->isValid());

    EXPECT_FALSE(parent2->isValid());

    EXPECT_TRUE(child->nothingToDraw());

    EXPECT_TRUE(parent1->nothingToDraw());

    EXPECT_TRUE(parent2->nothingToDraw());

}



TEST(RenderNode, releasedCallback) {

TEST(RenderNode, releasedCallback) {

    class DecRefOnReleased : public GlFunctorLifecycleListener {

    class DecRefOnReleased : public GlFunctorLifecycleListener {

    public:

    public: