color: Enable color correct rendering for GPU pixel integration tests |
|||
Issue descriptionRunning with color correct rendering changes the output of some of the GPU pixel integration tests. We should move these tests over to using color correct rendering (and rebase the results, etc), before we enable the flag (which may be done once, or gradually using Finch). AFAICT, this boils down to adding --force-color-profile=srgb and --enable-features=ColorCorrectRendering to _AddDefaultArgs in pixel_integration_test.py. On Linux and Windows this will likely be fairly simple. On Mac, things may be somewhat complicated by the fact that that the system may have a non-sRGB display, and the functions that we use to capture screenshots are likely capturing in whatever space this is. We may want to consider overriding the system's color profile to be sRGB for the duration of the test (much like the Webkit layout tests used to do).
,
Jun 20 2017
I was looking at making GrabViewSnapshot use -[CALayer renderInContext:]. That way we could specify a color space for the context, and always get a reasonable result. This scheme fails because that method doesn't work for remote CALayers. For future reference, there is a thread about "taking a snapshot of remote CALayers is hard" at https://bugs.webkit.org/show_bug.cgi?id=161450, but the issue is unresolved. We could pass in --disable-remote-core-animation to allow using this method, but that defeats the goal of testing what actually shows up on the screen. Of note is that using remote CALayers is not strictly necessary anymore, but removing it is still something of a project.
,
Jun 20 2017
I don't think that this can be robustly implemented at the screenshot level. That said, I think this can be robustly implemented by forcing a display profile (similar to what used to be done for layout tests, see deleted code in X). I implemented this in the Python code at https://codereview.chromium.org/2944733002/. The Python implementation is nice in that it doesn't require that we build a separate executable, but it is a bit messy (and I don't even know if the objc, Framework, and Quartz modules are on the bots). So, the menu of solutions for setting a color profile during testing are: A: Do it in Python (assuming it's possible) B: Do it in C++, and have run_gpu_integration_test.py depend on that new build target
,
Jun 20 2017
Great work putting this together. Commented on the CL. CC'ing dpranke@ as an FYI that this is going to start changing the system-wide display profile on the Swarming bots, so if something goes wrong during the test run it might affect subsequent runs. I wonder whether this is something we should guard against at the Swarming level (CC'ing maruel@, vadimsh@).
,
Jun 21 2017
Spent several minutes looking at ways to query the active ICC profile for each of the displays connected to a machine. system_profiler doesn't expose it. I "think" I found the right function to call but the documentation is really awesome: https://developer.apple.com/documentation/colorsync/1461807-colorsyncdevicecopydeviceinfo I've found an example in http://git.webkit.org/?p=WebKit-https.git;a=blob;f=Tools/DumpRenderTree/mac/LayoutTestHelper.m;h=958380e83dafebc10dd8d2f175e2159395644b6b;hb=HEAD#l61 So using the equivalent of the code in https://codereview.chromium.org/2944733002/diff/180001/content/test/gpu/gpu_tests/color_profile_manager_mac.py, it would be possible to surface this as a swarming dimension if desired, or you can put this in bot_config.py, as you prefer.
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b80ffde89677d5d2b83ca25c221a768fa0029a9b commit b80ffde89677d5d2b83ca25c221a768fa0029a9b Author: ccameron <ccameron@chromium.org> Date: Wed Jun 21 23:43:10 2017 Force sRGB color profile on Mac for pixel tests This is following the behavior that was used for layout tests up until its remove in crrev.com/416170 The pixel tests take screenshots using CGWindowListCreateImage, which is always in the output device's color space. To get deterministic results, force the output device's color space to be sRGB for the duration of these tests. Mark video-based tests on Mac as failing, since their output will change with this patch (because CoreAnimation will be doing different color conversion). BUG= 734255 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2944733002 Cr-Commit-Position: refs/heads/master@{#481358} [add] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/color_profile_manager.py [add] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/color_profile_manager_mac.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/maps_integration_test.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/pixel_integration_test.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/pixel_test_pages.py
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b80ffde89677d5d2b83ca25c221a768fa0029a9b commit b80ffde89677d5d2b83ca25c221a768fa0029a9b Author: ccameron <ccameron@chromium.org> Date: Wed Jun 21 23:43:10 2017 Force sRGB color profile on Mac for pixel tests This is following the behavior that was used for layout tests up until its remove in crrev.com/416170 The pixel tests take screenshots using CGWindowListCreateImage, which is always in the output device's color space. To get deterministic results, force the output device's color space to be sRGB for the duration of these tests. Mark video-based tests on Mac as failing, since their output will change with this patch (because CoreAnimation will be doing different color conversion). BUG= 734255 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2944733002 Cr-Commit-Position: refs/heads/master@{#481358} [add] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/color_profile_manager.py [add] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/color_profile_manager_mac.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/maps_integration_test.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/pixel_integration_test.py [modify] https://crrev.com/b80ffde89677d5d2b83ca25c221a768fa0029a9b/content/test/gpu/gpu_tests/pixel_test_pages.py
,
Jun 22 2017
Re #5, that sounds interesting! For the moment things are sort-of-tangled (this currently has to be used in conjunction with the --force-color-profile flag in Chrome, and this is Mac-specific, since the rest of the system compositors are color-oblivious for the most part).
,
Jun 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/363ae99ff183adae88c9642c450fc485808bf39c commit 363ae99ff183adae88c9642c450fc485808bf39c Author: ccameron <ccameron@chromium.org> Date: Sun Jun 25 09:01:13 2017 color: Remove pixel test suppressions Expectations have been updated. TBR=kbr BUG= 734255 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2953413002 Cr-Commit-Position: refs/heads/master@{#482171} [modify] https://crrev.com/363ae99ff183adae88c9642c450fc485808bf39c/content/test/gpu/gpu_tests/pixel_expectations.py
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/86e7894c3f1f9fe41e40e7f111206ea996fdcc3a commit 86e7894c3f1f9fe41e40e7f111206ea996fdcc3a Author: Christopher Cameron <ccameron@chromium.org> Date: Mon Jun 26 21:45:20 2017 color: Run GPU pixel tests with ColorCorrectRendering feature This feature will be enabled by default soon. Make these tests run using this feature so that they don't need to be rebaselined as the feature is rolled out. The --enable-features=ColorCorrectRendering flag will be removed once the feature is on by default. Also give the screenshot sync test a tolerance of 1 pixel because its results sometimes vary by that much due to quantization error. BUG= 734255 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I80997bbe0de5deafaea3deac66140a176b4bbe0f Reviewed-on: https://chromium-review.googlesource.com/548818 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#482421} [modify] https://crrev.com/86e7894c3f1f9fe41e40e7f111206ea996fdcc3a/content/test/gpu/gpu_tests/maps_integration_test.py [modify] https://crrev.com/86e7894c3f1f9fe41e40e7f111206ea996fdcc3a/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/86e7894c3f1f9fe41e40e7f111206ea996fdcc3a/content/test/gpu/gpu_tests/pixel_integration_test.py [modify] https://crrev.com/86e7894c3f1f9fe41e40e7f111206ea996fdcc3a/content/test/gpu/gpu_tests/pixel_test_pages.py [modify] https://crrev.com/86e7894c3f1f9fe41e40e7f111206ea996fdcc3a/content/test/gpu/gpu_tests/screenshot_sync_integration_test.py
,
Jun 27 2017
Pixel_Video_MP4 failed on: https://build.chromium.org/p/chromium.gpu.fyi/builders/Win10%20Debug%20%28Intel%20HD%20530%29/builds/1072 https://build.chromium.org/p/chromium.gpu.fyi/builders/Win10%20Debug%20%28NVIDIA%29/builds/1408 https://build.chromium.org/p/chromium.gpu.fyi/builders/Win10%20Release%20%28Intel%20HD%20530%29/builds/960 https://build.chromium.org/p/chromium.gpu.fyi/builders/Win10%20Release%20%28NVIDIA%29/builds/2324
,
Jun 27 2017
Thanks for the heads-up, updated https://chromium-review.googlesource.com/549718.
,
Jun 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cdf143624704a712fcc887b8325cfe79ee4ec7a commit 9cdf143624704a712fcc887b8325cfe79ee4ec7a Author: Christopher Cameron <ccameron@chromium.org> Date: Tue Jun 27 06:13:21 2017 color: Update MP4 video GPU pixel test expecations This was affected by color correct rendering, but not caught by tryjobs. TBR=kbr Bug: 734255 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: If76a9244495c742ca3fb4680c63038a6718ed47c Reviewed-on: https://chromium-review.googlesource.com/549718 Reviewed-by: ccameron chromium <ccameron@chromium.org> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#482548} [modify] https://crrev.com/9cdf143624704a712fcc887b8325cfe79ee4ec7a/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/9cdf143624704a712fcc887b8325cfe79ee4ec7a/content/test/gpu/gpu_tests/pixel_test_pages.py
,
Jun 28 2017
Still needs rebaseline, but otherwise this is done.
,
Jun 28 2017
Thanks for figuring this out and pushing it through!
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e647815da3389ad12b9c376ba9786ab9758cab0 commit 4e647815da3389ad12b9c376ba9786ab9758cab0 Author: Christopher Cameron <ccameron@chromium.org> Date: Wed Jul 19 18:54:32 2017 Remove pixel test suppressions from color correct rendering enable TBR=kbr Bug: 734255 Change-Id: Ia74542d870a037ce54a702735145cd64c0c923ba Reviewed-on: https://chromium-review.googlesource.com/574681 Reviewed-by: ccameron chromium <ccameron@chromium.org> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#487932} [modify] https://crrev.com/4e647815da3389ad12b9c376ba9786ab9758cab0/content/test/gpu/gpu_tests/pixel_expectations.py
,
Jul 21 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by kbr@chromium.org
, Jun 17 2017Labels: OS-Mac