Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 55b6f95e authored by Romain Guy's avatar Romain Guy
Browse files

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
parent 9feab843

libs/hwui/AssetAtlas.cpp

+4 −0
Original line number Diff line number Diff line
@@ -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<GraphicBuffer> buffer, int* map, int count) { 
        mImage = NULL;

        mTexture = NULL;

    }



    mGenerationId++;

}



void AssetAtlas::terminate() {

libs/hwui/AssetAtlas.h

+10 −1
Original line number Diff line number Diff line
@@ -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<SkBitmap*, Entry*> mEntries;

}; // class AssetAtlas

libs/hwui/DisplayListOp.h

+43 −17
Original line number Diff line number Diff line
@@ -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) {

            mBitmap(bitmap), mAtlas(Caches::getInstance().assetAtlas) {

        mEntry = mAtlas.getEntry(bitmap);

        if (mEntry) {

            mEntryGenerationId = mAtlas.getGenerationId();

            mUvMapper = mEntry->uvMapper;

        }



    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;

    }



    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;

};

libs/hwui/DisplayListRenderer.cpp

+2 −4
Original line number Diff line number Diff line
@@ -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;

    }

libs/hwui/OpenGLRenderer.cpp

+3 −5
Original line number Diff line number Diff line
@@ -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);