Skip to content

Commit

Permalink
Default GIF frames to ARGB_8888 and configure them via DecodeFormat.
Browse files Browse the repository at this point in the history
Fixes #2396.
  • Loading branch information
sjudd committed Oct 13, 2017
1 parent a9ab473 commit 6f91031
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.bumptech.glide.load.ImageHeaderParser;
import com.bumptech.glide.load.ImageHeaderParser.ImageType;
import com.bumptech.glide.load.ImageHeaderParserUtils;
import com.bumptech.glide.load.Option;
import com.bumptech.glide.load.Options;
import com.bumptech.glide.load.ResourceDecoder;
import com.bumptech.glide.load.Transformation;
Expand All @@ -32,15 +31,6 @@
public class ByteBufferGifDecoder implements ResourceDecoder<ByteBuffer, GifDrawable> {
private static final String TAG = "BufferGifDecoder";
private static final GifDecoderFactory GIF_DECODER_FACTORY = new GifDecoderFactory();

/**
* If set to {@code true}, disables this decoder
* ({@link #handles(ByteBuffer, Options)} will return {@code false}). Defaults to
* {@code false}.
*/
public static final Option<Boolean> DISABLE_ANIMATION = Option.memory(
"com.bumptech.glide.load.resource.gif.ByteBufferGifDecoder.DisableAnimation", false);

private static final GifHeaderParserPool PARSER_POOL = new GifHeaderParserPool();

private final Context context;
Expand Down Expand Up @@ -79,7 +69,7 @@ public ByteBufferGifDecoder(

@Override
public boolean handles(ByteBuffer source, Options options) throws IOException {
return !options.get(DISABLE_ANIMATION)
return !options.get(GifOptions.DISABLE_ANIMATION)
&& ImageHeaderParserUtils.getType(parsers, source) == ImageType.GIF;
}

Expand All @@ -102,7 +92,6 @@ private GifDrawableResource decode(ByteBuffer byteBuffer, int width, int height,
return null;
}


int sampleSize = getSampleSize(header, width, height);
GifDecoder gifDecoder = gifDecoderFactory.build(provider, header, byteBuffer, sampleSize);
gifDecoder.advance();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.bumptech.glide.load.resource.gif;

import com.bumptech.glide.load.DecodeFormat;
import com.bumptech.glide.load.Option;
import com.bumptech.glide.load.Options;
import com.bumptech.glide.load.ResourceDecoder;

/**
* Options related to decoding GIFs.
*/
public final class GifOptions {

/**
* Indicates the {@link com.bumptech.glide.load.DecodeFormat} that will be used in conjunction
* with the particular GIF to determine the {@link android.graphics.Bitmap.Config} to use when
* decoding frames of GIFs.
*/
public static final Option<DecodeFormat> DECODE_FORMAT = Option.memory(
"com.bumptech.glide.load.resource.gif.DecodeFormat", DecodeFormat.DEFAULT);

/**
* If set to {@code true}, disables the GIF {@link com.bumptech.glide.load.ResourceDecoder}s
* ({@link ResourceDecoder#handles(Object, Options)} will return {@code false}). Defaults to
* {@code false}.
*/
public static final Option<Boolean> DISABLE_ANIMATION = Option.memory(
"com.bumptech.glide.load.resource.gif.ByteBufferGifDecoder.DisableAnimation", false);

private GifOptions() {
// Utility class.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.bumptech.glide.load.ImageHeaderParser;
import com.bumptech.glide.load.ImageHeaderParser.ImageType;
import com.bumptech.glide.load.ImageHeaderParserUtils;
import com.bumptech.glide.load.Option;
import com.bumptech.glide.load.Options;
import com.bumptech.glide.load.ResourceDecoder;
import com.bumptech.glide.load.engine.Resource;
Expand All @@ -22,13 +21,6 @@
*/
public class StreamGifDecoder implements ResourceDecoder<InputStream, GifDrawable> {
private static final String TAG = "StreamGifDecoder";
/**
* If set to {@code true}, disables this decoder
* ({@link #handles(InputStream, Options)} will return {@code false}). Defaults to
* {@code false}.
*/
public static final Option<Boolean> DISABLE_ANIMATION = Option.memory(
"com.bumptech.glide.load.resource.gif.ByteBufferGifDecoder.DisableAnimation", false);

private final List<ImageHeaderParser> parsers;
private final ResourceDecoder<ByteBuffer, GifDrawable> byteBufferDecoder;
Expand All @@ -43,7 +35,7 @@ public StreamGifDecoder(List<ImageHeaderParser> parsers, ResourceDecoder<ByteBuf

@Override
public boolean handles(InputStream source, Options options) throws IOException {
return !options.get(DISABLE_ANIMATION)
return !options.get(GifOptions.DISABLE_ANIMATION)
&& ImageHeaderParserUtils.getType(parsers, source, byteArrayPool) == ImageType.GIF;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@
import com.bumptech.glide.load.resource.bitmap.Downsampler;
import com.bumptech.glide.load.resource.bitmap.FitCenter;
import com.bumptech.glide.load.resource.bitmap.VideoBitmapDecoder;
import com.bumptech.glide.load.resource.gif.ByteBufferGifDecoder;
import com.bumptech.glide.load.resource.gif.GifDrawable;
import com.bumptech.glide.load.resource.gif.GifDrawableTransformation;
import com.bumptech.glide.load.resource.gif.StreamGifDecoder;
import com.bumptech.glide.load.resource.gif.GifOptions;
import com.bumptech.glide.signature.EmptySignature;
import com.bumptech.glide.util.Preconditions;
import com.bumptech.glide.util.Util;
Expand Down Expand Up @@ -838,22 +837,31 @@ public RequestOptions frame(@IntRange(from = 0) long frameTimeMicros) {

/**
* Sets the {@link DecodeFormat} to use when decoding {@link Bitmap} objects using
* {@link Downsampler}.
* {@link Downsampler} and Glide's default GIF decoders.
*
* <p>{@link DecodeFormat} is a request, not a requirement. It's possible the resource will be
* decoded using a decoder that cannot control the format
* ({@link android.media.MediaMetadataRetriever} for example), or that the decoder may choose to
* ignore the requested format if it can't display the image (i.e. RGB_565 is requested, but the
* image has alpha).
*
* <p>This is a component option specific to {@link Downsampler}. If the defautlt Bitmap decoder
* is replaced or skipped because of your configuration, this option may be ignored.
* <p>This is a component option specific to {@link Downsampler} and Glide's GIF decoders. If the
* default Bitmap decoders are replaced or skipped because of your configuration, this option may
* be ignored.
*
* <p>To set only the format used when decoding {@link Bitmap}s, use
* {@link #option(Option, Object)} and {@link Downsampler#DECODE_FORMAT}. To set only the format
* used when decoding GIF frames, use {@link #option(Option, Object)} and
* {@link GifOptions#DECODE_FORMAT}.
*
* @see Downsampler#DECODE_FORMAT
* @see GifOptions#DECODE_FORMAT
*/
@CheckResult
public RequestOptions format(@NonNull DecodeFormat format) {
return set(Downsampler.DECODE_FORMAT, Preconditions.checkNotNull(format));
Preconditions.checkNotNull(format);
return set(Downsampler.DECODE_FORMAT, format)
.set(GifOptions.DECODE_FORMAT, format);
}

/**
Expand Down Expand Up @@ -1248,8 +1256,8 @@ public RequestOptions dontAnimate() {
return clone().dontAnimate();
}

set(ByteBufferGifDecoder.DISABLE_ANIMATION, true);
set(StreamGifDecoder.DISABLE_ANIMATION, true);
set(GifOptions.DISABLE_ANIMATION, true);
set(GifOptions.DISABLE_ANIMATION, true);

This comment has been minimized.

Copy link
@TWiStErRob

TWiStErRob Oct 14, 2017

Collaborator

Setting the same thing twice?

return selfOrThrowIfLocked();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ public void testHandlesStreamIfContainsGifHeaderAndDisabledIsNotSet() throws IOE

@Test
public void testHandlesStreamIfContainsGifHeaderAndDisabledIsFalse() throws IOException {
options.set(ByteBufferGifDecoder.DISABLE_ANIMATION, false);
options.set(GifOptions.DISABLE_ANIMATION, false);
assertThat(decoder.handles(ByteBuffer.wrap(GIF_HEADER), options)).isTrue();
}

@Test
public void testDoesNotHandleStreamIfDisabled() throws IOException {
options.set(ByteBufferGifDecoder.DISABLE_ANIMATION, true);
options.set(GifOptions.DISABLE_ANIMATION, true);
assertThat(decoder.handles(ByteBuffer.wrap(GIF_HEADER), options)).isFalse();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ public void testHandlesStreamIfContainsGifHeaderAndDisabledIsNotSet() throws IOE

@Test
public void testHandlesStreamIfContainsGifHeaderAndDisabledIsFalse() throws IOException {
options.set(StreamGifDecoder.DISABLE_ANIMATION, false);
options.set(GifOptions.DISABLE_ANIMATION, false);
assertThat(decoder.handles(new ByteArrayInputStream(GIF_HEADER), options)).isTrue();
}

@Test
public void testDoesNotHandleStreamIfDisabled() throws IOException {
options.set(StreamGifDecoder.DISABLE_ANIMATION, true);
options.set(GifOptions.DISABLE_ANIMATION, true);
assertThat(decoder.handles(new ByteArrayInputStream(GIF_HEADER), options)).isFalse();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Shared interface for GIF decoders.
*/
public interface GifDecoder {

/** File read status: No errors. */
int STATUS_OK = 0;
/** File read status: Error decoding file (may be partially decoded). */
Expand Down Expand Up @@ -216,4 +217,21 @@ public interface BitmapProvider {
@GifDecodeStatus
int read(byte[] data);


/**
* Sets the default {@link android.graphics.Bitmap.Config} to use when decoding frames of a GIF.
*
* <p>Valid options are {@link android.graphics.Bitmap.Config#ARGB_8888} and
* {@link android.graphics.Bitmap.Config#RGB_565}.
* {@link android.graphics.Bitmap.Config#ARGB_8888} will produce higher quality frames, but will
* also use 2x the memory of {@link android.graphics.Bitmap.Config#RGB_565}.
*
* <p>Defaults to {@link android.graphics.Bitmap.Config#ARGB_8888}
*
* <p>This value is not a guarantee. For example if set to
* {@link android.graphics.Bitmap.Config#RGB_565} and the GIF contains transparent pixels,
* {@link android.graphics.Bitmap.Config#ARGB_8888} will be used anyway to support the
* transparency.
*/
void setDefaultBitmapFormat(Bitmap.Config format);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
import static com.bumptech.glide.gifdecoder.GifFrame.DISPOSAL_UNSPECIFIED;

import android.graphics.Bitmap;
import android.graphics.Bitmap.Config;
import android.support.annotation.ColorInt;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.util.Log;
import java.io.ByteArrayOutputStream;
Expand Down Expand Up @@ -121,6 +123,8 @@ public class StandardGifDecoder implements GifDecoder {
private int downsampledHeight;
private int downsampledWidth;
private boolean isFirstFrameTransparent;
@NonNull
private Bitmap.Config bitmapConfig = Config.ARGB_8888;

public StandardGifDecoder(
GifDecoder.BitmapProvider provider, GifHeader gifHeader, ByteBuffer rawData) {
Expand Down Expand Up @@ -396,6 +400,16 @@ public synchronized int read(byte[] data) {
return status;
}

@Override
public void setDefaultBitmapFormat(Bitmap.Config config) {
if (config != Bitmap.Config.ARGB_8888 && config != Bitmap.Config.RGB_565) {
throw new IllegalArgumentException("Unsupported format: " + config
+ ", must be one of " + Bitmap.Config.ARGB_8888 + " or " + Bitmap.Config.RGB_565);
}

bitmapConfig = config;
}

/**
* Creates new frame image from current data (and previous frames as specified by their
* disposition codes).
Expand Down Expand Up @@ -779,7 +793,7 @@ private int readBlock() {

private Bitmap getNextBitmap() {
Bitmap.Config config = isFirstFrameTransparent
? Bitmap.Config.ARGB_8888 : Bitmap.Config.RGB_565;
? Bitmap.Config.ARGB_8888 : bitmapConfig;
Bitmap result = bitmapProvider.obtain(downsampledWidth, downsampledHeight, config);
result.setHasAlpha(true);
return result;
Expand Down

0 comments on commit 6f91031

Please sign in to comment.