1799391 - Regression: Colour management no longer working

Status: VERIFIED FIXED

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

Description

[colinlee10]

Attachment: colorman.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:107.0) Gecko/20100101 Firefox/107.0

Steps to reproduce:

1. Build Firefox from nightly or beta. I used 107.0b10 (64-bit)
2. Apply attached ICC profile in Windows colour management
3. Try a color management test after restarting the browser (e.g by using https://cameratico.com/tools/web-browser-color-management-test/, or just comparing to earlier Firefox version with gfx.color_management.mode = 1)

This ICC profile was exported by DisplayCal with standard settings after monitor profiling.

I believe this is the regression point. Reverting this change restores colour management:
https://bugzilla.mozilla.org/show_bug.cgi?id=1795045

See attached screenshots from https://cameratico.com/tools/web-browser-color-management-test/ for comparison between browsers.

Actual results:

Colour management is not visible - images appear oversaturated on wide gamut monitor. Tagged images do not have changed colors

Expected results:

Output images should have colour management applied.

Comment 1

[colinlee10]

Attachment: ICC profile (generated by displayCal)

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Graphics: Color Management' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

:jrmuizel, since you are the author of the regressor, bug 1795045, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: Bug 1799391. Allow negative XYZ on non-macOS.

It seems like Chrome allows this and it's relatively common.
We don't have a good reason to continue blocking these profiles.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a635a87e5896
Allow negative XYZ on non-macOS. r=aosmond

Comment 6

[Atila Butkovits]

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

Comment 7

[colinlee10]

Thanks. Verified fix on my end

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1795045

Comment 9

[Ryan VanderMeulen [:RyanVM]]

I assume you're going to want this in a dot release, so go ahead and nominate for Beta & Release approval when you get a chance. It grafts cleanly.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

colinlee10, did you have gfx.color_management.enablev4 set to true?

Comment 11

[colinlee10]

Yes

Comment 12

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9303350 [details]
Bug 1799391. Allow negative XYZ on non-macOS.

Beta/Release Uplift Approval Request

• User impact if declined: Color management will not work for some people with negative XYZ in their profiles
• Is this code covered by automated tests?: No
• 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): We've been shipping this change on macOS for many year.
• String changes made/needed:
• Is Android affected?: Yes

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9303350 [details]
Bug 1799391. Allow negative XYZ on non-macOS.

Approved for 108.0b4

Comment 14

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/60948e4d45e2

Comment 15

[Donal Meehan [:dmeehan]]

:jrmuizel could you add a release uplift request on this when ready?
Would like to consider including this in the planned dot release

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9303350 [details]
Bug 1799391. Allow negative XYZ on non-macOS.

Beta/Release Uplift Approval Request

• User impact if declined: Users that have set gfx.color_management.enablev4=true may get broken color management on Windows and Linux
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• 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): This will cause us to accept profiles that we previously rejected. As far as we know that's ok.
• String changes made/needed:
• Is Android affected?: Yes

Comment 17

[Jeff Muizelaar [:jrmuizel]]

https://www.reddit.com/r/firefox/comments/z1t28j/color_management_not_working_after_microsoft/ sounds like it might be another user running into this issue in 107.

Comment 18

[Brindusa Tot, Desktop QA]

Reproduced the bug as observed in screenshot from description, on Beta 108.0b2 on Windows 10. Also, on affected build, there are no changes on colors when set gfx.color_management.mode = 1 (default is 2).

I can confirm that bug is fixed on Beta 108.0b5 and latest Nightly build ID 20221122094606, on Windows 10 and Ubuntu 18.04 x64

Comment 19

[Donal Meehan [:dmeehan]]

Comment on attachment 9303350 [details]
Bug 1799391. Allow negative XYZ on non-macOS.

Approved for 107.0.1

Comment 20

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-release/rev/4adc7e089536

Comment 21

[Brindusa Tot, Desktop QA]

Verified as fixed on RC 107.0.1 on Windows 10 and Ubuntu 18.04 x64.