Update HWUI to store its own SkBitmap objects
This enables us to...
1) simplify the lifecycle/ownership between Java and HWUI
2) remove DisplayListRenderer::drawBitmapData and associated logic
3) track pixel lifecycle using standard SkPixelRef refcounting
4) Remove uncessary calls to ref/unref the bitmap's pixels and colorTable
Change-Id: I3c95078da20995444f6388a029414280fd654318
diff --git a/libs/hwui/DisplayList.cpp b/libs/hwui/DisplayList.cpp
index 249ada0..9832e60 100644
--- a/libs/hwui/DisplayList.cpp
+++ b/libs/hwui/DisplayList.cpp
@@ -46,12 +46,6 @@
resourceCache.decrementRefcountLocked(bitmapResources.itemAt(i));
}
- for (size_t i = 0; i < ownedBitmapResources.size(); i++) {
- const SkBitmap* bitmap = ownedBitmapResources.itemAt(i);
- resourceCache.decrementRefcountLocked(bitmap);
- resourceCache.destructorLocked(bitmap);
- }
-
for (size_t i = 0; i < patchResources.size(); i++) {
resourceCache.decrementRefcountLocked(patchResources.itemAt(i));
}
@@ -63,7 +57,6 @@
resourceCache.unlock();
bitmapResources.clear();
- ownedBitmapResources.clear();
patchResources.clear();
sourcePaths.clear();
paints.clear();
diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h
index cfd60ad..011b509 100644
--- a/libs/hwui/DisplayList.h
+++ b/libs/hwui/DisplayList.h
@@ -134,7 +134,6 @@
int projectionReceiveIndex;
Vector<const SkBitmap*> bitmapResources;
- Vector<const SkBitmap*> ownedBitmapResources;
Vector<const Res_png_9patch*> patchResources;
std::vector<std::unique_ptr<const SkPaint>> paints;
diff --git a/libs/hwui/DisplayListOp.h b/libs/hwui/DisplayListOp.h
index 1b5af49..36c14c4 100644
--- a/libs/hwui/DisplayListOp.h
+++ b/libs/hwui/DisplayListOp.h
@@ -734,27 +734,6 @@
Rect mSrc;
};
-class DrawBitmapDataOp : public DrawBitmapOp {
-public:
- DrawBitmapDataOp(const SkBitmap* bitmap, const SkPaint* paint)
- : DrawBitmapOp(bitmap, paint) {}
-
- virtual void applyDraw(OpenGLRenderer& renderer, Rect& dirty) override {
- renderer.drawBitmapData(mBitmap, mPaint);
- }
-
- virtual void output(int level, uint32_t logFlags) const override {
- OP_LOG("Draw bitmap %p", mBitmap);
- }
-
- virtual const char* name() override { return "DrawBitmapData"; }
-
- virtual void onDefer(OpenGLRenderer& renderer, DeferInfo& deferInfo,
- const DeferredDisplayState& state) override {
- deferInfo.batchId = DeferredDisplayList::kOpBatch_Bitmap;
- }
-};
-
class DrawBitmapMeshOp : public DrawBoundedOp {
public:
DrawBitmapMeshOp(const SkBitmap* bitmap, int meshWidth, int meshHeight,
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index ca21e4e..c7de03a 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -266,13 +266,6 @@
}
}
-void DisplayListRenderer::drawBitmapData(const SkBitmap* bitmap, const SkPaint* paint) {
- bitmap = refBitmapData(bitmap);
- paint = refPaint(paint);
-
- addDrawOp(new (alloc()) DrawBitmapDataOp(bitmap, paint));
-}
-
void DisplayListRenderer::drawBitmapMesh(const SkBitmap& bitmap, int meshWidth, int meshHeight,
const float* vertices, const int* colors, const SkPaint* paint) {
int vertexCount = (meshWidth + 1) * (meshHeight + 1);
diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h
index 01fcdd3..8f8d24c 100644
--- a/libs/hwui/DisplayListRenderer.h
+++ b/libs/hwui/DisplayListRenderer.h
@@ -101,8 +101,6 @@
// Bitmap-based
void drawBitmap(const SkBitmap* bitmap, const SkPaint* paint);
- // TODO: move drawBitmapData() to Canvas.h
- void drawBitmapData(const SkBitmap* bitmap, const SkPaint* paint);
// TODO: move drawPatch() to Canvas.h
void drawPatch(const SkBitmap* bitmap, const Res_png_9patch* patch,
float left, float top, float right, float bottom, const SkPaint* paint);
@@ -367,15 +365,9 @@
// 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.
- mDisplayListData->bitmapResources.add(bitmap);
- mResourceCache.incrementRefcount(bitmap);
- return bitmap;
- }
-
- inline const SkBitmap* refBitmapData(const SkBitmap* bitmap) {
- mDisplayListData->ownedBitmapResources.add(bitmap);
- mResourceCache.incrementRefcount(bitmap);
- return bitmap;
+ const SkBitmap* cachedBitmap = mResourceCache.insert(bitmap);
+ mDisplayListData->bitmapResources.add(cachedBitmap);
+ return cachedBitmap;
}
inline const Res_png_9patch* refPatch(const Res_png_9patch* patch) {
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index 3c8fb8b..d1e51e1 100755
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -2006,24 +2006,6 @@
mDirty = true;
}
-void OpenGLRenderer::drawBitmapData(const SkBitmap* bitmap, const SkPaint* paint) {
- if (quickRejectSetupScissor(0, 0, bitmap->width(), bitmap->height())) {
- return;
- }
-
- mCaches.activeTexture(0);
- Texture* texture = mCaches.textureCache.getTransient(bitmap);
- const AutoTexture autoCleanup(texture);
-
- if (CC_UNLIKELY(bitmap->colorType() == kAlpha_8_SkColorType)) {
- drawAlphaBitmap(texture, 0, 0, paint);
- } else {
- drawTextureRect(0, 0, bitmap->width(), bitmap->height(), texture, paint);
- }
-
- mDirty = true;
-}
-
void OpenGLRenderer::drawBitmapMesh(const SkBitmap* bitmap, int meshWidth, int meshHeight,
const float* vertices, const int* colors, const SkPaint* paint) {
if (!vertices || mState.currentlyIgnored()) {
diff --git a/libs/hwui/OpenGLRenderer.h b/libs/hwui/OpenGLRenderer.h
index 9d9b3d2..75e79d6 100755
--- a/libs/hwui/OpenGLRenderer.h
+++ b/libs/hwui/OpenGLRenderer.h
@@ -162,7 +162,6 @@
virtual void drawBitmap(const SkBitmap* bitmap, float srcLeft, float srcTop,
float srcRight, float srcBottom, float dstLeft, float dstTop,
float dstRight, float dstBottom, const SkPaint* paint) override;
- virtual void drawBitmapData(const SkBitmap* bitmap, const SkPaint* paint) override;
virtual void drawBitmapMesh(const SkBitmap* bitmap, int meshWidth, int meshHeight,
const float* vertices, const int* colors, const SkPaint* paint) override;
void drawPatches(const SkBitmap* bitmap, AssetAtlas::Entry* entry,
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index 5a7ea40..23462cc 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -338,11 +338,6 @@
if (subtree) {
TextureCache& cache = Caches::getInstance().textureCache;
info.out.hasFunctors |= subtree->functors.size();
- // TODO: Fix ownedBitmapResources to not require disabling prepareTextures
- // and thus falling out of async drawing path.
- if (subtree->ownedBitmapResources.size()) {
- info.prepareTextures = false;
- }
for (size_t i = 0; info.prepareTextures && i < subtree->bitmapResources.size(); i++) {
info.prepareTextures = cache.prefetchAndMarkInUse(subtree->bitmapResources[i]);
}
diff --git a/libs/hwui/Renderer.h b/libs/hwui/Renderer.h
index 3240bbc..48d83b9 100644
--- a/libs/hwui/Renderer.h
+++ b/libs/hwui/Renderer.h
@@ -155,7 +155,6 @@
virtual void drawBitmap(const SkBitmap* bitmap, float srcLeft, float srcTop,
float srcRight, float srcBottom, float dstLeft, float dstTop,
float dstRight, float dstBottom, const SkPaint* paint) = 0;
- virtual void drawBitmapData(const SkBitmap* bitmap, const SkPaint* paint) = 0;
virtual void drawBitmapMesh(const SkBitmap* bitmap, int meshWidth, int meshHeight,
const float* vertices, const int* colors, const SkPaint* paint) = 0;
virtual void drawPatch(const SkBitmap* bitmap, const Res_png_9patch* patch,
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 45c27c2..2d1adc5 100644
--- a/libs/hwui/ResourceCache.cpp
+++ b/libs/hwui/ResourceCache.cpp
@@ -16,7 +16,6 @@
#define LOG_TAG "OpenGLRenderer"
-#include <SkPixelRef.h>
#include "ResourceCache.h"
#include "Caches.h"
@@ -37,8 +36,8 @@
ResourceReference* ref = mCache->valueAt(i);
ALOGD(" ResourceCache: mCache(%zu): resource, ref = 0x%p, 0x%p",
i, mCache->keyAt(i), mCache->valueAt(i));
- ALOGD(" ResourceCache: mCache(%zu): refCount, recycled, destroyed, type = %d, %d, %d, %d",
- i, ref->refCount, ref->recycled, ref->destroyed, ref->resourceType);
+ ALOGD(" ResourceCache: mCache(%zu): refCount, destroyed, type = %d, %d, %d",
+ i, ref->refCount, ref->destroyed, ref->resourceType);
}
}
@@ -60,15 +59,26 @@
mLock.unlock();
}
+const SkBitmap* ResourceCache::insert(const SkBitmap* bitmapResource) {
+ Mutex::Autolock _l(mLock);
+
+ BitmapKey bitmapKey(bitmapResource);
+ ssize_t index = mBitmapCache.indexOfKey(bitmapKey);
+ if (index == NAME_NOT_FOUND) {
+ SkBitmap* cachedBitmap = new SkBitmap(*bitmapResource);
+ index = mBitmapCache.add(bitmapKey, cachedBitmap);
+ return cachedBitmap;
+ }
+
+ mBitmapCache.keyAt(index).mRefCount++;
+ return mBitmapCache.valueAt(index);
+}
+
void ResourceCache::incrementRefcount(void* resource, ResourceType resourceType) {
Mutex::Autolock _l(mLock);
incrementRefcountLocked(resource, resourceType);
}
-void ResourceCache::incrementRefcount(const SkBitmap* bitmapResource) {
- incrementRefcount((void*) bitmapResource, kBitmap);
-}
-
void ResourceCache::incrementRefcount(const SkPath* pathResource) {
incrementRefcount((void*) pathResource, kPath);
}
@@ -87,25 +97,14 @@
ref->refCount++;
}
-void ResourceCache::incrementRefcountLocked(const SkBitmap* bitmapResource) {
- incrementRefcountLocked((void*) bitmapResource, kBitmap);
-}
-
-void ResourceCache::incrementRefcountLocked(const SkPath* pathResource) {
- incrementRefcountLocked((void*) pathResource, kPath);
-}
-
-void ResourceCache::incrementRefcountLocked(const Res_png_9patch* patchResource) {
- incrementRefcountLocked((void*) patchResource, kNinePatch);
-}
-
void ResourceCache::decrementRefcount(void* resource) {
Mutex::Autolock _l(mLock);
decrementRefcountLocked(resource);
}
void ResourceCache::decrementRefcount(const SkBitmap* bitmapResource) {
- decrementRefcount((void*) bitmapResource);
+ Mutex::Autolock _l(mLock);
+ decrementRefcountLocked(bitmapResource);
}
void ResourceCache::decrementRefcount(const SkPath* pathResource) {
@@ -130,7 +129,20 @@
}
void ResourceCache::decrementRefcountLocked(const SkBitmap* bitmapResource) {
- decrementRefcountLocked((void*) bitmapResource);
+ BitmapKey bitmapKey(bitmapResource);
+ ssize_t index = mBitmapCache.indexOfKey(bitmapKey);
+
+ LOG_ALWAYS_FATAL_IF(index == NAME_NOT_FOUND,
+ "Decrementing the reference of an untracked Bitmap");
+
+ const BitmapKey& cacheEntry = mBitmapCache.keyAt(index);
+ if (cacheEntry.mRefCount == 1) {
+ // delete the bitmap and remove it from the cache
+ delete mBitmapCache.valueAt(index);
+ mBitmapCache.removeItemsAt(index);
+ } else {
+ cacheEntry.mRefCount--;
+ }
}
void ResourceCache::decrementRefcountLocked(const SkPath* pathResource) {
@@ -164,28 +176,6 @@
}
}
-void ResourceCache::destructor(const SkBitmap* resource) {
- Mutex::Autolock _l(mLock);
- destructorLocked(resource);
-}
-
-void ResourceCache::destructorLocked(const SkBitmap* resource) {
- ssize_t index = mCache->indexOfKey(resource);
- ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : nullptr;
- if (ref == nullptr) {
- // If we're not tracking this resource, just delete it
- if (Caches::hasInstance()) {
- Caches::getInstance().textureCache.releaseTexture(resource);
- }
- delete resource;
- return;
- }
- ref->destroyed = true;
- if (ref->refCount == 0) {
- deleteResourceReferenceLocked(resource, ref);
- }
-}
-
void ResourceCache::destructor(Res_png_9patch* resource) {
Mutex::Autolock _l(mLock);
destructorLocked(resource);
@@ -212,64 +202,12 @@
}
/**
- * Return value indicates whether resource was actually recycled, which happens when RefCnt
- * reaches 0.
- */
-bool ResourceCache::recycle(SkBitmap* resource) {
- Mutex::Autolock _l(mLock);
- return recycleLocked(resource);
-}
-
-/**
- * Return value indicates whether resource was actually recycled, which happens when RefCnt
- * reaches 0.
- */
-bool ResourceCache::recycleLocked(SkBitmap* resource) {
- ssize_t index = mCache->indexOfKey(resource);
- if (index < 0) {
- if (Caches::hasInstance()) {
- Caches::getInstance().textureCache.releaseTexture(resource);
- }
- // not tracking this resource; just recycle the pixel data
- resource->setPixels(nullptr, nullptr);
- return true;
- }
- ResourceReference* ref = mCache->valueAt(index);
- if (ref == nullptr) {
- // Should not get here - shouldn't get a call to recycle if we're not yet tracking it
- return true;
- }
- ref->recycled = true;
- if (ref->refCount == 0) {
- deleteResourceReferenceLocked(resource, ref);
- return true;
- }
- // Still referring to resource, don't recycle yet
- return false;
-}
-
-/**
* This method should only be called while the mLock mutex is held (that mutex is grabbed
* by the various destructor() and recycle() methods which call this method).
*/
void ResourceCache::deleteResourceReferenceLocked(const void* resource, ResourceReference* ref) {
- if (ref->recycled && ref->resourceType == kBitmap) {
- SkBitmap* bitmap = (SkBitmap*) resource;
- if (Caches::hasInstance()) {
- Caches::getInstance().textureCache.releaseTexture(bitmap);
- }
- bitmap->setPixels(nullptr, nullptr);
- }
if (ref->destroyed) {
switch (ref->resourceType) {
- case kBitmap: {
- SkBitmap* bitmap = (SkBitmap*) resource;
- if (Caches::hasInstance()) {
- Caches::getInstance().textureCache.releaseTexture(bitmap);
- }
- delete bitmap;
- }
- break;
case kPath: {
SkPath* path = (SkPath*) resource;
if (Caches::hasInstance()) {
@@ -296,5 +234,38 @@
delete ref;
}
+///////////////////////////////////////////////////////////////////////////////
+// Bitmap Key
+///////////////////////////////////////////////////////////////////////////////
+
+void BitmapKey::operator=(const BitmapKey& other) {
+ this->mRefCount = other.mRefCount;
+ this->mBitmapDimensions = other.mBitmapDimensions;
+ this->mPixelRefOrigin = other.mPixelRefOrigin;
+ this->mPixelRefStableID = other.mPixelRefStableID;
+}
+
+bool BitmapKey::operator==(const BitmapKey& other) const {
+ return mPixelRefStableID == other.mPixelRefStableID &&
+ mPixelRefOrigin == other.mPixelRefOrigin &&
+ mBitmapDimensions == other.mBitmapDimensions;
+}
+
+bool BitmapKey::operator<(const BitmapKey& other) const {
+ if (mPixelRefStableID != other.mPixelRefStableID) {
+ return mPixelRefStableID < other.mPixelRefStableID;
+ }
+ if (mPixelRefOrigin.x() != other.mPixelRefOrigin.x()) {
+ return mPixelRefOrigin.x() < other.mPixelRefOrigin.x();
+ }
+ if (mPixelRefOrigin.y() != other.mPixelRefOrigin.y()) {
+ return mPixelRefOrigin.y() < other.mPixelRefOrigin.y();
+ }
+ if (mBitmapDimensions.width() != other.mBitmapDimensions.width()) {
+ return mBitmapDimensions.width() < other.mBitmapDimensions.width();
+ }
+ return mBitmapDimensions.height() < other.mBitmapDimensions.height();
+}
+
}; // namespace uirenderer
}; // namespace android
diff --git a/libs/hwui/ResourceCache.h b/libs/hwui/ResourceCache.h
index a252f6ce..c6483ac 100644
--- a/libs/hwui/ResourceCache.h
+++ b/libs/hwui/ResourceCache.h
@@ -21,6 +21,7 @@
#include <SkBitmap.h>
#include <SkPath.h>
+#include <SkPixelRef.h>
#include <utils/KeyedVector.h>
#include <utils/Singleton.h>
@@ -36,7 +37,6 @@
* Type of Resource being cached
*/
enum ResourceType {
- kBitmap,
kNinePatch,
kPath
};
@@ -45,15 +45,45 @@
public:
ResourceReference(ResourceType type) {
- refCount = 0; recycled = false; destroyed = false; resourceType = type;
+ refCount = 0; destroyed = false; resourceType = type;
}
int refCount;
- bool recycled;
bool destroyed;
ResourceType resourceType;
};
+class BitmapKey {
+public:
+ BitmapKey(const SkBitmap* bitmap)
+ : mRefCount(1)
+ , mBitmapDimensions(bitmap->dimensions())
+ , mPixelRefOrigin(bitmap->pixelRefOrigin())
+ , mPixelRefStableID(bitmap->pixelRef()->getStableID()) { }
+
+ void operator=(const BitmapKey& other);
+ bool operator==(const BitmapKey& other) const;
+ bool operator<(const BitmapKey& other) const;
+
+private:
+ // This constructor is only used by the KeyedVector implementation
+ BitmapKey()
+ : mRefCount(-1)
+ , mBitmapDimensions(SkISize::Make(0,0))
+ , mPixelRefOrigin(SkIPoint::Make(0,0))
+ , mPixelRefStableID(0) { }
+
+ // reference count of all HWUI object using this bitmap
+ mutable int mRefCount;
+
+ SkISize mBitmapDimensions;
+ SkIPoint mPixelRefOrigin;
+ uint32_t mPixelRefStableID;
+
+ friend class ResourceCache;
+ friend class android::key_value_pair_t<BitmapKey, SkBitmap*>;
+};
+
class ANDROID_API ResourceCache: public Singleton<ResourceCache> {
ResourceCache();
~ResourceCache();
@@ -69,13 +99,14 @@
void lock();
void unlock();
- void incrementRefcount(const SkPath* resource);
- void incrementRefcount(const SkBitmap* resource);
- void incrementRefcount(const Res_png_9patch* resource);
+ /**
+ * The cache stores a copy of the provided resource or refs an existing resource
+ * if the bitmap has previously been inserted and returns the cached copy.
+ */
+ const SkBitmap* insert(const SkBitmap* resource);
- void incrementRefcountLocked(const SkPath* resource);
- void incrementRefcountLocked(const SkBitmap* resource);
- void incrementRefcountLocked(const Res_png_9patch* resource);
+ void incrementRefcount(const SkPath* resource);
+ void incrementRefcount(const Res_png_9patch* resource);
void decrementRefcount(const SkBitmap* resource);
void decrementRefcount(const SkPath* resource);
@@ -86,16 +117,11 @@
void decrementRefcountLocked(const Res_png_9patch* resource);
void destructor(SkPath* resource);
- void destructor(const SkBitmap* resource);
void destructor(Res_png_9patch* resource);
void destructorLocked(SkPath* resource);
- void destructorLocked(const SkBitmap* resource);
void destructorLocked(Res_png_9patch* resource);
- bool recycle(SkBitmap* resource);
- bool recycleLocked(SkBitmap* resource);
-
private:
void deleteResourceReferenceLocked(const void* resource, ResourceReference* ref);
@@ -115,6 +141,7 @@
mutable Mutex mLock;
KeyedVector<const void*, ResourceReference*>* mCache;
+ KeyedVector<BitmapKey, SkBitmap*> mBitmapCache;
};
}; // namespace uirenderer
diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp
index 7dc2480..b4d3708 100644
--- a/libs/hwui/TextureCache.cpp
+++ b/libs/hwui/TextureCache.cpp
@@ -233,11 +233,9 @@
return texture;
}
-void TextureCache::releaseTexture(const SkBitmap* bitmap) {
- if (!bitmap || !bitmap->pixelRef()) return;
-
+void TextureCache::releaseTexture(uint32_t pixelRefStableID) {
Mutex::Autolock _l(mLock);
- mGarbage.push(bitmap->pixelRef()->getStableID());
+ mGarbage.push(pixelRefStableID);
}
void TextureCache::clearGarbage() {
diff --git a/libs/hwui/TextureCache.h b/libs/hwui/TextureCache.h
index 0e33e4c..b97d92d 100644
--- a/libs/hwui/TextureCache.h
+++ b/libs/hwui/TextureCache.h
@@ -88,10 +88,10 @@
Texture* getTransient(const SkBitmap* bitmap);
/**
- * Removes the texture associated with the specified bitmap. This is meant
+ * Removes the texture associated with the specified pixelRef. This is meant
* to be called from threads that are not the EGL context thread.
*/
- void releaseTexture(const SkBitmap* bitmap);
+ ANDROID_API void releaseTexture(uint32_t pixelRefStableID);
/**
* Process deferred removals.
*/