Use RGB10A2 surface with BT2020 PQ color space for HDR

Use RGB10A2 with BT2020 PQ color space (aka HDR10) for Chrome's backbuffer. This fixes the performance issues with fullscreen HDR that are seen with RGBA FP16 backbuffer possibly because of a DWM or driver bug when direct flip is engaged. This requires the following changes and fixes: 1) Use BT2020 PQ color space by default and fallback to SCRGB linear if root render pass needs alpha blending with titlebar (taking into account SDR white level correctly). 2) Use R10G10B10A2_UNORM format for swap chain in direct composition root surface if color space is BT2020 PQ and R16G16B16A16_FLOAT with alpha blending if color space is SCRGB linear. 3) Add support for R10G10B10A2_UNORM textures in eglCreatePbufferFromClientBuffer in ANGLE. CL: https://chromium-review.googlesource.com/c/angle/angle/+/1565420 4) Add plumbing for HDR10 color space. Ensure that the blending and raster color space is correctly set similar to SCRGB linear. CL: https://chromium-review.googlesource.com/c/chromium/src/+/1573172 5) Fix incorrect has_transparent_background flag on color conversion render pass added by SurfaceAggregator. 6) Fix a null dereference bug in blink due to not checking that the output color space has a valid SkColorSpace. Bug: 937108 Change-Id: I2427674651800bb026b7c9327ca1d2fb43d5175a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565631 Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Robert Liao <robliao@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/heads/master@{#653896}
Denger-Network · Apr 25, 2019 · d89195f · d89195f
1 parent 4952c57
commit d89195f
Show file tree

Hide file tree

Showing 20 changed files with 182 additions and 54 deletions.
diff --git a/components/viz/service/display/surface_aggregator.cc b/components/viz/service/display/surface_aggregator.cc
@@ -659,6 +659,8 @@ void SurfaceAggregator::AddColorConversionPass() {
   color_conversion_pass->SetNew(color_conversion_render_pass_id_, output_rect,
                                 root_render_pass->damage_rect,
                                 root_render_pass->transform_to_root_target);
+  color_conversion_pass->has_transparent_background =
+      root_render_pass->has_transparent_background;
   color_conversion_pass->color_space = output_color_space_;
 
   auto* shared_quad_state =

diff --git a/gpu/ipc/service/direct_composition_child_surface_win.cc b/gpu/ipc/service/direct_composition_child_surface_win.cc
@@ -14,6 +14,7 @@
 #include "base/win/windows_version.h"
 #include "ui/display/display_switches.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/gl/color_space_utils.h"
 #include "ui/gl/egl_util.h"
 #include "ui/gl/gl_angle_util_win.h"
 #include "ui/gl/gl_context.h"
@@ -84,7 +85,6 @@ bool IsSwapChainTearingSupported() {
   }();
   return supported;
 }
-
 }  // namespace
 
 DirectCompositionChildSurfaceWin::DirectCompositionChildSurfaceWin() = default;
