1631615 - windy.com - colors missing from map layers

Status: VERIFIED FIXED

(Core :: Graphics: Color Management, defect, P2)

Description

[Mike Taylor [:miketaylr]]

Attachment: 76 left vs 77 right

(Sorry for the poor title, I don't know how to describe this)

Originally reported @ https://github.com/webcompat/web-bugs/issues/51892

STR:

1. load windy.com

Expected: You see a colorful map image with greens, yellows, reds, etc
Actual: None of that, just a blue and gray rain layer.

This gets SUPER obvious if you click on the temperature control on the top right.

In the console:

WebGL warning: texSubImage: Texture has not been initialized prior to a partial upload, forcing the browser to clear it. This may be slow. gl-particles.js:2:10223
WebGL warning: texSubImage: Tex image TEXTURE_2D level 0 is incurring lazy initialization.

11:04.45 INFO: Narrowed integration regression window from [ad3f58c9, 5f2454ac] (3 builds) to [ad3f58c9, 6392325e] (2 builds) (~1 steps left)
11:04.45 INFO: No more integration revisions, bisection finished.
11:04.45 INFO: Last good revision: ad3f58c996e59bc5ca0a0e6aac61c8f6a7e51096
11:04.45 INFO: First bad revision: 6392325ec39e5cb10edd4aaaa4afb7a9f061b0b2
11:04.45 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ad3f58c996e59bc5ca0a0e6aac61c8f6a7e51096&tochange=6392325ec39e5cb10edd4aaaa4afb7a9f061b0b2

Comment 1

[Mike Taylor [:miketaylr]]

[Tracking Requested - why for this release]: It's unclear how this change affects other sites, it breaks this one pretty much.

Comment 2

[Kris Taeleman (:ktaeleman)]

Can't reproduce on linux or windows, so might be mac specific.

@Mike: would you also be able to add your about:support information

Comment 3

[Mike Taylor [:miketaylr]]

Attachment: about-support-mike.txt

Comment 4

[Mike Taylor [:miketaylr]]

Also yeah, I can't reproduce on Windows either (on a VM)

Comment 5

[Jeff Muizelaar [:jrmuizel]]

I can reproduce on mac

Comment 6

[Jeff Muizelaar [:jrmuizel]]

I confirmed that setting gfx.color_management.mode=2 fixes the problem

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

I can reproduce on Linux if I force the browser to use the same color profile as the user.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

It appears the problem is related to color managing untagged JPEG images:

https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/image/decoders/nsJPEGDecoder.cpp#330

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

Adding nsLayoutUtils::SFE_NO_COLORSPACE_CONVERSION to CanvasRenderingContext2D::DrawImage's nsLayoutUtils::SurfaceFromElement call resolves the problem:

https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/dom/canvas/CanvasRenderingContext2D.cpp#4493

Before my pref change, it would have ignored any colorspace conversion because the JPEG has no ICC profile. Not sure what the actual fix should be yet.

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

As far as I can tell Chrome doesn't ask for any special behaviour for canvas. As such, it will enter this branch to try to load the ICC profile:

https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/platform/image-decoders/jpeg/jpeg_image_decoder.cc;l=574;drc=d685ea3c9ffcb18c781bc3a0bdbb92eb88842b1b;bpv=0;bpt=1?originalUrl=https:%2F%2Fcs.chromium.org%2F

It doesn't have one so it calls Decoder()->ColorTransform():

https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/platform/image-decoders/image_decoder.cc;drc=d685ea3c9ffcb18c781bc3a0bdbb92eb88842b1b;bpv=0;bpt=1;l=716?originalUrl=https:%2F%2Fcs.chromium.org%2F

source_to_target_color_transform_needs_update_ is false because we never added the ICC profile, so it doesn't actually fallback to assuming sRGB like we do. This explains why we see different behaviour (assuming I followed the lines correctly ;)).

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Okay, I'll update the decoders to not assume sRGB if there is no reasonable colorspace indication to match Chrome's behaviour.

Comment 12

[Pascal Chevrel:pascalc]

Andrew, will this 77 regression get fixed before the merge next Monday or should we expect an uplift in beta maybe? Thanks

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

I wrote the patch, but needed to fix some gtest related failures. Should be in before the merge but if not, it will be very upliftable/small change.

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1631615 - Don't color manage PNG/JPG/WebP if not tagged.

It appears some websites assume we will not color manage untagged images
and encode metadata in the image's surface data. Chrome matches this
behaviour. We should probably do the same for webcompat.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf2757bbcf71
Don't color manage PNG/JPG/WebP if not tagged. r=tnikkel

Comment 16

[Cristina Coroiu [:ccoroiu]]

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

Comment 17

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9145590 [details]
Bug 1631615 - Don't color manage PNG/JPG/WebP if not tagged.

Beta/Release Uplift Approval Request

• User impact if declined: We have too aggressively color manage images which content authors did not intend to be color managed; if storing data like windy.com, it is hard to predict what would happen; it is also a webcompat issue with Chrome
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is small well contained within imagelib and the individual decoders. We run a lot of tests through this path and the code coverage is high. We shipped for a very long time not color managing most untagged images.
• String changes made/needed:

Comment 18

[Pascal Chevrel:pascalc]

Comment on attachment 9145590 [details]
Bug 1631615 - Don't color manage PNG/JPG/WebP if not tagged.

Looks low risk for early betas and has tests, approved for 77 beta 2, thanks.

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/54ca458874c2

Comment 20

[Bogdan Maris, Desktop QA]

Attachment: Old behavior

I reproduced the initial issue on Ubuntu 18.04 64bit using an old Nightly from 2020-04-20 where the color was completely off (full red map), but on Windows 10 64bit and MacOS 10.15.5 it reproduced a bit differently, the map was divided in different squares with different color tones (attached image).
On all platforms I can confirm that setting gfx.color_management.mode on 2 fixes the problem on that old Nightly build and also on latest Nightly 78.0a1 from today and Firefox 77.0b3.

Comment 21

[Bogdan Maris, Desktop QA]

Attachment: New issue

What I've noticed now (Windows 10 and Ubuntu 18.04 only) that there are a few lines flashing when zooming in/out, same lines I saw 1 comment above where the map was devided in different squares, should I log a new issue on this or should this issue be reopen here?
I did not notice the same behavior on Mac.

Comment 22

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #21)

Created attachment 9146758 [details]
New issue

What I've noticed now (Windows 10 and Ubuntu 18.04 only) that there are a few lines flashing when zooming in/out, same lines I saw 1 comment above where the map was devided in different squares, should I log a new issue on this or should this issue be reopen here?
I did not notice the same behavior on Mac.

Sounds like a different bug. Does it happen when color management is set to 2? If so, I would be confident it is unrelated. Please attach your about:support as well, since it will contain the color profile which may be useful for reproducing.

Comment 23

[Bogdan Maris, Desktop QA]

(In reply to Andrew Osmond [:aosmond] from comment #22)

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #21)

Created attachment 9146758 [details]
New issue

What I've noticed now (Windows 10 and Ubuntu 18.04 only) that there are a few lines flashing when zooming in/out, same lines I saw 1 comment above where the map was devided in different squares, should I log a new issue on this or should this issue be reopen here?
I did not notice the same behavior on Mac.

Sounds like a different bug. Does it happen when color management is set to 2? If so, I would be confident it is unrelated. Please attach your about:support as well, since it will contain the color profile which may be useful for reproducing.

Yes, this does still happen with gfx.color_management.mode set to 2. I logged bug 1636856 on this issue and posted there the about:support content. I'll go ahead and close this one as verified fixed.

Comment 25

[mikeality]

Is there any temporary workaround for this issue on windy.com while keeping gfx.color_management.mode=1?

Comment 26

[Brett W.]

(In reply to Mike Taylor [:miketaylr] from comment #1)

[Tracking Requested - why for this release]: It's unclear how this change affects other sites, it breaks this one pretty much.

I wanted to note that:

(1) this bug still exists on FF version 106.0.1 on Windows 11; and,
(2) this bug can also be seen on other websites, such as https://www.windfinder.com/#3/39.5000/-98.3500

Thanks

Comment 27

[Brett W.]

(In reply to Brett W. from comment #26)

(In reply to Mike Taylor [:miketaylr] from comment #1)

[Tracking Requested - why for this release]: It's unclear how this change affects other sites, it breaks this one pretty much.

I wanted to note that:

(1) this bug still exists on FF version 106.0.1 on Windows 11; and,
(2) this bug can also be seen on other websites, such as https://www.windfinder.com/#3/39.5000/-98.3500

Thanks

Per my comment above, it seems like this should be reopened.

Comment 28

[Brett W.]

https://sf.atmo.ai seems to suffer from the same issue.

Comment 29

[Masatoshi Kimura [:emk]]

(In reply to Brett W. from comment #27)

Per my comment above, it seems like this should be reopened.

Unfortunately, no one can reopen bugs that are closed more than 1 year ago. Please create a clone of this bug.

Comment 30

[Brett W.]

(In reply to Masatoshi Kimura [:emk] from comment #29)

(In reply to Brett W. from comment #27)

Per my comment above, it seems like this should be reopened.

Unfortunately, no one can reopen bugs that are closed more than 1 year ago. Please create a clone of this bug.

Thanks for the heads up! The clone has been created at Bug 1797902.