Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update TransformationUtils#rotateImageExif to preserve gainmap and co… #5334

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<GifDrawable> resource;
@Mock private GifDecoder decoder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
18 changes: 18 additions & 0 deletions library/src/main/java/com/bumptech/glide/GlideBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*
* <p>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.
Expand Down Expand Up @@ -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)}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ByteBuffer, Bitmap> byteBufferBitmapDecoder;
ResourceDecoder<InputStream, Bitmap> streamBitmapDecoder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,39 @@ public void onDecodeComplete(BitmapPool bitmapPool, Bitmap downsampled) {
private final ArrayPool byteArrayPool;
private final List<ImageHeaderParser> parsers;
private final HardwareConfigState hardwareConfigState = HardwareConfigState.getInstance();
private final boolean preserveGainmapAndColorSpaceForTransformations;

public Downsampler(
List<ImageHeaderParser> 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<ImageHeaderParser> 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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,35 +304,72 @@ public static int getExifOrientationDegrees(int exifOrientation) {
/**
* Rotate and/or flip the image to match the given exif orientation.
*
* <p>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].
* @return The rotated and/or flipped image or toOrient if no rotation or flip was necessary.
*/
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;
}

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(
falhassen marked this conversation as resolved.
Show resolved Hide resolved
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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -206,6 +206,5 @@ public int[] obtainIntArray(int size) {
public void release(@NonNull int[] array) {
// Do Nothing
}

}
}
Loading