From e77562a432352da19741d93b8c1f6599b558676a Mon Sep 17 00:00:00 2001 From: falhassen Date: Wed, 15 Nov 2023 17:07:44 -0800 Subject: [PATCH] Update TransformationUtils#rotateImageExif to preserve gainmap and colorspace on Android U+. Also bump Robolectric Version to support testing on Android U. --- .../ReEncodingGifResourceEncoderTest.java | 2 +- .../volley/VolleyStreamFetcherServerTest.java | 2 +- .../java/com/bumptech/glide/GlideBuilder.java | 18 ++++++ .../com/bumptech/glide/RegistryFactory.java | 7 ++- .../load/resource/bitmap/Downsampler.java | 27 ++++++++- .../resource/bitmap/TransformationUtils.java | 57 +++++++++++++++---- .../bitmap/TransformationUtilsTest.java | 37 ++++++++++++ settings.gradle | 2 +- .../glide/gifdecoder/GifDecoderTest.java | 3 +- 9 files changed, 137 insertions(+), 18 deletions(-) diff --git a/integration/gifencoder/src/test/java/com/bumptech/glide/integration/gifencoder/ReEncodingGifResourceEncoderTest.java b/integration/gifencoder/src/test/java/com/bumptech/glide/integration/gifencoder/ReEncodingGifResourceEncoderTest.java index b23dc4bd7f..5074117820 100644 --- a/integration/gifencoder/src/test/java/com/bumptech/glide/integration/gifencoder/ReEncodingGifResourceEncoderTest.java +++ b/integration/gifencoder/src/test/java/com/bumptech/glide/integration/gifencoder/ReEncodingGifResourceEncoderTest.java @@ -44,7 +44,7 @@ /** Tests for {@link com.bumptech.glide.integration.gifencoder.ReEncodingGifResourceEncoder}. */ @RunWith(RobolectricTestRunner.class) -@Config(manifest = Config.NONE, sdk = 18) +@Config(manifest = Config.NONE, sdk = 19) public class ReEncodingGifResourceEncoderTest { @Mock private Resource resource; @Mock private GifDecoder decoder; diff --git a/integration/volley/src/test/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java b/integration/volley/src/test/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java index cab515fce3..d754aa1787 100644 --- a/integration/volley/src/test/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java +++ b/integration/volley/src/test/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java @@ -47,7 +47,7 @@ @RunWith(RobolectricTestRunner.class) @Config( manifest = Config.NONE, - sdk = 18, + sdk = 19, shadows = VolleyStreamFetcherServerTest.FakeSystemClock.class) public class VolleyStreamFetcherServerTest { private static final String DEFAULT_PATH = "/fakepath"; diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index c61d6be114..5429db1a56 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -487,6 +487,18 @@ public GlideBuilder setImageDecoderEnabledForBitmaps(boolean isEnabled) { return this; } + /** + * Preserves gainmap and color spaces while {@link Bitmap}s undergo transformation, i.e., in + * {@link com.bumptech.glide.load.resource.bitmap.TransformationUtils}. + * + *

