Skip to content

Commit

Permalink
Reverts Clean up preserveGainmapAndColorSpaceForTransformations flag
Browse files Browse the repository at this point in the history
  • Loading branch information
falhassen committed Jan 16, 2025
1 parent 9c9f56f commit ca64948
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,8 @@ private static String runScaleTest(
int expectedWidth,
int expectedHeight)
throws IOException {
Downsampler downsampler = buildDownsampler();
Downsampler downsampler =
hasGainmap ? buildDownsamplerWithGainmapBugFixes() : buildDownsampler();

InputStream is =
openBitmapStream(format, initialWidth, initialHeight, exifOrientation, hasGainmap);
Expand Down Expand Up @@ -552,6 +553,18 @@ private static Downsampler buildDownsampler() {
return new Downsampler(parsers, displayMetrics, bitmapPool, arrayPool);
}

private static Downsampler buildDownsamplerWithGainmapBugFixes() {
List<ImageHeaderParser> parsers =
Collections.<ImageHeaderParser>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, /* enableHardwareGainmapFixOnU= */ true);
}

private static InputStream openBitmapStream(
CompressFormat format, int width, int height, int exifOrientation, boolean hasGainmap) {
Preconditions.checkArgument(
Expand Down
11 changes: 8 additions & 3 deletions library/src/main/java/com/bumptech/glide/GlideBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -497,11 +497,13 @@ public GlideBuilder setPreserveGainmapAndColorSpaceForTransformations(boolean is
}

/**
* @deprecated This method does nothing. It will be hard coded and removed in a future release
* without further warning.
* Fixes decoding of hardware gainmaps from Ultra HDR images on Android U.
*
* <p>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.
*/
@Deprecated
public GlideBuilder setEnableHardwareGainmapFixOnU(boolean isEnabled) {
glideExperimentsBuilder.update(new EnableHardwareGainmapFixOnU(), isEnabled);
return this;
}

Expand Down Expand Up @@ -618,6 +620,9 @@ static final class ManualOverrideHardwareBitmapMaxFdCount 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)}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.gifdecoder.GifDecoder;
import com.bumptech.glide.load.ImageHeaderParser;
Expand Down Expand Up @@ -158,7 +159,8 @@ private static void initializeDefaults(
registry.getImageHeaderParsers(),
resources.getDisplayMetrics(),
bitmapPool,
arrayPool);
arrayPool,
experiments.isEnabled(EnableHardwareGainmapFixOnU.class));

ResourceDecoder<ByteBuffer, Bitmap> byteBufferBitmapDecoder;
ResourceDecoder<InputStream, Bitmap> streamBitmapDecoder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,35 @@ 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 enableHardwareGainmapFixOnU;

public Downsampler(
List<ImageHeaderParser> parsers,
DisplayMetrics displayMetrics,
BitmapPool bitmapPool,
ArrayPool byteArrayPool) {
this(
parsers,
displayMetrics,
bitmapPool,
byteArrayPool,
/* enableHardwareGainmapFixOnU= */ false);
}

/**
* @param enableHardwareGainmapFixOnU Fixes issues with hardware gainmaps on U.
*/
public Downsampler(
List<ImageHeaderParser> parsers,
DisplayMetrics displayMetrics,
BitmapPool bitmapPool,
ArrayPool byteArrayPool,
boolean enableHardwareGainmapFixOnU) {
this.parsers = parsers;
this.displayMetrics = Preconditions.checkNotNull(displayMetrics);
this.bitmapPool = Preconditions.checkNotNull(bitmapPool);
this.byteArrayPool = Preconditions.checkNotNull(byteArrayPool);
this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU;
}

