HDR: Do nonlinear blending in the compositor

HDR: Do nonlinear blending in the compositor

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

R=enne
TBR=sky (for mash)
BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2728173004
Cr-Commit-Position: refs/heads/master@{#455274}
Committed: https://chromium.googlesource.com/chromium/src/+/3f68a49d843d30417077a09ac37bff4491be4e0c

[ccameron, 2017-03-05 07:49:38]

Description was changed from

==========
cc: Add separate concept of blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are outputing to HDR color spaces.

BUG=687357
==========

to

==========
cc: Add separate concept of blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are outputing to HDR color spaces.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-03-05 07:54:49]

Description was changed from

==========
cc: Add separate concept of blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are outputing to HDR color spaces.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Add separate concept of blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are outputing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-03-05 07:54:49]

ccameron@chromium.org changed reviewers:
+ hubbe@chromium.org

[ccameron, 2017-03-05 07:55:04]

ccameron@chromium.org changed reviewers:
+ enne@chromium.org
- hubbe@chromium.org

[ccameron, 2017-03-05 08:00:03]

ptal

[ccameron, 2017-03-05 20:44:18]

Description was changed from

==========
cc: Add separate concept of blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are outputing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Separate blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-03-05 20:55:03]

Description was changed from

==========
cc: Separate blending and output color spaces

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
HDR: Do nonlinear blending in the compositor

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-03-05 20:55:03]

ccameron@chromium.org changed reviewers:
+ hubbe@chromium.org

[ccameron, 2017-03-05 21:08:49]

Somewhat enlarging this patch to include adding extended-sRGB support, cause
that's the space where we need to do this blending (and if I didn't add it, the
patch would clamp values).

Adding hubbe@ for the colorspace parts.

https://codereview.chromium.org/2728173004/diff/20001/cc/output/gl_renderer_u...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/cc/output/gl_renderer_u...
cc/output/gl_renderer_unittest.cc:141: gfx::ColorSpace::CreateREC709(),
gfx::ColorSpace::CreateExtendedSRGB(),
I think it was just dumb luck that this didn't crash all over the place before
(src_color_spaces[3] was just memset to 0...).

https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
ui/compositor/compositor.cc:166:
command_line->HasSwitch(cc::switches::kEnableTrueColorRendering);
This makes it so that HDR doesn't opt us into doing color-aware rasterization
(just compositing).

Color-aware rasterization won't be ready as early as HDR. We'll probably want to
limit rasterization to sRGB gamut in HDR mode, since that's the only way to get
colors to match.

https://codereview.chromium.org/2728173004/diff/20001/ui/gfx/color_space.h
File ui/gfx/color_space.h (right):

https://codereview.chromium.org/2728173004/diff/20001/ui/gfx/color_space.h#ne...
ui/gfx/color_space.h:134: static ColorSpace CreateExtendedSRGB();
I consciously didn't call this scRGB, because it is defined for all real values,
not just the more-limited scRGB gamut. It also matches the CG API names
https://developer.apple.com/reference/coregraphics/kcgcolorspaceextendedsrgb?...

[enne (OOO), 2017-03-06 19:56:12]

Looks good in general % my own confusion

https://codereview.chromium.org/2728173004/diff/20001/cc/output/gl_renderer_u...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/cc/output/gl_renderer_u...
cc/output/gl_renderer_unittest.cc:141: gfx::ColorSpace::CreateREC709(),
gfx::ColorSpace::CreateExtendedSRGB(),
On 2017/03/05 at 21:08:48, ccameron wrote:
> I think it was just dumb luck that this didn't crash all over the place before
(src_color_spaces[3] was just memset to 0...).

@_@

https://codereview.chromium.org/2728173004/diff/20001/cc/surfaces/surface_agg...
File cc/surfaces/surface_aggregator.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:335:
CHECK(root_render_pass->transform_to_root_target == gfx::Transform());
CHECK_EQ?

https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
ui/compositor/compositor.cc:166:
command_line->HasSwitch(cc::switches::kEnableTrueColorRendering);
On 2017/03/05 at 21:08:48, ccameron wrote:
> This makes it so that HDR doesn't opt us into doing color-aware rasterization
(just compositing).
> 
> Color-aware rasterization won't be ready as early as HDR. We'll probably want
to limit rasterization to sRGB gamut in HDR mode, since that's the only way to
get colors to match.

Sorry to be confused, I'm a little lost in the sea of settings we have now. 
There's now three command line flags (color correct, true color, hdr output) and
two settings (LayerTreeSetting, RendererSettings).  This is my understanding:

color correct switch = raster is done in output color space (but "legacy"
blending), all resources converted at compositor at draw time (draw time also
uses legacy blending, I think?)

true color switch = same as color correct switch, but with correct blending at
both raster and draw (this is not what we're planning to ship, so maybe will get
removed?)

hdr output switch = (requires one of the above? maybe it should CHECK that
somewhere if so?) changes the backbuffer format to floating point, so draw-time
blending has more precision

color correct LayerTreeSetting = turns on and off raster-time color conversion

color correct RendererSetting = turns on and off draw-time color conversion

[ccameron, 2017-03-07 00:13:51]

