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

Status: RESOLVED FIXED

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

Description

[Mason Chang [Inactive] [:mchang]]

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.

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

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

Comment 2

[Jerry Shih[:jerry] (UTC+8) (inactive)]

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

Comment 3

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: Update cms profile in SRGBOverrideObserver callback. v1

wait for try result

Comment 4

[Jerry Shih[:jerry] (UTC+8) (inactive)]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6187cad9835a

Comment 5

[Mason Chang [Inactive] [:mchang]]

Nice find!

Comment 6

[Glenn Randers-Pehrson]

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.

Comment 7

[Jerry Shih[:jerry] (UTC+8) (inactive)]

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.

Comment 8

[Bobby Holley (:bholley)]

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.

Comment 9

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Please land attachment 8560335 [details] [diff] [review] to m-c.
try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6187cad9835a

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ac0b61a1b02

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7ac0b61a1b02

Comment 12

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: fix friend class declaration. v1, r=jmuizelaar

Comment 13

[Jerry Shih[:jerry] (UTC+8) (inactive)]

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

Got r+ from :jrmuizel for this trivial fix.

Comment 14

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Please land attachment 8563915 [details] [diff] [review] to m-c.

Comment 15

[Carsten Book [:Tomcat]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/fe05446f8618

Comment 16

[Carsten Book [:Tomcat]]

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