From 162a0217563f4665da6eb183dfce0fef740f641f Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Thu, 21 Jul 2011 17:02:54 -0700 Subject: Decouple GLES20RecordingCanvas lifetime from GLES20DisplayList. Bug: 5062011 Previously, each GLES20DisplayList would hold onto an instance of GLES20RecordingCanvas. In turn, each GLES20RecordingCanvas held onto an SkWriter with a 16Kb buffer along with several other objects. With one display list per view and hundreds of views, the overhead could add up to a few megabytes. Ensured that the GLES20RecordingCanvas is reset as soon as the display list has been constructed, thereby promptly freeing the 16Kb buffer. Changed GLES20DisplayList so that it acquires a GLES20RecordingCanvas from a pool as needed and recycles it when done. Removed some dead code and cruft related to the construction of GLES20Canvas objects in general. Some code was written with the assumption that the underlying renderer object could change behind the scenes or might be lazily constructed, but that isn't actually the case so we can simplify things. Removed an unnecessary weak reference from GLES20DisplayList to the View. It isn't actually used anywhere. Fixed a bug in GLES20DisplayList where isValid() would return true while the display list was being recorded. This is incorrect because the native display list might not actually exist. Worse, even if the native display list does exist, it is stale and potentially refers to old Bitmaps that have been GC'd (because the mBitmaps list was cleared when recording started). Change-Id: Ib12d5483688cb253478edeb0156d34c476c2566b --- core/java/android/view/DisplayList.java | 9 --- core/java/android/view/GLES20Canvas.java | 51 +++++------- core/java/android/view/GLES20DisplayList.java | 86 ++++++++------------ core/java/android/view/GLES20RecordingCanvas.java | 96 ++++++++++++++++++----- core/java/android/view/HardwareRenderer.java | 6 +- core/java/android/view/View.java | 2 +- core/jni/android_view_GLES20Canvas.cpp | 23 +++--- libs/hwui/DisplayListRenderer.cpp | 11 ++- libs/hwui/DisplayListRenderer.h | 4 +- 9 files changed, 147 insertions(+), 141 deletions(-) diff --git a/core/java/android/view/DisplayList.java b/core/java/android/view/DisplayList.java index 4484d59..f4c0249 100644 --- a/core/java/android/view/DisplayList.java +++ b/core/java/android/view/DisplayList.java @@ -41,15 +41,6 @@ public abstract class DisplayList { abstract void end(); /** - * Indicates whether this display list can be replayed or not. - * - * @return True if the display list can be replayed, false otherwise. - * - * @see android.view.HardwareCanvas#drawDisplayList(DisplayList) - */ - abstract boolean isReady(); - - /** * Invalidates the display list, indicating that it should be repopulated * with new drawing commands prior to being used again. Calling this method * causes calls to {@link #isValid()} to return false. diff --git a/core/java/android/view/GLES20Canvas.java b/core/java/android/view/GLES20Canvas.java index 80244bb..d22fa6e 100644 --- a/core/java/android/view/GLES20Canvas.java +++ b/core/java/android/view/GLES20Canvas.java @@ -51,6 +51,7 @@ class GLES20Canvas extends HardwareCanvas { // The native renderer will be destroyed when this object dies. // DO NOT overwrite this reference once it is set. + @SuppressWarnings("unused") private CanvasFinalizer mFinalizer; private int mWidth; @@ -97,12 +98,8 @@ class GLES20Canvas extends HardwareCanvas { protected GLES20Canvas(boolean record, boolean translucent) { mOpaque = !translucent; - setupRenderer(record); - } - - protected void setupRenderer(boolean record) { if (record) { - mRenderer = nGetDisplayListRenderer(mRenderer); + mRenderer = nCreateDisplayListRenderer(); } else { mRenderer = nCreateRenderer(); } @@ -114,43 +111,31 @@ class GLES20Canvas extends HardwareCanvas { if (mRenderer == 0) { throw new IllegalStateException("Could not create GLES20Canvas renderer"); } else { - mFinalizer = CanvasFinalizer.getFinalizer(mFinalizer, mRenderer); + mFinalizer = new CanvasFinalizer(mRenderer); } } + protected void resetDisplayListRenderer() { + nResetDisplayListRenderer(mRenderer); + } + private static native int nCreateRenderer(); private static native int nCreateLayerRenderer(int layer); - private static native int nGetDisplayListRenderer(int renderer); + private static native int nCreateDisplayListRenderer(); + private static native void nResetDisplayListRenderer(int renderer); private static native void nDestroyRenderer(int renderer); - private static class CanvasFinalizer { - int mRenderer; - - // Factory method returns new instance if old one is null, or old instance - // otherwise, destroying native renderer along the way as necessary - static CanvasFinalizer getFinalizer(CanvasFinalizer oldFinalizer, int renderer) { - if (oldFinalizer == null) { - return new CanvasFinalizer(renderer); - } - oldFinalizer.replaceNativeObject(renderer); - return oldFinalizer; - } + private static final class CanvasFinalizer { + private final int mRenderer; - private CanvasFinalizer(int renderer) { + public CanvasFinalizer(int renderer) { mRenderer = renderer; } - private void replaceNativeObject(int newRenderer) { - if (mRenderer != 0 && newRenderer != mRenderer) { - nDestroyRenderer(mRenderer); - } - mRenderer = newRenderer; - } - @Override protected void finalize() throws Throwable { try { - replaceNativeObject(0); + nDestroyRenderer(mRenderer); } finally { super.finalize(); } @@ -322,11 +307,11 @@ class GLES20Canvas extends HardwareCanvas { // Display list /////////////////////////////////////////////////////////////////////////// - int getDisplayList() { - return nGetDisplayList(mRenderer); + int getDisplayList(int displayList) { + return nGetDisplayList(mRenderer, displayList); } - private static native int nGetDisplayList(int renderer); + private static native int nGetDisplayList(int renderer, int displayList); static void destroyDisplayList(int displayList) { nDestroyDisplayList(displayList); @@ -337,7 +322,7 @@ class GLES20Canvas extends HardwareCanvas { @Override public boolean drawDisplayList(DisplayList displayList, int width, int height, Rect dirty) { return nDrawDisplayList(mRenderer, - ((GLES20DisplayList) displayList).mNativeDisplayList, width, height, dirty); + ((GLES20DisplayList) displayList).getNativeDisplayList(), width, height, dirty); } private static native boolean nDrawDisplayList(int renderer, int displayList, @@ -345,7 +330,7 @@ class GLES20Canvas extends HardwareCanvas { @Override void outputDisplayList(DisplayList displayList) { - nOutputDisplayList(mRenderer, ((GLES20DisplayList) displayList).mNativeDisplayList); + nOutputDisplayList(mRenderer, ((GLES20DisplayList) displayList).getNativeDisplayList()); } private static native void nOutputDisplayList(int renderer, int displayList); diff --git a/core/java/android/view/GLES20DisplayList.java b/core/java/android/view/GLES20DisplayList.java index aeff31f..9e649ce 100644 --- a/core/java/android/view/GLES20DisplayList.java +++ b/core/java/android/view/GLES20DisplayList.java @@ -16,52 +16,50 @@ package android.view; -import java.lang.ref.WeakReference; +import android.graphics.Bitmap; + +import java.util.ArrayList; /** * An implementation of display list for OpenGL ES 2.0. */ class GLES20DisplayList extends DisplayList { - private GLES20Canvas mCanvas; - - private boolean mStarted = false; - private boolean mRecorded = false; - private boolean mValid = false; + // These lists ensure that any Bitmaps recorded by a DisplayList are kept alive as long + // as the DisplayList is alive. The Bitmaps are populated by the GLES20RecordingCanvas. + final ArrayList mBitmaps = new ArrayList(5); - int mNativeDisplayList; - WeakReference hostView; + private GLES20RecordingCanvas mCanvas; + private boolean mValid; // The native display list will be destroyed when this object dies. // DO NOT overwrite this reference once it is set. - @SuppressWarnings("unused") private DisplayListFinalizer mFinalizer; - public GLES20DisplayList(View view) { - hostView = new WeakReference(view); + int getNativeDisplayList() { + if (!mValid || mFinalizer == null) { + throw new IllegalStateException("The display list is not valid."); + } + return mFinalizer.mNativeDisplayList; } @Override HardwareCanvas start() { - if (mStarted) { - throw new IllegalStateException("Recording has already started"); - } - if (mCanvas != null) { - ((GLES20RecordingCanvas) mCanvas).reset(); - } else { - mCanvas = new GLES20RecordingCanvas(true); + throw new IllegalStateException("Recording has already started"); } - mStarted = true; - mRecorded = false; - mValid = true; + mValid = false; + mCanvas = GLES20RecordingCanvas.obtain(this); + mCanvas.start(); return mCanvas; } @Override void invalidate() { - mStarted = false; - mRecorded = false; + if (mCanvas != null) { + mCanvas.recycle(); + mCanvas = null; + } mValid = false; } @@ -73,48 +71,28 @@ class GLES20DisplayList extends DisplayList { @Override void end() { if (mCanvas != null) { - mStarted = false; - mRecorded = true; - - mNativeDisplayList = mCanvas.getDisplayList(); - mFinalizer = DisplayListFinalizer.getFinalizer(mFinalizer, mNativeDisplayList); + if (mFinalizer != null) { + mCanvas.end(mFinalizer.mNativeDisplayList); + } else { + mFinalizer = new DisplayListFinalizer(mCanvas.end(0)); + } + mCanvas.recycle(); + mCanvas = null; + mValid = true; } } - @Override - boolean isReady() { - return !mStarted && mRecorded; - } - private static class DisplayListFinalizer { - int mNativeDisplayList; - - // Factory method returns new instance if old one is null, or old instance - // otherwise, destroying native display list along the way as necessary - static DisplayListFinalizer getFinalizer(DisplayListFinalizer oldFinalizer, - int nativeDisplayList) { - if (oldFinalizer == null) { - return new DisplayListFinalizer(nativeDisplayList); - } - oldFinalizer.replaceNativeObject(nativeDisplayList); - return oldFinalizer; - } + final int mNativeDisplayList; - private DisplayListFinalizer(int nativeDisplayList) { + public DisplayListFinalizer(int nativeDisplayList) { mNativeDisplayList = nativeDisplayList; } - private void replaceNativeObject(int newNativeDisplayList) { - if (mNativeDisplayList != 0 && mNativeDisplayList != newNativeDisplayList) { - GLES20Canvas.destroyDisplayList(mNativeDisplayList); - } - mNativeDisplayList = newNativeDisplayList; - } - @Override protected void finalize() throws Throwable { try { - replaceNativeObject(0); + GLES20Canvas.destroyDisplayList(mNativeDisplayList); } finally { super.finalize(); } diff --git a/core/java/android/view/GLES20RecordingCanvas.java b/core/java/android/view/GLES20RecordingCanvas.java index ec94fe7..078222b 100644 --- a/core/java/android/view/GLES20RecordingCanvas.java +++ b/core/java/android/view/GLES20RecordingCanvas.java @@ -24,8 +24,10 @@ import android.graphics.Path; import android.graphics.Rect; import android.graphics.RectF; import android.graphics.Shader; - -import java.util.ArrayList; +import android.util.Pool; +import android.util.Poolable; +import android.util.PoolableManager; +import android.util.Pools; /** * An implementation of a GL canvas that records drawing operations. @@ -33,62 +35,94 @@ import java.util.ArrayList; * Bitmap objects that it draws, preventing the backing memory of Bitmaps from being freed while * the DisplayList is still holding a native reference to the memory. */ -class GLES20RecordingCanvas extends GLES20Canvas { - // These lists ensure that any Bitmaps recorded by a DisplayList are kept alive as long - // as the DisplayList is alive. - @SuppressWarnings({"MismatchedQueryAndUpdateOfCollection"}) - private final ArrayList mBitmaps = new ArrayList(5); +class GLES20RecordingCanvas extends GLES20Canvas implements Poolable { + // The recording canvas pool should be large enough to handle a deeply nested + // view hierarchy because display lists are generated recursively. + private static final int POOL_LIMIT = 50; + + private static final Pool sPool = Pools.synchronizedPool( + Pools.finitePool(new PoolableManager() { + public GLES20RecordingCanvas newInstance() { + return new GLES20RecordingCanvas(); + } + @Override + public void onAcquired(GLES20RecordingCanvas element) { + } + @Override + public void onReleased(GLES20RecordingCanvas element) { + } + }, POOL_LIMIT)); + + private GLES20RecordingCanvas mNextPoolable; + private boolean mIsPooled; + + private GLES20DisplayList mDisplayList; - GLES20RecordingCanvas(boolean translucent) { - super(true, translucent); + private GLES20RecordingCanvas() { + super(true /*record*/, true /*translucent*/); + } + + static GLES20RecordingCanvas obtain(GLES20DisplayList displayList) { + GLES20RecordingCanvas canvas = sPool.acquire(); + canvas.mDisplayList = displayList; + return canvas; + } + + void recycle() { + mDisplayList = null; + resetDisplayListRenderer(); + sPool.release(this); + } + + void start() { + mDisplayList.mBitmaps.clear(); + } + + int end(int nativeDisplayList) { + return getDisplayList(nativeDisplayList); } private void recordShaderBitmap(Paint paint) { if (paint != null) { final Shader shader = paint.getShader(); if (shader instanceof BitmapShader) { - mBitmaps.add(((BitmapShader) shader).mBitmap); + mDisplayList.mBitmaps.add(((BitmapShader) shader).mBitmap); } } } - void reset() { - mBitmaps.clear(); - setupRenderer(true); - } - @Override public void drawPatch(Bitmap bitmap, byte[] chunks, RectF dst, Paint paint) { super.drawPatch(bitmap, chunks, dst, paint); - mBitmaps.add(bitmap); + mDisplayList.mBitmaps.add(bitmap); // Shaders in the Paint are ignored when drawing a Bitmap } @Override public void drawBitmap(Bitmap bitmap, float left, float top, Paint paint) { super.drawBitmap(bitmap, left, top, paint); - mBitmaps.add(bitmap); + mDisplayList.mBitmaps.add(bitmap); // Shaders in the Paint are ignored when drawing a Bitmap } @Override public void drawBitmap(Bitmap bitmap, Matrix matrix, Paint paint) { super.drawBitmap(bitmap, matrix, paint); - mBitmaps.add(bitmap); + mDisplayList.mBitmaps.add(bitmap); // Shaders in the Paint are ignored when drawing a Bitmap } @Override public void drawBitmap(Bitmap bitmap, Rect src, Rect dst, Paint paint) { super.drawBitmap(bitmap, src, dst, paint); - mBitmaps.add(bitmap); + mDisplayList.mBitmaps.add(bitmap); // Shaders in the Paint are ignored when drawing a Bitmap } @Override public void drawBitmap(Bitmap bitmap, Rect src, RectF dst, Paint paint) { super.drawBitmap(bitmap, src, dst, paint); - mBitmaps.add(bitmap); + mDisplayList.mBitmaps.add(bitmap); // Shaders in the Paint are ignored when drawing a Bitmap } @@ -111,7 +145,7 @@ class GLES20RecordingCanvas extends GLES20Canvas { int vertOffset, int[] colors, int colorOffset, Paint paint) { super.drawBitmapMesh(bitmap, meshWidth, meshHeight, verts, vertOffset, colors, colorOffset, paint); - mBitmaps.add(bitmap); + mDisplayList.mBitmaps.add(bitmap); // Shaders in the Paint are ignored when drawing a Bitmap } @@ -270,4 +304,24 @@ class GLES20RecordingCanvas extends GLES20Canvas { colorOffset, indices, indexOffset, indexCount, paint); recordShaderBitmap(paint); } + + @Override + public GLES20RecordingCanvas getNextPoolable() { + return mNextPoolable; + } + + @Override + public void setNextPoolable(GLES20RecordingCanvas element) { + mNextPoolable = element; + } + + @Override + public boolean isPooled() { + return mIsPooled; + } + + @Override + public void setPooled(boolean isPooled) { + mIsPooled = isPooled; + } } diff --git a/core/java/android/view/HardwareRenderer.java b/core/java/android/view/HardwareRenderer.java index b865b50..503b54b 100644 --- a/core/java/android/view/HardwareRenderer.java +++ b/core/java/android/view/HardwareRenderer.java @@ -189,7 +189,7 @@ public abstract class HardwareRenderer { * * @return A new display list. */ - abstract DisplayList createDisplayList(View v); + abstract DisplayList createDisplayList(); /** * Creates a new hardware layer. A hardware layer built by calling this @@ -852,8 +852,8 @@ public abstract class HardwareRenderer { } @Override - DisplayList createDisplayList(View v) { - return new GLES20DisplayList(v); + DisplayList createDisplayList() { + return new GLES20DisplayList(); } @Override diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index 0108ecf..23bf4bb 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -9830,7 +9830,7 @@ public class View implements Drawable.Callback2, KeyEvent.Callback, Accessibilit // we copy in child display lists into ours in drawChild() mRecreateDisplayList = true; if (mDisplayList == null) { - mDisplayList = mAttachInfo.mHardwareRenderer.createDisplayList(this); + mDisplayList = mAttachInfo.mHardwareRenderer.createDisplayList(); // If we're creating a new display list, make sure our parent gets invalidated // since they will need to recreate their display list to account for this // new child display list. diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp index b0c2f2c..b06de9d 100644 --- a/core/jni/android_view_GLES20Canvas.cpp +++ b/core/jni/android_view_GLES20Canvas.cpp @@ -576,18 +576,18 @@ static void android_view_GLES20Canvas_drawTextRun(JNIEnv* env, jobject clazz, // ---------------------------------------------------------------------------- static DisplayList* android_view_GLES20Canvas_getDisplayList(JNIEnv* env, - jobject clazz, DisplayListRenderer* renderer) { - return renderer->getDisplayList(); + jobject clazz, DisplayListRenderer* renderer, DisplayList* displayList) { + return renderer->getDisplayList(displayList); +} + +static OpenGLRenderer* android_view_GLES20Canvas_createDisplayListRenderer(JNIEnv* env, + jobject clazz) { + return new DisplayListRenderer; } -static OpenGLRenderer* android_view_GLES20Canvas_getDisplayListRenderer(JNIEnv* env, +static void android_view_GLES20Canvas_resetDisplayListRenderer(JNIEnv* env, jobject clazz, DisplayListRenderer* renderer) { - if (renderer == NULL) { - renderer = new DisplayListRenderer; - } else { - renderer->reset(); - } - return renderer; + renderer->reset(); } static void android_view_GLES20Canvas_destroyDisplayList(JNIEnv* env, @@ -812,9 +812,10 @@ static JNINativeMethod gMethods[] = { { "nGetClipBounds", "(ILandroid/graphics/Rect;)Z", (void*) android_view_GLES20Canvas_getClipBounds }, - { "nGetDisplayList", "(I)I", (void*) android_view_GLES20Canvas_getDisplayList }, + { "nGetDisplayList", "(II)I", (void*) android_view_GLES20Canvas_getDisplayList }, { "nDestroyDisplayList", "(I)V", (void*) android_view_GLES20Canvas_destroyDisplayList }, - { "nGetDisplayListRenderer", "(I)I", (void*) android_view_GLES20Canvas_getDisplayListRenderer }, + { "nCreateDisplayListRenderer", "()I", (void*) android_view_GLES20Canvas_createDisplayListRenderer }, + { "nResetDisplayListRenderer", "(I)V", (void*) android_view_GLES20Canvas_resetDisplayListRenderer }, { "nDrawDisplayList", "(IIIILandroid/graphics/Rect;)Z", (void*) android_view_GLES20Canvas_drawDisplayList }, { "nOutputDisplayList", "(II)V", (void*) android_view_GLES20Canvas_outputDisplayList }, diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp index 8b1caeee..886c05c 100644 --- a/libs/hwui/DisplayListRenderer.cpp +++ b/libs/hwui/DisplayListRenderer.cpp @@ -883,7 +883,6 @@ bool DisplayList::replay(OpenGLRenderer& renderer, Rect& dirty, uint32_t level) /////////////////////////////////////////////////////////////////////////////// DisplayListRenderer::DisplayListRenderer(): mWriter(MIN_WRITER_SIZE) { - mDisplayList = NULL; } DisplayListRenderer::~DisplayListRenderer() { @@ -923,13 +922,13 @@ void DisplayListRenderer::reset() { // Operations /////////////////////////////////////////////////////////////////////////////// -DisplayList* DisplayListRenderer::getDisplayList() { - if (mDisplayList == NULL) { - mDisplayList = new DisplayList(*this); +DisplayList* DisplayListRenderer::getDisplayList(DisplayList* displayList) { + if (!displayList) { + displayList = new DisplayList(*this); } else { - mDisplayList->initFromDisplayListRenderer(*this, true); + displayList->initFromDisplayListRenderer(*this, true); } - return mDisplayList; + return displayList; } void DisplayListRenderer::setViewport(int width, int height) { diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h index b83259f..8157631 100644 --- a/libs/hwui/DisplayListRenderer.h +++ b/libs/hwui/DisplayListRenderer.h @@ -217,7 +217,7 @@ public: DisplayListRenderer(); ~DisplayListRenderer(); - DisplayList* getDisplayList(); + DisplayList* getDisplayList(DisplayList* displayList); void setViewport(int width, int height); void prepareDirty(float left, float top, float right, float bottom, bool opaque); @@ -474,8 +474,6 @@ private: SkWriter32 mWriter; - DisplayList *mDisplayList; - int mRestoreSaveCount; friend class DisplayList; -- cgit v1.1