hdr: Move more logic into gfx::ColorSpace

Add helper in gfx::ColorSpace to, given an output color space,
compute
- the space in which rasterization should be performed
- the space in which blending should be performed.

Remove a place in the ui::Compositor where we check the
kHighDyanmicRange feature, and use the above helper functions
instead.

Add logic in display::Display to automatically update the
bit depth for media queries based on whether or not the
output color space is HDR. Allow forcing an scrgb-linear
profile using the --force-color-profile argument.

Add logic in display::ScreenWin to enable HDR mode. This
currently queries the HDR feature, but will pull the logic
out of the GPU process in the next patch.

R=hubbe
TBR=avi (for content/)

Bug: 682416
Change-Id: Ieffca5a3e0fa01a98aea5558db251712989519e7
Reviewed-on: https://chromium-review.googlesource.com/596776Reviewed-by: ccameron chromium <ccameron@chromium.org>
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491211}

content/renderer/render_widget.cc

@@ -1242,7 +1242,7 @@ void RenderWidget::Resize(const ResizeParams& params) {
         params.top_controls_height, params.bottom_controls_height,
         params.browser_controls_shrink_blink_size);
     compositor_->SetRasterColorSpace(
-        screen_info_.color_space.GetParametricApproximation());
+        screen_info_.color_space.GetRasterColorSpace());
     // If surface synchronization is enable, then this will use the provided
     // |local_surface_id_| to submit the next generated CompositorFrame.
     // If the ID is not valid, then the compositor will defer commits until
@@ -1340,7 +1340,7 @@ blink::WebLayerTreeView* RenderWidget::InitializeLayerTreeView() {
   compositor_->SetViewportSize(physical_backing_size_);
   OnDeviceScaleFactorChanged();
   compositor_->SetRasterColorSpace(
-      screen_info_.color_space.GetParametricApproximation());
+      screen_info_.color_space.GetRasterColorSpace());
   compositor_->SetContentSourceId(current_content_source_id_);
   compositor_->SetLocalSurfaceId(local_surface_id_);
   // For background pages and certain tests, we don't want to trigger

ui/compositor/compositor.cc

