From d98aa2de9ab18e09c2be1997f41212740f51f6e6 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Mon, 25 Oct 2010 15:47:32 -0700 Subject: [PATCH] DisplayList optimizations and fixes. We now use a copy of SkPaint objects to avoid having it changed from under us. We reuse copies that have not changed. We also copy the SkMatrix every time to avoid the same problem. Change-Id: If3fd80698f2d43ea16d23302063e0fd8d0549027 --- core/jni/android/graphics/Matrix.cpp | 6 --- core/jni/android/graphics/Paint.cpp | 6 --- libs/hwui/DisplayListRenderer.cpp | 66 +++++++++++++--------------- libs/hwui/DisplayListRenderer.h | 53 +++++++++++++--------- libs/hwui/ResourceCache.cpp | 42 ------------------ libs/hwui/ResourceCache.h | 8 ---- libs/hwui/TextureCache.cpp | 5 +++ libs/hwui/TextureCache.h | 14 ++++++ 8 files changed, 82 insertions(+), 118 deletions(-) diff --git a/core/jni/android/graphics/Matrix.cpp b/core/jni/android/graphics/Matrix.cpp index b30550606be29..d0871ac5082fb 100644 --- a/core/jni/android/graphics/Matrix.cpp +++ b/core/jni/android/graphics/Matrix.cpp @@ -32,12 +32,6 @@ class SkMatrixGlue { public: static void finalizer(JNIEnv* env, jobject clazz, SkMatrix* obj) { -#ifdef USE_OPENGL_RENDERER - if (android::uirenderer::Caches::hasInstance()) { - android::uirenderer::Caches::getInstance().resourceCache.destructor(obj); - return; - } -#endif // USE_OPENGL_RENDERER delete obj; } diff --git a/core/jni/android/graphics/Paint.cpp b/core/jni/android/graphics/Paint.cpp index 79a02f106d826..e62b034cc6945 100644 --- a/core/jni/android/graphics/Paint.cpp +++ b/core/jni/android/graphics/Paint.cpp @@ -63,12 +63,6 @@ public: }; static void finalizer(JNIEnv* env, jobject clazz, SkPaint* obj) { -#ifdef USE_OPENGL_RENDERER - if (android::uirenderer::Caches::hasInstance()) { - android::uirenderer::Caches::getInstance().resourceCache.destructor(obj); - return; - } -#endif // USE_OPENGL_RENDERER delete obj; } diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp index a43f164ee8f18..a9cd5be5ff85c 100644 --- a/libs/hwui/DisplayListRenderer.cpp +++ b/libs/hwui/DisplayListRenderer.cpp @@ -108,18 +108,7 @@ DisplayList::DisplayList(const DisplayListRenderer& recorder) { mBitmapResources.add(resource); caches.resourceCache.incrementRefcount(resource); } - const Vector &matrixResources = recorder.getMatrixResources(); - for (size_t i = 0; i < matrixResources.size(); i++) { - SkMatrix* resource = matrixResources.itemAt(i); - mMatrixResources.add(resource); - caches.resourceCache.incrementRefcount(resource); - } - const Vector &paintResources = recorder.getPaintResources(); - for (size_t i = 0; i < paintResources.size(); i++) { - SkPaint* resource = paintResources.itemAt(i); - mPaintResources.add(resource); - caches.resourceCache.incrementRefcount(resource); - } + const Vector &shaderResources = recorder.getShaderResources(); for (size_t i = 0; i < shaderResources.size(); i++) { SkiaShader* resource = shaderResources.itemAt(i); @@ -127,6 +116,16 @@ DisplayList::DisplayList(const DisplayListRenderer& recorder) { caches.resourceCache.incrementRefcount(resource); } + const Vector &paints = recorder.getPaints(); + for (size_t i = 0; i < paints.size(); i++) { + mPaints.add(paints.itemAt(i)); + } + + const Vector &matrices = recorder.getMatrices(); + for (size_t i = 0; i < matrices.size(); i++) { + mMatrices.add(matrices.itemAt(i)); + } + mPathHeap = recorder.mPathHeap; mPathHeap->safeRef(); } @@ -137,25 +136,25 @@ DisplayList::~DisplayList() { Caches& caches = Caches::getInstance(); for (size_t i = 0; i < mBitmapResources.size(); i++) { - SkBitmap* resource = mBitmapResources.itemAt(i); - caches.resourceCache.decrementRefcount(resource); + caches.resourceCache.decrementRefcount(mBitmapResources.itemAt(i)); } mBitmapResources.clear(); - for (size_t i = 0; i < mMatrixResources.size(); i++) { - SkMatrix* resource = mMatrixResources.itemAt(i); - caches.resourceCache.decrementRefcount(resource); - } - mMatrixResources.clear(); - for (size_t i = 0; i < mPaintResources.size(); i++) { - SkPaint* resource = mPaintResources.itemAt(i); - caches.resourceCache.decrementRefcount(resource); - } - mPaintResources.clear(); + for (size_t i = 0; i < mShaderResources.size(); i++) { - SkiaShader* resource = mShaderResources.itemAt(i); - caches.resourceCache.decrementRefcount(resource); + caches.resourceCache.decrementRefcount(mShaderResources.itemAt(i)); } mShaderResources.clear(); + + for (size_t i = 0; i < mPaints.size(); i++) { + delete mPaints.itemAt(i); + } + mPaints.clear(); + + for (size_t i = 0; i < mMatrices.size(); i++) { + delete mMatrices.itemAt(i); + } + mMatrices.clear(); + mPathHeap->safeUnref(); } @@ -335,21 +334,16 @@ void DisplayListRenderer::reset() { caches.resourceCache.decrementRefcount(resource); } mBitmapResources.clear(); - for (size_t i = 0; i < mMatrixResources.size(); i++) { - SkMatrix* resource = mMatrixResources.itemAt(i); - caches.resourceCache.decrementRefcount(resource); - } - mMatrixResources.clear(); - for (size_t i = 0; i < mPaintResources.size(); i++) { - SkPaint* resource = mPaintResources.itemAt(i); - caches.resourceCache.decrementRefcount(resource); - } - mPaintResources.clear(); + for (size_t i = 0; i < mShaderResources.size(); i++) { SkiaShader* resource = mShaderResources.itemAt(i); caches.resourceCache.decrementRefcount(resource); } mShaderResources.clear(); + + mPaints.clear(); + mPaintMap.clear(); + mMatrices.clear(); } /////////////////////////////////////////////////////////////////////////////// diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h index c8cd8017e9cd3..ce4cfc5569844 100644 --- a/libs/hwui/DisplayListRenderer.h +++ b/libs/hwui/DisplayListRenderer.h @@ -182,10 +182,11 @@ private: PathHeap* mPathHeap; Vector mBitmapResources; - Vector mMatrixResources; - Vector mPaintResources; Vector mShaderResources; + Vector mPaints; + Vector mMatrices; + mutable SkFlattenableReadBuffer mReader; SkRefCntPlayback mRCPlayback; @@ -263,18 +264,18 @@ public: return mBitmapResources; } - const Vector& getMatrixResources() const { - return mMatrixResources; - } - - const Vector& getPaintResources() const { - return mPaintResources; - } - const Vector& getShaderResources() const { return mShaderResources; } + const Vector& getPaints() const { + return mPaints; + } + + const Vector& getMatrices() const { + return mMatrices; + } + private: inline void addOp(DisplayList::Op drawOp) { mWriter.writeInt(drawOp); @@ -334,20 +335,30 @@ private: } inline void addPaint(SkPaint* paint) { - addInt((int)paint); - mPaintResources.add(paint); - Caches& caches = Caches::getInstance(); - caches.resourceCache.incrementRefcount(paint); + if (paint == NULL) { + addInt((int)NULL); + return; + } + SkPaint *paintCopy = mPaintMap.valueFor(paint); + if (paintCopy == NULL || paintCopy->getGenerationID() != paint->getGenerationID()) { + paintCopy = new SkPaint(*paint); + mPaintMap.add(paint, paintCopy); + mPaints.add(paintCopy); + } + addInt((int)paintCopy); } inline void addMatrix(SkMatrix* matrix) { - addInt((int)matrix); - mMatrixResources.add(matrix); - Caches& caches = Caches::getInstance(); - caches.resourceCache.incrementRefcount(matrix); + // Copying the matrix is cheap and prevents against the user changing the original + // matrix before the operation that uses it + addInt((int) new SkMatrix(*matrix)); } 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 + // contents, and drawing again. The only fix would be to always copy it the first time, + // which doesn't seem worth the extra cycles for this unlikely case. addInt((int)bitmap); mBitmapResources.add(bitmap); Caches& caches = Caches::getInstance(); @@ -364,10 +375,12 @@ private: SkChunkAlloc mHeap; Vector mBitmapResources; - Vector mMatrixResources; - Vector mPaintResources; Vector mShaderResources; + Vector mPaints; + DefaultKeyedVector mPaintMap; + Vector mMatrices; + PathHeap* mPathHeap; SkWriter32 mWriter; diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp index 20b8d6ce5b831..b0fbe65339bbe 100644 --- a/libs/hwui/ResourceCache.cpp +++ b/libs/hwui/ResourceCache.cpp @@ -62,14 +62,6 @@ void ResourceCache::incrementRefcount(SkBitmap* bitmapResource) { incrementRefcount((void*)bitmapResource, kBitmap); } -void ResourceCache::incrementRefcount(SkMatrix* matrixResource) { - incrementRefcount((void*)matrixResource, kMatrix); -} - -void ResourceCache::incrementRefcount(SkPaint* paintResource) { - incrementRefcount((void*)paintResource, kPaint); -} - void ResourceCache::incrementRefcount(SkiaShader* shaderResource) { shaderResource->getSkShader()->safeRef(); incrementRefcount((void*)shaderResource, kShader); @@ -136,34 +128,6 @@ void ResourceCache::destructor(SkBitmap* resource) { } } -void ResourceCache::destructor(SkMatrix* resource) { - ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL; - if (ref == NULL) { - // If we're not tracking this resource, just delete it - delete resource; - return; - } - ref->destroyed = true; - if (ref->refCount == 0) { - deleteResourceReference(resource, ref); - return; - } -} - -void ResourceCache::destructor(SkPaint* resource) { - ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL; - if (ref == NULL) { - // If we're not tracking this resource, just delete it - delete resource; - return; - } - ref->destroyed = true; - if (ref->refCount == 0) { - deleteResourceReference(resource, ref); - return; - } -} - void ResourceCache::destructor(SkiaShader* resource) { ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL; if (ref == NULL) { @@ -196,12 +160,6 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r delete bitmap; } break; - case kMatrix: - delete (SkMatrix*) resource; - break; - case kPaint: - delete (SkPaint*) resource; - break; case kShader: SkiaShader* shader = (SkiaShader*)resource; if (Caches::hasInstance()) { diff --git a/libs/hwui/ResourceCache.h b/libs/hwui/ResourceCache.h index cda27188fe105..b5503674490bd 100644 --- a/libs/hwui/ResourceCache.h +++ b/libs/hwui/ResourceCache.h @@ -18,8 +18,6 @@ #define ANDROID_UI_RESOURCE_CACHE_H #include -#include -#include #include #include @@ -31,8 +29,6 @@ namespace uirenderer { */ enum ResourceType { kBitmap, - kMatrix, - kPaint, kShader, }; @@ -56,8 +52,6 @@ public: ResourceCache(); ~ResourceCache(); void incrementRefcount(SkBitmap* resource); - void incrementRefcount(SkMatrix* resource); - void incrementRefcount(SkPaint* resource); void incrementRefcount(SkiaShader* resource); void incrementRefcount(const void* resource, ResourceType resourceType); void decrementRefcount(void* resource); @@ -66,8 +60,6 @@ public: void recycle(void* resource); void recycle(SkBitmap* resource); void destructor(SkBitmap* resource); - void destructor(SkMatrix* resource); - void destructor(SkPaint* resource); void destructor(SkiaShader* resource); private: void deleteResourceReference(void* resource, ResourceReference* ref); diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp index 701df83589641..d860953bc247d 100644 --- a/libs/hwui/TextureCache.cpp +++ b/libs/hwui/TextureCache.cpp @@ -94,6 +94,8 @@ void TextureCache::operator()(SkBitmap*& bitmap, Texture*& texture) { // This will be called already locked if (texture) { mSize -= texture->bitmapSize; + TEXTURE_LOGD("TextureCache::callback: removed size, mSize = %d, %d", + texture->bitmapSize, mSize); glDeleteTextures(1, &texture->id); delete texture; } @@ -131,6 +133,8 @@ Texture* TextureCache::get(SkBitmap* bitmap) { if (size < mMaxSize) { mLock.lock(); mSize += size; + TEXTURE_LOGD("TextureCache::get: create texture(0x%p): size, mSize = %d, %d", + bitmap, size, mSize); mCache.put(bitmap, texture); mLock.unlock(); } else { @@ -151,6 +155,7 @@ void TextureCache::remove(SkBitmap* bitmap) { void TextureCache::clear() { Mutex::Autolock _l(mLock); mCache.clear(); + TEXTURE_LOGD("TextureCache:clear(), miSize = %d", mSize); } void TextureCache::generateTexture(SkBitmap* bitmap, Texture* texture, bool regenerate) { diff --git a/libs/hwui/TextureCache.h b/libs/hwui/TextureCache.h index 467e851f7200c..671859716d823 100644 --- a/libs/hwui/TextureCache.h +++ b/libs/hwui/TextureCache.h @@ -25,6 +25,20 @@ namespace android { namespace uirenderer { +/////////////////////////////////////////////////////////////////////////////// +// Defines +/////////////////////////////////////////////////////////////////////////////// + +// Debug +#define DEBUG_TEXTURES 0 + +// Debug +#if DEBUG_TEXTURES + #define TEXTURE_LOGD(...) LOGD(__VA_ARGS__) +#else + #define TEXTURE_LOGD(...) +#endif + /** * A simple LRU texture cache. The cache has a maximum size expressed in bytes. * Any texture added to the cache causing the cache to grow beyond the maximum