color: Rasterize layers with sRGB-only content into sRGB |
||||||
Issue descriptionIn the scheme for color correct rendering (design doc at [1]), we ideally rasterize all content into the display's color space, so that [A] We get maximum color fidelity [B] We don't pay the price of color conversion at display time Without --enable-color-correct-rendering, we do no color conversion for images that don't specify a color space. Based on the UMA at [2], this is ~90% of all images. Doing color conversion for this ~90% of images will be a substantial regression. From some initial numbers, CPU raster color conversion costs ~2-3x what image decode costs. A way around this is to identify layers that contain only sRGB content, rasterize them into sRGB buffers, and convert to display color space at composite time. This satisfies [A] -- we lose no fidelity by staying in sRGB, but does not satisfy [B] -- we will have to do color conversion at composite time. This bug is to - quantify the cost of color conversion for CPU raster (and see if optimization is possible) - quantify the cost of color conversion for GPU raster (and see if it's low enough to not bother with this workaround) - quantify the cost of composite-time color conversion - based on the above, determine if we should use this scheme [1] https://docs.google.com/a/chromium.org/document/d/1bTQKKCRXFV6UK1YiYMj7Yif114hcgPqq-mU49VnaljQ/edit?usp=sharing [2] https://uma.googleplex.com/p/chrome/histograms/?endDate=20170507&dayCount=1&histograms=Blink.ColorSpace.Source&fixupData=true&showMax=true&filters=platform%2Ceq%2CW%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial
,
May 9 2017
,
May 9 2017
+1 for the idea. Seems like a good approach. "Doing color conversion for this ~90% of images will be a substantial regression. From some initial numbers, CPU raster color conversion costs ~2-3x what image decode costs." I'm a little surprised by this. I don't remember the cost being this bad. Do you have any benchmarks/images that I can take a look at?
,
May 9 2017
This was from looking at about:tracing doing a tab-switch to a page with a new image. The image, IIRC, was the attached one, which may not have an sRGB transfer curve, which probably makes a substantial difference (and also represents an extremely uncommon type of input). I'll run some more precise benchmarks to get better numbers. I also want to check out the GPU raster situation.
,
May 9 2017
Sorry, my "2-3x" statement isn't accurate. Decode and color conversion are approximately equal in cost (color conversion is a bit less). You can see the decode breakdown by about:tracing "cc.debug" and "blink", and you'll get something like this screenshot. Also tested with some bizarre transfer functions, and it doesn't move the needle. IIRC (from the same experiments from which I mis-remembered the ~2-3x figure), it did not make an appreciable difference in cost if we did the transform during decode (as it currently is done) or if it was done after. I also want to check out what the difference is WRT in GPU raster. I suspect it will be very minor, but I'll need an accurate way of benchmarking on the GPU.
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa21c64f46ed363641a70bc230afbe21fd10c0e4 commit fa21c64f46ed363641a70bc230afbe21fd10c0e4 Author: ccameron <ccameron@chromium.org> Date: Wed May 10 19:38:36 2017 color: Add unit test to ensure sRGB SkColorSpaces match It turns out that Skia does fuzzy comparision for gamma and gamut internally. Add tests to be emphatic about this, since we don't want to promote layers to non-sRGB rendering just because they have an image that is tagged with an sRGB color space. BUG=719735 Review-Url: https://codereview.chromium.org/2876463003 Cr-Commit-Position: refs/heads/master@{#470671} [modify] https://crrev.com/fa21c64f46ed363641a70bc230afbe21fd10c0e4/ui/gfx/icc_profile_unittest.cc
,
May 12 2017
Re: #5 Thanks for the extra info Chris! AFAICT, it looks like the graph shows... Decode Time: 7.6ms Decode + Xform Time: 11.5ms That actually matches up pretty consistently with my benchmarks on z620. Decode Time: 5.37ms Decode + Xform Time: 7.87ms (obviously, how the image is encoded and the properties of the dst color space will affect this proportion) I'm not sure how to quantify the effect that this extra cost would have on the browser, but I definitely think it makes sense to try to optimize this further.
,
Jul 21 2017
I've updated the patch at https://chromium-review.googlesource.com/580309 But I'm still on the fence about what to do with it. It may still be a good idea for HDR (since we're already at low precision there). I also tested some pages around the internet and found that many pages (at least on desktop) have images with color profiles, so this doesn't win as much as I had hoped for. I may implement this in for HDR now and merge that -- this would let me keep the two branches in sync and also leave it as an option in the back pocket.
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e7ca16787f13fff437875b4446d25b2fc5b5776f commit e7ca16787f13fff437875b4446d25b2fc5b5776f Author: Christopher Cameron <ccameron@chromium.org> Date: Thu Jul 27 15:17:40 2017 cc: Detect raster sources with sRGB-only content In enabling the color correct rendering feature, we are now converting all sRGB images to the output device color space. Before enabling this feature, these images had no conversion applied. Track the fraction of images and pixels on each raster source that are in this category of "requires color conversion when previously none was required" and record it in a histogram. If it is the case that many raster sources have exclusively sRGB content, then we may want to consider rasterizing such layers in sRGB and doing color conversion at compositing time. Query image color spaces in the discardable image map generator, and record statistics about the percent of pixels and images that are sRGB versus non-sRGB. Record a histogram of both of these statistics. Bug: 719735 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ic180b46627987ac6badf96ddba61d0e5e9a19c25 Reviewed-on: https://chromium-review.googlesource.com/580309 Commit-Queue: ccameron chromium <ccameron@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/master@{#489928} [modify] https://crrev.com/e7ca16787f13fff437875b4446d25b2fc5b5776f/cc/paint/discardable_image_map.cc [modify] https://crrev.com/e7ca16787f13fff437875b4446d25b2fc5b5776f/cc/paint/discardable_image_map.h [modify] https://crrev.com/e7ca16787f13fff437875b4446d25b2fc5b5776f/cc/paint/discardable_image_map_unittest.cc [modify] https://crrev.com/e7ca16787f13fff437875b4446d25b2fc5b5776f/tools/metrics/histograms/histograms.xml
,
Jul 28 2017
,
Jul 29 2017
Requesting a merge to M61 for this, since the histogram generating data just missed the cutoff.
,
Aug 2 2017
Actually requesting 61 merge this time
,
Aug 3 2017
Pls apply appropriate OSs label. Thank you.
,
Aug 3 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 3 2017
,
Aug 3 2017
Pls merge you change to M61 branch 3163 by 5:00 PM PT, Friday (08/04) so we can take it in for next week M61 Beta release. Thank you
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/994325df098b18515e007a0cc44155c013c9fd4e commit 994325df098b18515e007a0cc44155c013c9fd4e Author: Christopher Cameron <ccameron@chromium.org> Date: Thu Aug 03 20:45:33 2017 cc: Detect raster sources with sRGB-only content In enabling the color correct rendering feature, we are now converting all sRGB images to the output device color space. Before enabling this feature, these images had no conversion applied. Track the fraction of images and pixels on each raster source that are in this category of "requires color conversion when previously none was required" and record it in a histogram. If it is the case that many raster sources have exclusively sRGB content, then we may want to consider rasterizing such layers in sRGB and doing color conversion at compositing time. Query image color spaces in the discardable image map generator, and record statistics about the percent of pixels and images that are sRGB versus non-sRGB. Record a histogram of both of these statistics. TBR=ccameron@chromium.org (cherry picked from commit e7ca16787f13fff437875b4446d25b2fc5b5776f) Bug: 719735 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ic180b46627987ac6badf96ddba61d0e5e9a19c25 Reviewed-on: https://chromium-review.googlesource.com/580309 Commit-Queue: ccameron chromium <ccameron@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#489928} Reviewed-on: https://chromium-review.googlesource.com/601058 Reviewed-by: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#286} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/994325df098b18515e007a0cc44155c013c9fd4e/cc/paint/discardable_image_map.cc [modify] https://crrev.com/994325df098b18515e007a0cc44155c013c9fd4e/cc/paint/discardable_image_map.h [modify] https://crrev.com/994325df098b18515e007a0cc44155c013c9fd4e/cc/paint/discardable_image_map_unittest.cc [modify] https://crrev.com/994325df098b18515e007a0cc44155c013c9fd4e/tools/metrics/histograms/histograms.xml
,
Nov 20 2017
I did some tests for power consumption effects of this on Mac, and for Mac, we do not want to do this. The display-time cost is too hard when using the CoreAnimation compositor. For non-CoreAnimation compositing, I haven't done this measurement yet, but I suspect that the result will be that we *will* want to do this. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ccameron@chromium.org
, May 8 2017