From 2c259f532bee14a4f3f6be419bcfb58ef5e22ff5 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Sat, 18 Oct 2014 11:20:44 -0700 Subject: [PATCH] Set transIndex for GIFs with transparent pixels. Fixes #201. --- .../load/resource/gif/GifDrawableTest.java | 11 +-- .../load/resource/gif/GifBitmapProvider.java | 2 +- .../glide/load/resource/gif/GifDrawable.java | 3 +- .../load/resource/gif/GifFrameManager.java | 39 +--------- .../bumptech/glide/gifdecoder/GifDecoder.java | 27 ++++--- .../bumptech/glide/gifdecoder/GifHeader.java | 1 - .../glide/gifdecoder/GifHeaderParser.java | 1 - .../glide/gifencoder/AnimatedGifEncoder.java | 72 ++++++++++--------- 8 files changed, 63 insertions(+), 93 deletions(-) diff --git a/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java index b0bb1fb2d..abfc06063 100644 --- a/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java +++ b/library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java @@ -246,19 +246,10 @@ public class GifDrawableTest { } @Test - public void testGetOpacityReturnsTransparentIfDecoderHasTransparency() { - when(gifDecoder.isTransparent()).thenReturn(true); - + public void testGetOpacityReturnsTransparent() { assertEquals(PixelFormat.TRANSPARENT, drawable.getOpacity()); } - @Test - public void testGetOpacityReturnsOpaqueIfDecoderDoesNotHaveTransparency() { - when(gifDecoder.isTransparent()).thenReturn(false); - - assertEquals(PixelFormat.OPAQUE, drawable.getOpacity()); - } - @Test public void testReturnsFrameCountFromDecoder() { int expected = 4; diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifBitmapProvider.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifBitmapProvider.java index 32008d3e8..be7381e53 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifBitmapProvider.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifBitmapProvider.java @@ -14,6 +14,6 @@ class GifBitmapProvider implements GifDecoder.BitmapProvider { @Override public Bitmap obtain(int width, int height, Bitmap.Config config) { - return bitmapPool.get(width, height, config); + return bitmapPool.getDirty(width, height, config); } } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java index 2d740c217..85c539ac1 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java @@ -197,7 +197,8 @@ public class GifDrawable extends GlideDrawable implements GifFrameManager.FrameC @Override public int getOpacity() { - return decoder.isTransparent() ? PixelFormat.TRANSPARENT : PixelFormat.OPAQUE; + // We can't tell, so default to transparent to be safe. + return PixelFormat.TRANSPARENT; } @TargetApi(Build.VERSION_CODES.HONEYCOMB) diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java index 773203b9a..32dce79a3 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java @@ -5,26 +5,17 @@ import android.graphics.Bitmap; import android.os.Handler; import android.os.Looper; import android.os.SystemClock; - import com.bumptech.glide.Glide; import com.bumptech.glide.gifdecoder.GifDecoder; import com.bumptech.glide.load.Encoder; -import com.bumptech.glide.load.ResourceDecoder; -import com.bumptech.glide.load.ResourceEncoder; import com.bumptech.glide.load.Transformation; import com.bumptech.glide.load.engine.DiskCacheStrategy; import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool; import com.bumptech.glide.load.engine.cache.MemorySizeCalculator; -import com.bumptech.glide.load.resource.NullDecoder; import com.bumptech.glide.load.resource.NullEncoder; -import com.bumptech.glide.load.resource.NullResourceEncoder; -import com.bumptech.glide.load.resource.bitmap.BitmapEncoder; -import com.bumptech.glide.load.resource.bitmap.StreamBitmapDecoder; -import com.bumptech.glide.load.resource.file.FileToStreamDecoder; import com.bumptech.glide.request.animation.GlideAnimation; import com.bumptech.glide.request.target.SimpleTarget; - -import java.io.File; +import com.bumptech.glide.util.Util; class GifFrameManager { /** 60fps is {@value #MIN_FRAME_DELAY}ms per frame. */ @@ -32,10 +23,8 @@ class GifFrameManager { private final MemorySizeCalculator calculator; private final GifFrameModelLoader frameLoader; private final GifFrameResourceDecoder frameResourceDecoder; - private final ResourceDecoder cacheDecoder; private final GifDecoder decoder; private final Handler mainHandler; - private final ResourceEncoder encoder; private final Context context; private final Encoder sourceEncoder; private final Transformation[] transformation; @@ -69,23 +58,11 @@ class GifFrameManager { this.transformation = new Transformation[] {transformation}; this.targetWidth = targetWidth; this.targetHeight = targetHeight; - this.totalFrameSize = frameWidth * frameHeight * (decoder.isTransparent() ? 4 : 2); + this.totalFrameSize = Util.getBitmapByteSize(frameWidth, frameHeight, Bitmap.Config.ARGB_8888); this.calculator = new MemorySizeCalculator(context); this.frameLoader = new GifFrameModelLoader(); this.sourceEncoder = NullEncoder.get(); - - if (!decoder.isTransparent()) { - // For non transparent gifs, we can beat the performance of our gif decoder for each frame by decoding jpegs - // from disk. - this.cacheDecoder = new FileToStreamDecoder(new StreamBitmapDecoder(context)); - this.encoder = new BitmapEncoder(); - } else { - // For transparent gifs, we would have to encode as pngs which is actually slower than our gif decoder so we - // avoid writing frames to the disk cache entirely. - this.cacheDecoder = NullDecoder.get(); - this.encoder = NullResourceEncoder.get(); - } } Transformation getTransformation() { @@ -95,17 +72,9 @@ class GifFrameManager { public void getNextFrame(FrameCallback cb) { decoder.advance(); - /** - * Note - Using the disk cache can potentially cause frames to be decoded incorrectly because the decoder is - * sequential. If earlier frames are evicted for some reason, later ones may then not be decoded correctly. - */ - // We don't want to blow out the entire memory cache with frames of gifs, so try to set some // maximum size beyond which we will always just decode one frame at a time. boolean skipCache = totalFrameSize > calculator.getMemoryCacheSize() / 2; - // We can decode non transparent (cached as jpegs) frames more quickly from cache, but transparent - // (cached as png) frames more quickly from the gif data. - boolean skipDiskCache = decoder.isTransparent(); long targetTime = SystemClock.uptimeMillis() + Math.max(MIN_FRAME_DELAY, decoder.getNextDelay()); next = new DelayTarget(cb, targetTime); @@ -117,11 +86,9 @@ class GifFrameManager { .as(Bitmap.class) .sourceEncoder(sourceEncoder) .decoder(frameResourceDecoder) - .cacheDecoder(cacheDecoder) - .encoder(encoder) .transform(transformation) .skipMemoryCache(skipCache) - .diskCacheStrategy(skipDiskCache ? DiskCacheStrategy.NONE : DiskCacheStrategy.RESULT) + .diskCacheStrategy(DiskCacheStrategy.NONE) .into(next); } diff --git a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java index efef2392b..e1f054554 100644 --- a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java +++ b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java @@ -24,8 +24,9 @@ package com.bumptech.glide.gifdecoder; * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +import android.annotation.TargetApi; import android.graphics.Bitmap; -import android.graphics.Color; +import android.os.Build; import android.util.Log; import java.io.ByteArrayOutputStream; @@ -144,10 +145,6 @@ public class GifDecoder { return header.height; } - public boolean isTransparent() { - return header.isTransparent; - } - public byte[] getData() { return data; } @@ -323,6 +320,7 @@ public class GifDecoder { rawData.rewind(); rawData.order(ByteOrder.LITTLE_ENDIAN); + // No point in specially saving an old frame if we're never going to use it. savePrevious = false; for (GifFrame frame : header.frames) { @@ -458,7 +456,7 @@ public class GifDecoder { } } - //Copy pixels into previous image + // Copy pixels into previous image if (savePrevious && currentFrame.dispose == DISPOSAL_UNSPECIFIED || currentFrame.dispose == DISPOSAL_NONE) { if (previousImage == null) { previousImage = getNextBitmap(); @@ -631,8 +629,10 @@ public class GifDecoder { } private Bitmap.Config getPreferredConfig() { - if (config == Bitmap.Config.RGB_565 && !header.isTransparent) { - return Bitmap.Config.RGB_565; + // We can't tell if a gif has transparency to decode a partial frame on top of a previous frame, or if the final + // frame will actually have transparent pixels, so we must always use a format that supports transparency. + if (config == Bitmap.Config.RGB_565 || config == Bitmap.Config.ARGB_4444) { + return Bitmap.Config.ARGB_4444; } else { return Bitmap.Config.ARGB_8888; } @@ -643,10 +643,15 @@ public class GifDecoder { Bitmap result = bitmapProvider.obtain(header.width, header.height, targetConfig); if (result == null) { result = Bitmap.createBitmap(header.width, header.height, targetConfig); - } else { - // If we're reusing a bitmap it may have other things drawn in it which we need to remove. - result.eraseColor(Color.TRANSPARENT); } + setAlpha(result); return result; } + + @TargetApi(12) + private static void setAlpha(Bitmap bitmap) { + if (Build.VERSION.SDK_INT >= 12) { + bitmap.setHasAlpha(true); + } + } } diff --git a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeader.java b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeader.java index 157ea57d8..961529014 100644 --- a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeader.java +++ b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeader.java @@ -34,7 +34,6 @@ public class GifHeader { int pixelAspect; //TODO: this is set both during reading the header and while decoding frames... int bgColor; - boolean isTransparent; int loopCount; public int getHeight() { diff --git a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java index e95738239..a74bb280e 100644 --- a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java +++ b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java @@ -143,7 +143,6 @@ public class GifHeaderParser { header.currentFrame.dispose = 1; } header.currentFrame.transparency = (packed & 1) != 0; - header.isTransparent |= header.currentFrame.transparency; // Delay in milliseconds. header.currentFrame.delay = readShort() * 10; // Transparent color index diff --git a/third_party/gif_encoder/src/main/java/com/bumptech/glide/gifencoder/AnimatedGifEncoder.java b/third_party/gif_encoder/src/main/java/com/bumptech/glide/gifencoder/AnimatedGifEncoder.java index 3db8a1864..2442a0d48 100644 --- a/third_party/gif_encoder/src/main/java/com/bumptech/glide/gifencoder/AnimatedGifEncoder.java +++ b/third_party/gif_encoder/src/main/java/com/bumptech/glide/gifencoder/AnimatedGifEncoder.java @@ -36,45 +36,47 @@ import java.io.OutputStream; public class AnimatedGifEncoder { - protected int width; // image size + private int width; // image size - protected int height; + private int height; - protected Integer transparent = null; // transparent color if given + private Integer transparent = null; // transparent color if given - protected int transIndex; // transparent index in color table + private int transIndex; // transparent index in color table - protected int repeat = -1; // no repeat + private int repeat = -1; // no repeat - protected int delay = 0; // frame delay (hundredths) + private int delay = 0; // frame delay (hundredths) - protected boolean started = false; // ready to output frames + private boolean started = false; // ready to output frames - protected OutputStream out; + private OutputStream out; - protected Bitmap image; // current frame + private Bitmap image; // current frame - protected byte[] pixels; // BGR byte array from frame + private byte[] pixels; // BGR byte array from frame - protected byte[] indexedPixels; // converted frame indexed to palette + private byte[] indexedPixels; // converted frame indexed to palette - protected int colorDepth; // number of bit planes + private int colorDepth; // number of bit planes - protected byte[] colorTab; // RGB palette + private byte[] colorTab; // RGB palette - protected boolean[] usedEntry = new boolean[256]; // active palette entries + private boolean[] usedEntry = new boolean[256]; // active palette entries - protected int palSize = 7; // color table size (bits-1) + private int palSize = 7; // color table size (bits-1) - protected int dispose = -1; // disposal code (-1 = use default) + private int dispose = -1; // disposal code (-1 = use default) - protected boolean closeStream = false; // close stream when finished + private boolean closeStream = false; // close stream when finished - protected boolean firstFrame = true; + private boolean firstFrame = true; - protected boolean sizeSet = false; // if false, get size from first frame + private boolean sizeSet = false; // if false, get size from first frame - protected int sample = 10; // default sample interval for quantizer + private int sample = 10; // default sample interval for quantizer + + private boolean hasTransparentPixels; /** * Sets the delay time between each frame, or changes it for subsequent frames @@ -300,7 +302,7 @@ public class AnimatedGifEncoder { /** * Analyzes image colors and creates color map. */ - protected void analyzePixels() { + private void analyzePixels() { int len = pixels.length; int nPix = len / 3; indexedPixels = new byte[nPix]; @@ -327,6 +329,8 @@ public class AnimatedGifEncoder { // get closest match to transparent color if specified if (transparent != null) { transIndex = findClosest(transparent); + } else if (hasTransparentPixels) { + transIndex = findClosest(Color.TRANSPARENT); } } @@ -334,7 +338,7 @@ public class AnimatedGifEncoder { * Returns index of palette color closest to c * */ - protected int findClosest(int color) { + private int findClosest(int color) { if (colorTab == null) return -1; int r = Color.red(color); @@ -361,7 +365,7 @@ public class AnimatedGifEncoder { /** * Extracts image pixels into byte array "pixels" */ - protected void getImagePixels() { + private void getImagePixels() { int w = image.getWidth(); int h = image.getHeight(); @@ -379,7 +383,11 @@ public class AnimatedGifEncoder { pixels = new byte[pixelsInt.length * 3]; int pixelsIndex = 0; + hasTransparentPixels = false; for (final int pixel : pixelsInt) { + if (pixel == Color.TRANSPARENT) { + hasTransparentPixels = true; + } pixels[pixelsIndex++] = (byte) (pixel & 0xFF); pixels[pixelsIndex++] = (byte) ((pixel >> 8) & 0xFF); pixels[pixelsIndex++] = (byte) ((pixel >> 16) & 0xFF); @@ -389,12 +397,12 @@ public class AnimatedGifEncoder { /** * Writes Graphic Control Extension */ - protected void writeGraphicCtrlExt() throws IOException { + private void writeGraphicCtrlExt() throws IOException { out.write(0x21); // extension introducer out.write(0xf9); // GCE label out.write(4); // data block size int transp, disp; - if (transparent == null) { + if (transparent == null && !hasTransparentPixels) { transp = 0; disp = 0; // dispose = no action } else { @@ -420,7 +428,7 @@ public class AnimatedGifEncoder { /** * Writes Image Descriptor */ - protected void writeImageDesc() throws IOException { + private void writeImageDesc() throws IOException { out.write(0x2c); // image separator writeShort(0); // image position x,y = 0,0 writeShort(0); @@ -443,7 +451,7 @@ public class AnimatedGifEncoder { /** * Writes Logical Screen Descriptor */ - protected void writeLSD() throws IOException { + private void writeLSD() throws IOException { // logical screen size writeShort(width); writeShort(height); @@ -460,7 +468,7 @@ public class AnimatedGifEncoder { /** * Writes Netscape application extension to define repeat count. */ - protected void writeNetscapeExt() throws IOException { + private void writeNetscapeExt() throws IOException { out.write(0x21); // extension introducer out.write(0xff); // app extension label out.write(11); // block size @@ -474,7 +482,7 @@ public class AnimatedGifEncoder { /** * Writes color table */ - protected void writePalette() throws IOException { + private void writePalette() throws IOException { out.write(colorTab, 0, colorTab.length); int n = (3 * 256) - colorTab.length; for (int i = 0; i < n; i++) { @@ -485,7 +493,7 @@ public class AnimatedGifEncoder { /** * Encodes and writes pixel data */ - protected void writePixels() throws IOException { + private void writePixels() throws IOException { LZWEncoder encoder = new LZWEncoder(width, height, indexedPixels, colorDepth); encoder.encode(out); } @@ -493,7 +501,7 @@ public class AnimatedGifEncoder { /** * Write 16-bit value to output stream, LSB first */ - protected void writeShort(int value) throws IOException { + private void writeShort(int value) throws IOException { out.write(value & 0xff); out.write((value >> 8) & 0xff); } @@ -501,7 +509,7 @@ public class AnimatedGifEncoder { /** * Writes string to output stream */ - protected void writeString(String s) throws IOException { + private void writeString(String s) throws IOException { for (int i = 0; i < s.length(); i++) { out.write((byte) s.charAt(i)); } -- GitLab