From 0e3d3df6915bd25b27b449ffb6143f6cb7ef26c9 Mon Sep 17 00:00:00 2001 From: falhassen Date: Mon, 22 Jan 2024 16:53:30 -0800 Subject: [PATCH] Add workaround for AOSP bug with decoding single channel hardware gainmaps using BitmapFactory. Android U devices that use skiagl for hwui renderering (currently the vast majority) have a bug where uplading single channel bitmaps to hardware bitmaps fails. This bug essentially breaks single channel gainmap rendering using hardware bitmaps, which is a common use case on devices that have Ultra HDR as the default capture format. See https://github.com/bumptech/glide/pull/5357 for more details. The workaround is as follows: 1. Detect the presence of the bug by attempting to copy a 1x1 single channel bitmap to hardware. 2. Memoize the result 3. For BitmapFactory.decode* calls, if the input bitmap has a gainmap, copy it to a three-channel gainmap. This step seems wasteful, but in practice, the Android OS already does some copying when creating hardware bitmaps. Also, more importantly, there is an AOSP fix for this bug, and as devices are updated with this fix, this flow will no longer be exercised. --- .../bitmap/DownsamplerEmulatorTest.java | 239 ++++++++++++-- .../java/com/bumptech/glide/GlideBuilder.java | 14 + .../com/bumptech/glide/RegistryFactory.java | 4 +- .../load/resource/bitmap/Downsampler.java | 42 ++- .../bitmap/GlideBitmapFactoryWrapper.java | 291 ++++++++++++++++++ .../load/resource/bitmap/ImageReader.java | 70 ++++- scripts/run_instrumentation_tests.sh | 1 + 7 files changed, 619 insertions(+), 42 deletions(-) create mode 100644 library/src/main/java/com/bumptech/glide/load/resource/bitmap/GlideBitmapFactoryWrapper.java diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java index 1ba9e54938..2e4716a88b 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java @@ -15,10 +15,13 @@ import android.graphics.Bitmap; import android.graphics.Bitmap.CompressFormat; import android.graphics.Bitmap.Config; +import android.graphics.Gainmap; import android.os.Build; +import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; import android.util.DisplayMetrics; import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; import androidx.exifinterface.media.ExifInterface; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -128,6 +131,26 @@ public void calculateScaling_withAtMost() throws IOException { .run(); } + @Test + public void calculateScaling_withGainmap_androidU_withAtMost() throws IOException { + new Tester(DownsampleStrategy.AT_MOST) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenGainmapImageWithDimensionsOf( + 3024, + 4032, + /* allowHardwareConfig= */ false, + atAndAbove(34) + .with(new Formats(new CompressFormat[] {CompressFormat.JPEG}, 1512, 2016))) + .givenGainmapImageWithDimensionsOf( + 3024, + 4032, + /* allowHardwareConfig= */ true, + atAndAbove(34) + .with(new Formats(new CompressFormat[] {CompressFormat.JPEG}, 1512, 2016))) + .run(); + } + @Test public void calculateScaling_withAtLeast() throws IOException { new Tester(DownsampleStrategy.AT_LEAST) @@ -417,14 +440,19 @@ private static String runScaleTest( int targetWidth, int targetHeight, int exifOrientation, + boolean hasGainmap, + boolean allowHardwareConfig, DownsampleStrategy strategy, int expectedWidth, int expectedHeight) throws IOException { - Downsampler downsampler = buildDownsampler(); + Downsampler downsampler = + hasGainmap ? buildDownsamplerWithGainmapBugFixes() : buildDownsampler(); - InputStream is = openBitmapStream(format, initialWidth, initialHeight, exifOrientation); + InputStream is = + openBitmapStream(format, initialWidth, initialHeight, exifOrientation, hasGainmap); Options options = new Options().set(DownsampleStrategy.OPTION, strategy); + options.set(Downsampler.ALLOW_HARDWARE_CONFIG, allowHardwareConfig); Bitmap bitmap; try { bitmap = downsampler.decode(is, targetWidth, targetHeight, options).get(); @@ -439,6 +467,8 @@ private static String runScaleTest( + strategy + ", orientation: " + exifOrientation + + ", allowHardwareConfig: " + + allowHardwareConfig + " -" + " Initial " + readableDimens(initialWidth, initialHeight) @@ -449,7 +479,35 @@ private static String runScaleTest( + " but threw OutOfMemoryError"; } try { - if (bitmap.getWidth() != expectedWidth || bitmap.getHeight() != expectedHeight) { + if (VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE + && (bitmap.getWidth() != expectedWidth + || bitmap.getHeight() != expectedHeight + || bitmap.hasGainmap() != hasGainmap)) { + return "API: " + + Build.VERSION.SDK_INT + + ", os: " + + Build.VERSION.RELEASE + + ", format: " + + format + + ", strategy: " + + strategy + + ", orientation: " + + exifOrientation + + ", hasGainmap: " + + hasGainmap + + ", allowHardwareConfig: " + + allowHardwareConfig + + " -" + + " Initial " + + readableDimens(initialWidth, initialHeight) + + " Target " + + readableDimens(targetWidth, targetHeight) + + " Expected " + + readableDimensAndHasGainmap(expectedWidth, expectedHeight, hasGainmap) + + ", but Received " + + readableDimensAndHasGainmap( + bitmap.getWidth(), bitmap.getHeight(), bitmap.hasGainmap()); + } else if (bitmap.getWidth() != expectedWidth || bitmap.getHeight() != expectedHeight) { return "API: " + Build.VERSION.SDK_INT + ", os: " @@ -460,6 +518,8 @@ private static String runScaleTest( + strategy + ", orientation: " + exifOrientation + + ", allowHardwareConfig: " + + allowHardwareConfig + " -" + " Initial " + readableDimens(initialWidth, initialHeight) @@ -480,6 +540,10 @@ private static String readableDimens(int width, int height) { return "[" + width + "x" + height + "]"; } + private static String readableDimensAndHasGainmap(int width, int height, boolean hasGainmap) { + return "[" + width + "x" + height + "], hasGainmap=" + hasGainmap; + } + private static Downsampler buildDownsampler() { List parsers = Collections.singletonList(new DefaultImageHeaderParser()); @@ -491,8 +555,25 @@ private static Downsampler buildDownsampler() { return new Downsampler(parsers, displayMetrics, bitmapPool, arrayPool); } + private static Downsampler buildDownsamplerWithGainmapBugFixes() { + List parsers = + Collections.singletonList(new DefaultImageHeaderParser()); + DisplayMetrics displayMetrics = new DisplayMetrics(); + // XHDPI. + displayMetrics.densityDpi = 320; + BitmapPool bitmapPool = new BitmapPoolAdapter(); + ArrayPool arrayPool = new LruArrayPool(); + return new Downsampler( + parsers, + displayMetrics, + bitmapPool, + arrayPool, + /* preserveGainmapAndColorSpaceForTransformations= */ true, + /* enableHardwareGainmapFixOnU= */ true); + } + private static InputStream openBitmapStream( - CompressFormat format, int width, int height, int exifOrientation) { + CompressFormat format, int width, int height, int exifOrientation, boolean hasGainmap) { Preconditions.checkArgument( format == CompressFormat.JPEG || exifOrientation == ExifInterface.ORIENTATION_UNDEFINED, "Can only orient JPEGs, but asked for orientation: " @@ -502,13 +583,14 @@ private static InputStream openBitmapStream( // TODO: support orientations for formats other than JPEG. if (format == CompressFormat.JPEG) { - return openFileStream(width, height, exifOrientation); + return openFileStream(width, height, exifOrientation, hasGainmap); } else { - return openInMemoryStream(format, width, height); + return openInMemoryStream(format, width, height, hasGainmap); } } - private static InputStream openFileStream(int width, int height, int exifOrientation) { + private static InputStream openFileStream( + int width, int height, int exifOrientation, boolean hasGainmap) { int rotationDegrees = TransformationUtils.getExifOrientationDegrees(exifOrientation); if (rotationDegrees == 270 || rotationDegrees == 90) { int temp = width; @@ -517,12 +599,15 @@ private static InputStream openFileStream(int width, int height, int exifOrienta } Bitmap bitmap = Bitmap.createBitmap(width, height, Config.ARGB_8888); + if (hasGainmap && VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE) { + bitmap.setGainmap(createGainmap(width / 2, height / 2)); + } OutputStream os = null; try { File tempFile = File.createTempFile( - "ds-" + width + "-" + height + "-" + exifOrientation, + "ds-" + width + "-" + height + "-" + exifOrientation + "-" + hasGainmap, ".jpeg", ApplicationProvider.getApplicationContext().getCacheDir()); os = new BufferedOutputStream(new FileOutputStream(tempFile)); @@ -551,8 +636,12 @@ private static InputStream openFileStream(int width, int height, int exifOrienta } } - private static InputStream openInMemoryStream(CompressFormat format, int width, int height) { + private static InputStream openInMemoryStream( + CompressFormat format, int width, int height, boolean hasGainmap) { Bitmap bitmap = Bitmap.createBitmap(width, height, Config.ARGB_8888); + if (hasGainmap && VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE) { + bitmap.setGainmap(createGainmap(width / 2, height / 2)); + } ByteArrayOutputStream os = new ByteArrayOutputStream(); bitmap.compress(format, 100 /*quality*/, os); bitmap.recycle(); @@ -560,6 +649,11 @@ private static InputStream openInMemoryStream(CompressFormat format, int width, return new ByteArrayInputStream(data); } + @RequiresApi(VERSION_CODES.UPSIDE_DOWN_CAKE) + private static Gainmap createGainmap(int width, int height) { + return new Gainmap(Bitmap.createBitmap(width, height, Config.ALPHA_8)); + } + static final class Tester { private final DownsampleStrategy strategy; private final List testCases = new ArrayList<>(); @@ -581,6 +675,21 @@ Tester givenSquareImageWithDimensionOf(int dimension, Api... apis) { return givenImageWithDimensionsOf(dimension, dimension, apis); } + Tester givenGainmapImageWithDimensionsOf( + int sourceWidth, int sourceHeight, boolean allowHardwareConfig, Api... apis) { + testCases.add( + new TestCase.Builder() + .setSourceWidth(sourceWidth) + .setSourceHeight(sourceHeight) + .setTargetWidth(targetWidth) + .setTargetHeight(targetHeight) + .setHasGainmap(true) + .setAllowHardwareConfig(allowHardwareConfig) + .setApis(apis) + .build()); + return this; + } + Tester givenImageWithDimensionsOf(int sourceWidth, int sourceHeight, Api... apis) { testCases.add(new TestCase(sourceWidth, sourceHeight, targetWidth, targetHeight, apis)); return this; @@ -608,23 +717,100 @@ private static final class TestCase { private final int sourceHeight; private final int targetWidth; private final int targetHeight; + private final boolean hasGainmap; + private final boolean allowHardwareConfig; private final Api[] apis; + /** + * @deprecated Use the {@link Builder}. + */ + @Deprecated TestCase(int sourceWidth, int sourceHeight, int targetWidth, int targetHeight, Api... apis) { this.sourceWidth = sourceWidth; this.sourceHeight = sourceHeight; this.targetWidth = targetWidth; this.targetHeight = targetHeight; + this.hasGainmap = false; + this.allowHardwareConfig = false; this.apis = apis; } + private TestCase(Builder builder) { + this.sourceWidth = builder.sourceWidth; + this.sourceHeight = builder.sourceHeight; + this.targetWidth = builder.targetWidth; + this.targetHeight = builder.targetHeight; + this.hasGainmap = builder.hasGainmap; + this.allowHardwareConfig = builder.allowHardwareConfig; + this.apis = builder.apis; + } + List test(DownsampleStrategy strategy) throws IOException { List results = new ArrayList<>(); for (Api api : apis) { - results.addAll(api.test(sourceWidth, sourceHeight, targetWidth, targetHeight, strategy)); + results.addAll( + api.test( + sourceWidth, + sourceHeight, + hasGainmap, + allowHardwareConfig, + targetWidth, + targetHeight, + strategy)); } return results; } + + private static final class Builder { + + private int sourceWidth; + private int sourceHeight; + private int targetWidth; + private int targetHeight; + private boolean hasGainmap; + private boolean allowHardwareConfig; + @Nullable private Api[] apis; + + public Builder setSourceWidth(int sourceWidth) { + this.sourceWidth = sourceWidth; + return this; + } + + public Builder setSourceHeight(int sourceHeight) { + this.sourceHeight = sourceHeight; + return this; + } + + public Builder setTargetWidth(int targetWidth) { + this.targetWidth = targetWidth; + return this; + } + + public Builder setTargetHeight(int targetHeight) { + this.targetHeight = targetHeight; + return this; + } + + public Builder setHasGainmap(boolean hasGainmap) { + this.hasGainmap = hasGainmap; + return this; + } + + public Builder setAllowHardwareConfig(boolean allowHardwareConfig) { + this.allowHardwareConfig = allowHardwareConfig; + return this; + } + + public Builder setApis(Api[] apis) { + this.apis = apis; + return this; + } + + public TestCase build() { + Preconditions.checkNotNull(apis); + return new TestCase(this); + } + } } } @@ -682,6 +868,8 @@ Api with(Formats... formats) { List test( int sourceWidth, int sourceHeight, + boolean hasGainmap, + boolean allowHardwareConfig, int targetWidth, int targetHeight, DownsampleStrategy strategy) @@ -693,7 +881,14 @@ List test( List results = new ArrayList<>(); for (Formats format : formats) { results.addAll( - format.runTest(sourceWidth, sourceHeight, targetWidth, targetHeight, strategy)); + format.runTest( + sourceWidth, + sourceHeight, + hasGainmap, + allowHardwareConfig, + targetWidth, + targetHeight, + strategy)); } return results; } @@ -705,15 +900,15 @@ static final class Formats { private final CompressFormat[] formats; private static final int[] ALL_EXIF_ORIENTATIONS = new int[] { - ExifInterface.ORIENTATION_UNDEFINED, - ExifInterface.ORIENTATION_NORMAL, - ExifInterface.ORIENTATION_FLIP_HORIZONTAL, - ExifInterface.ORIENTATION_ROTATE_180, - ExifInterface.ORIENTATION_FLIP_VERTICAL, - ExifInterface.ORIENTATION_TRANSPOSE, - ExifInterface.ORIENTATION_ROTATE_90, - ExifInterface.ORIENTATION_TRANSVERSE, - ExifInterface.ORIENTATION_ROTATE_270 + ExifInterface.ORIENTATION_UNDEFINED, + ExifInterface.ORIENTATION_NORMAL, + ExifInterface.ORIENTATION_FLIP_HORIZONTAL, + ExifInterface.ORIENTATION_ROTATE_180, + ExifInterface.ORIENTATION_FLIP_VERTICAL, + ExifInterface.ORIENTATION_TRANSPOSE, + ExifInterface.ORIENTATION_ROTATE_90, + ExifInterface.ORIENTATION_TRANSVERSE, + ExifInterface.ORIENTATION_ROTATE_270 }; private static final int[] UNDEFINED_EXIF_ORIENTATIONS = new int[] {ExifInterface.ORIENTATION_UNDEFINED}; @@ -747,6 +942,8 @@ Formats expect(int width, int height) { List runTest( int sourceWidth, int sourceHeight, + boolean hasGainmap, + boolean allowHardwareConfig, int targetWidth, int targetHeight, DownsampleStrategy strategy) @@ -764,6 +961,8 @@ List runTest( targetWidth, targetHeight, exifOrientation, + hasGainmap, + allowHardwareConfig, strategy, expectedWidth, expectedHeight); diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index 5429db1a56..b3b5d741e9 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -499,6 +499,17 @@ public GlideBuilder setPreserveGainmapAndColorSpaceForTransformations(boolean is return this; } + /** + * Fixes decoding of hardware gainmaps from Ultra HDR images on Android U. + * + *

