1124088 - Rename "decode-on-draw" to "decode-only-on-draw" to make its meaning more obvious

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

We will *always* decode-on-draw, in the sense that if we don't have a decoded version of an image and someone calls imgIContainer::Draw, we start decoding the image. The "image.mem.decodeondraw" pref actually didn't do anything.

It recently got repurposed to mean that we would *only* decode-on-draw, and ignore calls to imgIContainer::RequestDecode and the like. However, since we will always decode-on-draw, using "decode-on-draw" to mean this is a bit confusing.

In this bug we'll rename "decode-on-draw" to "decode-only-on-draw" everywhere that it occurs. This make it much clearer what this pref means.

Comment 1

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

Attachment: Rename decode-on-draw to decode-only-on-draw

Here's the patch.

Comment 2

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

Attachment: Rename decode-on-draw to decode-only-on-draw

Ugh, I shouldn't have waited so long to land this. Belated thanks for the
review. Here's a rebased version.

Comment 3

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

Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/77347e90d2c7

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/77347e90d2c7

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Release Note Request (optional, but appreciated)
[Why is this notable]: Renaming a preference, the users that have modified this preference should know about it.
[Suggested wording]: Preference image.mem.decodeondraw has been renamed to image.decode-only-on-draw.enabled
[Links (documentation, blog post, etc)]:

Comment 6

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

FWIW the old pref literally did not do anything. This one only works on B2G right now and I anticipate removing it soon. I'm not keen on publicizing it for that reason.

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Sounds like this may not need a release note, then. 
Milan, if you disagree can you needinfo me or renominate this for a note? Thanks!

Comment 8

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

(In reply to Seth Fowler [:seth] from comment #6)
> FWIW the old pref literally did not do anything. This one only works on B2G
> right now and I anticipate removing it soon. I'm not keen on publicizing it
> for that reason.

It will be good, as we now probably have 2 preferences for doing the same thing:
image.decode-only-on-draw.enabled
image.decode-immediately.enabled

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

I'm good with whatever you guys decide.

Comment 10

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

(In reply to Virtual_ManPL [:Virtual] from comment #8)
> (In reply to Seth Fowler [:seth] from comment #6)
> > FWIW the old pref literally did not do anything. This one only works on B2G
> > right now and I anticipate removing it soon. I'm not keen on publicizing it
> > for that reason.
> 
> It will be good, as we now probably have 2 preferences for doing the same
> thing:
> image.decode-only-on-draw.enabled
> image.decode-immediately.enabled

We don't, actually. "image.decode-immediately.enabled", just as it sounds, decodes images immediately when it's enabled. "image.decode-only-on-draw.enabled" decodes images only when something attempts to draw them. But the normal behavior isn't either of those things, it's "apply a bunch of heuristics to decide which images to decode and when". The reason I added "image.decode-immediately.enabled" was because lots of people thought that setting "image.decode-only-on-draw.enabled" to false would cause all images to be decoded immediately, but that is not true.

Comment 11

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

(In reply to Liz Henry (:lizzard) from comment #7)
> Sounds like this may not need a release note, then. 
> Milan, if you disagree can you needinfo me or renominate this for a note?
> Thanks!

Let's not add a release note for this.

We could consider adding one for bug 1149893, which added "image.decode-immediately.enabled". Though I'm not super enthusiastic about that, either. That one will arrive in 39.