1118092 - Manage invalidation policy directly in the image decoder

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

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.

Comment 1

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

Attachment: Manage invalidation policy directly in the image decoder

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.

Comment 2

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

Thanks for the review! Try job here:

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

Comment 3

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

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

[Carsten Book [:Tomcat]]

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

Comment 5

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

Repushed:

 https://hg.mozilla.org/integration/mozilla-inbound/rev/36e21bed08f2

Comment 6

[Tom S [:evilpie]]

Backout
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f3cc2c90893

Comment 7

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

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b61e84480e2e