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

Commit 5a7e8288 authored by Chet Haase's avatar Chet Haase
Browse files

Fix crash when Paths are GCd in hw accelerated apps 

A recent change to optimize path rendering didn't account for
the destruction of native objects by the VM finalizer. We may be
done with the Java level version before we're done with the native
structure that's used by the display list. For example, a drawing
method on a View that creates a temporary path to render into the
canvas will implicitly create a native structure that is put onto
the GL display list. That temporary path may go away, but the native
version should stick around as long as the display list does.

The fix is to refcount the original native version of the path
and only delete it when the refcoutn reaches zero (which means that
it is no longer needed by any display list). This is a similar mechanism
used for bitmaps and shaders.

Change-Id: I4de1047415066d425d1c689aa60827f97729b470
parent 23c907ca

core/jni/android/graphics/Path.cpp

+2 −1
Original line number Diff line number Diff line
@@ -36,7 +36,8 @@ public: 
    static void finalizer(JNIEnv* env, jobject clazz, SkPath* obj) {

#ifdef USE_OPENGL_RENDERER

        if (android::uirenderer::Caches::hasInstance()) {

            android::uirenderer::Caches::getInstance().pathCache.removeDeferred(obj);

            android::uirenderer::Caches::getInstance().resourceCache.destructor(obj);

            return;

        }

#endif

        delete obj;

libs/hwui/DisplayListRenderer.cpp

+17 −0
Original line number Diff line number Diff line
@@ -95,6 +95,10 @@ void DisplayList::clearResources() { 
        delete mPaths.itemAt(i);

    }

    mPaths.clear();

    for (size_t i = 0; i < mOriginalPaths.size(); i++) {

        caches.resourceCache.decrementRefcount(mOriginalPaths.itemAt(i));

    }

    mOriginalPaths.clear();



    for (size_t i = 0; i < mMatrices.size(); i++) {

        delete mMatrices.itemAt(i);
@@ -146,6 +150,13 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde 
        mPaths.add(paths.itemAt(i));

    }



    const Vector<SkPath*> &originalPaths = recorder.getOriginalPaths();

    for (size_t i = 0; i < originalPaths.size(); i++) {

        SkPath* path = originalPaths.itemAt(i);

        mOriginalPaths.add(path);

        caches.resourceCache.incrementRefcount(path);

    }



    const Vector<SkMatrix*> &matrices = recorder.getMatrices();

    for (size_t i = 0; i < matrices.size(); i++) {

        mMatrices.add(matrices.itemAt(i));
@@ -519,6 +530,12 @@ void DisplayListRenderer::reset() { 
    }

    mBitmapResources.clear();



    for (size_t i = 0; i < mOriginalPaths.size(); i++) {

        SkPath* resource = mOriginalPaths.itemAt(i);

        caches.resourceCache.decrementRefcount(resource);

    }

    mOriginalPaths.clear();



    for (size_t i = 0; i < mShaders.size(); i++) {

       caches.resourceCache.decrementRefcount(mShaders.itemAt(i));

    }

libs/hwui/DisplayListRenderer.h

+9 −0
Original line number Diff line number Diff line
@@ -190,6 +190,7 @@ private: 


    Vector<SkPaint*> mPaints;

    Vector<SkPath*> mPaths;

    Vector<SkPath*> mOriginalPaths;

    Vector<SkMatrix*> mMatrices;

    Vector<SkiaShader*> mShaders;
@@ -293,6 +294,10 @@ public: 
        return mPaths;

    }



    const Vector<SkPath*>& getOriginalPaths() const {

        return mOriginalPaths;

    }



    const Vector<SkMatrix*>& getMatrices() const {

        return mMatrices;

    }
@@ -371,6 +376,9 @@ private: 
        if (pathCopy == NULL || pathCopy->getGenerationID() != path->getGenerationID()) {

            if (pathCopy == NULL) {

                pathCopy = path;

                mOriginalPaths.add(path);

                Caches& caches = Caches::getInstance();

                caches.resourceCache.incrementRefcount(path);

            } else {

                pathCopy = new SkPath(*path);

                mPaths.add(pathCopy);
@@ -452,6 +460,7 @@ private: 
    Vector<SkPaint*> mPaints;

    DefaultKeyedVector<SkPaint*, SkPaint*> mPaintMap;



    Vector<SkPath*> mOriginalPaths;

    Vector<SkPath*> mPaths;

    DefaultKeyedVector<SkPath*, SkPath*> mPathMap;

libs/hwui/ResourceCache.cpp

+35 −0
Original line number Diff line number Diff line
@@ -65,6 +65,10 @@ void ResourceCache::incrementRefcount(SkBitmap* bitmapResource) { 
    incrementRefcount((void*)bitmapResource, kBitmap);

}



void ResourceCache::incrementRefcount(SkPath* pathResource) {

    incrementRefcount((void*)pathResource, kPath);

}



void ResourceCache::incrementRefcount(SkiaShader* shaderResource) {

    shaderResource->getSkShader()->safeRef();

    incrementRefcount((void*) shaderResource, kShader);
@@ -94,6 +98,10 @@ void ResourceCache::decrementRefcount(SkBitmap* bitmapResource) { 
    decrementRefcount((void*) bitmapResource);

}



void ResourceCache::decrementRefcount(SkPath* pathResource) {

    decrementRefcount((void*) pathResource);

}



void ResourceCache::decrementRefcount(SkiaShader* shaderResource) {

    shaderResource->getSkShader()->safeUnref();

    decrementRefcount((void*) shaderResource);
@@ -122,6 +130,24 @@ void ResourceCache::recycle(SkBitmap* resource) { 
    }

}



void ResourceCache::destructor(SkPath* resource) {

    Mutex::Autolock _l(mLock);

    ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL;

    if (ref == NULL) {

        // If we're not tracking this resource, just delete it

        if (Caches::hasInstance()) {

            Caches::getInstance().pathCache.removeDeferred(resource);

        }

        delete resource;

        return;

    }

    ref->destroyed = true;

    if (ref->refCount == 0) {

        deleteResourceReference(resource, ref);

        return;

    }

}



void ResourceCache::destructor(SkBitmap* resource) {

    Mutex::Autolock _l(mLock);

    ResourceReference* ref = mCache->indexOfKey(resource) >= 0 ? mCache->valueFor(resource) : NULL;
@@ -192,6 +218,15 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r 
                delete bitmap;

            }

            break;

            case kPath:

            {

                SkPath* path = (SkPath*)resource;

                if (Caches::hasInstance()) {

                    Caches::getInstance().pathCache.removeDeferred(path);

                }

                delete path;

            }

            break;

            case kShader:

            {

                SkiaShader* shader = (SkiaShader*)resource;

libs/hwui/ResourceCache.h

+4 −0
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ enum ResourceType { 
    kBitmap,

    kShader,

    kColorFilter,

    kPath,

};



class ResourceReference {
@@ -53,15 +54,18 @@ class ResourceCache { 
public:

    ResourceCache();

    ~ResourceCache();

    void incrementRefcount(SkPath* resource);

    void incrementRefcount(SkBitmap* resource);

    void incrementRefcount(SkiaShader* resource);

    void incrementRefcount(SkiaColorFilter* resource);

    void incrementRefcount(const void* resource, ResourceType resourceType);

    void decrementRefcount(void* resource);

    void decrementRefcount(SkBitmap* resource);

    void decrementRefcount(SkPath* resource);

    void decrementRefcount(SkiaShader* resource);

    void decrementRefcount(SkiaColorFilter* resource);

    void recycle(SkBitmap* resource);

    void destructor(SkPath* resource);

    void destructor(SkBitmap* resource);

    void destructor(SkiaShader* resource);

    void destructor(SkiaColorFilter* resource);