diff options
author | John Reck <jreck@google.com> | 2014-07-08 13:59:49 -0700 |
---|---|---|
committer | John Reck <jreck@google.com> | 2014-07-08 14:14:55 -0700 |
commit | dcba6725e8b9d3eba9ad7a01258d6aa974feafba (patch) | |
tree | 994a519302533fc1073eeafda8bf74933531dd56 /libs | |
parent | 114c68cec40a995fb6f3ef0ab110ee8b59ab0cba (diff) | |
download | frameworks_base-dcba6725e8b9d3eba9ad7a01258d6aa974feafba.zip frameworks_base-dcba6725e8b9d3eba9ad7a01258d6aa974feafba.tar.gz frameworks_base-dcba6725e8b9d3eba9ad7a01258d6aa974feafba.tar.bz2 |
Fix layers lifecycle issues
Bug: 16118540
Fix an issue where we could have a reference to a Layer after
the GL context was destroyed
Change-Id: I7bfd909d735ca6b942ebe188fc10099422eb6d95
Diffstat (limited to 'libs')
-rw-r--r-- | libs/hwui/RenderNode.cpp | 80 | ||||
-rw-r--r-- | libs/hwui/RenderNode.h | 14 | ||||
-rw-r--r-- | libs/hwui/TreeInfo.h | 9 | ||||
-rw-r--r-- | libs/hwui/renderthread/CanvasContext.cpp | 3 |
4 files changed, 69 insertions, 37 deletions
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp index 89105ea..fe03806 100644 --- a/libs/hwui/RenderNode.cpp +++ b/libs/hwui/RenderNode.cpp @@ -63,11 +63,12 @@ RenderNode::RenderNode() , mDisplayListData(0) , mStagingDisplayListData(0) , mAnimatorManager(*this) - , mLayer(0) { + , mLayer(0) + , mParentCount(0) { } RenderNode::~RenderNode() { - delete mDisplayListData; + deleteDisplayListData(); delete mStagingDisplayListData; LayerRenderer::destroyLayerDeferred(mLayer); } @@ -196,27 +197,12 @@ void RenderNode::pushLayerUpdate(TreeInfo& info) { void RenderNode::prepareTreeImpl(TreeInfo& info) { info.damageAccumulator->pushTransform(this); - switch (info.mode) { - case TreeInfo::MODE_FULL: - pushStagingPropertiesChanges(info); - mAnimatorManager.animate(info); - break; - case TreeInfo::MODE_MAYBE_DETACHING: + if (info.mode == TreeInfo::MODE_FULL) { pushStagingPropertiesChanges(info); - break; - case TreeInfo::MODE_RT_ONLY: - mAnimatorManager.animate(info); - break; - case TreeInfo::MODE_DESTROY_RESOURCES: - // This will also release the hardware layer if we have one as - // isRenderable() will return false, thus causing pushLayerUpdate - // to recycle the hardware layer - setStagingDisplayList(NULL); - break; } - + mAnimatorManager.animate(info); prepareLayer(info); - if (info.mode == TreeInfo::MODE_FULL || info.mode == TreeInfo::MODE_DESTROY_RESOURCES) { + if (info.mode == TreeInfo::MODE_FULL) { pushStagingDisplayListChanges(info); } prepareSubTree(info, mDisplayListData); @@ -258,17 +244,30 @@ void RenderNode::applyLayerPropertiesToLayer(TreeInfo& info) { void RenderNode::pushStagingDisplayListChanges(TreeInfo& info) { if (mNeedsDisplayListDataSync) { mNeedsDisplayListDataSync = false; - // Do a push pass on the old tree to handle freeing DisplayListData - // that are no longer used - TreeInfo oldTreeInfo(TreeInfo::MODE_MAYBE_DETACHING, info); - prepareSubTree(oldTreeInfo, mDisplayListData); - delete mDisplayListData; + // Make sure we inc first so that we don't fluctuate between 0 and 1, + // which would thrash the layer cache + if (mStagingDisplayListData) { + for (size_t i = 0; i < mStagingDisplayListData->children().size(); i++) { + mStagingDisplayListData->children()[i]->mRenderNode->incParentRefCount(); + } + } + deleteDisplayListData(); mDisplayListData = mStagingDisplayListData; - mStagingDisplayListData = 0; + mStagingDisplayListData = NULL; damageSelf(info); } } +void RenderNode::deleteDisplayListData() { + if (mDisplayListData) { + for (size_t i = 0; i < mDisplayListData->children().size(); i++) { + mDisplayListData->children()[i]->mRenderNode->decParentRefCount(); + } + } + delete mDisplayListData; + mDisplayListData = NULL; +} + void RenderNode::prepareSubTree(TreeInfo& info, DisplayListData* subtree) { if (subtree) { TextureCache& cache = Caches::getInstance().textureCache; @@ -291,6 +290,35 @@ void RenderNode::prepareSubTree(TreeInfo& info, DisplayListData* subtree) { } } +void RenderNode::destroyHardwareResources() { + if (mLayer) { + LayerRenderer::destroyLayer(mLayer); + mLayer = NULL; + } + if (mDisplayListData) { + for (size_t i = 0; i < mDisplayListData->children().size(); i++) { + mDisplayListData->children()[i]->mRenderNode->destroyHardwareResources(); + } + if (mNeedsDisplayListDataSync) { + // Next prepare tree we are going to push a new display list, so we can + // drop our current one now + deleteDisplayListData(); + } + } +} + +void RenderNode::decParentRefCount() { + LOG_ALWAYS_FATAL_IF(!mParentCount, "already 0!"); + mParentCount--; + if (!mParentCount) { + // 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(); + } +} + /* * For property operations, we pass a savecount of 0, since the operations aren't part of the * displaylist, and thus don't have to compensate for the record-time/playback-time discrepancy in diff --git a/libs/hwui/RenderNode.h b/libs/hwui/RenderNode.h index 7d42b59..54fa143 100644 --- a/libs/hwui/RenderNode.h +++ b/libs/hwui/RenderNode.h @@ -168,6 +168,7 @@ public: } ANDROID_API virtual void prepareTree(TreeInfo& info); + void destroyHardwareResources(); // UI thread only! ANDROID_API void addAnimator(const sp<BaseRenderNodeAnimator>& animator); @@ -248,6 +249,10 @@ private: void applyLayerPropertiesToLayer(TreeInfo& info); void prepareLayer(TreeInfo& info); void pushLayerUpdate(TreeInfo& info); + void deleteDisplayListData(); + + void incParentRefCount() { mParentCount++; } + void decParentRefCount(); String8 mName; @@ -256,6 +261,7 @@ private: RenderProperties mStagingProperties; bool mNeedsDisplayListDataSync; + // WARNING: Do not delete this directly, you must go through deleteDisplayListData()! DisplayListData* mDisplayListData; DisplayListData* mStagingDisplayListData; @@ -272,6 +278,14 @@ private: // for projection surfaces, contains a list of all children items Vector<DrawRenderNodeOp*> mProjectedNodes; + + // How many references our parent(s) have to us. Typically this should alternate + // between 2 and 1 (when a staging push happens we inc first then dec) + // When this hits 0 we are no longer in the tree, so any hardware resources + // (specifically Layers) should be released. + // This is *NOT* thread-safe, and should therefore only be tracking + // mDisplayListData, not mStagingDisplayListData. + uint32_t mParentCount; }; // class RenderNode } /* namespace uirenderer */ diff --git a/libs/hwui/TreeInfo.h b/libs/hwui/TreeInfo.h index 9746ac53..de09755 100644 --- a/libs/hwui/TreeInfo.h +++ b/libs/hwui/TreeInfo.h @@ -58,15 +58,6 @@ public: // animators, but potentially things like SurfaceTexture updates // could be handled by this as well if there are no listeners MODE_RT_ONLY, - // The subtree is being detached. Maybe. If the RenderNode is present - // in both the old and new display list's children then it will get a - // MODE_MAYBE_DETACHING followed shortly by a MODE_FULL. - // Push any pending display list changes in case it is detached, - // but don't evaluate animators and such as if it isn't detached as a - // MODE_FULL will follow shortly. - MODE_MAYBE_DETACHING, - // Destroy all hardware resources, including DisplayListData, in the tree. - MODE_DESTROY_RESOURCES, }; explicit TreeInfo(TraversalMode mode, RenderState& renderState) diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index f5d4f8b..57279b7 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/libs/hwui/renderthread/CanvasContext.cpp @@ -250,8 +250,7 @@ void CanvasContext::destroyHardwareResources() { stopDrawing(); if (mEglManager.hasEglContext()) { requireGlContext(); - TreeInfo info(TreeInfo::MODE_DESTROY_RESOURCES, mRenderThread.renderState()); - mRootRenderNode->prepareTree(info); + mRootRenderNode->destroyHardwareResources(); Caches::getInstance().flush(Caches::kFlushMode_Layers); } } |