897488 - Semi-Transparent, animated gifs don't play/loop correctly

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[silverwind]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130723 Firefox/25.0 (Nightly/Aurora)
Build ID: 20130723030205

Steps to reproduce:

Using current nightly, semi-transparent GIFs don't play correctly, at least on Windows. See this example:

https://dl.dropboxusercontent.com/u/34392223/gif-test.html

At least on my Windows machine, I see two frames of the animation switching back and forth here. Opening the image in a new tab or setting the opacity to 1 seems to correct the problem.

This may be a Windows only bug as Joe Drew reports this case as working on OSX.

The problem appeared maybe 1-2 weeks ago, and I'm pretty sure it may be a regression from a patch landed in bug 717872.

Comment 1

[beingalink]

I'm on OS X (10.8.4) and I can reproduce this behavior using Firefox Nightly 25.0a1 (2013-07-24).

Comment 2

[Joe Drew (not getting mail)]

Oh wow. It doesn't happen on my UX nightly, which is synced to mozilla-central only occasionally, but it *does* on my regular nightly!

Comment 3

[Joe Drew (not getting mail)]

Last good nightly: 2013-07-22
First bad nightly: 2013-07-23

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2268ff80683a&tochange=5ceea82a79c7

Which implies it can't be OMT gif!

Comment 4

[Alice0775 White]

(In reply to Joe Drew (:JOEDREW! \o/) from comment #3)
> Last good nightly: 2013-07-22
> First bad nightly: 2013-07-23
> 
> Pushlog:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=2268ff80683a&tochange=5ceea82a79c7
> 
> Which implies it can't be OMT gif!

???
I can reproduce the problem in your Good build....


Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a9d6d86a8090
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130704 Firefox/25.0 ID:20130704094644
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/77c0bebf47ed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130704 Firefox/25.0 ID:20130704114744
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a9d6d86a8090&tochange=77c0bebf47ed

Suspected: Bug 717872

Comment 5

[Joe Drew (not getting mail)]

I trust Alice way more than I trust me.

Comment 6

[Alice0775 White]

It works as expected if set 
gfx.direct2d.disabled = true
OR
layers.prefer-d3d9 = true .

Comment 7

[Alice0775 White]

In local build
Last Good: 8e2e52c72e42
First Bad: 4210dd438a36

Triggered by:
4210dd438a36	Joe Drew — Bug 717872 - Store a frame's raw image data pointer beside its imgFrame pointer so we can access it without having to lock the frame. r=seth This patch makes us store imgFrames in FrameBlender with a new sort-of-tuple, FrameDataPair, that is smart enough to be able to lock and unlock imgFrames, and can be transparently cast to an imgFrame, but doesn't do too much else. The alternative, storing a separate array of uint8_t pointers, seemed too complicated.

Comment 8

[Joe Drew (not getting mail)]

Attachment: apply frame dirt in GetDrawableImgFrame and ScalingDone

Aha! The problem is that, since this page is layerized, RasterImage::Draw() is never called. This means that our only way of applying dirt to frame surfaces is bypassed.

However, if we move it to the places that frames are gotten, i.e., GetDrawableImgFrame and ScalingDone, we always hit it when we need to.

Comment 9

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 780557 [details] [diff] [review]
apply frame dirt in GetDrawableImgFrame and ScalingDone

Review of attachment 780557 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 10

[Joe Drew (not getting mail)]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/362f621702e6

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/362f621702e6

Comment 12

[silverwind]

I can confirm this as working, thanks.

Comment 13

[u279076]

Calling this verified fixed for Firefox 25 based on comment 12.