Glide: black square background appears under the transparent gif.

[ourgit]

I am using glide to show multi emotion gifs, most of time the glide shows the gifs correctly,however,there would be black square background appearing under the transparent gif sometimes ,and the black square background will disappear if I scroll the listview. Any solution for that? thanks

[sjudd]

Can you provide the GIF in question? Can you reproduce the issue in a small sample app?

[ourgit]

thanks for reply.
here are the gifs I used: https://drive.google.com/file/d/0B5ghBZcVHgnKd2YxQ0lKZHNHd00/view?usp=sharing

I find it hard to reproduce the issue in a small sample app. I guess glide use too much cpu or memory which cause resource intense. I try to use android-gif-library which do not cause such issue. All I focus now is how to effectively release the memory and cpu when those gifs go off the screen. Maybe that would do some help.

[TWiStErRob]

@sjudd I gave him an example for releasing resources in #1062.

@ourgit Is this the same list? because if it's Html.ImageGetter that's an important info. Try my solution first, and if you can still reproduce this, please report back. It's possible that your UrlDrawable draws a black background and the real Drawable doesn't fully overdraw it. I used WrappingDrawable which doesn't do unnecessary drawing and was designed for this use case.

[ourgit]

@TWiStErRob
Yeah,the same list.

here is the UrlDrawable code:

/**
 * Implements {@link Drawable.Callback} in order to show animated GIFs in the TextView.
 * <p/>
 */
public final class UrlDrawable extends Drawable implements Drawable.Callback {

    private GlideDrawable mDrawable;

    public GlideDrawable getDrawable() {
        return mDrawable;
    }

    @Override
    public void draw(Canvas canvas) {
        if (mDrawable != null && mDrawable.isVisible()) {
            try {
                mDrawable.draw(canvas);
            } catch (RuntimeException e) {
                Log.e("UrlDrawable", "无法draw:" + e.getMessage());
            }
        }
    }

    @Override
    public void setAlpha(int alpha) {
        if (mDrawable != null) {
            mDrawable.setAlpha(alpha);
        }
    }

    @Override
    public void setColorFilter(ColorFilter cf) {
        if (mDrawable != null) {
            mDrawable.setColorFilter(cf);
        }
    }

    @Override
    public int getOpacity() {
        if (mDrawable != null) {
            return mDrawable.getOpacity();
        }
        return 0;
    }

    public void setDrawable(GlideDrawable drawable) {
        if(null == drawable){
            if (this.mDrawable != null) {
                this.mDrawable.setCallback(null);
            }
            return;
        }
        if (this.mDrawable != null) {
            this.mDrawable.setCallback(null);
        }
        drawable.setCallback(this);
        this.mDrawable = drawable;
    }

    @Override
    public void invalidateDrawable(Drawable who) {
        if (getCallback() != null) {
            getCallback().invalidateDrawable(who);
        }
    }

    @Override
    public void scheduleDrawable(Drawable who, Runnable what, long when) {
        if (getCallback() != null) {
            getCallback().scheduleDrawable(who, what, when);
        }
    }

    @Override
    public void unscheduleDrawable(Drawable who, Runnable what) {
        if (getCallback() != null) {
            getCallback().unscheduleDrawable(who, what);
        }
    }
}

help me review the code, I have no idea for UrlDrawable can draw black background through code.
Thank you.

[ourgit]

for better understand the issue, I shot a video,here is the link:
https://drive.google.com/file/d/0B5ghBZcVHgnKZXpLal9qM0dzX3c/view?usp=sharing

[TWiStErRob]

OK, so that's really close to what DrawableWrapper is. When I searched I found a version of UrlDrawable that was extending BitmapDrawable, that why I thought it may be that drawing black. One thing that stands out is that getDrawable(null) won't clear the reference to mDrawable, only its callback. Another thing is that you return from getOpacity 0 (ImageFormat.UNKNOWN), maybe you should change that to return ImageFormat.TRANSPARENT (or TRANSLUCENT). I would imagine that TextView asks first and the decides: OK, so it's unknown so I won't support alpha channel, and then a moment later it changes, and it gets confused for a moment. Please try these two changes (setDrawable and getOpacity) alone, as they're the most likely cause for now.

