User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022320 Minefield/3.0b4pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022320 Minefield/3.0b4pre It seems animated GIF can have display problems when changing the zoom level. Details below. Reproducible: Always Steps to Reproduce: 1. Go to http://stifu.free.fr/pics/zabu/wip/templateman.gif 2. Change the zoom level (anything but 100%) 3. Let the animation loop Actual Results: A black rectangle appears on the right of the image Expected Results: No black rectangle should be there

I went back to yesterday's nightly, and the bug isn't in it, as expected. So this is most likely a regression from bug 381661.

Works: 20080223_1426_firefox-3.0b4pre.en-US.win32 Broken: 20080223_1459_firefox-3.0b4pre.en-US.win32 Checkins to module PhoenixTinderbox between 2008-02-23 14:26 and 2008-02-23 14:58 : http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1203805560&maxdate=1203807539 --> bug 381661

We're not blocking at this point on any zoom artifacts, but I think what's going on is that the gif compositing code is drawing subframes possibly using Paint() and using EXTEND_PAD... that's a guess though, without looking at code.

It's just the selection cursor for me. Try clicking to the right of the image.

Is this guaranteed to only affect zoom and not affect high-DPI screens? Does this issue appear if layout.css.dpi is set to a large value?

Yes, this issue also appears with a large value for layout.css.dpi (192) for me.

Renominating, since high-DPI issues are much more something we should block on than zoom issues...

What's going on here is a little odd -- stuart tells me that old Image impls used to work around this as well. Basically, we have a first frame that's smaller than the actual gif canvas dimensions... but for some reason, the nsThebesImage that's created has the right mWidth/mHeight, but its decodedSize is the size of the canvas, and when Draw is called, the source rectangle assumes that it's the bigger size as well. When we turned on EXTEND_PAD, all of a sudden we were getting the padding effect from where the fist of the animation was hitting the right edge of the frame, because we were drawing it into a bigger rectangle. This patch corrects for this. One thing that I'm not sure of is that this might have invalidation problems, since we're not actually going to draw over the entire area that we'd be expected to. However, since images can be transparent, the background should be drawn anyway, and the part that's not drawn would normally just have a transparent region drawn on top... so I think that this should be safe.

Vlad, can you please change the patch to exclude OS/2, too, from the workaround in ThebesDrawTile? Like this: #if !defined(XP_MACOSX) && !defined(XP_WIN) && !defined(XP_OS2)

(In reply to comment #4) > We're not blocking at this point on any zoom artifacts I think we should block on some zoom artifacts, depending on how severe they are.

Wouldn't it be slightly cleaner (and work better with my patch in bug 403181, which I'm going to land in an hour or two) to do this fixup around where we clip the source rect to the mDecoded rect? You should just need to generalize the code there to remove the implicit assumption that aSourceRect is contained in (0,0,mWidth,mHeight). Does your imgContainer change work when blendMethod != imgIContainer::kBlendSource? Seems like the Fill() in that if-block will consume the path.

Nope, our Fill()/Stroke() are always fill_preserve()/stroke_preserve(). We had a really good reason for defining it that way way back, but I forget what it was now :p But yeah, I'll wait until you land 403181 and update this.

So now that roc's patch has landed and stuck, I don't seem to be able to reproduce this any more. Can someone confirm?

Yep, pretty sure this was fixed by roc's patch here: http://mxr.mozilla.org/seamonkey/source/gfx/src/thebes/nsThebesImage.cpp#492 subimageRect won't be != 0,0,mWidth,mHeight, so we'll get the temporary surface treatment. The temporary is created with subimageRect's size (which is bigger), but that's ok, since we end up painting into it essentially enlarging the frame, and everything then works correctly. So, going to call this WORKSFORME.

I'm not sure how to do a test for this, FWIW, since it relies on animated GIFs and specific frame sizes. It might be possible to create a testcase for it by trying to get the error to happen in the first frame, and then disabling animation.. but I think the first frame will always be drawn at full canvas size.