Closed
Bug 1118092
Opened 10 years ago
Closed 10 years ago
Manage invalidation policy directly in the image decoder
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: seth, Assigned: seth)
References
Details
Attachments
(1 file)
(deleted),
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
Right now, Decoder is fairly naive about sending invalidations - it just collects whatever the decoder implementation classes give it, and relies on RasterImage::FinishedSomeDecoding to apply policy. However, it's tricky to get the logic right in FinishedSomeDecoding, and in bug 1079627 it will become essentially impossible. On top of that, sending spurious invalidations from a decoder (which is running off-main-thread) to FinishedSomeDecoding running on the main thread is hugely wasteful if we're going to immediately drop those invalidations.
This patch gives Decoder the responsibility for managing this policy directly. It implements the following rules:
If Decoder::SetSendPartialInvalidations(true) was called before Decoder::Init, then we will send partial invalidations for the first frame, and nothing afterwards, because invalidations for subsequent frames should come from the animation playback code.
If Decoder::SetSendPartialInvalidations(false) was called before Decoder::Init (or if SetSendPartialInvalidations wasn't called at all; this is the default), then we will send no partial invalidations for the first frame, and instead send a single invalidation for the entire first frame when it's complete.
Assignee | ||
Comment 1•10 years ago
|
||
Here's the patch. Note we call SetSendPartialInvalidations(true) if mHasBeenDecoded is false. This should produce the same results as the existing code, where we avoid sending partial invalidations if we've been decoded before.
Attachment #8544318 -
Flags: review?(tnikkel)
Updated•10 years ago
|
Attachment #8544318 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 2•10 years ago
|
||
Thanks for the review! Try job here:
https://tbpl.mozilla.org/?tree=Try&rev=c805585ab778
Assignee | ||
Comment 3•10 years ago
|
||
Try job here looks good:
https://tbpl.mozilla.org/?tree=Try&rev=f02a89008e90
Went ahead and pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb2775fb362c
Comment 4•10 years ago
|
||
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b74c9afa3b4b since one of this changes caused test failures on windows like:
https://treeherder.mozilla.org/logviewer.html#?job_id=5263429&repo=mozilla-inbound
and
https://treeherder.mozilla.org/logviewer.html#?job_id=5263747&repo=mozilla-inbound
Assignee | ||
Comment 5•10 years ago
|
||
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Looks green on this try job:
https://tbpl.mozilla.org/?tree=Try&rev=aae64e1b4638
Repushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b61e84480e2e
Comment 8•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in
before you can comment on or make changes to this bug.
Description
•