From ef09a210dd6ea481158b7028ec2424a7f5769ed2 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Tue, 25 Sep 2012 12:17:14 -0700 Subject: [PATCH] Don't destroy the same texture twice Bug #7221449 SurfaceTexture already deletes the GL texture when detachFromContext is invoked. The newly introduced refcount would casue the Layer object to be destroyed later and attempt to delete the GL texture again. By the time the second cleanup occurs, the texture name might have been reused by somebody else, resulting in erroneous behaviors. Change-Id: I257c589fea64b34c00f46fbfaa7732e6854a5e41 --- core/java/android/view/GLES20Canvas.java | 1 + core/java/android/view/GLES20Layer.java | 5 ++++ .../java/android/view/GLES20TextureLayer.java | 2 +- core/java/android/view/HardwareLayer.java | 5 ++++ core/java/android/view/TextureView.java | 1 + core/jni/android_view_GLES20Canvas.cpp | 6 +++++ libs/hwui/Layer.h | 26 ++++++++++++++----- 7 files changed, 38 insertions(+), 8 deletions(-) diff --git a/core/java/android/view/GLES20Canvas.java b/core/java/android/view/GLES20Canvas.java index c703aaf88b50b..b64a06e9cc790 100644 --- a/core/java/android/view/GLES20Canvas.java +++ b/core/java/android/view/GLES20Canvas.java @@ -166,6 +166,7 @@ class GLES20Canvas extends HardwareCanvas { static native void nSetLayerColorFilter(int layerId, int nativeColorFilter); static native void nUpdateTextureLayer(int layerId, int width, int height, boolean opaque, SurfaceTexture surface); + static native void nClearLayerTexture(int layerId); static native void nSetTextureLayerTransform(int layerId, int matrix); static native void nDestroyLayer(int layerId); static native void nDestroyLayerDeferred(int layerId); diff --git a/core/java/android/view/GLES20Layer.java b/core/java/android/view/GLES20Layer.java index a462ed635ea5c..812fb977421ed 100644 --- a/core/java/android/view/GLES20Layer.java +++ b/core/java/android/view/GLES20Layer.java @@ -66,6 +66,11 @@ abstract class GLES20Layer extends HardwareLayer { mLayer = 0; } + @Override + void clearStorage() { + if (mLayer != 0) GLES20Canvas.nClearLayerTexture(mLayer); + } + static class Finalizer { private int mLayerId; diff --git a/core/java/android/view/GLES20TextureLayer.java b/core/java/android/view/GLES20TextureLayer.java index 797c7342c947c..e863e49a01411 100644 --- a/core/java/android/view/GLES20TextureLayer.java +++ b/core/java/android/view/GLES20TextureLayer.java @@ -39,7 +39,7 @@ class GLES20TextureLayer extends GLES20Layer { mFinalizer = new Finalizer(mLayer); } else { mFinalizer = null; - } + } } @Override diff --git a/core/java/android/view/HardwareLayer.java b/core/java/android/view/HardwareLayer.java index d6868ca16f9a9..d3bc35a2af42d 100644 --- a/core/java/android/view/HardwareLayer.java +++ b/core/java/android/view/HardwareLayer.java @@ -204,4 +204,9 @@ abstract class HardwareLayer { * @param dirtyRect The dirty region of the layer that needs to be redrawn */ abstract void redrawLater(DisplayList displayList, Rect dirtyRect); + + /** + * Indicates that this layer has lost its underlying storage. + */ + abstract void clearStorage(); } diff --git a/core/java/android/view/TextureView.java b/core/java/android/view/TextureView.java index 7e335f042deb6..876b7d8479199 100644 --- a/core/java/android/view/TextureView.java +++ b/core/java/android/view/TextureView.java @@ -224,6 +224,7 @@ public class TextureView extends View { private void destroySurface() { if (mLayer != null) { mSurface.detachFromGLContext(); + mLayer.clearStorage(); boolean shouldRelease = true; if (mListener != null) { diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp index a3834acba2bb4..1b71b4329649b 100644 --- a/core/jni/android_view_GLES20Canvas.cpp +++ b/core/jni/android_view_GLES20Canvas.cpp @@ -822,6 +822,11 @@ static void android_view_GLES20Canvas_updateRenderLayer(JNIEnv* env, jobject cla layer->updateDeferred(renderer, displayList, left, top, right, bottom); } +static void android_view_GLES20Canvas_clearLayerTexture(JNIEnv* env, jobject clazz, + Layer* layer) { + layer->clearTexture(); +} + static void android_view_GLES20Canvas_setTextureLayerTransform(JNIEnv* env, jobject clazz, Layer* layer, SkMatrix* matrix) { @@ -1016,6 +1021,7 @@ static JNINativeMethod gMethods[] = { { "nUpdateTextureLayer", "(IIIZLandroid/graphics/SurfaceTexture;)V", (void*) android_view_GLES20Canvas_updateTextureLayer }, { "nUpdateRenderLayer", "(IIIIIII)V", (void*) android_view_GLES20Canvas_updateRenderLayer }, + { "nClearLayerTexture", "(I)V", (void*) android_view_GLES20Canvas_clearLayerTexture }, { "nDestroyLayer", "(I)V", (void*) android_view_GLES20Canvas_destroyLayer }, { "nDestroyLayerDeferred", "(I)V", (void*) android_view_GLES20Canvas_destroyLayerDeferred }, { "nDrawLayer", "(IIFFI)V", (void*) android_view_GLES20Canvas_drawLayer }, diff --git a/libs/hwui/Layer.h b/libs/hwui/Layer.h index d2cd4407ed94a..9b6205d31f28c 100644 --- a/libs/hwui/Layer.h +++ b/libs/hwui/Layer.h @@ -134,10 +134,6 @@ struct Layer { return fbo; } - inline GLuint* getTexturePointer() { - return &texture.id; - } - inline GLuint getTexture() { return texture.id; } @@ -181,15 +177,31 @@ struct Layer { ANDROID_API void setColorFilter(SkiaColorFilter* filter); inline void bindTexture() { - glBindTexture(renderTarget, texture.id); + if (texture.id) { + glBindTexture(renderTarget, texture.id); + } } inline void generateTexture() { - glGenTextures(1, &texture.id); + if (!texture.id) { + glGenTextures(1, &texture.id); + } } inline void deleteTexture() { - if (texture.id) glDeleteTextures(1, &texture.id); + if (texture.id) { + glDeleteTextures(1, &texture.id); + texture.id = 0; + } + } + + /** + * When the caller frees the texture itself, the caller + * must call this method to tell this layer that it lost + * the texture. + */ + void clearTexture() { + texture.id = 0; } inline void deleteFbo() {