I would try putting a "canvas.drawCircle(red)" in the else brach in the draw method to see on the UI when the Drawable is null.

@sjudd It really looks like a single frame sometimes gets a black background (there were some white flashes as well, I think), but we don't know if it's black pixels in the bitmap or behind the bitmap.

@ourgit so to reproduce this there needs to be a lot of different gifs displayed, but some reused. Do you think it's possible that all that clearing and stopping that's going on is interfering with Glide? Note that messing with a GifDrawable can have an effect later because it may be put in memory cache, and if its state changed before putting in maybe needs a little time to fix itself.

[ourgit]

@TWiStErRob

you really broaden my knowledge , you have done a great job! thank you.

I will try your advice and give you feedback.

[ourgit]

I integrated your sample code to my project,replace all of previous code,however,the issue still exists,black square and white splash still appear and disappear quickly now and then.

[ourgit]

I do some test with your advice, here is the code:
in the UrlDrawable:

@Override
    public void draw(Canvas canvas) {
        if (mDrawable != null && mDrawable.isVisible()) {
            try {
                mDrawable.draw(canvas);
            } catch (RuntimeException e) {
                Log.e("UrlDrawable",  e.getMessage());
            }
        }else{
            Paint paint = new Paint();
            paint.setColor(Color.RED);
            canvas.drawCircle(20,20,100,paint);
        }
    }

    @Override
    public int getOpacity() {
        if (mDrawable != null) {
            return mDrawable.getOpacity();
        }
        return PixelFormat.TRANSPARENT;
    }

and here is the result:
when the gif does not load,the red background will appear,and disappear quickly after the gif loaded.
but the black square still appear and disappear now and then, When it appear,the red background did not appear.

I also test with your sample code without UrlDrawable, get the same result,the black square still appear and disappear now and then.

[TWiStErRob]

The opacity is handled well by DrawableWrapper, the default "null object" in the target is transparent, so the TextView will receive the right constant, and the gifs are transparent as well, so there shouldn't be a jump. I think the next step is to confirm that the individual bitmaps in gif are tainted. You should somehow dump the frames, I'll think about how.

[TWiStErRob]

Hmm, is it the same emoticons all the time, or any one of them can flash?

[ourgit]

I have upload the gifs in the post. Maybe some of the gifs have problem, but I have no idea which one has the problem.

any one of them can flash,especially the moment loaded. If you need more detail,I will shot a new video.

[TWiStErRob]

I rewatched the video: the ones that I noticed:

• last :sob: in the row is black
• two :sunglasses: are black
• :frowning: is white and :eyelashes: is black
• then after a moment :cry: is white on the same row
• :sealed-lips: is black
• :sleeping: is black
• :giggle: is black before big scroll
• :open_mouth: is black
• :danceing: is black, :sunglasses: is black (when you say "ah, ah, ah, ah" )
  (the sunglasses frame had only partially black bg)
• :fearful: is black
• :danceing: is black again
• huge :vomit: is black, for a long time?!
• :heart_eyes: is black twice
• :sob: and :heart_eyes: again
• :danceing:, :sunglasses: and :sealed-lips: at the end

[ourgit]

thanks for your time.And do you have solution right now?

[TWiStErRob]

I tried with your GIFs but they render fine (Galaxy S4 4.4.2), here's a way to dump:

private static boolean dump = false;
private static Method getCurrentFrame;
private static Field frameLoader;
static {
    try {
        getCurrentFrame = Class.forName("com.bumptech.glide.load.resource.gif.GifFrameLoader")
                               .getMethod("getCurrentFrame");
        frameLoader = GifDrawable.class.getDeclaredField("frameLoader");
        frameLoader.setAccessible(true);
    } catch (Exception ignore) {
        ignore.printStackTrace();
    }
}