@@ -342,13 +342,9 @@ void Compositor::SetScaleAndSize(float scale, const gfx::Size& size_in_pixel) {
 }
 
 void Compositor::SetDisplayColorSpace(const gfx::ColorSpace& color_space) {
-  blending_color_space_ = color_space;
-  output_color_space_ = blending_color_space_;
-  if (base::FeatureList::IsEnabled(features::kHighDynamicRange)) {
-    blending_color_space_ = gfx::ColorSpace::CreateExtendedSRGB();
-    output_color_space_ = gfx::ColorSpace::CreateSCRGBLinear();
-  }
-  host_->SetRasterColorSpace(color_space.GetParametricApproximation());
+  output_color_space_ = color_space;
+  blending_color_space_ = output_color_space_.GetBlendingColorSpace();
+  host_->SetRasterColorSpace(output_color_space_.GetRasterColorSpace());
   // Color space is reset when the output surface is lost, so this must also be
   // updated then.
   // TODO(fsamuel): Get rid of this.

ui/display/display.cc

@@ -109,6 +109,8 @@ gfx::ColorSpace Display::GetForcedColorProfile() {
           switches::kForceColorProfile);
   if (value == "srgb") {
     return gfx::ColorSpace::CreateSRGB();
+  } else if (value == "scrgb-linear") {
+    return gfx::ColorSpace::CreateSCRGBLinear();
   } else if (value == "generic-rgb") {
     return gfx::ColorSpace(gfx::ColorSpace::PrimaryID::APPLE_GENERIC_RGB,
                            gfx::ColorSpace::TransferID::GAMMA18);
@@ -144,14 +146,11 @@ Display::Display(int64_t id, const gfx::Rect& bounds)
       bounds_(bounds),
       work_area_(bounds),
       device_scale_factor_(GetForcedDeviceScaleFactor()),
-      color_space_(HasForceColorProfile() ? GetForcedColorProfile()
-                                          : gfx::ColorSpace::CreateSRGB()),
+      color_space_(gfx::ColorSpace::CreateSRGB()),
       color_depth_(DEFAULT_BITS_PER_PIXEL),
       depth_per_component_(DEFAULT_BITS_PER_COMPONENT) {
-  if (base::FeatureList::IsEnabled(features::kHighDynamicRange)) {
-    color_depth_ = HDR_BITS_PER_PIXEL;
-    depth_per_component_ = HDR_BITS_PER_COMPONENT;
-  }
+  if (HasForceColorProfile())
+    SetColorSpaceAndDepth(GetForcedColorProfile());
 #if defined(USE_AURA)
   SetScaleAndBounds(device_scale_factor_, bounds);
 #endif
@@ -232,6 +231,17 @@ void Display::SetSize(const gfx::Size& size_in_pixel) {
   SetScaleAndBounds(device_scale_factor_, gfx::Rect(origin, size_in_pixel));
 }
 
+void Display::SetColorSpaceAndDepth(const gfx::ColorSpace& color_space) {
+  color_space_ = color_space;
+  if (color_space_.IsHDR()) {
+    color_depth_ = HDR_BITS_PER_PIXEL;
+    depth_per_component_ = HDR_BITS_PER_COMPONENT;
+  } else {
+    color_depth_ = DEFAULT_BITS_PER_PIXEL;
+    depth_per_component_ = DEFAULT_BITS_PER_COMPONENT;
+  }
+}
+
 void Display::UpdateWorkAreaFromInsets(const gfx::Insets& insets) {
   work_area_ = bounds_;
   work_area_.Inset(insets);

ui/display/display.h

@@ -198,6 +198,10 @@ class DISPLAY_EXPORT Display final {
     color_space_ = color_space;
   }
 
+  // Set the color space of the display and reset the color depth and depth per
+  // component based on whether or not the color space is HDR.
+  void SetColorSpaceAndDepth(const gfx::ColorSpace& color_space);
+
   // The number of bits per pixel. Used by media query APIs.
   int color_depth() const { return color_depth_; }
   void set_color_depth(int color_depth) {

ui/display/display_unittest.cc

@@ -71,18 +71,16 @@ TEST(DisplayTest, ForcedDeviceScaleFactor) {
   Display::ResetForceDeviceScaleFactorForTesting();
 }
 
-// TODO(ccameron): Re-enable this test after refactoring display::Display to
-// set color space and bit depth together.
-TEST(DisplayTest, DISABLED_DisplayHDRValues) {
+TEST(DisplayTest, DisplayHDRValues) {
   Display display;
   EXPECT_EQ(24, display.color_depth());
   EXPECT_EQ(8, display.depth_per_component());
 
-  display.set_color_space(gfx::ColorSpace::CreateSCRGBLinear());
+  display.SetColorSpaceAndDepth(gfx::ColorSpace::CreateSCRGBLinear());
   EXPECT_EQ(48, display.color_depth());
   EXPECT_EQ(16, display.depth_per_component());
 
-  display.set_color_space(gfx::ColorSpace::CreateSRGB());
+  display.SetColorSpaceAndDepth(gfx::ColorSpace::CreateSRGB());
   EXPECT_EQ(24, display.color_depth());
   EXPECT_EQ(8, display.depth_per_component());
 }

ui/display/win/screen_win.cc

@@ -17,6 +17,7 @@
 #include "ui/display/display.h"
 #include "ui/display/display_layout.h"
 #include "ui/display/display_layout_builder.h"
+#include "ui/display/display_switches.h"
 #include "ui/display/win/display_info.h"
 #include "ui/display/win/dpi.h"
 #include "ui/display/win/scaling_util.h"
@@ -81,7 +82,8 @@ std::vector<DisplayInfo> FindAndRemoveTouchingDisplayInfos(
 }
 
 Display CreateDisplayFromDisplayInfo(const DisplayInfo& display_info,
-                                     ColorProfileReader* color_profile_reader) {
+                                     ColorProfileReader* color_profile_reader,
+                                     bool hdr_enabled) {
   Display display(display_info.id());
   float scale_factor = display_info.device_scale_factor();
   display.set_device_scale_factor(scale_factor);
@@ -92,8 +94,12 @@ Display CreateDisplayFromDisplayInfo(const DisplayInfo& display_info,
                      1.0f / scale_factor));
   display.set_rotation(display_info.rotation());
   if (!Display::HasForceColorProfile()) {
-    display.set_color_space(
-        color_profile_reader->GetDisplayColorSpace(display_info.id()));
+    if (hdr_enabled) {
+      display.SetColorSpaceAndDepth(gfx::ColorSpace::CreateSCRGBLinear());
+    } else {
+      display.SetColorSpaceAndDepth(
+          color_profile_reader->GetDisplayColorSpace(display_info.id()));
+    }
   }
   return display;
 }
@@ -117,7 +123,8 @@ Display CreateDisplayFromDisplayInfo(const DisplayInfo& display_info,
 // will take precedence.
 std::vector<ScreenWinDisplay> DisplayInfosToScreenWinDisplays(
     const std::vector<DisplayInfo>& display_infos,
-    ColorProfileReader* color_profile_reader) {
+    ColorProfileReader* color_profile_reader,
+    bool hdr_enabled) {
   // Find and extract the primary display.
   std::vector<DisplayInfo> display_infos_remaining = display_infos;
   auto primary_display_iter = std::find_if(
@@ -145,9 +152,10 @@ std::vector<ScreenWinDisplay> DisplayInfosToScreenWinDisplays(
 
   // Layout and create the ScreenWinDisplays.
   std::vector<Display> displays;
-  for (const auto& display_info : display_infos)
-    displays.push_back(
-        CreateDisplayFromDisplayInfo(display_info, color_profile_reader));
+  for (const auto& display_info : display_infos) {
+    displays.push_back(CreateDisplayFromDisplayInfo(
+        display_info, color_profile_reader, hdr_enabled));
+  }
 
   std::unique_ptr<DisplayLayout> layout(builder.Build());
   layout->ApplyToDisplayList(&displays, nullptr, 0);
@@ -215,7 +223,8 @@ gfx::Point ScalePointRelative(const gfx::Point& from_origin,
 ScreenWin::ScreenWin() : ScreenWin(true) {}
 
 ScreenWin::ScreenWin(bool initialize)
-    : color_profile_reader_(new ColorProfileReader(this)) {
+    : color_profile_reader_(new ColorProfileReader(this)),
+      hdr_enabled_(base::FeatureList::IsEnabled(features::kHighDynamicRange)) {
   DCHECK(!g_screen_win_instance);
   g_screen_win_instance = this;
   if (initialize)
@@ -458,7 +467,7 @@ gfx::Rect ScreenWin::DIPToScreenRectInWindow(gfx::NativeView view,
 void ScreenWin::UpdateFromDisplayInfos(
     const std::vector<DisplayInfo>& display_infos) {
   screen_win_displays_ = DisplayInfosToScreenWinDisplays(
-      display_infos, color_profile_reader_.get());
+      display_infos, color_profile_reader_.get(), hdr_enabled_);
   displays_ = ScreenWinDisplaysToDisplays(screen_win_displays_);
 }
 

ui/display/win/screen_win.h

@@ -203,6 +203,11 @@ class DISPLAY_EXPORT ScreenWin : public Screen,
   // A helper to read color profiles from the filesystem.
   std::unique_ptr<ColorProfileReader> color_profile_reader_;
 
+  // Whether or not HDR mode is enabled.
+  // TODO(ccameron): Set this via the GPU process when the system "HDR and
+  // advanced color" setting is enabled.
+  bool hdr_enabled_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ScreenWin);
 };
 

ui/gfx/color_space.cc

@@ -303,6 +303,26 @@ ColorSpace ColorSpace::GetAsFullRangeRGB() const {
   return result;
 }
 
+ColorSpace ColorSpace::GetRasterColorSpace() const {
+  // Rasterization can only be done into parametric color spaces.
+  if (!IsParametric())
+    return GetParametricApproximation();
+  // Rasterization doesn't support more than 8 bit unorm values. If the output
+  // space has an extended range, use Display P3 for the rasterization space,
+  // to get a somewhat wider color gamut.
+  if (HasExtendedSkTransferFn())
+    return CreateDisplayP3D65();
+  return *this;
+}
+
+ColorSpace ColorSpace::GetBlendingColorSpace() const {
+  // HDR output on windows requires output have a linear transfer function.
+  // Linear blending breaks the web, so use extended-sRGB for blending.
+  if (transfer_ == TransferID::LINEAR_HDR)
+    return CreateExtendedSRGB();
+  return *this;
+}
+
 sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace() const {
   // If we got a specific SkColorSpace from the ICCProfile that this color space
   // was created from, use that.

ui/gfx/color_space.h

@@ -143,6 +143,7 @@ class COLOR_SPACE_EXPORT ColorSpace {
   // Extended sRGB matches sRGB for values in [0, 1], and extends the transfer
   // function to all real values.
   static ColorSpace CreateExtendedSRGB();
+
   // scRGB uses the same primaries as sRGB but has a linear transfer function
   // for all real values.
   static ColorSpace CreateSCRGBLinear();
@@ -160,18 +161,28 @@ class COLOR_SPACE_EXPORT ColorSpace {
 
   // Returns true if the decoded values can be outside of the 0.0-1.0 range.
   bool IsHDR() const;
+
   // Returns true if the encoded values can be outside of the 0.0-1.0 range.
   bool FullRangeEncodedValues() const;
 
   // Returns true if this color space can be represented parametrically.
   bool IsParametric() const;
+
   // Return a parametric approximation of this color space (if it is not already
   // parametric).
-  gfx::ColorSpace GetParametricApproximation() const;
+  ColorSpace GetParametricApproximation() const;
 
   // Return this color space with any range adjust or YUV to RGB conversion
   // stripped off.
-  gfx::ColorSpace GetAsFullRangeRGB() const;
+  ColorSpace GetAsFullRangeRGB() const;
+
+  // If |this| is the final output color space, return the color space that
+  // would be appropriate for rasterization.
+  ColorSpace GetRasterColorSpace() const;
+
+  // If |this| is the final output color space, return the color space that
+  // would be appropriate for blending.
+  ColorSpace GetBlendingColorSpace() const;
 
   // This will return nullptr for non-RGB spaces, spaces with non-FULL
   // range, and unspecified spaces.
@@ -216,7 +227,7 @@ class COLOR_SPACE_EXPORT ColorSpace {
   friend class ColorTransform;
   friend class ColorTransformInternal;
   friend class ColorSpaceWin;
-  friend struct IPC::ParamTraits<gfx::ColorSpace>;
+  friend struct IPC::ParamTraits<ColorSpace>;
   FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, GetColorSpace);
 };
 

ui/gfx/color_space_unittest.cc

@@ -172,6 +172,23 @@ TEST(ColorSpace, ApproximateTransferFnBadMatch) {
   }
 }
 
+TEST(ColorSpace, RasterAndBlend) {
+  ColorSpace display_color_space;
+
+  // A linear transfer function being used for HDR should be blended using an
+  // sRGB-like transfer function.
+  display_color_space = ColorSpace::CreateSCRGBLinear();
+  EXPECT_EQ(ColorSpace::CreateExtendedSRGB(),
+            display_color_space.GetBlendingColorSpace());
+  EXPECT_EQ(ColorSpace::CreateDisplayP3D65(),
+            display_color_space.GetRasterColorSpace());
+
+  // If not used for HDR, a linear transfer function should be left unchanged.
+  display_color_space = ColorSpace::CreateXYZD50();
+  EXPECT_EQ(display_color_space, display_color_space.GetBlendingColorSpace());
+  EXPECT_EQ(display_color_space, display_color_space.GetRasterColorSpace());
+}
+
 TEST(ColorSpace, ToSkColorSpace) {
   const size_t kNumTests = 4;
   ColorSpace color_spaces[kNumTests] = {