From dcba6725e8b9d3eba9ad7a01258d6aa974feafba Mon Sep 17 00:00:00 2001 From: John Reck Date: Tue, 8 Jul 2014 13:59:49 -0700 Subject: [PATCH] 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 --- core/java/android/view/View.java | 8 ++-- libs/hwui/RenderNode.cpp | 80 +++++++++++++++++++++----------- libs/hwui/RenderNode.h | 14 ++++++ libs/hwui/TreeInfo.h | 9 ---- libs/hwui/renderthread/CanvasContext.cpp | 3 +- 5 files changed, 74 insertions(+), 40 deletions(-) diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index f1a0913dcdc0..1e5f448beaa8 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -13625,9 +13625,11 @@ public class View implements Drawable.Callback, KeyEvent.Callback, * @hide */ protected void destroyHardwareResources() { - // Intentionally empty. RenderNode's lifecycle is now fully managed - // by the hardware renderer. - // However some subclasses (eg, WebView, TextureView) still need this signal + // Although the Layer will be destroyed by RenderNode, we want to release + // the staging display list, which is also a signal to RenderNode that it's + // safe to free its copy of the display list as it knows that we will + // push an updated DisplayList if we try to draw again + resetDisplayList(); } /** diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp index 89105ea19fbd..fe03806903a5 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 7d42b59054ad..54fa143f9aa8 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& 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 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 9746ac53906e..de09755803b6 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 f5d4f8bc954c..57279b778d92 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); } } -- 2.11.0