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

6 files changed, 80 insertions, 104 deletions

diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index a43f164..a9cd5be 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<SkMatrix*> &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<SkPaint*> &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<SkiaShader*> &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<SkPaint*> &paints = recorder.getPaints();
+    for (size_t i = 0; i < paints.size(); i++) {
+        mPaints.add(paints.itemAt(i));
+    }
+
+    const Vector<SkMatrix*> &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 c8cd801..ce4cfc5 100644
--- a/libs/hwui/DisplayListRenderer.h
+++ b/libs/hwui/DisplayListRenderer.h
@@ -182,10 +182,11 @@ private:
     PathHeap* mPathHeap;
 
     Vector<SkBitmap*> mBitmapResources;
-    Vector<SkMatrix*> mMatrixResources;
-    Vector<SkPaint*> mPaintResources;
     Vector<SkiaShader*> mShaderResources;
 
+    Vector<SkPaint*> mPaints;
+    Vector<SkMatrix*> mMatrices;
+
     mutable SkFlattenableReadBuffer mReader;
 
     SkRefCntPlayback mRCPlayback;
@@ -263,16 +264,16 @@ public:
         return mBitmapResources;
     }
 
-    const Vector<SkMatrix*>& getMatrixResources() const {
-        return mMatrixResources;
+    const Vector<SkiaShader*>& getShaderResources() const {
+        return mShaderResources;
     }
 
-    const Vector<SkPaint*>& getPaintResources() const {
-        return mPaintResources;
+    const Vector<SkPaint*>& getPaints() const {
+        return mPaints;
     }
 
-    const Vector<SkiaShader*>& getShaderResources() const {
-        return mShaderResources;
+    const Vector<SkMatrix*>& getMatrices() const {
+        return mMatrices;
     }
 
 private:
@@ -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<SkBitmap*> mBitmapResources;
-    Vector<SkMatrix*> mMatrixResources;
-    Vector<SkPaint*> mPaintResources;
     Vector<SkiaShader*> mShaderResources;
 
+    Vector<SkPaint*> mPaints;
+    DefaultKeyedVector<SkPaint *, SkPaint *> mPaintMap;
+    Vector<SkMatrix*> mMatrices;
+
     PathHeap* mPathHeap;
     SkWriter32 mWriter;
 
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 20b8d6c..b0fbe65 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 cda2718..b550367 100644
--- a/libs/hwui/ResourceCache.h
+++ b/libs/hwui/ResourceCache.h
@@ -18,8 +18,6 @@
 #define ANDROID_UI_RESOURCE_CACHE_H
 
 #include <SkBitmap.h>
-#include <SkMatrix.h>
-#include <SkPaint.h>
 #include <SkiaShader.h>
 #include <utils/KeyedVector.h>
 
@@ -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 701df83..d860953 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 467e851..6718597 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