https://codereview.chromium.org/2728173004/diff/20001/cc/surfaces/surface_agg...
File cc/surfaces/surface_aggregator.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:335:
CHECK(root_render_pass->transform_to_root_target == gfx::Transform());
On 2017/03/06 19:56:12, enne wrote:
> CHECK_EQ?

Hmm, just tried this, and it complained about not being able to
MakeCheckOpValueString of a gfx::Transform.

https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
ui/compositor/compositor.cc:166:
command_line->HasSwitch(cc::switches::kEnableTrueColorRendering);
So, I complicated this a lot. Eventually all the flags will go away... but in
the mean time they're multiplying.

This part that you wrote is the key part:

> color correct LayerTreeSetting = turns on and off raster-time color conversion
> 
> color correct RendererSetting = turns on and off draw-time color conversion

The "--enable-color-correct-rendering" flag turns on both of these.

The "--enable-hdr-output" will turn on just the RendererSetting (because the
rasterization APIs aren't stable enough to use). This is sort-of-hacky because
the rasterization results that we're compositing aren't in a coherent color
space ... we (will) work around this by rasterizing to sRGB all the time.

The "--enable-true-color-rendering" will enable linear blending as well (in
fact, we should just call it --enable-linear-blending), and is mostly being
ignored for now.

[enne (OOO), 2017-03-07 01:43:18]

On 2017/03/07 at 00:13:51, ccameron wrote:
>
https://codereview.chromium.org/2728173004/diff/20001/cc/surfaces/surface_agg...
> File cc/surfaces/surface_aggregator.cc (right):
> 
>
https://codereview.chromium.org/2728173004/diff/20001/cc/surfaces/surface_agg...
> cc/surfaces/surface_aggregator.cc:335:
CHECK(root_render_pass->transform_to_root_target == gfx::Transform());
> On 2017/03/06 19:56:12, enne wrote:
> > CHECK_EQ?
> 
> Hmm, just tried this, and it complained about not being able to
MakeCheckOpValueString of a gfx::Transform.
> 
>
https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
> File ui/compositor/compositor.cc (right):
> 
>
https://codereview.chromium.org/2728173004/diff/20001/ui/compositor/composito...
> ui/compositor/compositor.cc:166:
command_line->HasSwitch(cc::switches::kEnableTrueColorRendering);
> So, I complicated this a lot. Eventually all the flags will go away... but in
the mean time they're multiplying.
> 
> This part that you wrote is the key part:
> 
> > color correct LayerTreeSetting = turns on and off raster-time color
conversion
> > 
> > color correct RendererSetting = turns on and off draw-time color conversion
> 
> The "--enable-color-correct-rendering" flag turns on both of these.
> 
> The "--enable-hdr-output" will turn on just the RendererSetting (because the
rasterization APIs aren't stable enough to use). This is sort-of-hacky because
the rasterization results that we're compositing aren't in a coherent color
space ... we (will) work around this by rasterizing to sRGB all the time.
> 
> The "--enable-true-color-rendering" will enable linear blending as well (in
fact, we should just call it --enable-linear-blending), and is mostly being
ignored for now.

Ah, ok, thanks.  That clarifies for me that you want to have hdr output without
doing the rasterization part, at least in the short term.

lgtm

[ccameron, 2017-03-07 18:47:50]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-03-07 18:47:59]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2017-03-07 18:48:22]

Description was changed from

==========
HDR: Do nonlinear blending in the compositor

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
HDR: Do nonlinear blending in the compositor

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

R=enne
TBR=sky (for mash)
BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-03-07 18:48:32]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-03-07 18:49:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 20:19:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 20:19:47]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[ccameron, 2017-03-07 21:20:23]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-03-07 21:20:24]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2728173004/#ps40001
(title: "Fix windows build")

[commit-bot: I haz the power, 2017-03-07 21:21:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 23:05:13]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488921623542510,
"parent_rev": "a69a95ffd08ee6447a804526e1bf8c228f7a6952", "commit_rev":
"3f68a49d843d30417077a09ac37bff4491be4e0c"}

[commit-bot: I haz the power, 2017-03-07 23:06:45]

Description was changed from

==========
HDR: Do nonlinear blending in the compositor

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

R=enne
TBR=sky (for mash)
BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
HDR: Do nonlinear blending in the compositor

Specify a color space for blending to the SurfaceAggregator, in addition
to the output color space. If the color space for blending is not the
same as the output color space, then add a separate render pass to
convert from the blending color space to the output color space.
Add a SurfaceAggregator test for this.

This is used by HDR output, because the output is in linear space, while
web content expects to be blended according in sRGB value space.
Add gfx::ColorSpace::CreateExtendedSRGB as the default space in which
to do blending for HDR content.

Change the meaning of the flag kEnableHDROutput to not enable color
aware rasterization, only compositing.

Change the GLRenderer to use half-float textures for render passes that
are output-ing to HDR color spaces. This can't be tested (yet) because
of Mesa limitations.

R=enne
TBR=sky (for mash)
BUG=687357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2728173004
Cr-Commit-Position: refs/heads/master@{#455274}
Committed:
https://chromium.googlesource.com/chromium/src/+/3f68a49d843d30417077a09ac37b...
==========

[commit-bot: I haz the power, 2017-03-07 23:06:46]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/3f68a49d843d30417077a09ac37b...