Without this flag on, gainmap and color space information may be dropped in transformations, + * leading to unexpected behavior when transforming wide gamut images or Ultra HDR images. + */ + public GlideBuilder setPreserveGainmapAndColorSpaceForTransformations(boolean isEnabled) { + glideExperimentsBuilder.update(new PreserveGainmapAndColorSpaceForTransformations(), isEnabled); + return this; + } + /** * @deprecated This method does nothing. It will be hard coded and removed in a future release * without further warning. @@ -600,6 +612,12 @@ static final class ManualOverrideHardwareBitmapMaxFdCount implements Experiment } } + /** + * Preserves gainmap and color spaces while {@link Bitmap}s undergo transformation, i.e., in + * {@link com.bumptech.glide.load.resource.bitmap.TransformationUtils}. + */ + static final class PreserveGainmapAndColorSpaceForTransformations 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 a4b23e17d1..36fa96fe21 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -13,6 +13,7 @@ import androidx.annotation.Nullable; import androidx.tracing.Trace; import com.bumptech.glide.GlideBuilder.EnableImageDecoderForBitmaps; +import com.bumptech.glide.GlideBuilder.PreserveGainmapAndColorSpaceForTransformations; import com.bumptech.glide.gifdecoder.GifDecoder; import com.bumptech.glide.load.ImageHeaderParser; import com.bumptech.glide.load.ResourceDecoder; @@ -155,7 +156,11 @@ private static void initializeDefaults( // TODO(judds): Make ParcelFileDescriptorBitmapDecoder work with ImageDecoder. Downsampler downsampler = new Downsampler( - registry.getImageHeaderParsers(), resources.getDisplayMetrics(), bitmapPool, arrayPool); + registry.getImageHeaderParsers(), + resources.getDisplayMetrics(), + bitmapPool, + arrayPool, + experiments.isEnabled(PreserveGainmapAndColorSpaceForTransformations.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 df1a7e0d1d..98fcd5a041 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 @@ -139,18 +139,39 @@ public void onDecodeComplete(BitmapPool bitmapPool, Bitmap downsampled) { private final ArrayPool byteArrayPool; private final List parsers; private final HardwareConfigState hardwareConfigState = HardwareConfigState.getInstance(); + private final boolean preserveGainmapAndColorSpaceForTransformations; public Downsampler( List parsers, DisplayMetrics displayMetrics, BitmapPool bitmapPool, ArrayPool byteArrayPool) { + this( + parsers, + displayMetrics, + bitmapPool, + byteArrayPool, + /* preserveGainmapAndColorSpaceForTransformations= */ 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. + */ + public Downsampler( + List parsers, + DisplayMetrics displayMetrics, + BitmapPool bitmapPool, + ArrayPool byteArrayPool, + boolean preserveGainmapAndColorSpaceForTransformations) { this.parsers = parsers; this.displayMetrics = Preconditions.checkNotNull(displayMetrics); this.bitmapPool = Preconditions.checkNotNull(bitmapPool); this.byteArrayPool = Preconditions.checkNotNull(byteArrayPool); + this.preserveGainmapAndColorSpaceForTransformations = + preserveGainmapAndColorSpaceForTransformations; } - public boolean handles(@SuppressWarnings("unused") InputStream is) { // We expect Downsampler to handle any available type Android supports. return true; @@ -447,7 +468,9 @@ private Bitmap decodeFromWrappedStreams( // the expected density dpi. downsampled.setDensity(displayMetrics.densityDpi); - rotated = TransformationUtils.rotateImageExif(bitmapPool, downsampled, orientation); + rotated = + TransformationUtils.rotateImageExif( + bitmapPool, downsampled, orientation, preserveGainmapAndColorSpaceForTransformations); if (!downsampled.equals(rotated)) { bitmapPool.put(downsampled); } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/TransformationUtils.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/TransformationUtils.java index 1d10d949d9..be82279693 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/TransformationUtils.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/TransformationUtils.java @@ -304,6 +304,9 @@ public static int getExifOrientationDegrees(int exifOrientation) { /** * Rotate and/or flip the image to match the given exif orientation. * + *

Note that this method will not preserve the image's color gamut and gainmap. Use {@link + * #rotateImageExif(BitmapPool, Bitmap, int, boolean)} to enable that functionality. + * * @param pool A pool that may or may not contain an image of the necessary dimensions. * @param inBitmap The bitmap to rotate/flip. * @param exifOrientation the exif orientation [1-8]. @@ -311,6 +314,25 @@ public static int getExifOrientationDegrees(int exifOrientation) { */ public static Bitmap rotateImageExif( @NonNull BitmapPool pool, @NonNull Bitmap inBitmap, int exifOrientation) { + return rotateImageExif( + pool, inBitmap, exifOrientation, /* preserveGainmapAndColorSpace= */ false); + } + + /** + * Rotate and/or flip the image to match the given exif orientation. + * + * @param pool A pool that may or may not contain an image of the necessary dimensions. + * @param inBitmap The bitmap to rotate/flip. + * @param exifOrientation the exif orientation [1-8]. + * @param preserveGainmapAndColorSpace whether to preserve gainmap and color space information + * post-transformation. + * @return The rotated and/or flipped image or inBitmap if no rotation or flip was necessary. + */ + public static Bitmap rotateImageExif( + @NonNull BitmapPool pool, + @NonNull Bitmap inBitmap, + int exifOrientation, + boolean preserveGainmapAndColorSpace) { if (!isExifOrientationRequired(exifOrientation)) { return inBitmap; } @@ -318,21 +340,36 @@ public static Bitmap rotateImageExif( final Matrix matrix = new Matrix(); initializeMatrixForRotation(exifOrientation, matrix); - // From Bitmap.createBitmap. - final RectF newRect = new RectF(0, 0, inBitmap.getWidth(), inBitmap.getHeight()); - matrix.mapRect(newRect); + Bitmap result; + if (preserveGainmapAndColorSpace) { + // BitmapPool doesn't preserve gainmaps and color space, so use Bitmap.create to apply the + // matrix. + result = + Bitmap.createBitmap( + inBitmap, + /* x= */ 0, + /* y= */ 0, + inBitmap.getWidth(), + inBitmap.getHeight(), + matrix, + /* filter= */ true); + } else { + // From Bitmap.createBitmap. + final RectF newRect = new RectF(0, 0, inBitmap.getWidth(), inBitmap.getHeight()); + matrix.mapRect(newRect); - final int newWidth = Math.round(newRect.width()); - final int newHeight = Math.round(newRect.height()); + final int newWidth = Math.round(newRect.width()); + final int newHeight = Math.round(newRect.height()); - Bitmap.Config config = getNonNullConfig(inBitmap); - Bitmap result = pool.get(newWidth, newHeight, config); + Bitmap.Config config = getNonNullConfig(inBitmap); + result = pool.get(newWidth, newHeight, config); - matrix.postTranslate(-newRect.left, -newRect.top); + matrix.postTranslate(-newRect.left, -newRect.top); - result.setHasAlpha(inBitmap.hasAlpha()); + result.setHasAlpha(inBitmap.hasAlpha()); - applyMatrix(inBitmap, result, matrix); + applyMatrix(inBitmap, result, matrix); + } return result; } diff --git a/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/TransformationUtilsTest.java b/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/TransformationUtilsTest.java index 2ddea80612..2f3188f121 100644 --- a/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/TransformationUtilsTest.java +++ b/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/TransformationUtilsTest.java @@ -14,7 +14,10 @@ import android.graphics.Bitmap; import android.graphics.Color; +import android.graphics.ColorSpace; +import android.graphics.ColorSpace.Named; import android.graphics.Matrix; +import android.os.Build.VERSION_CODES; import androidx.exifinterface.media.ExifInterface; import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool; import com.bumptech.glide.tests.Util; @@ -409,6 +412,40 @@ public void testRotateImageExifHandlesBitmapsWithNullConfigs() { assertEquals(Bitmap.Config.ARGB_8888, rotated.getConfig()); } + @Test + @Config(sdk = VERSION_CODES.UPSIDE_DOWN_CAKE) + public void rotateImageExif_flagOff_doesNotPreserveColorSpace() { + Bitmap toRotate = Bitmap.createBitmap(200, 100, Bitmap.Config.ARGB_8888); + toRotate.setColorSpace(ColorSpace.get(ColorSpace.Named.DISPLAY_P3)); + + Bitmap rotated = + TransformationUtils.rotateImageExif( + bitmapPool, + toRotate, + ExifInterface.ORIENTATION_ROTATE_90, + /* preserveGainmapAndColorSpace= */ false); + + assertEquals(ColorSpace.get(Named.SRGB), rotated.getColorSpace()); + } + + @Test + @Config(sdk = VERSION_CODES.UPSIDE_DOWN_CAKE) + public void rotateImageExif_flagOm_preservesColorSpace() { + Bitmap toRotate = Bitmap.createBitmap(200, 100, Bitmap.Config.ARGB_8888); + toRotate.setColorSpace(ColorSpace.get(ColorSpace.Named.DISPLAY_P3)); + + Bitmap rotated = + TransformationUtils.rotateImageExif( + bitmapPool, + toRotate, + ExifInterface.ORIENTATION_ROTATE_90, + /* preserveGainmapAndColorSpace= */ true); + + assertEquals(ColorSpace.get(ColorSpace.Named.DISPLAY_P3), rotated.getColorSpace()); + } + + // TODO: Add gainmap-based tests once Robolectric has sufficient support. + @Test public void testInitializeMatrixSetsScaleIfFlipHorizontal() { Matrix matrix = mock(Matrix.class); diff --git a/settings.gradle b/settings.gradle index d0fbad1dfc..b0f554661e 100644 --- a/settings.gradle +++ b/settings.gradle @@ -138,7 +138,7 @@ dependencyResolutionManagement { library('retrofit', 'com.squareup.retrofit2', 'retrofit').versionRef('retrofit') library('retrofit-gson', 'com.squareup.retrofit2', 'converter-gson').versionRef('retrofit') library('retrofit-rxjava', 'com.squareup.retrofit2', 'adapter-rxjava').versionRef('retrofit') - library('robolectric', 'org.robolectric:robolectric:4.8.1') + library('robolectric', 'org.robolectric:robolectric:4.11.1') library('rx-android', 'io.reactivex:rxandroid:1.2.1') library('rx-java', 'io.reactivex:rxjava:1.3.8') library('svg', 'com.caverock:androidsvg:1.2.1') diff --git a/third_party/gif_decoder/src/test/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java b/third_party/gif_decoder/src/test/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java index 4972f44ec9..ac9fadedac 100644 --- a/third_party/gif_decoder/src/test/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java +++ b/third_party/gif_decoder/src/test/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java @@ -17,7 +17,7 @@ /** Tests for {@link com.bumptech.glide.gifdecoder.GifDecoder}. */ @RunWith(RobolectricTestRunner.class) -@Config(sdk = 18) +@Config(sdk = 19) public class GifDecoderTest { private MockProvider provider; @@ -206,6 +206,5 @@ public int[] obtainIntArray(int size) { public void release(@NonNull int[] array) { // Do Nothing } - } }