Merge "Fix occasional crash bug with layers" into jb-mr1-dev

chethaase · Android (Google) Code Review · commit f74d2f2fe8a1 · 2012-09-17T13:29:03.000-07:00
diff --git a/core/java/android/view/GLES20Canvas.java b/core/java/android/view/GLES20Canvas.java
@@ -150,7 +150,7 @@ protected void finalize() throws Throwable {
     
     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
@@ -54,20 +54,26 @@ boolean isValid() {
     }
 
     @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;
         
         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
@@ -48,7 +48,8 @@ boolean isValid() {
     }
 
     @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
@@ -135,8 +135,9 @@ boolean isOpaque() {
      * 
      * @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
@@ -12213,8 +12213,9 @@ HardwareLayer getHardwareLayer() {
                 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 @@ HardwareLayer getHardwareLayer() {
                 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
@@ -1402,6 +1402,7 @@ private void performTraversals() {
                                     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
@@ -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
@@ -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
@@ -496,6 +496,7 @@ class DisplayList {
     SortedVector<SkPath*> mSourcePaths;
     Vector<SkMatrix*> mMatrices;
     Vector<SkiaShader*> mShaders;
+    Vector<Layer*> mLayers;
 
     mutable SkFlattenableReadBuffer mReader;
 
@@ -652,6 +653,10 @@ class DisplayListRenderer: public OpenGLRenderer {
         return mSourcePaths;
     }
 
+    const Vector<Layer*>& getLayers() const {
+        return mLayers;
+    }
+
     const Vector<SkMatrix*>& getMatrices() const {
         return mMatrices;
     }
@@ -804,6 +809,12 @@ class DisplayListRenderer: public OpenGLRenderer {
         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 @@ class DisplayListRenderer: public OpenGLRenderer {
 
     Vector<SkMatrix*> mMatrices;
 
+    Vector<Layer*> mLayers;
+
     uint32_t mBufferSize;
 
     int mRestoreSaveCount;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/libs/hwui/ResourceCache.h b/libs/hwui/ResourceCache.h

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,8 @@ boolean isValid() {`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`@Override`
`51`		`- void resize(int width, int height) {`
	`51`	`+ boolean resize(int width, int height) {`
	`52`	`+ return isValid();`
`52`	`53`	`}`
`53`	`54`
`54`	`55`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -1402,6 +1402,7 @@ private void performTraversals() {`
`1402`	`1402`	`mResizeBuffer.getHeight() != mHeight) {`
`1403`	`1403`	`mResizeBuffer.resize(mWidth, mHeight);`
`1404`	`1404`	`}`
	`1405`	`+ // TODO: should handle create/resize failure`
`1405`	`1406`	`layerCanvas = mResizeBuffer.start(hwRendererCanvas);`
`1406`	`1407`	`layerCanvas.setViewport(mWidth, mHeight);`
`1407`	`1408`	`layerCanvas.onPreDraw(null);`