If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Ensure animated images can be made into layers

RESOLVED FIXED in mozilla24

Status

()

Core
ImageLib
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Joe Drew (not getting mail), Assigned: mattwoodrow)

Tracking

unspecified
mozilla24
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
It currently looks like animated images can be made into layers without too much (any?) work, but we need to make sure this is the case. In particular, GetCurrentImage() needs to be called to update the Image on the ImageContainer whenever the frame is changed by RequestRefresh() (which is itself called by the refresh driver).
(Assignee)

Comment 1

5 years ago
The code for turning nsDisplayImage into an active layer definitely exists (and works last time I checked). I don't believe it has ever been used by default for anything though, so bugs likely exist.

We're using the equivalent path in nsDisplayBackgroundImage now, and hit a few issues when enabling that.
(Reporter)

Comment 2

5 years ago
I'm running a try build of bug 867770, which enables layerizing of at least some animated images. If it works I'll just close this bug as worksforme. https://tbpl.mozilla.org/?tree=Try&rev=5229887b24c4
(Reporter)

Comment 3

5 years ago
12:50:24 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/image/test/reftest/gif/delaytest.html?transparent-animation.gif | load failed: timed out waiting for pending paint count to reach zero (waiting for MozAfterPaint)

Looks like I need to fix the reftest framework to handle this situation.
(Reporter)

Comment 4

4 years ago
The problem that the reftest framework is running in to is that it thinks there's a draw pending (isMozAfterPaintPending is true) but one never comes. I presume this is because we've converted the Thebes layer into an Image layer before the draw has a chance to be cleared.

Matt, is this something we've ever had to deal with before?
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 5

4 years ago
MozAfterPaint should still be fired, even if the only changes are to non-ThebesLayer layers.

In fact, if we return true to isMozAfterPaintPending then we add an empty rect to the MozAfterPaint list to make sure that we always get something fired.

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#137
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 6

4 years ago
Created attachment 755719 [details] [diff] [review]
Recycle image layers instead of creating them every time

The paint events were happening, the problem was that they didn't *stop* happening.

Every time we built layers, we created a new ImageLayer, which DLBI picks up and sends a MozAfterPaint.

Reftest takes a new snapshot, which builds layers, and triggers another MozAfterPaint. Repeat forever.
Attachment #755719 - Flags: review?(roc)
Comment on attachment 755719 [details] [diff] [review]
Recycle image layers instead of creating them every time

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

ooh, nice catch.
Attachment #755719 - Flags: review?(roc) → review+
(Assignee)

Comment 8

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dfdef811e24
(Assignee)

Updated

4 years ago
Assignee: joe → matt.woodrow
(Reporter)

Comment 9

4 years ago
hey matt, next time can you just fix all my bugs so I can go on holiday
https://hg.mozilla.org/mozilla-central/rev/1dfdef811e24
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.