Reftest test/build/tests/reftest/tests/image/test/reftest/pngsuite-ancillary/ccwn3p08.png Fails with vsync refresh driver on os x

RESOLVED FIXED in Firefox 38

Status

()

Core
GFX: Color Management
P2
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mchang, Assigned: jerry)

Tracking

37 Branch
mozilla38
ARM
All
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
From https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa36f268dd4e

file:///builds/slave/talos-slave/test/build/tests/reftest/tests/image/test/reftest/pngsuite-ancillary/ccwn3p08.png | image comparison (==), max difference: 8, number of differing pixels: 577

This only occurs with the silk refresh driver enabled on OSX. Mochitests all passed with the compositor.
(Reporter)

Updated

3 years ago
Summary: file:///builds/slave/talos-slave/test/build/tests/reftest/tests/image/test/reftest/pngsuite-ancillary/ccwn3p08.png | image comparison (==), max difference: 8, number of differing pixels: 577 Fails with vsync refresh driver on os x → Reftest test/build/tests/reftest/tests/image/test/reftest/pngsuite-ancillary/ccwn3p08.png Fails with vsync refresh driver on os x
(Assignee)

Comment 1

3 years ago
In reftest-cmdline.js, we will set the following pref[1]. With silk, we might got the wrong value since we init the gfxPlatform at very early stage.

With silk:
1) init gfxPlatform at [2]
2) get "gfx.color_management.force_srgb" at [3], and we got false here.
3) set "gfx.color_management.force_srgb" to true in [1], but we don't update the profiler to system.


[1]
in reftest-cmdline.js
branch.setBoolPref("gfx.color_management.force_srgb", true);
[2]
https://hg.mozilla.org/mozilla-central/annotate/58ce6051edf5/layout/base/nsRefreshDriver.cpp#l291
[3]
https://hg.mozilla.org/mozilla-central/annotate/58ce6051edf5/gfx/thebes/gfxPlatform.cpp#l1854
Assignee: mchang → hshih
(Assignee)

Comment 2

3 years ago
> 3) set "gfx.color_management.force_srgb" to true in [1], but we don't update the profiler to system.
s/profiler/profile
(Assignee)

Comment 3

3 years ago
Created attachment 8560335 [details] [diff] [review]
Update cms profile in SRGBOverrideObserver callback. v1

wait for try result
(Assignee)

Comment 4

3 years ago
try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6187cad9835a
See Also: → bug 452125
(Assignee)

Updated

3 years ago
Attachment #8560335 - Flags: review?(vladimir)
(Assignee)

Updated

3 years ago
See Also: → bug 856452
(Reporter)

Comment 5

3 years ago
Nice find!
The image mentioned in the initial report (image/test/reftest/pngsuite-ancillary/ccwn3p08.png) has a gAMA chunk with gamma=1.0 so it should not be surprising that the image is rendered differently if it's forced to be interpreted as sRGB.
(Reporter)

Updated

3 years ago
See Also: → bug 1130681
(Reporter)

Updated

3 years ago
See Also: → bug 1130680
(Assignee)

Comment 7

3 years ago
Bobby, could you help to review the attachment 8560335 [details] [diff] [review] patch? It is related to the color management. When we change the "gfx.color_management.force_srgb", we just call ShutdownCMS(), but we don't call CreateCMSOutputProfile() to update the new pref value.
Flags: needinfo?(bobbyholley)
(Assignee)

Updated

3 years ago
Component: Graphics → GFX: Color Management
(Assignee)

Updated

3 years ago
Attachment #8560335 - Flags: review?(bobbyholley)
Comment on attachment 8560335 [details] [diff] [review]
Update cms profile in SRGBOverrideObserver callback. v1

Review of attachment 8560335 [details] [diff] [review]:
-----------------------------------------------------------------

Jeff owns color management now.
Attachment #8560335 - Flags: review?(vladimir)
Attachment #8560335 - Flags: review?(jmuizelaar)
Attachment #8560335 - Flags: review?(bobbyholley)
Flags: needinfo?(bobbyholley)
Attachment #8560335 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 9

3 years ago
Please land attachment 8560335 [details] [diff] [review] to m-c.
try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6187cad9835a
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ac0b61a1b02
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7ac0b61a1b02
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 12

3 years ago
Created attachment 8563915 [details] [diff] [review]
fix friend class declaration. v1, r=jmuizelaar
Attachment #8563915 - Flags: review?(jmuizelaar)
(Assignee)

Comment 13

3 years ago
Comment on attachment 8563915 [details] [diff] [review]
fix friend class declaration. v1, r=jmuizelaar

Got r+ from :jrmuizel for this trivial fix.
Attachment #8563915 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

3 years ago
Attachment #8563915 - Attachment description: fix friend class declaration. v1 → fix friend class declaration. v1, r=jmuizelaar
(Assignee)

Comment 14

3 years ago
Please land attachment 8563915 [details] [diff] [review] to m-c.
Keywords: checkin-needed
   https://hg.mozilla.org/integration/mozilla-inbound/rev/fe05446f8618
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fe05446f8618
Duplicate of this bug: 1133399
You need to log in before you can comment on or make changes to this bug.