From 55b6f95ee4ace96c97508bcd14483fb4e9dbeaa0 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Thu, 27 Jun 2013 15:27:09 -0700 Subject: Track the atlas' generation ID Bug #9589379 If the atlas is terminated/reinitialized and a view does not invalidate in between it might end up using a stale AssetAtlas::Entry. This change is similar to how 9patch meshes are cached in DrawPatchOp: we simply track the generation ID of the cache to make sure we always use the latest data. Change-Id: Ib5abb3769d2ce0eabe9adc04e320ca27c422019e --- libs/hwui/AssetAtlas.cpp | 4 +++ libs/hwui/AssetAtlas.h | 11 ++++++- libs/hwui/DisplayListOp.h | 60 ++++++++++++++++++++++++++++----------- libs/hwui/DisplayListRenderer.cpp | 6 ++-- libs/hwui/OpenGLRenderer.cpp | 8 ++---- libs/hwui/OpenGLRenderer.h | 4 +-- 6 files changed, 64 insertions(+), 29 deletions(-) (limited to 'libs/hwui') diff --git a/libs/hwui/AssetAtlas.cpp b/libs/hwui/AssetAtlas.cpp index 782c052..d98a538 100644 --- a/libs/hwui/AssetAtlas.cpp +++ b/libs/hwui/AssetAtlas.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#define LOG_TAG "OpenGLRenderer" + #include "AssetAtlas.h" #include "Caches.h" @@ -49,6 +51,8 @@ void AssetAtlas::init(sp buffer, int* map, int count) { mImage = NULL; mTexture = NULL; } + + mGenerationId++; } void AssetAtlas::terminate() { diff --git a/libs/hwui/AssetAtlas.h b/libs/hwui/AssetAtlas.h index 2624907..9afc54d 100644 --- a/libs/hwui/AssetAtlas.h +++ b/libs/hwui/AssetAtlas.h @@ -97,7 +97,7 @@ public: friend class AssetAtlas; }; - AssetAtlas(): mTexture(NULL), mImage(NULL) { } + AssetAtlas(): mTexture(NULL), mImage(NULL), mGenerationId(0) { } ~AssetAtlas() { terminate(); } /** @@ -158,12 +158,21 @@ public: */ Texture* getEntryTexture(SkBitmap* const bitmap) const; + /** + * Returns the current generation id of the atlas. + */ + uint32_t getGenerationId() const { + return mGenerationId; + } + private: void createEntries(Caches& caches, int* map, int count); Texture* mTexture; Image* mImage; + uint32_t mGenerationId; + KeyedVector mEntries; }; // class AssetAtlas diff --git a/libs/hwui/DisplayListOp.h b/libs/hwui/DisplayListOp.h index 6099f5d..2b6f4cd 100644 --- a/libs/hwui/DisplayListOp.h +++ b/libs/hwui/DisplayListOp.h @@ -732,14 +732,12 @@ class DrawBitmapOp : public DrawBoundedOp { public: DrawBitmapOp(SkBitmap* bitmap, float left, float top, SkPaint* paint) : DrawBoundedOp(left, top, left + bitmap->width(), top + bitmap->height(), paint), - mBitmap(bitmap), mAtlasEntry(NULL) { - } - - DrawBitmapOp(SkBitmap* bitmap, float left, float top, SkPaint* paint, - const AssetAtlas::Entry* entry) - : DrawBoundedOp(left, top, left + bitmap->width(), top + bitmap->height(), paint), - mBitmap(bitmap), mAtlasEntry(entry) { - if (entry) mUvMapper = entry->uvMapper; + mBitmap(bitmap), mAtlas(Caches::getInstance().assetAtlas) { + mEntry = mAtlas.getEntry(bitmap); + if (mEntry) { + mEntryGenerationId = mAtlas.getGenerationId(); + mUvMapper = mEntry->uvMapper; + } } virtual status_t applyDraw(OpenGLRenderer& renderer, Rect& dirty) { @@ -747,6 +745,16 @@ public: getPaint(renderer)); } + AssetAtlas::Entry* getAtlasEntry() { + // The atlas entry is stale, let's get a new one + if (mEntry && mEntryGenerationId != mAtlas.getGenerationId()) { + mEntryGenerationId = mAtlas.getGenerationId(); + mEntry = mAtlas.getEntry(mBitmap); + mUvMapper = mEntry->uvMapper; + } + return mEntry; + } + #define SET_TEXTURE(ptr, posRect, offsetRect, texCoordsRect, xDim, yDim) \ TextureVertex::set(ptr++, posRect.xDim - offsetRect.left, posRect.yDim - offsetRect.top, \ texCoordsRect.xDim, texCoordsRect.yDim) @@ -791,7 +799,7 @@ public: } } - return renderer.drawBitmaps(mBitmap, ops.size(), &vertices[0], + return renderer.drawBitmaps(mBitmap, mEntry, ops.size(), &vertices[0], transformed, bounds, mPaint); } @@ -803,7 +811,7 @@ public: virtual void onDefer(OpenGLRenderer& renderer, DeferInfo& deferInfo) { deferInfo.batchId = DeferredDisplayList::kOpBatch_Bitmap; - deferInfo.mergeId = mAtlasEntry ? (mergeid_t) &mAtlasEntry->atlas : (mergeid_t) mBitmap; + deferInfo.mergeId = getAtlasEntry() ? (mergeid_t) &mEntry->atlas : (mergeid_t) mBitmap; // Don't merge A8 bitmaps - the paint's color isn't compared by mergeId, or in // MergingDrawBatch::canMergeWith() @@ -816,7 +824,9 @@ public: const SkBitmap* bitmap() { return mBitmap; } protected: SkBitmap* mBitmap; - const AssetAtlas::Entry* mAtlasEntry; + const AssetAtlas& mAtlas; + uint32_t mEntryGenerationId; + AssetAtlas::Entry* mEntry; UvMapper mUvMapper; }; @@ -934,14 +944,27 @@ public: DrawPatchOp(SkBitmap* bitmap, Res_png_9patch* patch, float left, float top, float right, float bottom, SkPaint* paint) : DrawBoundedOp(left, top, right, bottom, paint), - mBitmap(bitmap), mPatch(patch), mGenerationId(0), mMesh(NULL) { - mEntry = Caches::getInstance().assetAtlas.getEntry(bitmap); + mBitmap(bitmap), mPatch(patch), mGenerationId(0), mMesh(NULL), + mAtlas(Caches::getInstance().assetAtlas) { + mEntry = mAtlas.getEntry(bitmap); + if (mEntry) { + mEntryGenerationId = mAtlas.getGenerationId(); + } }; + AssetAtlas::Entry* getAtlasEntry() { + // The atlas entry is stale, let's get a new one + if (mEntry && mEntryGenerationId != mAtlas.getGenerationId()) { + mEntryGenerationId = mAtlas.getGenerationId(); + mEntry = mAtlas.getEntry(mBitmap); + } + return mEntry; + } + const Patch* getMesh(OpenGLRenderer& renderer) { if (!mMesh || renderer.getCaches().patchCache.getGenerationId() != mGenerationId) { PatchCache& cache = renderer.getCaches().patchCache; - mMesh = cache.get(mEntry, mBitmap->width(), mBitmap->height(), + mMesh = cache.get(getAtlasEntry(), mBitmap->width(), mBitmap->height(), mLocalBounds.getWidth(), mLocalBounds.getHeight(), mPatch); mGenerationId = cache.getGenerationId(); } @@ -1021,13 +1044,14 @@ public: indexCount += opMesh->indexCount; } - return renderer.drawPatches(mBitmap, mEntry, &vertices[0], indexCount, getPaint(renderer)); + return renderer.drawPatches(mBitmap, getAtlasEntry(), + &vertices[0], indexCount, getPaint(renderer)); } virtual status_t applyDraw(OpenGLRenderer& renderer, Rect& dirty) { // We're not calling the public variant of drawPatch() here // This method won't perform the quickReject() since we've already done it at this point - return renderer.drawPatch(mBitmap, getMesh(renderer), mEntry, + return renderer.drawPatch(mBitmap, getMesh(renderer), getAtlasEntry(), mLocalBounds.left, mLocalBounds.top, mLocalBounds.right, mLocalBounds.bottom, getPaint(renderer)); } @@ -1040,7 +1064,7 @@ public: virtual void onDefer(OpenGLRenderer& renderer, DeferInfo& deferInfo) { deferInfo.batchId = DeferredDisplayList::kOpBatch_Patch; - deferInfo.mergeId = mEntry ? (mergeid_t) &mEntry->atlas : (mergeid_t) mBitmap; + deferInfo.mergeId = getAtlasEntry() ? (mergeid_t) &mEntry->atlas : (mergeid_t) mBitmap; deferInfo.mergeable = state.mMatrix.isPureTranslate() && OpenGLRenderer::getXfermodeDirect(mPaint) == SkXfermode::kSrcOver_Mode; deferInfo.opaqueOverBounds = isOpaqueOverBounds() && mBitmap->isOpaque(); @@ -1053,6 +1077,8 @@ private: uint32_t mGenerationId; const Patch* mMesh; + const AssetAtlas& mAtlas; + uint32_t mEntryGenerationId; AssetAtlas::Entry* mEntry; }; diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp index 9113092..ba4c2a0 100644 --- a/libs/hwui/DisplayListRenderer.cpp +++ b/libs/hwui/DisplayListRenderer.cpp @@ -262,8 +262,7 @@ status_t DisplayListRenderer::drawBitmap(SkBitmap* bitmap, float left, float top bitmap = refBitmap(bitmap); paint = refPaint(paint); - const AssetAtlas::Entry* entry = mCaches.assetAtlas.getEntry(bitmap); - addDrawOp(new (alloc()) DrawBitmapOp(bitmap, left, top, paint, entry)); + addDrawOp(new (alloc()) DrawBitmapOp(bitmap, left, top, paint)); return DrawGlInfo::kStatusDone; } @@ -287,8 +286,7 @@ status_t DisplayListRenderer::drawBitmap(SkBitmap* bitmap, float srcLeft, float (srcBottom - srcTop == dstBottom - dstTop) && (srcRight - srcLeft == dstRight - dstLeft)) { // transform simple rect to rect drawing case into position bitmap ops, since they merge - const AssetAtlas::Entry* entry = mCaches.assetAtlas.getEntry(bitmap); - addDrawOp(new (alloc()) DrawBitmapOp(bitmap, dstLeft, dstTop, paint, entry)); + addDrawOp(new (alloc()) DrawBitmapOp(bitmap, dstLeft, dstTop, paint)); return DrawGlInfo::kStatusDone; } diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp index 6f1dc6f..73c0453 100644 --- a/libs/hwui/OpenGLRenderer.cpp +++ b/libs/hwui/OpenGLRenderer.cpp @@ -1168,8 +1168,6 @@ void OpenGLRenderer::composeLayerRegion(Layer* layer, const Rect& rect) { return; } - // TODO: See LayerRenderer.cpp::generateMesh() for important - // information about this implementation if (CC_LIKELY(!layer->region.isEmpty())) { size_t count; const android::Rect* rects; @@ -2055,10 +2053,10 @@ void OpenGLRenderer::drawAlphaBitmap(Texture* texture, float left, float top, Sk * will not set the scissor enable or dirty the current layer, if any. * The caller is responsible for properly dirtying the current layer. */ -status_t OpenGLRenderer::drawBitmaps(SkBitmap* bitmap, int bitmapCount, TextureVertex* vertices, - bool transformed, const Rect& bounds, SkPaint* paint) { +status_t OpenGLRenderer::drawBitmaps(SkBitmap* bitmap, AssetAtlas::Entry* entry, int bitmapCount, + TextureVertex* vertices, bool transformed, const Rect& bounds, SkPaint* paint) { mCaches.activeTexture(0); - Texture* texture = getTexture(bitmap); + Texture* texture = entry ? entry->texture : mCaches.textureCache.get(bitmap); if (!texture) return DrawGlInfo::kStatusDone; const AutoTexture autoCleanup(texture); diff --git a/libs/hwui/OpenGLRenderer.h b/libs/hwui/OpenGLRenderer.h index aa0f9fc..5e731b4 100644 --- a/libs/hwui/OpenGLRenderer.h +++ b/libs/hwui/OpenGLRenderer.h @@ -288,8 +288,8 @@ public: virtual void outputDisplayList(DisplayList* displayList); virtual status_t drawLayer(Layer* layer, float x, float y); virtual status_t drawBitmap(SkBitmap* bitmap, float left, float top, SkPaint* paint); - status_t drawBitmaps(SkBitmap* bitmap, int bitmapCount, TextureVertex* vertices, - bool transformed, const Rect& bounds, SkPaint* paint); + status_t drawBitmaps(SkBitmap* bitmap, AssetAtlas::Entry* entry, int bitmapCount, + TextureVertex* vertices, bool transformed, const Rect& bounds, SkPaint* paint); virtual status_t drawBitmap(SkBitmap* bitmap, SkMatrix* matrix, SkPaint* paint); virtual status_t drawBitmap(SkBitmap* bitmap, float srcLeft, float srcTop, float srcRight, float srcBottom, float dstLeft, float dstTop, -- cgit v1.1