Without this flag on, gainmaps may be dropped when decoding Ultra HDR on Android U devices + * using skiagl for hwui as described in https://github.com/bumptech/glide/issues/5362. + */ + public GlideBuilder setEnableHardwareGainmapFixOnU(boolean isEnabled) { + glideExperimentsBuilder.update(new EnableHardwareGainmapFixOnU(), isEnabled); + return this; + } + /** * @deprecated This method does nothing. It will be hard coded and removed in a future release * without further warning. @@ -618,6 +629,9 @@ static final class ManualOverrideHardwareBitmapMaxFdCount implements Experiment */ static final class PreserveGainmapAndColorSpaceForTransformations implements Experiment {} + /** Fixes decoding of hardware gainmaps from Ultra HDR images on Android U. */ + static final class EnableHardwareGainmapFixOnU implements Experiment {} + static final class EnableImageDecoderForBitmaps implements Experiment {} /** See {@link #setLogRequestOrigins(boolean)}. */ diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index 36fa96fe21..d74626977d 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -12,6 +12,7 @@ import android.os.ParcelFileDescriptor; import androidx.annotation.Nullable; import androidx.tracing.Trace; +import com.bumptech.glide.GlideBuilder.EnableHardwareGainmapFixOnU; import com.bumptech.glide.GlideBuilder.EnableImageDecoderForBitmaps; import com.bumptech.glide.GlideBuilder.PreserveGainmapAndColorSpaceForTransformations; import com.bumptech.glide.gifdecoder.GifDecoder; @@ -160,7 +161,8 @@ private static void initializeDefaults( resources.getDisplayMetrics(), bitmapPool, arrayPool, - experiments.isEnabled(PreserveGainmapAndColorSpaceForTransformations.class)); + experiments.isEnabled(PreserveGainmapAndColorSpaceForTransformations.class), + experiments.isEnabled(EnableHardwareGainmapFixOnU.class)); ResourceDecoder byteBufferBitmapDecoder; ResourceDecoder streamBitmapDecoder; diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java index 98fcd5a041..56507918f6 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java @@ -65,6 +65,7 @@ public final class Downsampler { */ public static final Option PREFERRED_COLOR_SPACE = Option.memory("com.bumptech.glide.load.resource.bitmap.Downsampler.PreferredColorSpace"); + /** * Indicates the {@link com.bumptech.glide.load.resource.bitmap.DownsampleStrategy} option that * will be used to calculate the sample size to use to downsample an image given the original and @@ -74,6 +75,7 @@ public final class Downsampler { */ @Deprecated public static final Option DOWNSAMPLE_STRATEGY = DownsampleStrategy.OPTION; + /** * Ensure that the size of the bitmap is fixed to the requested width and height of the resource * from the caller. The final resource dimensions may differ from the requested width and height, @@ -140,6 +142,7 @@ public void onDecodeComplete(BitmapPool bitmapPool, Bitmap downsampled) { private final List parsers; private final HardwareConfigState hardwareConfigState = HardwareConfigState.getInstance(); private final boolean preserveGainmapAndColorSpaceForTransformations; + private final boolean enableHardwareGainmapFixOnU; public Downsampler( List parsers, @@ -151,7 +154,8 @@ public Downsampler( displayMetrics, bitmapPool, byteArrayPool, - /* preserveGainmapAndColorSpaceForTransformations= */ false); + /* preserveGainmapAndColorSpaceForTransformations= */ false, + /* enableHardwareGainmapFixOnU= */ false); } /** @@ -165,13 +169,37 @@ public Downsampler( BitmapPool bitmapPool, ArrayPool byteArrayPool, boolean preserveGainmapAndColorSpaceForTransformations) { + this( + parsers, + displayMetrics, + bitmapPool, + byteArrayPool, + preserveGainmapAndColorSpaceForTransformations, + /* enableHardwareGainmapFixOnU= */ false); + } + + /** + * @param preserveGainmapAndColorSpaceForTransformations Preserves gainmap and color space for + * transformation, e.g., the color space of wide gamut images or the gainmap of Ultra HDR + * images. + * @param enableHardwareGainmapFixOnU Fixes issues with hardware gainmaps on U. + */ + public Downsampler( + List parsers, + DisplayMetrics displayMetrics, + BitmapPool bitmapPool, + ArrayPool byteArrayPool, + boolean preserveGainmapAndColorSpaceForTransformations, + boolean enableHardwareGainmapFixOnU) { this.parsers = parsers; this.displayMetrics = Preconditions.checkNotNull(displayMetrics); this.bitmapPool = Preconditions.checkNotNull(bitmapPool); this.byteArrayPool = Preconditions.checkNotNull(byteArrayPool); this.preserveGainmapAndColorSpaceForTransformations = preserveGainmapAndColorSpaceForTransformations; + this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU; } + public boolean handles(@SuppressWarnings("unused") InputStream is) { // We expect Downsampler to handle any available type Android supports. return true; @@ -206,7 +234,8 @@ public Resource decode( ByteBuffer buffer, int requestedWidth, int requestedHeight, Options options) throws IOException { return decode( - new ImageReader.ByteBufferReader(buffer, parsers, byteArrayPool), + new ImageReader.ByteBufferReader( + buffer, parsers, byteArrayPool, enableHardwareGainmapFixOnU), requestedWidth, requestedHeight, options, @@ -241,7 +270,8 @@ public Resource decode( DecodeCallbacks callbacks) throws IOException { return decode( - new ImageReader.InputStreamImageReader(is, parsers, byteArrayPool), + new ImageReader.InputStreamImageReader( + is, parsers, byteArrayPool, enableHardwareGainmapFixOnU), requestedWidth, requestedHeight, options, @@ -252,7 +282,7 @@ public Resource decode( void decode(byte[] bytes, int requestedWidth, int requestedHeight, Options options) throws IOException { decode( - new ImageReader.ByteArrayReader(bytes, parsers, byteArrayPool), + new ImageReader.ByteArrayReader(bytes, parsers, byteArrayPool, enableHardwareGainmapFixOnU), requestedWidth, requestedHeight, options, @@ -263,7 +293,7 @@ void decode(byte[] bytes, int requestedWidth, int requestedHeight, Options optio void decode(File file, int requestedWidth, int requestedHeight, Options options) throws IOException { decode( - new ImageReader.FileReader(file, parsers, byteArrayPool), + new ImageReader.FileReader(file, parsers, byteArrayPool, enableHardwareGainmapFixOnU), requestedWidth, requestedHeight, options, @@ -276,7 +306,7 @@ public Resource decode( throws IOException { return decode( new ImageReader.ParcelFileDescriptorImageReader( - parcelFileDescriptor, parsers, byteArrayPool), + parcelFileDescriptor, parsers, byteArrayPool, enableHardwareGainmapFixOnU), outWidth, outHeight, options, diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/GlideBitmapFactoryWrapper.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/GlideBitmapFactoryWrapper.java new file mode 100644 index 0000000000..607d9946d2 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/GlideBitmapFactoryWrapper.java @@ -0,0 +1,291 @@ +package com.bumptech.glide.load.resource.bitmap; + +import android.graphics.Bitmap; +import android.graphics.Bitmap.Config; +import android.graphics.BitmapFactory; +import android.graphics.BitmapFactory.Options; +import android.graphics.Gainmap; +import android.graphics.Rect; +import android.os.Build.VERSION; +import android.os.Build.VERSION_CODES; +import android.util.Log; +import androidx.annotation.GuardedBy; +import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; +import com.bumptech.glide.util.Preconditions; +import java.io.FileDescriptor; +import java.io.InputStream; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Wrapper around {@link BitmapFactory} to work around known issues with {@link BitmapFactory} + * across Android SDK levels. + * + *

