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 \