diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java index a4daa3be3b..2b18540210 100644 --- a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java +++ b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java @@ -69,6 +69,15 @@ public static void init(@NonNull Context context, @NonNull GlideBuilder builder) Glide.init(context, builder); } + /** + * @see Glide#enableHardwareBitmaps() + */ + @VisibleForTesting + @SuppressLint("VisibleForTests") + public static void enableHardwareBitmaps() { + Glide.enableHardwareBitmaps(); + } + /** * @see Glide#tearDown() */ diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java index 1da10d6cb6..2c023b3755 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java @@ -121,6 +121,7 @@ public void loadTransparentGifResource_asHardware_withNoOtherLoaders_decodesReso assumeTrue( "Hardware Bitmaps are only supported on O+", Build.VERSION.SDK_INT >= Build.VERSION_CODES.O); + Glide.enableHardwareBitmaps(); Glide.get(context) .getRegistry() diff --git a/library/src/main/java/com/bumptech/glide/Glide.java b/library/src/main/java/com/bumptech/glide/Glide.java index 554e7b37eb..8e9160fc6d 100644 --- a/library/src/main/java/com/bumptech/glide/Glide.java +++ b/library/src/main/java/com/bumptech/glide/Glide.java @@ -63,6 +63,7 @@ import com.bumptech.glide.load.resource.bitmap.DefaultImageHeaderParser; import com.bumptech.glide.load.resource.bitmap.Downsampler; import com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser; +import com.bumptech.glide.load.resource.bitmap.HardwareConfigState; import com.bumptech.glide.load.resource.bitmap.InputStreamBitmapImageDecoderResourceDecoder; import com.bumptech.glide.load.resource.bitmap.ParcelFileDescriptorBitmapDecoder; import com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder; @@ -235,6 +236,19 @@ public static void init(@NonNull Context context, @NonNull GlideBuilder builder) } } + /** + * Allows hardware Bitmaps to be used prior to the first frame in the app being drawn as soon as + * this method is called. + * + *

If you use this method in non-test code, your app will experience native crashes on some + * versions of Android if you try to decode a hardware Bitmap. This method is only useful for + * testing. + */ + @VisibleForTesting + public static void enableHardwareBitmaps() { + HardwareConfigState.getInstance().unblockHardwareBitmaps(); + } + @VisibleForTesting public static void tearDown() { synchronized (Glide.class) { @@ -373,7 +387,6 @@ private static void throwIncorrectGlideModule(Exception e) { @NonNull List> defaultRequestListeners, boolean isLoggingRequestOriginsEnabled, boolean isImageDecoderEnabledForBitmaps, - boolean blockHardwareBitmaps, int manualOverrideHardwareBitmapMaxFdCount) { this.engine = engine; this.bitmapPool = bitmapPool; diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index 50c774e5d0..46aee07786 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -66,9 +66,7 @@ public RequestOptions build() { private boolean isLoggingRequestOriginsEnabled; private boolean isImageDecoderEnabledForBitmaps; - private boolean waitForFirstFrameBeforeEnablingHardwareBitmaps; private int manualOverrideHardwareBitmapMaxFdCount = HardwareConfigState.NO_MAX_FD_COUNT; - private boolean waitForCallBeforeEnablingHardwareBitmaps; /** * Sets the {@link com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool} implementation to use @@ -563,11 +561,7 @@ Glide build(@NonNull Context context) { } RequestManagerRetriever requestManagerRetriever = - new RequestManagerRetriever( - requestManagerFactory, waitForFirstFrameBeforeEnablingHardwareBitmaps); - - boolean blockHardwareBitmaps = - waitForFirstFrameBeforeEnablingHardwareBitmaps || waitForCallBeforeEnablingHardwareBitmaps; + new RequestManagerRetriever(requestManagerFactory); return new Glide( context, @@ -583,7 +577,6 @@ Glide build(@NonNull Context context) { defaultRequestListeners, isLoggingRequestOriginsEnabled, isImageDecoderEnabledForBitmaps, - blockHardwareBitmaps, manualOverrideHardwareBitmapMaxFdCount); } } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java index d7e62c5ba0..eef183a4b4 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java @@ -15,6 +15,15 @@ * Android O+. */ public final class HardwareConfigState { + /** + * Force the state to wait until a call to allow hardware Bitmaps to be used when they'd otherwise + * be eligible to work around a framework issue pre Q that can cause a native crash when + * allocating a hardware Bitmap in this specific circumstance. See b/126573603#comment12 for + * details. + */ + private static final boolean BLOCK_HARDWARE_BITMAPS_BY_DEFAULT = + Build.VERSION.SDK_INT < Build.VERSION_CODES.Q; + /** * The minimum size in pixels a {@link Bitmap} must be in both dimensions to be created with the * {@link Bitmap.Config#HARDWARE} configuration. @@ -66,7 +75,6 @@ public final class HardwareConfigState { public static final int NO_MAX_FD_COUNT = -1; private static volatile HardwareConfigState instance; - private static volatile boolean blockHardwareBitmapsByDefault; private static volatile int manualOverrideMaxFdCount = NO_MAX_FD_COUNT; private final boolean isHardwareConfigAllowedByDeviceModel; @@ -104,6 +112,10 @@ public static HardwareConfigState getInstance() { } } + public void unblockHardwareBitmaps() { + areHardwareBitmapsUnblocked = true; + } + public boolean isHardwareConfigAllowed( int targetWidth, int targetHeight, @@ -112,7 +124,7 @@ public boolean isHardwareConfigAllowed( if (!isHardwareConfigAllowed || !isHardwareConfigAllowedByDeviceModel || Build.VERSION.SDK_INT < Build.VERSION_CODES.O - || (blockHardwareBitmapsByDefault && !areHardwareBitmapsUnblocked) + || areHardwareBitmapsBlockedByAppState() || isExifOrientationRequired) { return false; } @@ -123,6 +135,10 @@ public boolean isHardwareConfigAllowed( && isFdSizeBelowHardwareLimit(); } + private boolean areHardwareBitmapsBlockedByAppState() { + return BLOCK_HARDWARE_BITMAPS_BY_DEFAULT && !areHardwareBitmapsUnblocked; + } + @TargetApi(Build.VERSION_CODES.O) boolean setHardwareConfigIfAllowed( int targetWidth, diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java index 0a4c02f108..fe747a7c0b 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java @@ -70,12 +70,10 @@ public class RequestManagerRetriever implements Handler.Callback { // This is really misplaced here, but to put it anywhere else means duplicating all of the // Fragment/Activity extraction logic that already exists here. It's gross, but less likely to // break. - @Nullable private final FirstFrameWaiter firstFrameWaiter; + private final FirstFrameWaiter firstFrameWaiter = new FirstFrameWaiter(); - public RequestManagerRetriever( - @Nullable RequestManagerFactory factory, boolean addFirstFrameWaiter) { + public RequestManagerRetriever(@Nullable RequestManagerFactory factory) { this.factory = factory != null ? factory : DEFAULT_FACTORY; - firstFrameWaiter = addFirstFrameWaiter ? new FirstFrameWaiter() : null; handler = new Handler(Looper.getMainLooper(), this /* Callback */); } @@ -126,19 +124,13 @@ public RequestManager get(@NonNull Context context) { return getApplicationManager(context); } - private void maybeRegisterFirstFrameWaiter(@NonNull Activity activity) { - if (firstFrameWaiter != null) { - firstFrameWaiter.registerSelf(activity); - } - } - @NonNull public RequestManager get(@NonNull FragmentActivity activity) { if (Util.isOnBackgroundThread()) { return get(activity.getApplicationContext()); } else { assertNotDestroyed(activity); - maybeRegisterFirstFrameWaiter(activity); + firstFrameWaiter.registerSelf(activity); FragmentManager fm = activity.getSupportFragmentManager(); return supportFragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity)); } @@ -152,7 +144,13 @@ public RequestManager get(@NonNull Fragment fragment) { if (Util.isOnBackgroundThread()) { return get(fragment.getContext().getApplicationContext()); } else { - maybeRegisterFirstFrameWaiter(fragment.getActivity()); + // In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's + // not all that much we can do here. Most apps will be started with a standard activity. If + // we manage not to register the first frame waiter for a while, the consequences are not + // catastrophic, we'll just use some extra memory. + if (fragment.getActivity() != null) { + firstFrameWaiter.registerSelf(fragment.getActivity()); + } FragmentManager fm = fragment.getChildFragmentManager(); return supportFragmentGet(fragment.getContext(), fm, fragment, fragment.isVisible()); } @@ -167,7 +165,7 @@ public RequestManager get(@NonNull Activity activity) { return get((FragmentActivity) activity); } else { assertNotDestroyed(activity); - maybeRegisterFirstFrameWaiter(activity); + firstFrameWaiter.registerSelf(activity); android.app.FragmentManager fm = activity.getFragmentManager(); return fragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity)); } @@ -347,7 +345,13 @@ public RequestManager get(@NonNull android.app.Fragment fragment) { if (Util.isOnBackgroundThread() || Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) { return get(fragment.getActivity().getApplicationContext()); } else { - maybeRegisterFirstFrameWaiter(fragment.getActivity()); + // In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's + // not all that much we can do here. Most apps will be started with a standard activity. If + // we manage not to register the first frame waiter for a while, the consequences are not + // catastrophic, we'll just use some extra memory. + if (fragment.getActivity() != null) { + firstFrameWaiter.registerSelf(fragment.getActivity()); + } android.app.FragmentManager fm = fragment.getChildFragmentManager(); return fragmentGet(fragment.getActivity(), fm, fragment, fragment.isVisible()); } diff --git a/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java b/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java index 39fd47ff8e..aca894e069 100644 --- a/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java +++ b/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java @@ -21,6 +21,7 @@ public class HardwareConfigStateTest { public void setHardwareConfigIfAllowed_withAllowedState_setsInPreferredConfigAndMutable_returnsFalse() { HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); boolean result = state.setHardwareConfigIfAllowed( @@ -39,6 +40,7 @@ public class HardwareConfigStateTest { @Test public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_doesNotSetValues() { HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -60,6 +62,7 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_does @Test public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doesNotSetValues() { HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -82,6 +85,7 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe public void setHardwareConfigIfAllowed_withHardwareConfigDisallowed_returnsFalse_doesNotSetValues() { HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -104,6 +108,7 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe public void setHardwareConfigIfAllowed_withExifOrientationRequired_returnsFalse_doesNotSetValues() { HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -125,6 +130,29 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe @Test public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetValues() { HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); + BitmapFactory.Options options = new BitmapFactory.Options(); + options.inPreferredConfig = null; + options.inMutable = true; + + boolean result = + state.setHardwareConfigIfAllowed( + /*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION_O, + /*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION_O, + options, + /*isHardwareConfigAllowed=*/ true, + /*isExifOrientationRequired=*/ false); + + assertThat(result).isFalse(); + assertThat(options.inMutable).isTrue(); + assertThat(options.inPreferredConfig).isNull(); + } + + @Config(sdk = Build.VERSION_CODES.P) + @Test + public void + setHardwareConfigIfAllowed_withOsLessThanQ_beforeUnblockingHardwareBitmaps_returnsFalseAndDoesNotSetValues() { + HardwareConfigState state = new HardwareConfigState(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -152,6 +180,7 @@ public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetVa }) { ShadowBuild.setModel(model); HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -179,6 +208,7 @@ public void setHardwareConfigIfAllowed_withDisallowedSamsungDevices_OMR1_returns }) { ShadowBuild.setModel(model); HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; @@ -206,6 +236,7 @@ public void setHardwareConfigIfAllowed_withShortEmptyOrNullModelNames_returnsTru new String[] {null, ".", "-", "", "S", "SM", "SM-", "SM-G", "SM-G9.", "SM-G93"}) { ShadowBuild.setModel(model); HardwareConfigState state = new HardwareConfigState(); + state.unblockHardwareBitmaps(); BitmapFactory.Options options = new BitmapFactory.Options(); options.inPreferredConfig = null; options.inMutable = true; diff --git a/library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java b/library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java index 1fb865b5f9..e2ffbcfab3 100644 --- a/library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java +++ b/library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java @@ -58,7 +58,7 @@ public class RequestManagerRetrieverTest { public void setUp() { appContext = ApplicationProvider.getApplicationContext(); - retriever = new RequestManagerRetriever(/*factory=*/ null, /* addFirstFrameWaiter= */ false); + retriever = new RequestManagerRetriever(/*factory=*/ null); harnesses = new RetrieverHarness[] {new DefaultRetrieverHarness(), new SupportRetrieverHarness()};