public boolean handles(@SuppressWarnings("unused") InputStream is) {
Expand Down Expand Up @@ -187,7 +206,8 @@ public Resource<Bitmap> 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,
Expand Down Expand Up @@ -222,7 +242,8 @@ public Resource<Bitmap> decode(
DecodeCallbacks callbacks)
throws IOException {
return decode(
new ImageReader.InputStreamImageReader(is, parsers, byteArrayPool),
new ImageReader.InputStreamImageReader(
is, parsers, byteArrayPool, enableHardwareGainmapFixOnU),
requestedWidth,
requestedHeight,
options,
Expand All @@ -233,7 +254,7 @@ public Resource<Bitmap> 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,
Expand All @@ -244,7 +265,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,
Expand All @@ -257,7 +278,7 @@ public Resource<Bitmap> decode(
throws IOException {
return decode(
new ImageReader.ParcelFileDescriptorImageReader(
parcelFileDescriptor, parsers, byteArrayPool),
parcelFileDescriptor, parsers, byteArrayPool, enableHardwareGainmapFixOnU),
outWidth,
outHeight,
options,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ public static Gainmap convertSingleChannelGainmapToTripleChannelGainmap(Gainmap
private static Bitmap copyAlpha8ToOpaqueArgb888(Bitmap bitmap) {
Preconditions.checkArgument(bitmap.getConfig() == Config.ALPHA_8);
// We have to use a canvas operation with an opaque alpha filter to draw the gainmap. We can't
// use bitmap.copy(Config.ARGB_8888, /* isMutable= */ false) because copying from A8 to RBGA
// will result in zero-valued RGB values.
// use bitmap.copy(Config.ARGB_8888, /* isMutable= */ false) because the output bitmap will
// have zero values for alpha.
Bitmap newContents =
Bitmap.createBitmap(bitmap.getWidth(), bitmap.getHeight(), Config.ARGB_8888);
Canvas canvas = new Canvas(newContents);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,25 @@ final class ByteArrayReader implements ImageReader {
private final byte[] bytes;
private final List<ImageHeaderParser> parsers;
private final ArrayPool byteArrayPool;
private final boolean enableHardwareGainmapFixOnU;

ByteArrayReader(byte[] bytes, List<ImageHeaderParser> parsers, ArrayPool byteArrayPool) {
ByteArrayReader(
byte[] bytes,
List<ImageHeaderParser> 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 GlideBitmapFactory.decodeByteArray(bytes, options);
return enableHardwareGainmapFixOnU
? GlideBitmapFactory.decodeByteArray(bytes, options)
: BitmapFactory.decodeByteArray(bytes, /* offset= */ 0, bytes.length, options);
}

@Override
Expand All @@ -68,18 +76,25 @@ public int getImageOrientation() throws IOException {

@Override
public void stopGrowingBuffers() {}

}

final class FileReader implements ImageReader {

private final File file;
private final List<ImageHeaderParser> parsers;
private final ArrayPool byteArrayPool;
private final boolean enableHardwareGainmapFixOnU;

FileReader(File file, List<ImageHeaderParser> parsers, ArrayPool byteArrayPool) {
FileReader(
File file,
List<ImageHeaderParser> parsers,
ArrayPool byteArrayPool,
boolean enableHardwareGainmapFixOnU) {
this.file = file;
this.parsers = parsers;
this.byteArrayPool = byteArrayPool;
this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU;
}

@Nullable
Expand All @@ -88,7 +103,9 @@ public Bitmap decodeBitmap(Options options) throws FileNotFoundException {
InputStream is = null;
try {
is = new RecyclableBufferedInputStream(new FileInputStream(file), byteArrayPool);
return GlideBitmapFactory.decodeStream(is, options);
return enableHardwareGainmapFixOnU
? GlideBitmapFactory.decodeStream(is, options)
: BitmapFactory.decodeStream(is, /* outPadding= */ null, options);
} finally {
if (is != null) {
try {
Expand Down Expand Up @@ -143,18 +160,26 @@ final class ByteBufferReader implements ImageReader {
private final ByteBuffer buffer;
private final List<ImageHeaderParser> parsers;
private final ArrayPool byteArrayPool;
private final boolean enableHardwareGainmapFixOnU;

ByteBufferReader(ByteBuffer buffer, List<ImageHeaderParser> parsers, ArrayPool byteArrayPool) {
ByteBufferReader(
ByteBuffer buffer,
List<ImageHeaderParser> parsers,
ArrayPool byteArrayPool,
boolean enableHardwareGainmapFixOnU) {
this.buffer = buffer;
this.parsers = parsers;
this.byteArrayPool = byteArrayPool;
this.enableHardwareGainmapFixOnU = enableHardwareGainmapFixOnU;
}

@Nullable
@Override
public Bitmap decodeBitmap(Options options) {
InputStream inputStream = stream();
return GlideBitmapFactory.decodeStream(inputStream, options);
return enableHardwareGainmapFixOnU
? GlideBitmapFactory.decodeStream(inputStream, options)
: BitmapFactory.decodeStream(inputStream, /* outPadding= */ null, options);
}

@Override
Expand All @@ -180,20 +205,27 @@ final class InputStreamImageReader implements ImageReader {
private final InputStreamRewinder dataRewinder;
private final ArrayPool byteArrayPool;
private final List<ImageHeaderParser> parsers;
private final boolean enableHardwareGainmapFixOnU;

InputStreamImageReader(
InputStream is, List<ImageHeaderParser> parsers, ArrayPool byteArrayPool) {
InputStream is,
List<ImageHeaderParser> 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 {
InputStream inputStream = dataRewinder.rewindAndGet();
return GlideBitmapFactory.decodeStream(inputStream, options);
return enableHardwareGainmapFixOnU
? GlideBitmapFactory.decodeStream(inputStream, options)
: BitmapFactory.decodeStream(inputStream, /* outPadding= */ null, options);
}

@Override
Expand All @@ -217,23 +249,29 @@ final class ParcelFileDescriptorImageReader implements ImageReader {
private final ArrayPool byteArrayPool;
private final List<ImageHeaderParser> parsers;
private final ParcelFileDescriptorRewinder dataRewinder;
private final boolean enableHardwareGainmapFixOnU;

ParcelFileDescriptorImageReader(
ParcelFileDescriptor parcelFileDescriptor,
List<ImageHeaderParser> 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 {
ParcelFileDescriptor parcelFileDescriptor = dataRewinder.rewindAndGet();
FileDescriptor fileDescriptor = parcelFileDescriptor.getFileDescriptor();
return GlideBitmapFactory.decodeFileDescriptor(fileDescriptor, options);
return enableHardwareGainmapFixOnU
? GlideBitmapFactory.decodeFileDescriptor(fileDescriptor, options)
: BitmapFactory.decodeFileDescriptor(
parcelFileDescriptor.getFileDescriptor(), /* outPadding= */ null, options);
}

@Override
Expand Down

0 comments on commit ca64948

Please sign in to comment.