Overhaul RenderNode's DisplayList management
* Move mValid to native
* Have destroyHardwareResources destroy everything
* Remove flaky mParentCount checks in setStaging
* All tree updates have an internal observer to
ensure onRemovedFromTree() is a reliable signal
* onRemovedFromTree() immediately releases resources
to avoid displaylist "leaks"
Test: Unit tests for validity added & pass, manually
verified that b/34072929 doesn't repro
Bug: 34072929
Change-Id: I856534b4ed1b7f009fc4b7cd13209b97fa42a71c
diff --git a/libs/hwui/DisplayList.cpp b/libs/hwui/DisplayList.cpp
index 5e4a7f7..3853356 100644
--- a/libs/hwui/DisplayList.cpp
+++ b/libs/hwui/DisplayList.cpp
@@ -104,15 +104,15 @@
}
}
-bool DisplayList::prepareListAndChildren(TreeInfo& info, bool functorsNeedLayer,
- std::function<void(RenderNode*, TreeInfo&, bool)> childFn) {
+bool DisplayList::prepareListAndChildren(TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
+ std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn) {
info.prepareTextures = info.canvasContext.pinImages(bitmapResources);
for (auto&& op : children) {
RenderNode* childNode = op->renderNode;
info.damageAccumulator->pushTransform(&op->localMatrix);
bool childFunctorsNeedLayer = functorsNeedLayer; // TODO! || op->mRecordedWithPotentialStencilClip;
- childFn(childNode, info, childFunctorsNeedLayer);
+ childFn(childNode, observer, info, childFunctorsNeedLayer);
info.damageAccumulator->popTransform();
}
diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h
index cab092f..ef0fd31 100644
--- a/libs/hwui/DisplayList.h
+++ b/libs/hwui/DisplayList.h
@@ -125,8 +125,8 @@
virtual void syncContents();
virtual void updateChildren(std::function<void(RenderNode*)> updateFn);
- virtual bool prepareListAndChildren(TreeInfo& info, bool functorsNeedLayer,
- std::function<void(RenderNode*, TreeInfo&, bool)> childFn);
+ virtual bool prepareListAndChildren(TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
+ std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn);
protected:
// allocator into which all ops and LsaVector arrays allocated
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index a5443d9..7d8f046 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -22,6 +22,7 @@
#include "OpDumper.h"
#include "RecordedOp.h"
#include "TreeInfo.h"
+#include "utils/FatVector.h"
#include "utils/MathUtils.h"
#include "utils/StringUtils.h"
#include "utils/TraceUtils.h"
@@ -39,6 +40,20 @@
namespace android {
namespace uirenderer {
+// Used for tree mutations that are purely destructive.
+// Generic tree mutations should use MarkAndSweepObserver instead
+class ImmediateRemoved : public TreeObserver {
+public:
+ explicit ImmediateRemoved(TreeInfo* info) : mTreeInfo(info) {}
+
+ void onMaybeRemovedFromTree(RenderNode* node) override {
+ node->onRemovedFromTree(mTreeInfo);
+ }
+
+private:
+ TreeInfo* mTreeInfo;
+};
+
RenderNode::RenderNode()
: mDirtyPropertyFields(0)
, mNeedsDisplayListSync(false)
@@ -49,20 +64,17 @@
}
RenderNode::~RenderNode() {
- deleteDisplayList(nullptr);
+ ImmediateRemoved observer(nullptr);
+ deleteDisplayList(observer);
delete mStagingDisplayList;
LOG_ALWAYS_FATAL_IF(hasLayer(), "layer missed detachment!");
}
-void RenderNode::setStagingDisplayList(DisplayList* displayList, TreeObserver* observer) {
+void RenderNode::setStagingDisplayList(DisplayList* displayList) {
+ mValid = (displayList != nullptr);
mNeedsDisplayListSync = true;
delete mStagingDisplayList;
mStagingDisplayList = displayList;
- // If mParentCount == 0 we are the sole reference to this RenderNode,
- // so immediately free the old display list
- if (!mParentCount && !mStagingDisplayList) {
- deleteDisplayList(observer);
- }
}
/**
@@ -187,12 +199,13 @@
void RenderNode::prepareTree(TreeInfo& info) {
ATRACE_CALL();
LOG_ALWAYS_FATAL_IF(!info.damageAccumulator, "DamageAccumulator missing");
+ MarkAndSweepRemoved observer(&info);
// The OpenGL renderer reserves the stencil buffer for overdraw debugging. Functors
// will need to be drawn in a layer.
bool functorsNeedLayer = Properties::debugOverdraw && !Properties::isSkiaEnabled();
- prepareTreeImpl(info, functorsNeedLayer);
+ prepareTreeImpl(observer, info, functorsNeedLayer);
}
void RenderNode::addAnimator(const sp<BaseRenderNodeAnimator>& animator) {
@@ -283,7 +296,7 @@
* While traversing down the tree, functorsNeedLayer flag is set to true if anything that uses the
* stencil buffer may be needed. Views that use a functor to draw will be forced onto a layer.
*/
-void RenderNode::prepareTreeImpl(TreeInfo& info, bool functorsNeedLayer) {
+void RenderNode::prepareTreeImpl(TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer) {
info.damageAccumulator->pushTransform(this);
if (info.mode == TreeInfo::MODE_FULL) {
@@ -309,14 +322,14 @@
prepareLayer(info, animatorDirtyMask);
if (info.mode == TreeInfo::MODE_FULL) {
- pushStagingDisplayListChanges(info);
+ pushStagingDisplayListChanges(observer, info);
}
if (mDisplayList) {
info.out.hasFunctors |= mDisplayList->hasFunctor();
- bool isDirty = mDisplayList->prepareListAndChildren(info, childFunctorsNeedLayer,
- [](RenderNode* child, TreeInfo& info, bool functorsNeedLayer) {
- child->prepareTreeImpl(info, functorsNeedLayer);
+ bool isDirty = mDisplayList->prepareListAndChildren(observer, info, childFunctorsNeedLayer,
+ [](RenderNode* child, TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer) {
+ child->prepareTreeImpl(observer, info, functorsNeedLayer);
});
if (isDirty) {
damageSelf(info);
@@ -353,7 +366,7 @@
}
}
-void RenderNode::syncDisplayList(TreeInfo* info) {
+void RenderNode::syncDisplayList(TreeObserver& observer, TreeInfo* info) {
// Make sure we inc first so that we don't fluctuate between 0 and 1,
// which would thrash the layer cache
if (mStagingDisplayList) {
@@ -361,7 +374,7 @@
child->incParentRefCount();
});
}
- deleteDisplayList(info ? info->observer : nullptr, info);
+ deleteDisplayList(observer, info);
mDisplayList = mStagingDisplayList;
mStagingDisplayList = nullptr;
if (mDisplayList) {
@@ -369,20 +382,20 @@
}
}
-void RenderNode::pushStagingDisplayListChanges(TreeInfo& info) {
+void RenderNode::pushStagingDisplayListChanges(TreeObserver& observer, TreeInfo& info) {
if (mNeedsDisplayListSync) {
mNeedsDisplayListSync = false;
// Damage with the old display list first then the new one to catch any
// changes in isRenderable or, in the future, bounds
damageSelf(info);
- syncDisplayList(&info);
+ syncDisplayList(observer, &info);
damageSelf(info);
}
}
-void RenderNode::deleteDisplayList(TreeObserver* observer, TreeInfo* info) {
+void RenderNode::deleteDisplayList(TreeObserver& observer, TreeInfo* info) {
if (mDisplayList) {
- mDisplayList->updateChildren([observer, info](RenderNode* child) {
+ mDisplayList->updateChildren([&observer, info](RenderNode* child) {
child->decParentRefCount(observer, info);
});
if (!mDisplayList->reuseDisplayList(this, info ? &info->canvasContext : nullptr)) {
@@ -392,38 +405,53 @@
mDisplayList = nullptr;
}
-void RenderNode::destroyHardwareResources(TreeObserver* observer, TreeInfo* info) {
+void RenderNode::destroyHardwareResources(TreeInfo* info) {
+ ImmediateRemoved observer(info);
+ destroyHardwareResourcesImpl(observer, info);
+}
+
+void RenderNode::destroyHardwareResourcesImpl(TreeObserver& observer, TreeInfo* info) {
if (hasLayer()) {
renderthread::CanvasContext::destroyLayer(this);
}
if (mDisplayList) {
- mDisplayList->updateChildren([observer, info](RenderNode* child) {
- child->destroyHardwareResources(observer, info);
+ mDisplayList->updateChildren([&observer, info](RenderNode* child) {
+ child->destroyHardwareResourcesImpl(observer, info);
});
- if (mNeedsDisplayListSync) {
- // Next prepare tree we are going to push a new display list, so we can
- // drop our current one now
- deleteDisplayList(observer, info);
+ setStagingDisplayList(nullptr);
+ deleteDisplayList(observer, info);
+ }
+}
+
+void RenderNode::destroyLayers() {
+ if (hasLayer()) {
+ renderthread::CanvasContext::destroyLayer(this);
+ }
+ if (mDisplayList) {
+ mDisplayList->updateChildren([](RenderNode* child) {
+ child->destroyLayers();
+ });
+ }
+}
+
+void RenderNode::decParentRefCount(TreeObserver& observer, TreeInfo* info) {
+ LOG_ALWAYS_FATAL_IF(!mParentCount, "already 0!");
+ mParentCount--;
+ if (!mParentCount) {
+ observer.onMaybeRemovedFromTree(this);
+ if (CC_UNLIKELY(mPositionListener.get())) {
+ mPositionListener->onPositionLost(*this, info);
}
}
}
-void RenderNode::decParentRefCount(TreeObserver* observer, TreeInfo* info) {
- LOG_ALWAYS_FATAL_IF(!mParentCount, "already 0!");
- mParentCount--;
- if (!mParentCount) {
- if (observer) {
- observer->onMaybeRemovedFromTree(this);
- }
- if (CC_UNLIKELY(mPositionListener.get())) {
- mPositionListener->onPositionLost(*this, info);
- }
- // If a child of ours is being attached to our parent then this will incorrectly
- // destroy its hardware resources. However, this situation is highly unlikely
- // and the failure is "just" that the layer is re-created, so this should
- // be safe enough
- destroyHardwareResources(observer, info);
- }
+void RenderNode::onRemovedFromTree(TreeInfo* info) {
+ destroyHardwareResources(info);
+}
+
+void RenderNode::clearRoot() {
+ ImmediateRemoved observer(nullptr);
+ decParentRefCount(observer);
}
/**
diff --git a/libs/hwui/RenderNode.h b/libs/hwui/RenderNode.h
index b8964f0..14a664c 100644
--- a/libs/hwui/RenderNode.h
+++ b/libs/hwui/RenderNode.h
@@ -34,6 +34,7 @@
#include "RenderProperties.h"
#include "pipeline/skia/SkiaDisplayList.h"
#include "pipeline/skia/SkiaLayer.h"
+#include "utils/FatVector.h"
#include <vector>
@@ -101,7 +102,7 @@
kReplayFlag_ClipChildren = 0x1
};
- ANDROID_API void setStagingDisplayList(DisplayList* newData, TreeObserver* observer);
+ ANDROID_API void setStagingDisplayList(DisplayList* newData);
void computeOrdering();
@@ -164,6 +165,10 @@
return mStagingProperties;
}
+ bool isValid() {
+ return mValid;
+ }
+
int getWidth() const {
return properties().getWidth();
}
@@ -173,7 +178,8 @@
}
ANDROID_API virtual void prepareTree(TreeInfo& info);
- void destroyHardwareResources(TreeObserver* observer, TreeInfo* info = nullptr);
+ void destroyHardwareResources(TreeInfo* info = nullptr);
+ void destroyLayers();
// UI thread only!
ANDROID_API void addAnimator(const sp<BaseRenderNodeAnimator>& animator);
@@ -232,24 +238,35 @@
return mParentCount;
}
+ void onRemovedFromTree(TreeInfo* info);
+
+ // Called by CanvasContext to promote a RenderNode to be a root node
+ void makeRoot() {
+ incParentRefCount();
+ }
+
+ // Called by CanvasContext when it drops a RenderNode from being a root node
+ void clearRoot();
+
private:
void computeOrderingImpl(RenderNodeOp* opState,
std::vector<RenderNodeOp*>* compositedChildrenOfProjectionSurface,
const mat4* transformFromProjectionSurface);
void syncProperties();
- void syncDisplayList(TreeInfo* info);
+ void syncDisplayList(TreeObserver& observer, TreeInfo* info);
- void prepareTreeImpl(TreeInfo& info, bool functorsNeedLayer);
+ void prepareTreeImpl(TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer);
void pushStagingPropertiesChanges(TreeInfo& info);
- void pushStagingDisplayListChanges(TreeInfo& info);
+ void pushStagingDisplayListChanges(TreeObserver& observer, TreeInfo& info);
void prepareLayer(TreeInfo& info, uint32_t dirtyMask);
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 incParentRefCount() { mParentCount++; }
- void decParentRefCount(TreeObserver* observer, TreeInfo* info = nullptr);
+ void decParentRefCount(TreeObserver& observer, TreeInfo* info = nullptr);
void output(std::ostream& output, uint32_t level);
String8 mName;
@@ -259,6 +276,10 @@
RenderProperties mProperties;
RenderProperties mStagingProperties;
+ // Owned by UI. Set when DL is set, cleared when DL cleared or when node detached
+ // (likely by parent re-record/removal)
+ bool mValid = false;
+
bool mNeedsDisplayListSync;
// WARNING: Do not delete this directly, you must go through deleteDisplayList()!
DisplayList* mDisplayList;
@@ -361,5 +382,28 @@
std::unique_ptr<skiapipeline::SkiaLayer> mSkiaLayer;
}; // class RenderNode
+class MarkAndSweepRemoved : public TreeObserver {
+PREVENT_COPY_AND_ASSIGN(MarkAndSweepRemoved);
+
+public:
+ explicit MarkAndSweepRemoved(TreeInfo* info) : mTreeInfo(info) {}
+
+ void onMaybeRemovedFromTree(RenderNode* node) override {
+ mMarked.emplace_back(node);
+ }
+
+ ~MarkAndSweepRemoved() {
+ for (auto& node : mMarked) {
+ if (!node->hasParents()) {
+ node->onRemovedFromTree(mTreeInfo);
+ }
+ }
+ }
+
+private:
+ FatVector<sp<RenderNode>, 10> mMarked;
+ TreeInfo* mTreeInfo;
+};
+
} /* namespace uirenderer */
} /* namespace android */
diff --git a/libs/hwui/TreeInfo.h b/libs/hwui/TreeInfo.h
index 749efdd..c6fbe2bd5 100644
--- a/libs/hwui/TreeInfo.h
+++ b/libs/hwui/TreeInfo.h
@@ -47,9 +47,9 @@
// Due to the unordered nature of tree pushes, once prepareTree
// is finished it is possible that the node was "resurrected" and has
// a non-zero parent count.
- virtual void onMaybeRemovedFromTree(RenderNode* node) {}
+ virtual void onMaybeRemovedFromTree(RenderNode* node) = 0;
protected:
- ~TreeObserver() {}
+ virtual ~TreeObserver() {}
};
// This would be a struct, but we want to PREVENT_COPY_AND_ASSIGN
@@ -91,10 +91,6 @@
LayerUpdateQueue* layerUpdateQueue = nullptr;
ErrorHandler* errorHandler = nullptr;
- // Optional, may be nullptr. Used to allow things to observe interesting
- // tree state changes
- TreeObserver* observer = nullptr;
-
int32_t windowInsetLeft = 0;
int32_t windowInsetTop = 0;
bool updateWindowPositions = false;
diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.cpp b/libs/hwui/pipeline/skia/SkiaDisplayList.cpp
index 9db8cd3..36d02ecb 100644
--- a/libs/hwui/pipeline/skia/SkiaDisplayList.cpp
+++ b/libs/hwui/pipeline/skia/SkiaDisplayList.cpp
@@ -51,8 +51,9 @@
}
}
-bool SkiaDisplayList::prepareListAndChildren(TreeInfo& info, bool functorsNeedLayer,
- std::function<void(RenderNode*, TreeInfo&, bool)> childFn) {
+bool SkiaDisplayList::prepareListAndChildren(TreeObserver& observer, TreeInfo& info,
+ bool functorsNeedLayer,
+ std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn) {
// If the prepare tree is triggered by the UI thread and no previous call to
// pinImages has failed then we must pin all mutable images in the GPU cache
// until the next UI thread draw.
@@ -74,7 +75,7 @@
info.damageAccumulator->pushTransform(&mat4);
// TODO: a layer is needed if the canvas is rotated or has a non-rect clip
info.hasBackwardProjectedNodes = false;
- childFn(childNode, info, functorsNeedLayer);
+ childFn(childNode, observer, info, functorsNeedLayer);
hasBackwardProjectedNodesSubtree |= info.hasBackwardProjectedNodes;
info.damageAccumulator->popTransform();
}
diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.h b/libs/hwui/pipeline/skia/SkiaDisplayList.h
index ff86fd1..2a01330 100644
--- a/libs/hwui/pipeline/skia/SkiaDisplayList.h
+++ b/libs/hwui/pipeline/skia/SkiaDisplayList.h
@@ -113,8 +113,8 @@
* to subclass from DisplayList
*/
- bool prepareListAndChildren(TreeInfo& info, bool functorsNeedLayer,
- std::function<void(RenderNode*, TreeInfo&, bool)> childFn) override;
+ bool prepareListAndChildren(TreeObserver& observer, TreeInfo& info, bool functorsNeedLayer,
+ std::function<void(RenderNode*, TreeObserver&, TreeInfo&, bool)> childFn) override;
/**
* Calls the provided function once for each child of this DisplayList
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 1b3bf96..a53e5e0 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -146,21 +146,38 @@
, mProfiler(mFrames)
, mContentDrawBounds(0, 0, 0, 0)
, mRenderPipeline(std::move(renderPipeline)) {
+ rootRenderNode->makeRoot();
mRenderNodes.emplace_back(rootRenderNode);
mRenderThread.renderState().registerCanvasContext(this);
mProfiler.setDensity(mRenderThread.mainDisplayInfo().density);
}
CanvasContext::~CanvasContext() {
- destroy(nullptr);
+ destroy();
mRenderThread.renderState().unregisterCanvasContext(this);
+ for (auto& node : mRenderNodes) {
+ node->clearRoot();
+ }
+ mRenderNodes.clear();
}
-void CanvasContext::destroy(TreeObserver* observer) {
+void CanvasContext::addRenderNode(RenderNode* node, bool placeFront) {
+ int pos = placeFront ? 0 : static_cast<int>(mRenderNodes.size());
+ node->makeRoot();
+ mRenderNodes.emplace(mRenderNodes.begin() + pos, node);
+}
+
+void CanvasContext::removeRenderNode(RenderNode* node) {
+ node->clearRoot();
+ mRenderNodes.erase(std::remove(mRenderNodes.begin(), mRenderNodes.end(), node),
+ mRenderNodes.end());
+}
+
+void CanvasContext::destroy() {
stopDrawing();
setSurface(nullptr);
- freePrefetchedLayers(observer);
- destroyHardwareResources(observer);
+ freePrefetchedLayers();
+ destroyHardwareResources();
mAnimationContext->destroy();
}
@@ -320,7 +337,7 @@
mAnimationContext->runRemainingAnimations(info);
GL_CHECKPOINT(MODERATE);
- freePrefetchedLayers(info.observer);
+ freePrefetchedLayers();
GL_CHECKPOINT(MODERATE);
mIsDirty = true;
@@ -504,19 +521,19 @@
}
}
-void CanvasContext::freePrefetchedLayers(TreeObserver* observer) {
+void CanvasContext::freePrefetchedLayers() {
if (mPrefetchedLayers.size()) {
for (auto& node : mPrefetchedLayers) {
ALOGW("Incorrectly called buildLayer on View: %s, destroying layer...",
node->getName());
- node->destroyHardwareResources(observer);
- node->decStrong(observer);
+ node->destroyLayers();
+ node->decStrong(nullptr);
}
mPrefetchedLayers.clear();
}
}
-void CanvasContext::buildLayer(RenderNode* node, TreeObserver* observer) {
+void CanvasContext::buildLayer(RenderNode* node) {
ATRACE_CALL();
if (!mRenderPipeline->isContextReady()) return;
@@ -525,7 +542,6 @@
TreeInfo info(TreeInfo::MODE_FULL, *this);
info.damageAccumulator = &mDamageAccumulator;
- info.observer = observer;
info.layerUpdateQueue = &mLayerUpdateQueue;
info.runAnimations = false;
node->prepareTree(info);
@@ -545,12 +561,12 @@
return mRenderPipeline->copyLayerInto(layer, bitmap);
}
-void CanvasContext::destroyHardwareResources(TreeObserver* observer) {
+void CanvasContext::destroyHardwareResources() {
stopDrawing();
if (mRenderPipeline->isContextReady()) {
- freePrefetchedLayers(observer);
+ freePrefetchedLayers();
for (const sp<RenderNode>& node : mRenderNodes) {
- node->destroyHardwareResources(observer);
+ node->destroyHardwareResources();
}
mRenderPipeline->onDestroyHardwareResources();
}
diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index 0174b86..aa01caa 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -132,17 +132,17 @@
void prepareTree(TreeInfo& info, int64_t* uiFrameInfo,
int64_t syncQueued, RenderNode* target);
void draw();
- void destroy(TreeObserver* observer);
+ void destroy();
// IFrameCallback, Choreographer-driven frame callback entry point
virtual void doFrame() override;
void prepareAndDraw(RenderNode* node);
- void buildLayer(RenderNode* node, TreeObserver* observer);
+ void buildLayer(RenderNode* node);
bool copyLayerInto(DeferredLayerUpdater* layer, SkBitmap* bitmap);
void markLayerInUse(RenderNode* node);
- void destroyHardwareResources(TreeObserver* observer);
+ void destroyHardwareResources();
static void trimMemory(RenderThread& thread, int level);
DeferredLayerUpdater* createTextureLayer();
@@ -160,15 +160,8 @@
void serializeDisplayListTree();
- void addRenderNode(RenderNode* node, bool placeFront) {
- int pos = placeFront ? 0 : static_cast<int>(mRenderNodes.size());
- mRenderNodes.emplace(mRenderNodes.begin() + pos, node);
- }
-
- void removeRenderNode(RenderNode* node) {
- mRenderNodes.erase(std::remove(mRenderNodes.begin(), mRenderNodes.end(), node),
- mRenderNodes.end());
- }
+ void addRenderNode(RenderNode* node, bool placeFront);
+ void removeRenderNode(RenderNode* node);
void setContentDrawBounds(int left, int top, int right, int bottom) {
mContentDrawBounds.set(left, top, right, bottom);
@@ -213,7 +206,7 @@
void setSurface(Surface* window);
- void freePrefetchedLayers(TreeObserver* observer);
+ void freePrefetchedLayers();
bool isSwapChainStuffed();
diff --git a/libs/hwui/renderthread/DrawFrameTask.cpp b/libs/hwui/renderthread/DrawFrameTask.cpp
index 4ff54a5..7d641d3 100644
--- a/libs/hwui/renderthread/DrawFrameTask.cpp
+++ b/libs/hwui/renderthread/DrawFrameTask.cpp
@@ -65,12 +65,11 @@
}
}
-int DrawFrameTask::drawFrame(TreeObserver* observer) {
+int DrawFrameTask::drawFrame() {
LOG_ALWAYS_FATAL_IF(!mContext, "Cannot drawFrame with no CanvasContext!");
mSyncResult = SyncResult::OK;
mSyncQueued = systemTime(CLOCK_MONOTONIC);
- mObserver = observer;
postAndWait();
return mSyncResult;
@@ -89,7 +88,6 @@
bool canDrawThisFrame;
{
TreeInfo info(TreeInfo::MODE_FULL, *mContext);
- info.observer = mObserver;
canUnblockUiThread = syncFrameState(info);
canDrawThisFrame = info.out.canDrawThisFrame;
}
diff --git a/libs/hwui/renderthread/DrawFrameTask.h b/libs/hwui/renderthread/DrawFrameTask.h
index c02d376..fb48062 100644
--- a/libs/hwui/renderthread/DrawFrameTask.h
+++ b/libs/hwui/renderthread/DrawFrameTask.h
@@ -65,7 +65,7 @@
void pushLayerUpdate(DeferredLayerUpdater* layer);
void removeLayerUpdate(DeferredLayerUpdater* layer);
- int drawFrame(TreeObserver* observer);
+ int drawFrame();
int64_t* frameInfo() { return mFrameInfo; }
@@ -90,7 +90,6 @@
int mSyncResult;
int64_t mSyncQueued;
- TreeObserver* mObserver;
int64_t mFrameInfo[UI_THREAD_FRAME_INFO_SIZE];
};
diff --git a/libs/hwui/renderthread/RenderProxy.cpp b/libs/hwui/renderthread/RenderProxy.cpp
index 022e871..fb79272 100644
--- a/libs/hwui/renderthread/RenderProxy.cpp
+++ b/libs/hwui/renderthread/RenderProxy.cpp
@@ -230,19 +230,18 @@
return mDrawFrameTask.frameInfo();
}
-int RenderProxy::syncAndDrawFrame(TreeObserver* observer) {
- return mDrawFrameTask.drawFrame(observer);
+int RenderProxy::syncAndDrawFrame() {
+ return mDrawFrameTask.drawFrame();
}
-CREATE_BRIDGE2(destroy, CanvasContext* context, TreeObserver* observer) {
- args->context->destroy(args->observer);
+CREATE_BRIDGE1(destroy, CanvasContext* context) {
+ args->context->destroy();
return nullptr;
}
-void RenderProxy::destroy(TreeObserver* observer) {
+void RenderProxy::destroy() {
SETUP_TASK(destroy);
args->context = mContext;
- args->observer = observer;
// destroyCanvasAndSurface() needs a fence as when it returns the
// underlying BufferQueue is going to be released from under
// the render thread.
@@ -282,16 +281,15 @@
return layer;
}
-CREATE_BRIDGE3(buildLayer, CanvasContext* context, RenderNode* node, TreeObserver* observer) {
- args->context->buildLayer(args->node, args->observer);
+CREATE_BRIDGE2(buildLayer, CanvasContext* context, RenderNode* node) {
+ args->context->buildLayer(args->node);
return nullptr;
}
-void RenderProxy::buildLayer(RenderNode* node, TreeObserver* observer) {
+void RenderProxy::buildLayer(RenderNode* node) {
SETUP_TASK(buildLayer);
args->context = mContext;
args->node = node;
- args->observer = observer;
postAndWait(task);
}
@@ -328,15 +326,14 @@
postAndWait(task);
}
-CREATE_BRIDGE2(destroyHardwareResources, CanvasContext* context, TreeObserver* observer) {
- args->context->destroyHardwareResources(args->observer);
+CREATE_BRIDGE1(destroyHardwareResources, CanvasContext* context) {
+ args->context->destroyHardwareResources();
return nullptr;
}
-void RenderProxy::destroyHardwareResources(TreeObserver* observer) {
+void RenderProxy::destroyHardwareResources() {
SETUP_TASK(destroyHardwareResources);
args->context = mContext;
- args->observer = observer;
postAndWait(task);
}
diff --git a/libs/hwui/renderthread/RenderProxy.h b/libs/hwui/renderthread/RenderProxy.h
index 44a5a14..1629090 100644
--- a/libs/hwui/renderthread/RenderProxy.h
+++ b/libs/hwui/renderthread/RenderProxy.h
@@ -44,7 +44,6 @@
class DisplayList;
class Layer;
class Rect;
-class TreeObserver;
namespace renderthread {
@@ -87,19 +86,19 @@
ANDROID_API void setLightCenter(const Vector3& lightCenter);
ANDROID_API void setOpaque(bool opaque);
ANDROID_API int64_t* frameInfo();
- ANDROID_API int syncAndDrawFrame(TreeObserver* observer);
- ANDROID_API void destroy(TreeObserver* observer);
+ ANDROID_API int syncAndDrawFrame();
+ ANDROID_API void destroy();
ANDROID_API static void invokeFunctor(Functor* functor, bool waitForCompletion);
ANDROID_API DeferredLayerUpdater* createTextureLayer();
- ANDROID_API void buildLayer(RenderNode* node, TreeObserver* observer);
+ ANDROID_API void buildLayer(RenderNode* node);
ANDROID_API bool copyLayerInto(DeferredLayerUpdater* layer, SkBitmap& bitmap);
ANDROID_API void pushLayerUpdate(DeferredLayerUpdater* layer);
ANDROID_API void cancelLayerUpdate(DeferredLayerUpdater* layer);
ANDROID_API void detachSurfaceTexture(DeferredLayerUpdater* layer);
- ANDROID_API void destroyHardwareResources(TreeObserver* observer);
+ ANDROID_API void destroyHardwareResources();
ANDROID_API static void trimMemory(int level);
ANDROID_API static void overrideProperty(const char* name, const char* value);
diff --git a/libs/hwui/tests/common/TestListViewSceneBase.cpp b/libs/hwui/tests/common/TestListViewSceneBase.cpp
index 6d2e8599..38c4848 100644
--- a/libs/hwui/tests/common/TestListViewSceneBase.cpp
+++ b/libs/hwui/tests/common/TestListViewSceneBase.cpp
@@ -69,7 +69,7 @@
// draw it to parent DisplayList
canvas->drawRenderNode(mListItems[ci].get());
}
- mListView->setStagingDisplayList(canvas->finishRecording(), nullptr);
+ mListView->setStagingDisplayList(canvas->finishRecording());
}
} // namespace test
diff --git a/libs/hwui/tests/common/TestUtils.h b/libs/hwui/tests/common/TestUtils.h
index 8b287de..16bc6f7 100644
--- a/libs/hwui/tests/common/TestUtils.h
+++ b/libs/hwui/tests/common/TestUtils.h
@@ -156,6 +156,11 @@
int* mSignal;
};
+ class MockTreeObserver : public TreeObserver {
+ public:
+ virtual void onMaybeRemovedFromTree(RenderNode* node) {}
+ };
+
static bool matricesAreApproxEqual(const Matrix4& a, const Matrix4& b) {
for (int i = 0; i < 16; i++) {
if (!MathUtils::areEqual(a[i], b[i])) {
@@ -215,7 +220,7 @@
std::unique_ptr<Canvas> canvas(Canvas::create_recording_canvas(props.getWidth(),
props.getHeight()));
setup(props, *canvas.get());
- node->setStagingDisplayList(canvas->finishRecording(), nullptr);
+ node->setStagingDisplayList(canvas->finishRecording());
}
node->setPropertyFieldsDirty(0xFFFFFFFF);
return node;
@@ -236,7 +241,7 @@
if (setup) {
RecordingCanvasType canvas(props.getWidth(), props.getHeight());
setup(props, canvas);
- node->setStagingDisplayList(canvas.finishRecording(), nullptr);
+ node->setStagingDisplayList(canvas.finishRecording());
}
node->setPropertyFieldsDirty(0xFFFFFFFF);
return node;
@@ -247,7 +252,7 @@
std::unique_ptr<Canvas> canvas(Canvas::create_recording_canvas(
node.stagingProperties().getWidth(), node.stagingProperties().getHeight()));
contentCallback(*canvas.get());
- node.setStagingDisplayList(canvas->finishRecording(), nullptr);
+ node.setStagingDisplayList(canvas->finishRecording());
}
static sp<RenderNode> createSkiaNode(int left, int top, int right, int bottom,
@@ -265,14 +270,14 @@
RenderProperties& props = node->mutateStagingProperties();
props.setLeftTopRightBottom(left, top, right, bottom);
if (displayList) {
- node->setStagingDisplayList(displayList, nullptr);
+ node->setStagingDisplayList(displayList);
}
if (setup) {
std::unique_ptr<skiapipeline::SkiaRecordingCanvas> canvas(
new skiapipeline::SkiaRecordingCanvas(nullptr,
props.getWidth(), props.getHeight()));
setup(props, *canvas.get());
- node->setStagingDisplayList(canvas->finishRecording(), nullptr);
+ node->setStagingDisplayList(canvas->finishRecording());
}
node->setPropertyFieldsDirty(0xFFFFFFFF);
TestUtils::syncHierarchyPropertiesAndDisplayList(node);
@@ -350,8 +355,9 @@
private:
static void syncHierarchyPropertiesAndDisplayListImpl(RenderNode* node) {
+ MarkAndSweepRemoved observer(nullptr);
node->syncProperties();
- node->syncDisplayList(nullptr);
+ node->syncDisplayList(observer, nullptr);
auto displayList = node->getDisplayList();
if (displayList) {
for (auto&& childOp : displayList->getChildren()) {
diff --git a/libs/hwui/tests/common/scenes/GlyphStressAnimation.cpp b/libs/hwui/tests/common/scenes/GlyphStressAnimation.cpp
index c0d9450..5b685bb 100644
--- a/libs/hwui/tests/common/scenes/GlyphStressAnimation.cpp
+++ b/libs/hwui/tests/common/scenes/GlyphStressAnimation.cpp
@@ -60,6 +60,6 @@
0, 100 * (i + 2), minikin::kBidi_Force_LTR, paint, nullptr);
}
- container->setStagingDisplayList(canvas->finishRecording(), nullptr);
+ container->setStagingDisplayList(canvas->finishRecording());
}
};
diff --git a/libs/hwui/tests/macrobench/TestSceneRunner.cpp b/libs/hwui/tests/macrobench/TestSceneRunner.cpp
index 396e896..f8d6397 100644
--- a/libs/hwui/tests/macrobench/TestSceneRunner.cpp
+++ b/libs/hwui/tests/macrobench/TestSceneRunner.cpp
@@ -151,7 +151,7 @@
testContext.waitForVsync();
nsecs_t vsync = systemTime(CLOCK_MONOTONIC);
UiFrameInfoBuilder(proxy->frameInfo()).setVsync(vsync, vsync);
- proxy->syncAndDrawFrame(nullptr);
+ proxy->syncAndDrawFrame();
}
proxy->resetProfileInfo();
@@ -167,7 +167,7 @@
ATRACE_NAME("UI-Draw Frame");
UiFrameInfoBuilder(proxy->frameInfo()).setVsync(vsync, vsync);
scene->doFrame(i);
- proxy->syncAndDrawFrame(nullptr);
+ proxy->syncAndDrawFrame();
}
if (opts.reportFrametimeWeight) {
proxy->fence();
diff --git a/libs/hwui/tests/unit/CanvasContextTests.cpp b/libs/hwui/tests/unit/CanvasContextTests.cpp
index 42ba3db..ef5ce0d 100644
--- a/libs/hwui/tests/unit/CanvasContextTests.cpp
+++ b/libs/hwui/tests/unit/CanvasContextTests.cpp
@@ -40,7 +40,7 @@
ASSERT_FALSE(canvasContext->hasSurface());
- canvasContext->destroy(nullptr);
+ canvasContext->destroy();
}
RENDERTHREAD_TEST(CanvasContext, invokeFunctor) {
diff --git a/libs/hwui/tests/unit/FrameBuilderTests.cpp b/libs/hwui/tests/unit/FrameBuilderTests.cpp
index 5a2791c..a391d1e 100644
--- a/libs/hwui/tests/unit/FrameBuilderTests.cpp
+++ b/libs/hwui/tests/unit/FrameBuilderTests.cpp
@@ -322,10 +322,14 @@
}
for (auto& node : nodes) {
+ EXPECT_TRUE(node->isValid());
EXPECT_FALSE(node->nothingToDraw());
- node->setStagingDisplayList(nullptr, nullptr);
- node->destroyHardwareResources(nullptr);
+ node->setStagingDisplayList(nullptr);
+ EXPECT_FALSE(node->isValid());
+ EXPECT_FALSE(node->nothingToDraw());
+ node->destroyHardwareResources();
EXPECT_TRUE(node->nothingToDraw());
+ EXPECT_FALSE(node->isValid());
}
{
diff --git a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
index 2f1eae3..f21b3f7 100644
--- a/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeDrawableTests.cpp
@@ -321,7 +321,6 @@
TreeInfo info(TreeInfo::MODE_RT_ONLY, *canvasContext.get());
DamageAccumulator damageAccumulator;
info.damageAccumulator = &damageAccumulator;
- info.observer = nullptr;
parent->prepareTree(info);
//parent(A) -> (receiverBackground, child)
@@ -437,7 +436,6 @@
TreeInfo info(TreeInfo::MODE_RT_ONLY, *canvasContext.get());
DamageAccumulator damageAccumulator;
info.damageAccumulator = &damageAccumulator;
- info.observer = nullptr;
parent->prepareTree(info);
int drawCounter = 0;
@@ -527,7 +525,6 @@
TreeInfo info(TreeInfo::MODE_RT_ONLY, *canvasContext.get());
DamageAccumulator damageAccumulator;
info.damageAccumulator = &damageAccumulator;
- info.observer = nullptr;
parent->prepareTree(info);
std::unique_ptr<ProjectionChildScrollTestCanvas> canvas(new ProjectionChildScrollTestCanvas());
@@ -545,7 +542,6 @@
TreeInfo info(TreeInfo::MODE_RT_ONLY, *canvasContext.get());
DamageAccumulator damageAccumulator;
info.damageAccumulator = &damageAccumulator;
- info.observer = nullptr;
renderNode->prepareTree(info);
//create a canvas not backed by any device/pixels, but with dimensions to avoid quick rejection
diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp
index ab8e4e1..8af4bbe 100644
--- a/libs/hwui/tests/unit/RenderNodeTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeTests.cpp
@@ -66,6 +66,70 @@
EXPECT_FALSE(parent->hasParents()) << "Root node shouldn't have any parents";
}
+TEST(RenderNode, validity) {
+ auto child = TestUtils::createNode(0, 0, 200, 400,
+ [](RenderProperties& props, Canvas& canvas) {
+ canvas.drawColor(Color::Red_500, SkBlendMode::kSrcOver);
+ });
+ auto parent = TestUtils::createNode(0, 0, 200, 400,
+ [&child](RenderProperties& props, Canvas& canvas) {
+ canvas.drawRenderNode(child.get());
+ });
+
+ EXPECT_TRUE(child->isValid());
+ EXPECT_TRUE(parent->isValid());
+ EXPECT_TRUE(child->nothingToDraw());
+ EXPECT_TRUE(parent->nothingToDraw());
+
+ TestUtils::syncHierarchyPropertiesAndDisplayList(parent);
+
+ EXPECT_TRUE(child->isValid());
+ EXPECT_TRUE(parent->isValid());
+ EXPECT_FALSE(child->nothingToDraw());
+ EXPECT_FALSE(parent->nothingToDraw());
+
+ TestUtils::recordNode(*parent, [](Canvas& canvas) {
+ canvas.drawColor(Color::Amber_500, SkBlendMode::kSrcOver);
+ });
+
+ EXPECT_TRUE(child->isValid());
+ EXPECT_TRUE(parent->isValid());
+ EXPECT_FALSE(child->nothingToDraw());
+ EXPECT_FALSE(parent->nothingToDraw());
+
+ TestUtils::syncHierarchyPropertiesAndDisplayList(parent);
+
+ EXPECT_FALSE(child->isValid());
+ EXPECT_TRUE(parent->isValid());
+ EXPECT_TRUE(child->nothingToDraw());
+ EXPECT_FALSE(parent->nothingToDraw());
+
+ TestUtils::recordNode(*child, [](Canvas& canvas) {
+ canvas.drawColor(Color::Amber_500, SkBlendMode::kSrcOver);
+ });
+
+ EXPECT_TRUE(child->isValid());
+ EXPECT_TRUE(child->nothingToDraw());
+
+ TestUtils::recordNode(*parent, [&child](Canvas& canvas) {
+ canvas.drawRenderNode(child.get());
+ });
+
+ TestUtils::syncHierarchyPropertiesAndDisplayList(parent);
+
+ EXPECT_TRUE(child->isValid());
+ EXPECT_TRUE(parent->isValid());
+ EXPECT_FALSE(child->nothingToDraw());
+ EXPECT_FALSE(parent->nothingToDraw());
+
+ parent->destroyHardwareResources();
+
+ EXPECT_FALSE(child->isValid());
+ EXPECT_FALSE(parent->isValid());
+ EXPECT_TRUE(child->nothingToDraw());
+ EXPECT_TRUE(parent->nothingToDraw());
+}
+
TEST(RenderNode, releasedCallback) {
class DecRefOnReleased : public GlFunctorLifecycleListener {
public:
@@ -112,7 +176,6 @@
TreeInfo info(TreeInfo::MODE_RT_ONLY, *canvasContext.get());
DamageAccumulator damageAccumulator;
info.damageAccumulator = &damageAccumulator;
- info.observer = nullptr;
{
auto nonNullDLNode = TestUtils::createNode(0, 0, 200, 400,
@@ -131,7 +194,7 @@
nullDLNode->prepareTree(info);
}
- canvasContext->destroy(nullptr);
+ canvasContext->destroy();
}
RENDERTHREAD_TEST(RenderNode, prepareTree_HwLayer_AVD_enqueueDamage) {
@@ -151,7 +214,6 @@
LayerUpdateQueue layerUpdateQueue;
info.damageAccumulator = &damageAccumulator;
info.layerUpdateQueue = &layerUpdateQueue;
- info.observer = nullptr;
// Put node on HW layer
rootNode->mutateStagingProperties().mutateLayerProperties().setType(LayerType::RenderLayer);
@@ -165,5 +227,5 @@
EXPECT_FALSE(info.layerUpdateQueue->entries().empty());
EXPECT_EQ(rootNode.get(), info.layerUpdateQueue->entries().at(0).renderNode);
EXPECT_EQ(uirenderer::Rect(0, 0, 200, 400), info.layerUpdateQueue->entries().at(0).damage);
- canvasContext->destroy(nullptr);
+ canvasContext->destroy();
}
diff --git a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
index 8f6fc8b..be460bf 100644
--- a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
+++ b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
@@ -126,7 +126,6 @@
TreeInfo info(TreeInfo::MODE_FULL, *canvasContext.get());
DamageAccumulator damageAccumulator;
info.damageAccumulator = &damageAccumulator;
- info.observer = nullptr;
SkiaDisplayList skiaDL(SkRect::MakeWH(200, 200));
@@ -137,7 +136,9 @@
ASSERT_FALSE(cleanVD.isDirty());
ASSERT_FALSE(cleanVD.getPropertyChangeWillBeConsumed());
- ASSERT_FALSE(skiaDL.prepareListAndChildren(info, false, [](RenderNode*, TreeInfo&, bool) {}));
+ TestUtils::MockTreeObserver observer;
+ ASSERT_FALSE(skiaDL.prepareListAndChildren(observer, info, false,
+ [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
ASSERT_TRUE(cleanVD.getPropertyChangeWillBeConsumed());
// prepare again this time adding a dirty VD
@@ -146,7 +147,8 @@
ASSERT_TRUE(dirtyVD.isDirty());
ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
- ASSERT_TRUE(skiaDL.prepareListAndChildren(info, false, [](RenderNode*, TreeInfo&, bool) {}));
+ ASSERT_TRUE(skiaDL.prepareListAndChildren(observer, info, false,
+ [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
ASSERT_TRUE(dirtyVD.getPropertyChangeWillBeConsumed());
// prepare again this time adding a RenderNode and a callback
@@ -155,8 +157,8 @@
SkCanvas dummyCanvas;
skiaDL.mChildNodes.emplace_back(renderNode.get(), &dummyCanvas);
bool hasRun = false;
- ASSERT_TRUE(skiaDL.prepareListAndChildren(info, false,
- [&hasRun, renderNode, infoPtr](RenderNode* n, TreeInfo& i, bool r) {
+ ASSERT_TRUE(skiaDL.prepareListAndChildren(observer, info, false,
+ [&hasRun, renderNode, infoPtr](RenderNode* n, TreeObserver& observer, TreeInfo& i, bool r) {
hasRun = true;
ASSERT_EQ(renderNode.get(), n);
ASSERT_EQ(infoPtr, &i);
@@ -164,7 +166,7 @@
}));
ASSERT_TRUE(hasRun);
- canvasContext->destroy(nullptr);
+ canvasContext->destroy();
}
TEST(SkiaDisplayList, updateChildren) {
diff --git a/libs/hwui/utils/TestWindowContext.cpp b/libs/hwui/utils/TestWindowContext.cpp
index 8b80d69..79fc864 100644
--- a/libs/hwui/utils/TestWindowContext.cpp
+++ b/libs/hwui/utils/TestWindowContext.cpp
@@ -98,8 +98,8 @@
}
void finishDrawing() {
- mRootNode->setStagingDisplayList(mCanvas->finishRecording(), nullptr);
- mProxy->syncAndDrawFrame(nullptr);
+ mRootNode->setStagingDisplayList(mCanvas->finishRecording());
+ mProxy->syncAndDrawFrame();
// Surprisingly, calling mProxy->fence() here appears to make no difference to
// the timings we record.
}