In particular, this class works around these known issues: + * + *

    + *
  • Ultra HDR image single-channel gainmaps not being decoded on Android U when hardware + * bitmaps are enabled. This issue is further described in + * https://github.com/bumptech/glide/issues/5362. + *
+ * + *

New usages of {@link BitmapFactory} APIs within Glide should be added here rather than called + * directly. + */ +final class GlideBitmapFactoryWrapper { + + private GlideBitmapFactoryWrapper() {} + + /** Wrapper for {@link BitmapFactory#decodeStream}. */ + @Nullable + public static Bitmap decodeStream(InputStream inputStream, BitmapFactory.Options options) { + if (VERSION.SDK_INT == VERSION_CODES.UPSIDE_DOWN_CAKE + && GainmapDecoderWorkaroundStateCalculator.needsGainmapDecodeWorkaround(options)) { + return safeDecodeHardwareBitmapWithGainmap(inputStream, options); + } + return BitmapFactory.decodeStream(inputStream, /* outPadding= */ null, options); + } + + /** Wrapper for {@link BitmapFactory#decodeByteArray}. */ + @Nullable + public static Bitmap decodeByteArray(byte[] bytes, BitmapFactory.Options options) { + if (VERSION.SDK_INT == VERSION_CODES.UPSIDE_DOWN_CAKE + && GainmapDecoderWorkaroundStateCalculator.needsGainmapDecodeWorkaround(options)) { + return safeDecodeHardwareBitmapWithGainmap(bytes, options); + } + return BitmapFactory.decodeByteArray(bytes, /* offset= */ 0, bytes.length, options); + } + + /** Wrapper for {@link BitmapFactory#decodeFileDescriptor}. */ + @Nullable + public static Bitmap decodeFileDescriptor( + FileDescriptor fileDescriptor, BitmapFactory.Options options) { + if (VERSION.SDK_INT == VERSION_CODES.UPSIDE_DOWN_CAKE + && GainmapDecoderWorkaroundStateCalculator.needsGainmapDecodeWorkaround(options)) { + return safeDecodeHardwareBitmapWithGainmap(fileDescriptor, options); + } + return BitmapFactory.decodeFileDescriptor(fileDescriptor, /* outPadding= */ null, options); + } + + /** + * Returns a decoded bitmap for the input stream, ensuring that any associated gainmap is decoded + * without errors on Android U if it is a valid gainamp. + * + *

This method safely wraps BitmapFactory#decodeStream(InputStream, Rect, Options)} on Android + * U. + * + * @param inputStream for the bitmap to be decoded. + * @param options to be applied in the {@link BitmapFactory#decodeStream} call. + */ + @RequiresApi(VERSION_CODES.UPSIDE_DOWN_CAKE) + @Nullable + private static Bitmap safeDecodeHardwareBitmapWithGainmap( + InputStream inputStream, Options options) { + Preconditions.checkArgument(options.inPreferredConfig == Config.HARDWARE); + options.inPreferredConfig = Bitmap.Config.ARGB_8888; + Bitmap softwareBitmap = null; + try { + options.inPreferredConfig = Config.ARGB_8888; + softwareBitmap = BitmapFactory.decodeStream(inputStream, /* outPadding= */ null, options); + if (softwareBitmap == null) { + return null; + } + return safeDecodeBitmapWithGainmap(softwareBitmap); + } finally { + if (softwareBitmap != null) { + softwareBitmap.recycle(); + } + options.inPreferredConfig = Config.HARDWARE; + } + } + + /** + * Returns a decoded bitmap for the input byte array, ensuring that any associated gainmap is + * decoded without errors on Android U if it is a valid gainmap. + * + *

This method safely wraps BitmapFactory#decodeByteArray(byte[], int, int)} on Android U. + * + * @param bytes for the bitmap to be decoded. + * @param options to be applied in the {@link BitmapFactory#decodeByteArray} call. This must be + * set to {@link Config#HARDWARE}. + * @throws IllegalArgumentException if {@link Options#inPreferredConfig} is set to any state other + * than {@link Config#HARDWARE}. + */ + @RequiresApi(VERSION_CODES.UPSIDE_DOWN_CAKE) + @Nullable + private static Bitmap safeDecodeHardwareBitmapWithGainmap(byte[] bytes, Options options) { + Preconditions.checkArgument(options.inPreferredConfig == Config.HARDWARE); + options.inPreferredConfig = Bitmap.Config.ARGB_8888; + Bitmap softwareBitmap = null; + try { + options.inPreferredConfig = Bitmap.Config.ARGB_8888; + softwareBitmap = BitmapFactory.decodeByteArray(bytes, /* offset= */ 0, bytes.length, options); + if (softwareBitmap == null) { + return null; + } + return GlideBitmapFactoryWrapper.safeDecodeBitmapWithGainmap(softwareBitmap); + } finally { + if (softwareBitmap != null) { + softwareBitmap.recycle(); + } + options.inPreferredConfig = Config.HARDWARE; + } + } + + /** + * Returns a decoded bitmap for the input file descriptor, ensuring that any associated gainmap is + * decoded without errors on Android U if it is a valid gainmap. + * + *

This method safely wraps {@link BitmapFactory#decodeFileDescriptor(FileDescriptor, Rect, + * Options)} on Android U. + * + * @param fileDescriptor from which the bitmap will be decoded. + * @param options to be applied in the {@link BitmapFactory#decodeFileDescriptor} call. This must + * be set to {@link Config#HARDWARE}. + * @throws IllegalArgumentException if {@link Options#inPreferredConfig} is set to any state other + * than {@link Config#HARDWARE}. + */ + @RequiresApi(VERSION_CODES.UPSIDE_DOWN_CAKE) + @Nullable + private static Bitmap safeDecodeHardwareBitmapWithGainmap( + FileDescriptor fileDescriptor, Options options) { + Preconditions.checkArgument(options.inPreferredConfig == Config.HARDWARE); + options.inPreferredConfig = Bitmap.Config.ARGB_8888; + Bitmap softwareBitmap = null; + try { + options.inPreferredConfig = Bitmap.Config.ARGB_8888; + softwareBitmap = + BitmapFactory.decodeFileDescriptor(fileDescriptor, /* outPadding= */ null, options); + if (softwareBitmap == null) { + return null; + } + return GlideBitmapFactoryWrapper.safeDecodeBitmapWithGainmap(softwareBitmap); + } finally { + if (softwareBitmap != null) { + softwareBitmap.recycle(); + } + options.inPreferredConfig = Config.HARDWARE; + } + } + + /** + * Returns a decoded bitmap for the input software bitmap, ensuring that any associated gainmap is + * decoded without errors on Android U if it is a valid gainmap. + * + * @param softwareBitmap The bitmap to be decoded. Must not be a hardware bitmap. + * @throws IllegalArgumentException if {@link Options#inPreferredConfig} is set to any state other + * than {@link Config#HARDWARE}. + */ + @RequiresApi(VERSION_CODES.UPSIDE_DOWN_CAKE) + @Nullable + private static Bitmap safeDecodeBitmapWithGainmap(Bitmap softwareBitmap) { + try { + Gainmap gainmap = softwareBitmap.getGainmap(); + if (gainmap != null) { + Bitmap gainmapContents = gainmap.getGainmapContents(); + if (gainmapContents.getConfig() == Config.ALPHA_8) { + softwareBitmap.setGainmap( + GainmapCopier.convertSingleChannelGainmapToTripleChannelGainmap(gainmap)); + } + } + return softwareBitmap.copy(Config.HARDWARE, /* isMutable= */ false); + } finally { + softwareBitmap.recycle(); + } + } + + /** Utils to copy gainmaps. */ + @RequiresApi(VERSION_CODES.UPSIDE_DOWN_CAKE) + private static final class GainmapCopier { + + private GainmapCopier() {} + + /** + * Converts single channel gainmap to triple channel, where a single channel gainmap is defined + * as a gainmap with a bitmap config of {@link Config#ALPHA_8}. + * + *

If the input gainmap is not single channel, then this method will just return the original + * gainmap. + */ + public static Gainmap convertSingleChannelGainmapToTripleChannelGainmap(Gainmap gainmap) { + Bitmap gainmapContents = gainmap.getGainmapContents(); + if (gainmapContents.getConfig() != Config.ALPHA_8) { + return gainmap; + } + Bitmap newContents = gainmapContents.copy(Config.ARGB_8888, /* isMutable= */ false); + Gainmap newGainmap = new Gainmap(newContents); + float[] tempFloatArray = gainmap.getRatioMin(); + newGainmap.setRatioMin(tempFloatArray[0], tempFloatArray[1], tempFloatArray[2]); + tempFloatArray = gainmap.getRatioMax(); + newGainmap.setRatioMax(tempFloatArray[0], tempFloatArray[1], tempFloatArray[2]); + tempFloatArray = gainmap.getGamma(); + newGainmap.setGamma(tempFloatArray[0], tempFloatArray[1], tempFloatArray[2]); + tempFloatArray = gainmap.getEpsilonSdr(); + newGainmap.setEpsilonSdr(tempFloatArray[0], tempFloatArray[1], tempFloatArray[2]); + tempFloatArray = gainmap.getEpsilonHdr(); + newGainmap.setEpsilonHdr(tempFloatArray[0], tempFloatArray[1], tempFloatArray[2]); + newGainmap.setDisplayRatioForFullHdr(gainmap.getDisplayRatioForFullHdr()); + newGainmap.setMinDisplayRatioForHdrTransition(gainmap.getMinDisplayRatioForHdrTransition()); + return newGainmap; + } + } + + /** + * Determines if a gainmap decoding workaround is required to mitigate an Android U bug with + * decoding bitmaps with gainmaps. When the following conditions are present, Android U will not be + * able to decode a gainmap, with hardware bitmap operation failing for the gainmap: + * + *

    + *
  • The HWUI is configured to use skiagl. + *
  • The gainmap is single channel. + *
  • The bitmap owning the bitmap is a hardware bitmap. + *
+ * + *

Callers should use this class to determine whether to apply a workaround, e.g., modifying the + * gainmap to be triple channel and software decode it. + */ + public static final class GainmapDecoderWorkaroundStateCalculator { + private static final String TAG = "GainmapWorkaroundCalc"; + + /** Meomizes result of test to see if the device is susceptible to the gainmap decoding bug. */ + @GuardedBy("GainmapDecoderWorkaroundStateCalculator.class") + private static final AtomicReference REQUIRES_GAIN_MAP_FIX = new AtomicReference<>(); + + private GainmapDecoderWorkaroundStateCalculator() {} + + /** + * Returns true if a gainmap decoding workaround is required to mitigate an Android U bug. This + * method tests for the presence of the bug, which only affects hardware bitmaps, and caches the + * result in memory. + * + *

This method is thread-safe. + * + * @param options which will be used to decode the gainmap. + */ + static synchronized boolean needsGainmapDecodeWorkaround(Options options) { + if (VERSION.SDK_INT != VERSION_CODES.UPSIDE_DOWN_CAKE) { + return false; + } + if (options.inPreferredConfig != Config.HARDWARE) { + return false; + } + Boolean requiresGainmapDecodeWorkaround = REQUIRES_GAIN_MAP_FIX.get(); + if (requiresGainmapDecodeWorkaround == null) { + // Create a 1x1 single channel, A8 bitmap and attempt to copy to a hardware bitmap. If the + // copy operation fails, then the device requires a workaround to decode hardware gainmaps. + Bitmap a8Source = Bitmap.createBitmap(/* width= */ 1, /* height= */ 1, Config.ALPHA_8); + Bitmap a8HardwareBitmap = a8Source.copy(Config.HARDWARE, /* isMutable= */ false); + a8Source.recycle(); + requiresGainmapDecodeWorkaround = a8HardwareBitmap == null; + if (Log.isLoggable(TAG, Log.DEBUG)) { + Log.d(TAG, "requiresGainmapDecodeWorkaround=" + requiresGainmapDecodeWorkaround); + } + if (a8HardwareBitmap != null) { + a8HardwareBitmap.recycle(); + } + REQUIRES_GAIN_MAP_FIX.set(requiresGainmapDecodeWorkaround); + } + return requiresGainmapDecodeWorkaround; + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/ImageReader.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/ImageReader.java index 240279e9f2..041a3b776b 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/ImageReader.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/ImageReader.java @@ -3,10 +3,8 @@ import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.graphics.BitmapFactory.Options; -import android.os.Build; import android.os.ParcelFileDescriptor; import androidx.annotation.Nullable; -import androidx.annotation.RequiresApi; import com.bumptech.glide.load.ImageHeaderParser; import com.bumptech.glide.load.ImageHeaderParser.ImageType; import com.bumptech.glide.load.ImageHeaderParserUtils; @@ -17,6 +15,7 @@ import com.bumptech.glide.util.ByteBufferUtil; import com.bumptech.glide.util.Preconditions; import java.io.File; +import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; @@ -29,6 +28,7 @@ * type wrapped into a {@link DataRewinder}. */ interface ImageReader { + @Nullable Bitmap decodeBitmap(BitmapFactory.Options options) throws IOException; @@ -43,17 +43,25 @@ final class ByteArrayReader implements ImageReader { private final byte[] bytes; private final List parsers; private final ArrayPool byteArrayPool; + private final boolean enableHardwareGainmapFixOnU; - ByteArrayReader(byte[] bytes, List parsers, ArrayPool byteArrayPool) { + ByteArrayReader( + byte[] bytes, + List parsers, + ArrayPool byteArrayPool, + boolean enableHardwareGainmapFixOnU) { this.bytes = bytes; this.parsers = parsers; this.byteArrayPool = byteArrayPool; + this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU; } @Nullable @Override public Bitmap decodeBitmap(Options options) { - return BitmapFactory.decodeByteArray(bytes, /* offset= */ 0, bytes.length, options); + return enableHardwareGainmapFixOnU + ? GlideBitmapFactoryWrapper.decodeByteArray(bytes, options) + : BitmapFactory.decodeByteArray(bytes, /* offset= */ 0, bytes.length, options); } @Override @@ -68,6 +76,7 @@ public int getImageOrientation() throws IOException { @Override public void stopGrowingBuffers() {} + } final class FileReader implements ImageReader { @@ -75,11 +84,17 @@ final class FileReader implements ImageReader { private final File file; private final List parsers; private final ArrayPool byteArrayPool; + private final boolean enableHardwareGainmapFixOnU; - FileReader(File file, List parsers, ArrayPool byteArrayPool) { + FileReader( + File file, + List parsers, + ArrayPool byteArrayPool, + boolean enableHardwareGainmapFixOnU) { this.file = file; this.parsers = parsers; this.byteArrayPool = byteArrayPool; + this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU; } @Nullable @@ -88,7 +103,9 @@ public Bitmap decodeBitmap(Options options) throws FileNotFoundException { InputStream is = null; try { is = new RecyclableBufferedInputStream(new FileInputStream(file), byteArrayPool); - return BitmapFactory.decodeStream(is, /* outPadding= */ null, options); + return enableHardwareGainmapFixOnU + ? GlideBitmapFactoryWrapper.decodeStream(is, options) + : BitmapFactory.decodeStream(is, /* outPadding= */ null, options); } finally { if (is != null) { try { @@ -143,17 +160,26 @@ final class ByteBufferReader implements ImageReader { private final ByteBuffer buffer; private final List parsers; private final ArrayPool byteArrayPool; + private final boolean enableHardwareGainmapFixOnU; - ByteBufferReader(ByteBuffer buffer, List parsers, ArrayPool byteArrayPool) { + ByteBufferReader( + ByteBuffer buffer, + List parsers, + ArrayPool byteArrayPool, + boolean enableHardwareGainmapFixOnU) { this.buffer = buffer; this.parsers = parsers; this.byteArrayPool = byteArrayPool; + this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU; } @Nullable @Override public Bitmap decodeBitmap(Options options) { - return BitmapFactory.decodeStream(stream(), /* outPadding= */ null, options); + InputStream inputStream = stream(); + return enableHardwareGainmapFixOnU + ? GlideBitmapFactoryWrapper.decodeStream(inputStream, options) + : BitmapFactory.decodeStream(inputStream, /* outPadding= */ null, options); } @Override @@ -179,19 +205,27 @@ final class InputStreamImageReader implements ImageReader { private final InputStreamRewinder dataRewinder; private final ArrayPool byteArrayPool; private final List parsers; + private final boolean enableHardwareGainmapFixOnU; InputStreamImageReader( - InputStream is, List parsers, ArrayPool byteArrayPool) { + InputStream is, + List parsers, + ArrayPool byteArrayPool, + boolean enableHardwareGainmapFixOnU) { this.byteArrayPool = Preconditions.checkNotNull(byteArrayPool); this.parsers = Preconditions.checkNotNull(parsers); dataRewinder = new InputStreamRewinder(is, byteArrayPool); + this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU; } @Nullable @Override public Bitmap decodeBitmap(BitmapFactory.Options options) throws IOException { - return BitmapFactory.decodeStream(dataRewinder.rewindAndGet(), null, options); + InputStream inputStream = dataRewinder.rewindAndGet(); + return enableHardwareGainmapFixOnU + ? GlideBitmapFactoryWrapper.decodeStream(inputStream, options) + : BitmapFactory.decodeStream(inputStream, /* outPadding= */ null, options); } @Override @@ -211,27 +245,33 @@ public void stopGrowingBuffers() { } } - @RequiresApi(Build.VERSION_CODES.LOLLIPOP) final class ParcelFileDescriptorImageReader implements ImageReader { private final ArrayPool byteArrayPool; private final List parsers; private final ParcelFileDescriptorRewinder dataRewinder; + private final boolean enableHardwareGainmapFixOnU; ParcelFileDescriptorImageReader( ParcelFileDescriptor parcelFileDescriptor, List parsers, - ArrayPool byteArrayPool) { + ArrayPool byteArrayPool, + boolean enableHardwareGainmapFixOnU) { this.byteArrayPool = Preconditions.checkNotNull(byteArrayPool); this.parsers = Preconditions.checkNotNull(parsers); dataRewinder = new ParcelFileDescriptorRewinder(parcelFileDescriptor); + this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU; } @Nullable @Override public Bitmap decodeBitmap(BitmapFactory.Options options) throws IOException { - return BitmapFactory.decodeFileDescriptor( - dataRewinder.rewindAndGet().getFileDescriptor(), null, options); + ParcelFileDescriptor parcelFileDescriptor = dataRewinder.rewindAndGet(); + FileDescriptor fileDescriptor = parcelFileDescriptor.getFileDescriptor(); + return enableHardwareGainmapFixOnU + ? GlideBitmapFactoryWrapper.decodeFileDescriptor(fileDescriptor, options) + : BitmapFactory.decodeFileDescriptor( + parcelFileDescriptor.getFileDescriptor(), /* outPadding= */ null, options); } @Override @@ -244,9 +284,9 @@ public int getImageOrientation() throws IOException { return ImageHeaderParserUtils.getOrientation(parsers, dataRewinder, byteArrayPool); } - @Override public void stopGrowingBuffers() { // Nothing to do here. } } + } diff --git a/scripts/run_instrumentation_tests.sh b/scripts/run_instrumentation_tests.sh index 412690a8ae..c7296e93c7 100755 --- a/scripts/run_instrumentation_tests.sh +++ b/scripts/run_instrumentation_tests.sh @@ -16,5 +16,6 @@ gcloud firebase test android run \ --device model=Nexus6P,version=23,locale=en,orientation=portrait \ --device model=Nexus6,version=22,locale=en,orientation=portrait \ --device model=Nexus5,version=19,locale=en,orientation=portrait \ + --device model=MediumPhone.arm,version=34,locale=en,orientation=portrait \ --project android-glide \ --no-auto-google-login \