@Override public void invalidateDrawable(Drawable who) {
    if (dump) { // attach debugger here and set the static variable to true,
        // then detach (stop icon) to start dumping,
        // and scroll to until you see black then exit the activity
        dump(who);
    }
    targetView.invalidate();
}
private void dump(Drawable who) {
    Drawable wrapped = ((DrawableWrapper)who).getWrappedDrawable();
    if (!(wrapped instanceof GifDrawable)) {
        return; // the wrapped drawable is sometimes color
    }
    GifDrawable gif = (GifDrawable)wrapped;
    File folder = new File("/sdcard/gif_dump");
    if (!folder.exists()) {
        folder.mkdirs();
    }
    String file = String.format("%s/%d_%08x_%d", folder,
            System.currentTimeMillis(), gif.hashCode(), gif.getDecoder().getCurrentFrameIndex());

    int w = who.getIntrinsicWidth(), h = who.getIntrinsicHeight();
    Bitmap bm = Bitmap.createBitmap(w, h, Config.ARGB_8888);
    Canvas canvas = new Canvas(bm);
    try {
        gif.draw(canvas);
        bm.compress(CompressFormat.PNG, 100, new FileOutputStream(file + "_draw.png"));
    } catch (Exception ignore) {
        ignore.printStackTrace();
    } finally {
        bm.recycle();
    }

    try {
        bm = (Bitmap)getCurrentFrame.invoke(frameLoader.get(gif)); // == gif.frameLoader.getCurrentFrame()
        if (bm != null) {
            bm.compress(CompressFormat.PNG, 100, new FileOutputStream(file + "_frame.png"));
        }
    } catch (Exception ignore) {
        ignore.printStackTrace();
    }
}

This should dump all the rendered frames (and the original bitmap inside GifDrawable), but it is painfully slow, and turn of StrictMode if you have it.

[ourgit]

after dump the frame ,there are more than 5000 pngs generated.However I don't find them any problem ,I can see all of them inside the file manager,then what should I do now?

[TWiStErRob]

Did you see any problems on the UI? If not, don't expect problems in dumped frames either.

Clean up, try to scroll until you see a black/white backgrounded emoticon, immediately press the back button and then look for that frame from the end of the dumped list.

[ourgit]

ok,I will try once more.you comment" attach debugger here and set the static variable to true,
then detach (stop icon) to start dumping, and scroll to until you see black then exit the activity",why need debug,may I just set the static variable to true,and scroll to until see black then exit the activity?

[TWiStErRob]

Yes you can do that too.

With debug flip you can set up your test by interacting with the UI: scrolling to a position where you suspect it will happen and only start dumping after that, so the number of images will be less. The detach step is really important, so the app is not slowed down by the debugger, only the dumping.

[ourgit]

filename:1458644700052_42877e20_0_draw.png

filename:1458644700052_42877e20_0_frame.png

here is the frame I found after try again your solution. Then what should I do now? @TWiStErRob

[TWiStErRob]

Sadly, this just confirmed that the blackness comes from the Bitmap object which was decoded in GifDrawable, not a drawing artifact from TextView/ImageSpan, nor from WrappingDrawable.
You just graduated this issue to a bug...

[ourgit]

I analyse the file name and compare with the generated file name:

 String file = String.format("%s/%d_%08x_%d", folder,
                System.currentTimeMillis(), gif.hashCode(), gif.getDecoder().getCurrentFrameIndex());

does it mean the first frame of the gif corrupt?

[TWiStErRob]

Yes, that's also a good clue, if you can please try to find more black ones and confirm that they're always the 0th frame. Also finding a whitey may be helpful too.

[ourgit]

As for the consideration of product, I prefer to change the gif if I am sure which frame corrupt. It will be much more efficiency by redesign than dozens of code,right?

[ourgit]

But there are many questions come to me,if the frame corrupt,then the gif should behave the same , that is it will show the black square every time,but the facts deny . well I am really puzzled right now...

[TWiStErRob]

change the gif if I am sure which frame corrupt

That could be good short-term solution. Remember that Glide is a public library, so fixing the bug is a better course of action. Think about how much time we spent figuring it out and confirming this is a bug, not even fixed yet. If everyone who uses GIFs may needs to do this it's a huge waste of their time.

really puzzled right now

Yes, the frame seems to render correctly most of the time. To summarize what we know to date:

_When a GIF is loaded and then possibly reused the first? frame may be corrupted (black or white background instead of transparent)._

We could clean up the "possibly reused" part if you can double-check that there's another dump file with a name: *_42877e20_0_frame.png which has the timestamp before. hashCode is the "memory address" of the Drawable, so if it was reused then it's was from memory cache its address didn't change.

[TWiStErRob]

@sjudd please read the text in italics in the previous comment. My suspicion is around GifDecoder, but couldn't pinpoint an exact location. I'm suspecting previousFrame nullity and mainScratch interaction in setPixels.

[ourgit]

I don't really catch you,what I got now is the gif_dump folder which contains the problem gif.
Well,here is the gif_dump folder rar,maybe could some help to fix the bug.

https://drive.google.com/file/d/0B5ghBZcVHgnKUy1SdUdpb0xORDg/view?usp=sharing

[TWiStErRob]

Here's a visual of my last paragraph in https://github.com/bumptech/glide/issues/1068#issuecomment-199776115

It's weird to see those 0th frame draws without having an actual frame backing it up...

[TWiStErRob]

I figured out that 0th frame, there's a firstFrame in GifDrawable which is displayed when the current frame in frameLoader is null. So that's normal startup behavior.

After corresponding in email to get an APK I was able to reproduce the issue on the production version of the app @ourgit is developing, many times, and also I was able to reproduce it once using his debug build which has dumping turned on. The first frame was black after loading the images. There's no need to scroll up and up and up, it's possible to repro by just finding a place where there are some (the more the easier to repro) emojis and then only text below them. Then start scrolling back and forth to allow the emojis to enter and then scroll them out, and repeat. I was using Galaxy S4 4.4.2

I have also seen the black background using the glide-support 1062 example, so it's possible to reproduce, but not consistently enough yet. It looks like it mostly happens when the emojis are loaded from non-memory cache, so likely a .skipMemoryCache(true) would increase the repro-ratio (yet to be tested).

[TWiStErRob]

I continued the investigation and after some backtracking in debug mode, I narrowed it down to the two if (currentFrame.transparency) in GifDecoder.getNextFrame().

The steps were:

• Where do the pixels come from in getNextFrame()? -> dest
• Breakpoint when dest[0] == 0xFF000000 at result.setPixels(dest) (plain black)
  confirmed with visuals that's a good condition
• Why does dest[0] become black? -> there are a few write occurences of dest, but previousFrame is null, so those are out, that leaves dest[dx] = c
• Why does c become black? -> act[31] is black
• What is act and 31? -> act == header.gct and 31 == currentFrame.transIndex

Current theory is, since no frames have local color table, act is always the global color table:

• thread A saves black to save, and replaces it with transparent
• thread A starts decoding
• thread B saves transparent to save, and replaces it with transparent
• thread B starts decoding (with transparent at transparent index)
• thread A finishes decoding
• thread A restores save == black
• thread B continues decoding (with black at transparent index)
• thread B restores saved transparent

This explains why sometimes only half of the frame's background is black.

Up next:

• Why is the header shared between decoders if they modify it? -> GifHeader is shared in GifDrawable.GifState

TODO: also noticed that if (act == null) is after act[currentFrame.transIndex], so it will be NPE before logging.

[ourgit]

@TWiStErRob

you have done a good job, I am looking forward to your solution!

[TWiStErRob]

Confirmed my thread theory with logging, added logging to those two ifs and near the usage of act. There are 4 threads in Glide's pool, 6 instances of the same animation displayed, between these header and hence header.gct == act is shared.
The colors and circles are there to visually match each thread to each other and each decoder to each other. A pairing a single frame load. I logged hashCode(), but replaced them with small numbers for readability.

It's clear that it's possible to have black at transparent index because of other threads, and we're not getting them later on because eventually the transparent index becomes #00000000. This would manifest itself if a GIF frame had different transparent indices into the color table, but most of the time all frames have the same index.

[TWiStErRob]

Fixed with #1090, available as 3.8.0-SNAPSHOT. Give it a spin, and let us know please.

[ourgit]

bingo! the black square don't appear any more after update to the 3.8.0-snapshot. Thank you guys.