diff options
author | Ruben Brunk <rubenbrunk@google.com> | 2013-03-13 00:47:03 -0700 |
---|---|---|
committer | Ruben Brunk <rubenbrunk@google.com> | 2013-03-13 12:32:24 -0700 |
commit | 2d0c9a59d3d742b9b42a133b29a6cfb9e3936ff7 (patch) | |
tree | 259fb802c74391cd4169fb8b1b91743b6876b50d /src/com/android/gallery3d/filtershow | |
parent | b2edf4808b52024ca6f84bfa63e9d167407dc68d (diff) | |
download | android_packages_apps_Snap-2d0c9a59d3d742b9b42a133b29a6cfb9e3936ff7.tar.gz android_packages_apps_Snap-2d0c9a59d3d742b9b42a133b29a6cfb9e3936ff7.tar.bz2 android_packages_apps_Snap-2d0c9a59d3d742b9b42a133b29a6cfb9e3936ff7.zip |
Added locking for FilteringPipeline and ImageFilterRS. Gets rid of leaked threads.
Bug: 8373600
Bug: 8363001
Bug: 8344345
Bug: 8264659
This CL fixes several things:
- FilteringPipeline no longer leaks a handler thread every time photoeditor is reopened.
- ImageFilterRS no longer leaks an RS context and thread every time photoeditor is reopened.
- ImageFilterRS now uses locking.
- ImageFilterRS no longer leaks Bitmap references or static renderscript allocations.
Change-Id: I79ebde1a8ba8ad689791c2af8db0c3c907e06399
Diffstat (limited to 'src/com/android/gallery3d/filtershow')
4 files changed, 90 insertions, 74 deletions
diff --git a/src/com/android/gallery3d/filtershow/FilterShowActivity.java b/src/com/android/gallery3d/filtershow/FilterShowActivity.java index 93bb02483..37b2cd9da 100644 --- a/src/com/android/gallery3d/filtershow/FilterShowActivity.java +++ b/src/com/android/gallery3d/filtershow/FilterShowActivity.java @@ -574,6 +574,7 @@ public class FilterShowActivity extends Activity implements OnItemClickListener, // TODO: Using singletons is a bad design choice for many of these // due static reference leaks and in general. Please refactor. MasterImage.reset(); + ImageFilterRS.destroyRenderScriptContext(); FilteringPipeline.reset(); ImageFilter.resetStatics(); FiltersManager.reset(); diff --git a/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java b/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java index 4aeb580e6..d7e9a62a7 100644 --- a/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java +++ b/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java @@ -30,7 +30,7 @@ import com.android.gallery3d.filtershow.presets.ImagePreset; public class FilteringPipeline implements Handler.Callback { - private static FilteringPipeline sPipeline; + private static volatile FilteringPipeline sPipeline = null; private static final String LOGTAG = "FilteringPipeline"; private ImagePreset mPreviousGeometryPreset = null; private ImagePreset mPreviousFiltersPreset = null; @@ -120,7 +120,7 @@ public class FilteringPipeline implements Handler.Callback { mProcessingHandler = new Handler(mHandlerThread.getLooper(), this); } - public static FilteringPipeline getPipeline() { + public synchronized static FilteringPipeline getPipeline() { if (sPipeline == null) { sPipeline = new FilteringPipeline(); } @@ -173,8 +173,6 @@ public class FilteringPipeline implements Handler.Callback { Allocation.MipmapControl.MIPMAP_NONE, Allocation.USAGE_SCRIPT); mPreviousGeometry = new GeometryMetadata(geometry); - - FiltersManager.getManager().resetBitmapsRS(); return true; } @@ -259,17 +257,19 @@ public class FilteringPipeline implements Handler.Callback { preset.setupEnvironment(); if (request.getType() == RenderingRequest.PARTIAL_RENDERING) { - bitmap = MasterImage.getImage().getImageLoader().getScaleOneImageForPreset(null, preset, + ImageLoader loader = MasterImage.getImage().getImageLoader(); + if (loader == null) { + Log.w(LOGTAG, "loader not yet setup, cannot handle: " + getType(request)); + return; + } + bitmap = loader.getScaleOneImageForPreset(null, preset, request.getBounds(), request.getDestination(), false); if (bitmap == null) { + Log.w(LOGTAG, "could not get bitmap for: " + getType(request)); return; } } - if (request.getType() == RenderingRequest.FILTERS_RENDERING) { - FiltersManager.getManager().resetBitmapsRS(); - } - if (request.getType() != RenderingRequest.ICON_RENDERING && request.getType() != RenderingRequest.PARTIAL_RENDERING) { updateOriginalAllocation(preset); @@ -296,9 +296,6 @@ public class FilteringPipeline implements Handler.Callback { FiltersManager.getManager().freeFilterResources(preset); } - if (request.getType() == RenderingRequest.FILTERS_RENDERING) { - FiltersManager.getManager().resetBitmapsRS(); - } } private void compute(TripleBufferBitmap buffer, ImagePreset preset, int type) { @@ -356,7 +353,8 @@ public class FilteringPipeline implements Handler.Callback { return mPreviewScaleFactor; } - public static void reset() { + public static synchronized void reset() { + sPipeline.mHandlerThread.quit(); sPipeline = null; } } diff --git a/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java b/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java index 215d5d438..f6c3bdd89 100644 --- a/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java +++ b/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java @@ -61,16 +61,6 @@ public abstract class BaseFiltersManager { return null; } - public void resetBitmapsRS() { - for (Class c : mFilters.keySet()) { - ImageFilter filter = mFilters.get(c); - if (filter instanceof ImageFilterRS) { - ImageFilterRS filterRS = (ImageFilterRS) filter; - filterRS.resetBitmap(); - } - } - } - public void freeFilterResources(ImagePreset preset) { if (preset == null) { return; diff --git a/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java b/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java index 978bc8bd1..595aa9b30 100644 --- a/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java +++ b/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java @@ -21,72 +21,76 @@ import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.support.v8.renderscript.*; import android.util.Log; +import android.content.res.Resources; import com.android.gallery3d.R; public abstract class ImageFilterRS extends ImageFilter { - private final String LOGTAG = "ImageFilterRS"; + private static final String LOGTAG = "ImageFilterRS"; - private static RenderScript mRS = null; - protected static Allocation mInPixelsAllocation; - protected static Allocation mOutPixelsAllocation; - private static android.content.res.Resources mResources = null; - private static Bitmap sOldBitmap = null; - private Bitmap mOldBitmap = null; + protected static volatile Allocation mInPixelsAllocation; + protected static volatile Allocation mOutPixelsAllocation; - private boolean mResourcesLoaded = false; + private static volatile RenderScript sRS = null; + private static volatile int sWidth = 0; + private static volatile int sHeight = 0; - private final Bitmap.Config mBitmapConfig = Bitmap.Config.ARGB_8888; + private static volatile Resources sResources = null; + private boolean mResourcesLoaded = false; - public void resetBitmap() { - mOldBitmap = null; - } + private static final Bitmap.Config BITMAP_CONFIG = Bitmap.Config.ARGB_8888; + // This must be used inside block synchronized on ImageFilterRS class object public void prepare(Bitmap bitmap, float scaleFactor, int quality) { - if (sOldBitmap == null - || (bitmap.getWidth() != sOldBitmap.getWidth()) - || (bitmap.getHeight() != sOldBitmap.getHeight())) { - if (mInPixelsAllocation != null) { - mInPixelsAllocation.destroy(); - mInPixelsAllocation = null; + if (mOutPixelsAllocation == null || mInPixelsAllocation == null || + bitmap.getWidth() != sWidth || bitmap.getHeight() != sHeight) { + destroyPixelAllocations(); + Bitmap bitmapBuffer = bitmap; + if (bitmap.getConfig() == null || bitmap.getConfig() != BITMAP_CONFIG) { + bitmapBuffer = bitmap.copy(BITMAP_CONFIG, true); } - if (mOutPixelsAllocation != null) { - mOutPixelsAllocation.destroy(); - mOutPixelsAllocation = null; - } - Bitmap bitmapBuffer = bitmap.copy(mBitmapConfig, true); - mOutPixelsAllocation = Allocation.createFromBitmap(mRS, bitmapBuffer, + mOutPixelsAllocation = Allocation.createFromBitmap(sRS, bitmapBuffer, Allocation.MipmapControl.MIPMAP_NONE, Allocation.USAGE_SCRIPT); - mInPixelsAllocation = Allocation.createTyped(mRS, + mInPixelsAllocation = Allocation.createTyped(sRS, mOutPixelsAllocation.getType()); - sOldBitmap = bitmap; } mInPixelsAllocation.copyFrom(bitmap); - if (mOldBitmap != sOldBitmap || !isResourcesLoaded()) { + if (bitmap.getWidth() != sWidth + || bitmap.getHeight() != sHeight || !isResourcesLoaded()) { freeResources(); - createFilter(mResources, scaleFactor, quality); - mOldBitmap = sOldBitmap; + createFilter(sResources, scaleFactor, quality); + sWidth = bitmap.getWidth(); + sHeight = bitmap.getHeight(); setResourcesLoaded(true); } } + // This must be used inside block synchronized on ImageFilterRS class object abstract public void createFilter(android.content.res.Resources res, float scaleFactor, int quality); + // This must be used inside block synchronized on ImageFilterRS class object abstract public void runFilter(); + // This must be used inside block synchronized on ImageFilterRS class object public void update(Bitmap bitmap) { mOutPixelsAllocation.copyTo(bitmap); } @Override public Bitmap apply(Bitmap bitmap, float scaleFactor, int quality) { - if (bitmap == null) { + if (bitmap == null || bitmap.getWidth() == 0 || bitmap.getHeight() == 0) { return bitmap; } try { - prepare(bitmap, scaleFactor, quality); - runFilter(); - update(bitmap); + synchronized(ImageFilterRS.class) { + if (sRS == null) { + Log.w(LOGTAG, "Cannot apply before calling setRenderScriptContext"); + return bitmap; + } + prepare(bitmap, scaleFactor, quality); + runFilter(); + update(bitmap); + } } catch (android.renderscript.RSIllegalArgumentException e) { Log.e(LOGTAG, "Illegal argument? " + e); } catch (android.renderscript.RSRuntimeException e) { @@ -100,15 +104,21 @@ public abstract class ImageFilterRS extends ImageFilter { return bitmap; } - public static RenderScript getRenderScriptContext() { - return mRS; + public static synchronized RenderScript getRenderScriptContext() { + return sRS; } - public static void setRenderScriptContext(Activity context) { - if (mRS == null) { - mRS = RenderScript.create(context); + public static synchronized void setRenderScriptContext(Activity context) { + if( sRS != null) { + Log.w(LOGTAG, "A prior RS context exists when calling setRenderScriptContext"); + destroyRenderScriptContext(); } - mResources = context.getResources(); + sRS = RenderScript.create(context); + sResources = context.getResources(); + destroyPixelAllocations(); + } + + private static synchronized void destroyPixelAllocations() { if (mInPixelsAllocation != null) { mInPixelsAllocation.destroy(); mInPixelsAllocation = null; @@ -117,36 +127,49 @@ public abstract class ImageFilterRS extends ImageFilter { mOutPixelsAllocation.destroy(); mOutPixelsAllocation = null; } - sOldBitmap = null; + sWidth = 0; + sHeight = 0; + } + + public static synchronized void destroyRenderScriptContext() { + destroyPixelAllocations(); + sRS.destroy(); + sRS = null; + sResources = null; } - private Allocation convertBitmap(Bitmap bitmap) { - return Allocation.createFromBitmap(mRS, bitmap, + private static synchronized Allocation convertBitmap(Bitmap bitmap) { + return Allocation.createFromBitmap(sRS, bitmap, Allocation.MipmapControl.MIPMAP_NONE, Allocation.USAGE_SCRIPT); } - private Allocation convertRGBAtoA(Bitmap bitmap) { - Type.Builder tb_a8 = new Type.Builder(mRS, Element.U8(mRS)); - ScriptC_grey greyConvert = new ScriptC_grey(mRS, mResources, R.raw.grey); + private static synchronized Allocation convertRGBAtoA(Bitmap bitmap) { + Type.Builder tb_a8 = new Type.Builder(sRS, Element.U8(sRS)); + ScriptC_grey greyConvert = new ScriptC_grey(sRS, + sRS.getApplicationContext().getResources(), R.raw.grey); Allocation bitmapTemp = convertBitmap(bitmap); - if (bitmapTemp.getType().getElement().isCompatible(Element.U8(mRS))) { + if (bitmapTemp.getType().getElement().isCompatible(Element.U8(sRS))) { return bitmapTemp; } tb_a8.setX(bitmapTemp.getType().getX()); tb_a8.setY(bitmapTemp.getType().getY()); - Allocation bitmapAlloc = Allocation.createTyped(mRS, tb_a8.create()); + Allocation bitmapAlloc = Allocation.createTyped(sRS, tb_a8.create()); greyConvert.forEach_RGBAtoA(bitmapTemp, bitmapAlloc); return bitmapAlloc; } public Allocation loadResourceAlpha(int resource) { + Resources res = null; + synchronized(ImageFilterRS.class) { + res = sRS.getApplicationContext().getResources(); + } final BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = Bitmap.Config.ALPHA_8; Bitmap bitmap = BitmapFactory.decodeResource( - mRS.getApplicationContext().getResources(), + res, resource, options); Allocation ret = convertRGBAtoA(bitmap); bitmap.recycle(); @@ -154,21 +177,25 @@ public abstract class ImageFilterRS extends ImageFilter { } public Allocation loadResource(int resource) { + Resources res = null; + synchronized(ImageFilterRS.class) { + res = sRS.getApplicationContext().getResources(); + } final BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = Bitmap.Config.ARGB_8888; Bitmap bitmap = BitmapFactory.decodeResource( - mRS.getApplicationContext().getResources(), + res, resource, options); Allocation ret = convertBitmap(bitmap); bitmap.recycle(); return ret; } - public boolean isResourcesLoaded() { + private boolean isResourcesLoaded() { return mResourcesLoaded; } - public void setResourcesLoaded(boolean resourcesLoaded) { + private void setResourcesLoaded(boolean resourcesLoaded) { mResourcesLoaded = resourcesLoaded; } |