diff options
author | Chet Haase <chet@google.com> | 2012-09-14 15:31:25 -0700 |
---|---|---|
committer | Chet Haase <chet@google.com> | 2012-09-17 11:21:34 -0700 |
commit | 603f6de35f21d74ae242d52d501f4f5c25ff4f4c (patch) | |
tree | b249a7750a60b445a43670b7ba540beecbe715db | |
parent | cc5dd18d15a174799ad79d26633c268e8860c8ab (diff) | |
download | frameworks_base-603f6de35f21d74ae242d52d501f4f5c25ff4f4c.zip frameworks_base-603f6de35f21d74ae242d52d501f4f5c25ff4f4c.tar.gz frameworks_base-603f6de35f21d74ae242d52d501f4f5c25ff4f4c.tar.bz2 |
Fix occasional crash bug with layers
Launcher occasionally crashes with a stack trace indicating that the memory
of a Layer object is corrupt. It is possible for us to delete a Layer
structure and then, briefly, use it to draw a DisplayList again before
that DisplayList gets recreated (without the layer that got deleted).
When this happens, if the memory got corrupted, it's possible to crash.
The fix is to add Layer to the other objects which we currently refcount
(bitmaps, shaders, etc.). Then instead of deleting a Layer, we decrement the
refcount. We increment when creating it, then increment it again when it's
referenced from a DisplayList. Then we decrement the refcount instead of
deleting it, and decrement when we clear a DisplayList that refers to it.
Then when the refcount reaches 0, we delete it.
Issue #6994632 Native crash in launcher when trying to launch all apps screen
Change-Id: I0627be8d49bb2f9ba8d158a84b764bb4e7df934c
-rw-r--r-- | core/java/android/view/GLES20Canvas.java | 2 | ||||
-rw-r--r-- | core/java/android/view/GLES20RenderLayer.java | 18 | ||||
-rw-r--r-- | core/java/android/view/GLES20TextureLayer.java | 3 | ||||
-rw-r--r-- | core/java/android/view/HardwareLayer.java | 3 | ||||
-rw-r--r-- | core/java/android/view/View.java | 7 | ||||
-rw-r--r-- | core/java/android/view/ViewRootImpl.java | 1 | ||||
-rw-r--r-- | core/jni/android_view_GLES20Canvas.cpp | 18 | ||||
-rw-r--r-- | libs/hwui/DisplayListRenderer.cpp | 19 | ||||
-rw-r--r-- | libs/hwui/DisplayListRenderer.h | 13 | ||||
-rw-r--r-- | libs/hwui/Layer.cpp | 19 | ||||
-rw-r--r-- | libs/hwui/Layer.h | 16 | ||||
-rw-r--r-- | libs/hwui/LayerCache.cpp | 11 | ||||
-rw-r--r-- | libs/hwui/LayerRenderer.cpp | 10 | ||||
-rw-r--r-- | libs/hwui/OpenGLRenderer.cpp | 7 | ||||
-rw-r--r-- | libs/hwui/ResourceCache.cpp | 23 | ||||
-rw-r--r-- | libs/hwui/ResourceCache.h | 6 |
16 files changed, 119 insertions, 57 deletions
diff --git a/core/java/android/view/GLES20Canvas.java b/core/java/android/view/GLES20Canvas.java index 869cd00..032ff7b 100644 --- a/core/java/android/view/GLES20Canvas.java +++ b/core/java/android/view/GLES20Canvas.java @@ -150,7 +150,7 @@ class GLES20Canvas extends HardwareCanvas { static native int nCreateTextureLayer(boolean opaque, int[] layerInfo); static native int nCreateLayer(int width, int height, boolean isOpaque, int[] layerInfo); - static native void nResizeLayer(int layerId, int width, int height, int[] layerInfo); + static native boolean nResizeLayer(int layerId, int width, int height, int[] layerInfo); static native void nSetOpaqueLayer(int layerId, boolean isOpaque); static native void nSetLayerPaint(int layerId, int nativePaint); static native void nSetLayerColorFilter(int layerId, int nativeColorFilter); diff --git a/core/java/android/view/GLES20RenderLayer.java b/core/java/android/view/GLES20RenderLayer.java index a77425a..fcfc8e1 100644 --- a/core/java/android/view/GLES20RenderLayer.java +++ b/core/java/android/view/GLES20RenderLayer.java @@ -54,8 +54,8 @@ class GLES20RenderLayer extends GLES20Layer { } @Override - void resize(int width, int height) { - if (!isValid() || width <= 0 || height <= 0) return; + boolean resize(int width, int height) { + if (!isValid() || width <= 0 || height <= 0) return false; mWidth = width; mHeight = height; @@ -63,11 +63,17 @@ class GLES20RenderLayer extends GLES20Layer { if (width != mLayerWidth || height != mLayerHeight) { int[] layerInfo = new int[2]; - GLES20Canvas.nResizeLayer(mLayer, width, height, layerInfo); - - mLayerWidth = layerInfo[0]; - mLayerHeight = layerInfo[1]; + if (GLES20Canvas.nResizeLayer(mLayer, width, height, layerInfo)) { + mLayerWidth = layerInfo[0]; + mLayerHeight = layerInfo[1]; + } else { + // Failure: not enough GPU resources for requested size + mLayer = 0; + mLayerWidth = 0; + mLayerHeight = 0; + } } + return isValid(); } @Override diff --git a/core/java/android/view/GLES20TextureLayer.java b/core/java/android/view/GLES20TextureLayer.java index e198ef6..b0ee2aa 100644 --- a/core/java/android/view/GLES20TextureLayer.java +++ b/core/java/android/view/GLES20TextureLayer.java @@ -48,7 +48,8 @@ class GLES20TextureLayer extends GLES20Layer { } @Override - void resize(int width, int height) { + boolean resize(int width, int height) { + return isValid(); } @Override diff --git a/core/java/android/view/HardwareLayer.java b/core/java/android/view/HardwareLayer.java index 6e763b2..d798e73 100644 --- a/core/java/android/view/HardwareLayer.java +++ b/core/java/android/view/HardwareLayer.java @@ -135,8 +135,9 @@ abstract class HardwareLayer { * * @param width The new desired minimum width for this layer * @param height The new desired minimum height for this layer + * @return True if the resulting layer is valid, false otherwise */ - abstract void resize(int width, int height); + abstract boolean resize(int width, int height); /** * Returns a hardware canvas that can be used to render onto diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index 03f9b72..d93c1be 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -12213,8 +12213,9 @@ public class View implements Drawable.Callback, KeyEvent.Callback, mLocalDirtyRect.set(0, 0, width, height); } else { if (mHardwareLayer.getWidth() != width || mHardwareLayer.getHeight() != height) { - mHardwareLayer.resize(width, height); - mLocalDirtyRect.set(0, 0, width, height); + if (mHardwareLayer.resize(width, height)) { + mLocalDirtyRect.set(0, 0, width, height); + } } // This should not be necessary but applications that change @@ -12225,7 +12226,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback, computeOpaqueFlags(); final boolean opaque = isOpaque(); - if (mHardwareLayer.isOpaque() != opaque) { + if (mHardwareLayer.isValid() && mHardwareLayer.isOpaque() != opaque) { mHardwareLayer.setOpaque(opaque); mLocalDirtyRect.set(0, 0, width, height); } diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index d6a0203..27fd374 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -1402,6 +1402,7 @@ public final class ViewRootImpl implements ViewParent, mResizeBuffer.getHeight() != mHeight) { mResizeBuffer.resize(mWidth, mHeight); } + // TODO: should handle create/resize failure layerCanvas = mResizeBuffer.start(hwRendererCanvas); layerCanvas.setViewport(mWidth, mHeight); layerCanvas.onPreDraw(null); diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp index 7dbf9ec..2ff886e 100644 --- a/core/jni/android_view_GLES20Canvas.cpp +++ b/core/jni/android_view_GLES20Canvas.cpp @@ -766,14 +766,16 @@ static Layer* android_view_GLES20Canvas_createLayer(JNIEnv* env, jobject clazz, return layer; } -static void android_view_GLES20Canvas_resizeLayer(JNIEnv* env, jobject clazz, +static bool android_view_GLES20Canvas_resizeLayer(JNIEnv* env, jobject clazz, Layer* layer, jint width, jint height, jintArray layerInfo) { - LayerRenderer::resizeLayer(layer, width, height); - - jint* storage = env->GetIntArrayElements(layerInfo, NULL); - storage[0] = layer->getWidth(); - storage[1] = layer->getHeight(); - env->ReleaseIntArrayElements(layerInfo, storage, 0); + if (LayerRenderer::resizeLayer(layer, width, height)) { + jint* storage = env->GetIntArrayElements(layerInfo, NULL); + storage[0] = layer->getWidth(); + storage[1] = layer->getHeight(); + env->ReleaseIntArrayElements(layerInfo, storage, 0); + return true; + } + return false; } static void android_view_GLES20Canvas_setLayerPaint(JNIEnv* env, jobject clazz, @@ -992,7 +994,7 @@ static JNINativeMethod gMethods[] = { { "nCreateLayerRenderer", "(I)I", (void*) android_view_GLES20Canvas_createLayerRenderer }, { "nCreateLayer", "(IIZ[I)I", (void*) android_view_GLES20Canvas_createLayer }, - { "nResizeLayer", "(III[I)V" , (void*) android_view_GLES20Canvas_resizeLayer }, + { "nResizeLayer", "(III[I)Z" , (void*) android_view_GLES20Canvas_resizeLayer }, { "nSetLayerPaint", "(II)V", (void*) android_view_GLES20Canvas_setLayerPaint }, { "nSetLayerColorFilter", "(II)V", (void*) android_view_GLES20Canvas_setLayerColorFilter }, { "nSetOpaqueLayer", "(IZ)V", (void*) android_view_GLES20Canvas_setOpaqueLayer }, diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp index c3444e6..cc72df6 100644 --- a/libs/hwui/DisplayListRenderer.cpp +++ b/libs/hwui/DisplayListRenderer.cpp @@ -182,6 +182,10 @@ void DisplayList::clearResources() { caches.resourceCache.decrementRefcountLocked(mSourcePaths.itemAt(i)); } + for (size_t i = 0; i < mLayers.size(); i++) { + caches.resourceCache.decrementRefcountLocked(mLayers.itemAt(i)); + } + caches.resourceCache.unlock(); for (size_t i = 0; i < mPaints.size(); i++) { @@ -206,6 +210,7 @@ void DisplayList::clearResources() { mPaints.clear(); mPaths.clear(); mMatrices.clear(); + mLayers.clear(); } void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorder, bool reusing) { @@ -264,6 +269,12 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde caches.resourceCache.incrementRefcountLocked(sourcePaths.itemAt(i)); } + const Vector<Layer*>& layers = recorder.getLayers(); + for (size_t i = 0; i < layers.size(); i++) { + mLayers.add(layers.itemAt(i)); + caches.resourceCache.incrementRefcountLocked(layers.itemAt(i)); + } + caches.resourceCache.unlock(); const Vector<SkPaint*>& paints = recorder.getPaints(); @@ -1361,6 +1372,10 @@ void DisplayListRenderer::reset() { mCaches.resourceCache.decrementRefcountLocked(mSourcePaths.itemAt(i)); } + for (size_t i = 0; i < mLayers.size(); i++) { + mCaches.resourceCache.decrementRefcountLocked(mLayers.itemAt(i)); + } + mCaches.resourceCache.unlock(); mBitmapResources.clear(); @@ -1379,6 +1394,8 @@ void DisplayListRenderer::reset() { mMatrices.clear(); + mLayers.clear(); + mHasDrawOps = false; } @@ -1539,7 +1556,7 @@ status_t DisplayListRenderer::drawDisplayList(DisplayList* displayList, status_t DisplayListRenderer::drawLayer(Layer* layer, float x, float y, SkPaint* paint) { addOp(DisplayList::DrawLayer); - addInt((int) layer); + addLayer(layer); addPoint(x, y); addPaint(paint); return DrawGlInfo::kStatusDone; diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h index 8e4f2d3..a0b1630 100644 --- a/libs/hwui/DisplayListRenderer.h +++ b/libs/hwui/DisplayListRenderer.h @@ -496,6 +496,7 @@ private: SortedVector<SkPath*> mSourcePaths; Vector<SkMatrix*> mMatrices; Vector<SkiaShader*> mShaders; + Vector<Layer*> mLayers; mutable SkFlattenableReadBuffer mReader; @@ -652,6 +653,10 @@ public: return mSourcePaths; } + const Vector<Layer*>& getLayers() const { + return mLayers; + } + const Vector<SkMatrix*>& getMatrices() const { return mMatrices; } @@ -804,6 +809,12 @@ private: mMatrices.add(copy); } + inline void addLayer(Layer* layer) { + addInt((int) layer); + mLayers.add(layer); + mCaches.resourceCache.incrementRefcount(layer); + } + inline void addBitmap(SkBitmap* bitmap) { // Note that this assumes the bitmap is immutable. There are cases this won't handle // correctly, such as creating the bitmap from scratch, drawing with it, changing its @@ -862,6 +873,8 @@ private: Vector<SkMatrix*> mMatrices; + Vector<Layer*> mLayers; + uint32_t mBufferSize; int mRestoreSaveCount; diff --git a/libs/hwui/Layer.cpp b/libs/hwui/Layer.cpp index 31e169b..76b274b 100644 --- a/libs/hwui/Layer.cpp +++ b/libs/hwui/Layer.cpp @@ -25,10 +25,29 @@ namespace android { namespace uirenderer { +Layer::Layer(const uint32_t layerWidth, const uint32_t layerHeight) { + mesh = NULL; + meshIndices = NULL; + meshElementCount = 0; + cacheable = true; + textureLayer = false; + renderTarget = GL_TEXTURE_2D; + texture.width = layerWidth; + texture.height = layerHeight; + colorFilter = NULL; + deferredUpdateScheduled = false; + renderer = NULL; + displayList = NULL; + fbo = 0; + Caches::getInstance().resourceCache.incrementRefcount(this); +} + Layer::~Layer() { if (mesh) delete mesh; if (meshIndices) delete meshIndices; if (colorFilter) Caches::getInstance().resourceCache.decrementRefcount(colorFilter); + if (fbo) Caches::getInstance().fboCache.put(fbo); + deleteTexture(); } void Layer::setPaint(SkPaint* paint) { diff --git a/libs/hwui/Layer.h b/libs/hwui/Layer.h index 76da671..420073a 100644 --- a/libs/hwui/Layer.h +++ b/libs/hwui/Layer.h @@ -46,21 +46,7 @@ class DisplayList; */ struct Layer { - Layer(const uint32_t layerWidth, const uint32_t layerHeight) { - mesh = NULL; - meshIndices = NULL; - meshElementCount = 0; - cacheable = true; - textureLayer = false; - renderTarget = GL_TEXTURE_2D; - texture.width = layerWidth; - texture.height = layerHeight; - colorFilter = NULL; - deferredUpdateScheduled = false; - renderer = NULL; - displayList = NULL; - } - + Layer(const uint32_t layerWidth, const uint32_t layerHeight); ~Layer(); /** diff --git a/libs/hwui/LayerCache.cpp b/libs/hwui/LayerCache.cpp index eea707e..ce74cee 100644 --- a/libs/hwui/LayerCache.cpp +++ b/libs/hwui/LayerCache.cpp @@ -69,15 +69,10 @@ void LayerCache::setMaxSize(uint32_t maxSize) { void LayerCache::deleteLayer(Layer* layer) { if (layer) { - GLuint fbo = layer->getFbo(); - LAYER_LOGD("Destroying layer %dx%d, fbo %d", layer->getWidth(), layer->getHeight(), fbo); - + LAYER_LOGD("Destroying layer %dx%d, fbo %d", layer->getWidth(), layer->getHeight(), + layer->getFbo()); mSize -= layer->getWidth() * layer->getHeight() * 4; - - if (fbo) Caches::getInstance().fboCache.put(fbo); - layer->deleteTexture(); - - delete layer; + Caches::getInstance().resourceCache.decrementRefcount(layer); } } diff --git a/libs/hwui/LayerRenderer.cpp b/libs/hwui/LayerRenderer.cpp index 02af5e2..b57d806 100644 --- a/libs/hwui/LayerRenderer.cpp +++ b/libs/hwui/LayerRenderer.cpp @@ -211,10 +211,8 @@ Layer* LayerRenderer::createLayer(uint32_t width, uint32_t height, bool isOpaque fbo, width, height); glBindFramebuffer(GL_FRAMEBUFFER, previousFbo); - caches.fboCache.put(fbo); - layer->deleteTexture(); - delete layer; + Caches::getInstance().resourceCache.decrementRefcount(layer); return NULL; } @@ -240,8 +238,7 @@ bool LayerRenderer::resizeLayer(Layer* layer, uint32_t width, uint32_t height) { layer->texCoords.set(0.0f, height / float(layer->getHeight()), width / float(layer->getWidth()), 0.0f); } else { - layer->deleteTexture(); - delete layer; + Caches::getInstance().resourceCache.decrementRefcount(layer); return false; } } @@ -303,8 +300,7 @@ void LayerRenderer::destroyLayer(Layer* layer) { if (!Caches::getInstance().layerCache.put(layer)) { LAYER_RENDERER_LOGD(" Destroyed!"); - layer->deleteTexture(); - delete layer; + Caches::getInstance().resourceCache.decrementRefcount(layer); } else { LAYER_RENDERER_LOGD(" Cached!"); #if DEBUG_LAYER_RENDERER diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp index 02448e8..4aefcba 100644 --- a/libs/hwui/OpenGLRenderer.cpp +++ b/libs/hwui/OpenGLRenderer.cpp @@ -643,10 +643,8 @@ bool OpenGLRenderer::createFboLayer(Layer* layer, Rect& bounds, Rect& clip, GLui ALOGE("Framebuffer incomplete (GL error code 0x%x)", status); glBindFramebuffer(GL_FRAMEBUFFER, previousFbo); - layer->deleteTexture(); - mCaches.fboCache.put(layer->getFbo()); - delete layer; + Caches::getInstance().resourceCache.decrementRefcount(layer); return false; } @@ -732,8 +730,7 @@ void OpenGLRenderer::composeLayer(sp<Snapshot> current, sp<Snapshot> previous) { // Failing to add the layer to the cache should happen only if the layer is too large if (!mCaches.layerCache.put(layer)) { LAYER_LOGD("Deleting layer"); - layer->deleteTexture(); - delete layer; + Caches::getInstance().resourceCache.decrementRefcount(layer); } } diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp index b0c57d1..1c83ea4 100644 --- a/libs/hwui/ResourceCache.cpp +++ b/libs/hwui/ResourceCache.cpp @@ -79,6 +79,10 @@ void ResourceCache::incrementRefcount(SkiaColorFilter* filterResource) { incrementRefcount((void*) filterResource, kColorFilter); } +void ResourceCache::incrementRefcount(Layer* layerResource) { + incrementRefcount((void*) layerResource, kLayer); +} + void ResourceCache::incrementRefcountLocked(void* resource, ResourceType resourceType) { ssize_t index = mCache->indexOfKey(resource); ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL; @@ -109,6 +113,10 @@ void ResourceCache::incrementRefcountLocked(SkiaColorFilter* filterResource) { incrementRefcountLocked((void*) filterResource, kColorFilter); } +void ResourceCache::incrementRefcountLocked(Layer* layerResource) { + incrementRefcountLocked((void*) layerResource, kLayer); +} + void ResourceCache::decrementRefcount(void* resource) { Mutex::Autolock _l(mLock); decrementRefcountLocked(resource); @@ -134,6 +142,10 @@ void ResourceCache::decrementRefcount(SkiaColorFilter* filterResource) { decrementRefcount((void*) filterResource); } +void ResourceCache::decrementRefcount(Layer* layerResource) { + decrementRefcount((void*) layerResource); +} + void ResourceCache::decrementRefcountLocked(void* resource) { ssize_t index = mCache->indexOfKey(resource); ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL; @@ -167,6 +179,10 @@ void ResourceCache::decrementRefcountLocked(SkiaColorFilter* filterResource) { decrementRefcountLocked((void*) filterResource); } +void ResourceCache::decrementRefcountLocked(Layer* layerResource) { + decrementRefcountLocked((void*) layerResource); +} + void ResourceCache::destructor(SkPath* resource) { Mutex::Autolock _l(mLock); destructorLocked(resource); @@ -280,7 +296,7 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r if (ref->recycled && ref->resourceType == kBitmap) { ((SkBitmap*) resource)->setPixels(NULL, NULL); } - if (ref->destroyed) { + if (ref->destroyed || ref->resourceType == kLayer) { switch (ref->resourceType) { case kBitmap: { SkBitmap* bitmap = (SkBitmap*) resource; @@ -308,6 +324,11 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r delete filter; } break; + case kLayer: { + Layer* layer = (Layer*) resource; + delete layer; + } + break; } } mCache->removeItem(resource); diff --git a/libs/hwui/ResourceCache.h b/libs/hwui/ResourceCache.h index 60ffa7d..2053d96 100644 --- a/libs/hwui/ResourceCache.h +++ b/libs/hwui/ResourceCache.h @@ -23,6 +23,7 @@ #include <SkiaColorFilter.h> #include <SkiaShader.h> #include <utils/KeyedVector.h> +#include "Layer.h" namespace android { namespace uirenderer { @@ -35,6 +36,7 @@ enum ResourceType { kShader, kColorFilter, kPath, + kLayer }; class ResourceReference { @@ -67,21 +69,25 @@ public: void incrementRefcount(SkBitmap* resource); void incrementRefcount(SkiaShader* resource); void incrementRefcount(SkiaColorFilter* resource); + void incrementRefcount(Layer* resource); void incrementRefcountLocked(SkPath* resource); void incrementRefcountLocked(SkBitmap* resource); void incrementRefcountLocked(SkiaShader* resource); void incrementRefcountLocked(SkiaColorFilter* resource); + void incrementRefcountLocked(Layer* resource); void decrementRefcount(SkBitmap* resource); void decrementRefcount(SkPath* resource); void decrementRefcount(SkiaShader* resource); void decrementRefcount(SkiaColorFilter* resource); + void decrementRefcount(Layer* resource); void decrementRefcountLocked(SkBitmap* resource); void decrementRefcountLocked(SkPath* resource); void decrementRefcountLocked(SkiaShader* resource); void decrementRefcountLocked(SkiaColorFilter* resource); + void decrementRefcountLocked(Layer* resource); void destructor(SkPath* resource); void destructor(SkBitmap* resource); |