@@ -285,16 +285,16 @@ bool DirectCompositionChildSurfaceWin::SetDrawRectangle(
   // |real_surface_|.
   ui::ScopedReleaseCurrent release_current;
 
-  DXGI_FORMAT output_format =
-      is_hdr_ ? DXGI_FORMAT_R16G16B16A16_FLOAT : DXGI_FORMAT_B8G8R8A8_UNORM;
+  DXGI_FORMAT dxgi_format = gl::ColorSpaceUtils::GetDXGIFormat(color_space_);
+
   if (enable_dc_layers_ && !dcomp_surface_) {
     TRACE_EVENT2("gpu", "DirectCompositionChildSurfaceWin::CreateSurface",
                  "width", size_.width(), "height", size_.height());
     swap_chain_.Reset();
     // Always treat as premultiplied, because an underlay could cause it to
     // become transparent.
     HRESULT hr = dcomp_device_->CreateSurface(
-        size_.width(), size_.height(), output_format,
+        size_.width(), size_.height(), dxgi_format,
         DXGI_ALPHA_MODE_PREMULTIPLIED, &dcomp_surface_);
     if (FAILED(hr)) {
       DLOG(ERROR) << "CreateSurface failed with error " << std::hex << hr;
@@ -305,8 +305,6 @@ bool DirectCompositionChildSurfaceWin::SetDrawRectangle(
                  "width", size_.width(), "height", size_.height());
     dcomp_surface_.Reset();
 
-    DXGI_ALPHA_MODE alpha_mode =
-        has_alpha_ ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE;
     Microsoft::WRL::ComPtr<IDXGIDevice> dxgi_device;
     d3d11_device_.As(&dxgi_device);
     DCHECK(dxgi_device);
@@ -320,14 +318,15 @@ bool DirectCompositionChildSurfaceWin::SetDrawRectangle(
     DXGI_SWAP_CHAIN_DESC1 desc = {};
     desc.Width = size_.width();
     desc.Height = size_.height();
-    desc.Format = output_format;
+    desc.Format = dxgi_format;
     desc.Stereo = FALSE;
     desc.SampleDesc.Count = 1;
     desc.BufferCount = 2;
     desc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT;
     desc.Scaling = DXGI_SCALING_STRETCH;
     desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL;
-    desc.AlphaMode = alpha_mode;
+    desc.AlphaMode =
+        has_alpha_ ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE;
     desc.Flags =
         IsSwapChainTearingSupported() ? DXGI_SWAP_CHAIN_FLAG_ALLOW_TEARING : 0;
     HRESULT hr = dxgi_factory->CreateSwapChainForComposition(
@@ -338,6 +337,11 @@ bool DirectCompositionChildSurfaceWin::SetDrawRectangle(
                   << std::hex << hr;
       return false;
     }
+    Microsoft::WRL::ComPtr<IDXGISwapChain3> swap_chain;
+    if (SUCCEEDED(swap_chain_.As(&swap_chain))) {
+      swap_chain->SetColorSpace1(
+          gl::ColorSpaceUtils::GetDXGIColorSpace(color_space_));
+    }
   }
 
   swap_rect_ = rectangle;
@@ -396,26 +400,23 @@ bool DirectCompositionChildSurfaceWin::Resize(const gfx::Size& size,
                                               float scale_factor,
                                               ColorSpace color_space,
                                               bool has_alpha) {
-  bool size_changed = size != size_;
-  bool is_hdr = color_space == ColorSpace::SCRGB_LINEAR;
-  bool hdr_changed = is_hdr != is_hdr_;
-  bool alpha_changed = has_alpha != has_alpha_;
-  if (!size_changed && !hdr_changed && !alpha_changed)
+  if (size_ == size && has_alpha_ == has_alpha && color_space_ == color_space)
     return true;
 
   // This will release indirect references to swap chain (|real_surface_|) by
   // binding |default_surface_| as the default framebuffer.
   if (!ReleaseDrawTexture(true /* will_discard */))
     return false;
 
+  bool resize_only = has_alpha_ == has_alpha && color_space_ == color_space;
+
   size_ = size;
-  is_hdr_ = is_hdr;
+  color_space_ = color_space;
   has_alpha_ = has_alpha;
 
   // ResizeBuffers can't change alpha blending mode.
-  if (swap_chain_ && !alpha_changed) {
-    DXGI_FORMAT format =
-        is_hdr_ ? DXGI_FORMAT_R16G16B16A16_FLOAT : DXGI_FORMAT_B8G8R8A8_UNORM;
+  if (swap_chain_ && resize_only) {
+    DXGI_FORMAT format = gl::ColorSpaceUtils::GetDXGIFormat(color_space_);
     UINT flags =
         IsSwapChainTearingSupported() ? DXGI_SWAP_CHAIN_FLAG_ALLOW_TEARING : 0;
     HRESULT hr = swap_chain_->ResizeBuffers(2 /* BufferCount */, size.width(),

diff --git a/gpu/ipc/service/direct_composition_child_surface_win.h b/gpu/ipc/service/direct_composition_child_surface_win.h
@@ -61,9 +61,9 @@ class GPU_IPC_SERVICE_EXPORT DirectCompositionChildSurfaceWin
 
   gfx::Size size_ = gfx::Size(1, 1);
   bool enable_dc_layers_ = false;
-  bool is_hdr_ = false;
   bool has_alpha_ = true;
   bool vsync_enabled_ = true;
+  ColorSpace color_space_ = ColorSpace::UNSPECIFIED;
 
   // This is a placeholder surface used when not rendering to the
   // DirectComposition surface.

diff --git a/third_party/blink/renderer/platform/graphics/color_space_gamut.cc b/third_party/blink/renderer/platform/graphics/color_space_gamut.cc
@@ -15,11 +15,17 @@ ColorSpaceGamut GetColorSpaceGamut(const WebScreenInfo& screen_info) {
   const gfx::ColorSpace& color_space = screen_info.color_space;
   if (!color_space.IsValid())
     return ColorSpaceGamut::kUnknown;
+
   // Return the gamut of the color space used for raster (this will return a
   // wide gamut for HDR profiles).
+  sk_sp<SkColorSpace> sk_color_space =
+      color_space.GetRasterColorSpace().ToSkColorSpace();
+  if (!sk_color_space)
+    return ColorSpaceGamut::kUnknown;
+
   skcms_ICCProfile color_profile;
-  color_space.GetRasterColorSpace().ToSkColorSpace()->toProfile(&color_profile);
-  return color_space_utilities::GetColorSpaceGamut(&color_profile);
+  sk_color_space->toProfile(&color_profile);
+  return GetColorSpaceGamut(&color_profile);
 }
 
 ColorSpaceGamut GetColorSpaceGamut(const skcms_ICCProfile* color_profile) {

diff --git a/ui/aura/test/test_screen.cc b/ui/aura/test/test_screen.cc
@@ -77,9 +77,10 @@ void TestScreen::SetDeviceScaleFactor(float device_scale_factor) {
   host_->OnHostResizedInPixels(bounds_in_pixel.size());
 }
 
-void TestScreen::SetColorSpace(const gfx::ColorSpace& color_space) {
+void TestScreen::SetColorSpace(const gfx::ColorSpace& color_space,
+                               float sdr_white_level) {
   display::Display display(GetPrimaryDisplay());
-  display.set_color_space(color_space);
+  display.SetColorSpaceAndDepth(color_space, sdr_white_level);
   display_list().UpdateDisplay(display);
 }
 

diff --git a/ui/aura/test/test_screen.h b/ui/aura/test/test_screen.h
@@ -37,7 +37,9 @@ class TestScreen : public display::ScreenBase, public WindowObserver {
   WindowTreeHost* CreateHostForPrimaryDisplay(Env* env = nullptr);
 
   void SetDeviceScaleFactor(float device_scale_fator);
-  void SetColorSpace(const gfx::ColorSpace& color_space);
+  void SetColorSpace(
+      const gfx::ColorSpace& color_space,
+      float sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevel);
   void SetDisplayRotation(display::Display::Rotation rotation);
   void SetUIScale(float ui_scale);
   void SetWorkAreaInsets(const gfx::Insets& insets);

diff --git a/ui/aura/window_tree_host.cc b/ui/aura/window_tree_host.cc
@@ -422,7 +422,8 @@ void WindowTreeHost::InitCompositor() {
 
   display::Display display =
       display::Screen::GetScreen()->GetDisplayNearestWindow(window());
-  compositor_->SetDisplayColorSpace(display.color_space());
+  compositor_->SetDisplayColorSpace(display.color_space(),
+                                    display.sdr_white_level());
 }
 
 void WindowTreeHost::OnAcceleratedWidgetAvailable() {
@@ -480,7 +481,8 @@ void WindowTreeHost::OnHostDisplayChanged() {
     return;
   display::Display display =
       display::Screen::GetScreen()->GetDisplayNearestWindow(window());
-  compositor_->SetDisplayColorSpace(display.color_space());
+  compositor_->SetDisplayColorSpace(display.color_space(),
+                                    display.sdr_white_level());
 }
 
 void WindowTreeHost::OnHostCloseRequested() {
@@ -506,7 +508,8 @@ void WindowTreeHost::OnDisplayMetricsChanged(const display::Display& display,
     display::Screen* screen = display::Screen::GetScreen();
     if (compositor_ &&
         display.id() == screen->GetDisplayNearestView(window()).id()) {
-      compositor_->SetDisplayColorSpace(display.color_space());
+      compositor_->SetDisplayColorSpace(display.color_space(),
+                                        display.sdr_white_level());
     }
   }
 }

diff --git a/ui/aura/window_tree_host_unittest.cc b/ui/aura/window_tree_host_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "build/build_config.h"
 #include "ui/aura/test/aura_test_base.h"
 #include "ui/aura/test/test_cursor_client.h"
 #include "ui/aura/test/test_screen.h"
@@ -83,10 +84,41 @@ TEST_F(WindowTreeHostTest, NoRewritesPostIME) {
 TEST_F(WindowTreeHostTest, ColorSpace) {
   EXPECT_EQ(gfx::ColorSpace::CreateSRGB(),
             host()->compositor()->output_color_space());
+
+  test_screen()->SetColorSpace(gfx::ColorSpace::CreateDisplayP3D65());
+  EXPECT_EQ(gfx::ColorSpace::CreateDisplayP3D65(),
+            host()->compositor()->output_color_space());
+}
+
+#if defined(OS_WIN)
+TEST_F(WindowTreeHostTest, ColorSpaceHDR) {
+  EXPECT_EQ(gfx::ColorSpace::CreateSRGB(),
+            host()->compositor()->output_color_space());
+
+  // UI compositor overrides HDR color space based on whether alpha blending is
+  // needed or not.
   test_screen()->SetColorSpace(gfx::ColorSpace::CreateSCRGBLinear());
+  host()->compositor()->SetBackgroundColor(SK_ColorBLACK);
+  EXPECT_EQ(gfx::ColorSpace::CreateHDR10(),
+            host()->compositor()->output_color_space());
+
+  test_screen()->SetColorSpace(gfx::ColorSpace::CreateHDR10());
+  host()->compositor()->SetBackgroundColor(SK_ColorTRANSPARENT);
   EXPECT_EQ(gfx::ColorSpace::CreateSCRGBLinear(),
             host()->compositor()->output_color_space());
+
+  // Setting SDR white level scales HDR color spaces but not SDR color spaces.
+  host()->compositor()->SetBackgroundColor(SK_ColorTRANSPARENT);
+  test_screen()->SetColorSpace(gfx::ColorSpace::CreateSCRGBLinear(), 200.f);
+  EXPECT_EQ(gfx::ColorSpace::CreateSCRGBLinear().GetScaledColorSpace(
+                gfx::ColorSpace::kDefaultSDRWhiteLevel / 200.f),
+            host()->compositor()->output_color_space());
+
+  test_screen()->SetColorSpace(gfx::ColorSpace::CreateSRGB(), 200.f);
+  EXPECT_EQ(gfx::ColorSpace::CreateSRGB(),
+            host()->compositor()->output_color_space());
 }
+#endif  // OS_WIN
 
 class TestWindow : public ui::StubWindow {
  public:

diff --git a/ui/compositor/compositor.cc b/ui/compositor/compositor.cc
@@ -444,11 +444,30 @@ viz::LocalSurfaceIdAllocation Compositor::RequestNewChildLocalSurfaceId() {
       base::TimeTicks::Now());
 }
 
-void Compositor::SetDisplayColorSpace(const gfx::ColorSpace& color_space) {
-  if (output_color_space_ == color_space)
+void Compositor::SetDisplayColorSpace(const gfx::ColorSpace& color_space,
+                                      float sdr_white_level) {
+  gfx::ColorSpace output_color_space = color_space;
+
+#if defined(OS_WIN)
+  // Ensure output color space for HDR is linear if we need alpha blending, and
+  // HDR10 (BT.2020 primaries with PQ transfer function) otherwise.
+  if (color_space.IsHDR()) {
+    bool transparent = SkColorGetA(host_->background_color()) != SK_AlphaOPAQUE;
+    output_color_space = transparent ? gfx::ColorSpace::CreateSCRGBLinear()
+                                     : gfx::ColorSpace::CreateHDR10();
+    output_color_space = output_color_space.GetScaledColorSpace(
+        gfx::ColorSpace::kDefaultSDRWhiteLevel / sdr_white_level);
+  }
+#endif  // OS_WIN
+
+  if (output_color_space_ == output_color_space &&
+      sdr_white_level_ == sdr_white_level) {
     return;
-  output_color_space_ = color_space;
+  }
+
+  output_color_space_ = output_color_space;
   blending_color_space_ = output_color_space_.GetBlendingColorSpace();
+  sdr_white_level_ = sdr_white_level;
   // Do all ui::Compositor rasterization to sRGB because UI resources will not
   // have their color conversion results cached, and will suffer repeated
   // image color conversions.
@@ -471,6 +490,8 @@ void Compositor::SetDisplayColorSpace(const gfx::ColorSpace& color_space) {
 
 void Compositor::SetBackgroundColor(SkColor color) {
   host_->set_background_color(color);
+  // Update color space based on whether background color is transparent.
+  SetDisplayColorSpace(output_color_space_, sdr_white_level_);
   ScheduleDraw();
 }
 

diff --git a/ui/compositor/compositor.h b/ui/compositor/compositor.h
@@ -293,8 +293,11 @@ class COMPOSITOR_EXPORT Compositor : public cc::LayerTreeHostClient,
   // number.
   viz::LocalSurfaceIdAllocation RequestNewChildLocalSurfaceId();
 
-  // Set the output color profile into which this compositor should render.
-  void SetDisplayColorSpace(const gfx::ColorSpace& color_space);
+  // Set the output color profile into which this compositor should render. Also
+  // sets the SDR white level (in nits) used to scale HDR color space primaries.
+  void SetDisplayColorSpace(
+      const gfx::ColorSpace& color_space,
+      float sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevel);
 
   // Returns the size of the widget that is being drawn to in pixel coordinates.
   const gfx::Size& size() const { return size_; }
@@ -506,6 +509,8 @@ class COMPOSITOR_EXPORT Compositor : public cc::LayerTreeHostClient,
   gfx::ColorSpace output_color_space_;
   gfx::ColorSpace blending_color_space_;
 
+  float sdr_white_level_ = gfx::ColorSpace::kDefaultSDRWhiteLevel;
+
   // If true, all paint commands are recorded at pixel size instead of DIP.
   const bool is_pixel_canvas_;
 

diff --git a/ui/display/display.cc b/ui/display/display.cc
@@ -24,8 +24,9 @@ namespace {
 constexpr int DEFAULT_BITS_PER_PIXEL = 24;
 constexpr int DEFAULT_BITS_PER_COMPONENT = 8;
 
-constexpr int HDR_BITS_PER_PIXEL = 48;
-constexpr int HDR_BITS_PER_COMPONENT = 16;
+// Assuming HDR10 color space with RGB10A2 backbuffer.
+constexpr int HDR_BITS_PER_PIXEL = 30;
+constexpr int HDR_BITS_PER_COMPONENT = 10;
 
 // This variable tracks whether the forced device scale factor switch needs to
 // be read from the command line, i.e. if it is set to -1 then the command line
@@ -209,12 +210,10 @@ Display::Display(int64_t id, const gfx::Rect& bounds)
     : id_(id),
       bounds_(bounds),
       work_area_(bounds),
-      device_scale_factor_(GetForcedDeviceScaleFactor()),
-      color_space_(gfx::ColorSpace::CreateSRGB()),
-      color_depth_(DEFAULT_BITS_PER_PIXEL),
-      depth_per_component_(DEFAULT_BITS_PER_COMPONENT) {
-  if (HasForceDisplayColorProfile())
-    SetColorSpaceAndDepth(GetForcedDisplayColorProfile());
+      device_scale_factor_(GetForcedDeviceScaleFactor()) {
+  SetColorSpaceAndDepth(HasForceDisplayColorProfile()
+                            ? GetForcedDisplayColorProfile()
+                            : gfx::ColorSpace::CreateSRGB());
 #if defined(USE_AURA)
   SetScaleAndBounds(device_scale_factor_, bounds);
 #endif
@@ -298,8 +297,11 @@ 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) {
+void Display::SetColorSpaceAndDepth(const gfx::ColorSpace& color_space,
+                                    float sdr_white_level) {
   color_space_ = color_space;
+  sdr_white_level_ = sdr_white_level;
+  // Assuming HDR10 color space and buffer format.
   if (color_space_.IsHDR()) {
     color_depth_ = HDR_BITS_PER_PIXEL;
     depth_per_component_ = HDR_BITS_PER_COMPONENT;

diff --git a/ui/display/display.h b/ui/display/display.h
@@ -228,9 +228,14 @@ 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);
+  // SDR white level used to scale HDR color spaces.
+  float sdr_white_level() const { return sdr_white_level_; }
+
+  // Set the color space and SDR white level of the display, and reset the color
+  // depth and depth per component based on whether the color space is HDR.
+  void SetColorSpaceAndDepth(
+      const gfx::ColorSpace& color_space,
+      float sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevel);
 
   // The number of bits per pixel. Used by media query APIs.
   int color_depth() const { return color_depth_; }
@@ -256,7 +261,7 @@ class DISPLAY_EXPORT Display final {
  private:
   friend struct mojo::StructTraits<mojom::DisplayDataView, Display>;
 
-  int64_t id_;
+  int64_t id_ = kInvalidDisplayId;
   gfx::Rect bounds_;
   // If non-empty, then should be same size as |bounds_|. Used to avoid rounding
   // errors.
@@ -270,6 +275,7 @@ class DISPLAY_EXPORT Display final {
   // NOTE: this is not currently written to the mojom as it is not used in
   // aura.
   gfx::ColorSpace color_space_;
+  float sdr_white_level_;
   int color_depth_;
   int depth_per_component_;
   bool is_monochrome_ = false;