888607 - Animated gif repaints badly

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Jan Skrasek]

Attachment: bug.gif

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20130628 Firefox/25.0 (Nightly/Aurora)
Build ID: 20130628031149

Steps to reproduce:

open gif in browser


Actual results:

some frames are not repainted -> bug is shown more than one, so it makes "path", not motion.


Expected results:

See bug moving.

Comment 1

[Jan Skrasek]

Attachment: Screenshot of wrong repaint

Comment 2

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/8ea92aeab783
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130619 Firefox/24.0 ID:20130619164023
Bad:
http://hg.mozilla.org/mozilla-central/rev/83aa31ec53d9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130620 Firefox/24.0 ID:20130620005958
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8ea92aeab783&tochange=83aa31ec53d9


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5b3196ad66f4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130619 Firefox/24.0 ID:20130619143654
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f5f8e2d6d991
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130619 Firefox/24.0 ID:20130619144755
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5b3196ad66f4&tochange=f5f8e2d6d991



REGRESSED BY: Bug 867774

Comment 3

[Joe Drew (not getting mail)]

Attachment: correctly clear subframes

This fixes the bug. We were simply wrong when clearing subframes. Now we're not.

Comment 4

[Joe Drew (not getting mail)]

Attachment: test

Here's a test based on the ants gif in this bug.

Comment 5

[Joe Drew (not getting mail)]

https://tbpl.mozilla.org/?tree=Try&rev=246b9323a6ac

Comment 6

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

Comment on attachment 771480 [details] [diff] [review]
correctly clear subframes

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

Makes sense.

Comment 7

[Joe Drew (not getting mail)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c0ce0ab4c3ad
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/18278826d0c0

Comment 8

[Joe Drew (not getting mail)]

Comment on attachment 771480 [details] [diff] [review]
correctly clear subframes

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 867774
User impact if declined: some gifs will paint wrong
Testing completed (on m-c, etc.): tested locally, has new test, just pushed
Risk to taking this patch (and alternatives if risky): quite low risk, though if it was wrong it could cause crashes
String or IDL/UUID changes made by this patch: none

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c0ce0ab4c3ad
https://hg.mozilla.org/mozilla-central/rev/18278826d0c0

Comment 10

[bhavana bajaj [:bajaj]]

Comment on attachment 771480 [details] [diff] [review]
correctly clear subframes

Given where we are in the cycle ok to take.If this results in crashes we have an opportunity to resolve/backout as needed in the next few days.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5a62d5ff8abb
https://hg.mozilla.org/releases/mozilla-aurora/rev/d0b270f86f3e

Comment 13

[u279076]

Assuming this does not need QA verification due to in-testsuite coverage. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs to be manually verified.