viz: Fix SkiaRenderer output with PQ/HDR10 color space

SkiaRenderer renders washed out colors when using PQ color space with a
RGB10A2 surface.  This happens in M81 because the default color space
and buffer format for HDR was changed to PQ and RGB10A2 on Windows.

The root cause is that the default PQ SkColorSpace uses normalized
luminance values whereas Chrome needs to output scaled values based on
the white point.  The fix is to construct a PQish SkColorSpace using the
scaling factor.

This CL includes pixel tests for HDR10 and scRGB linear.  The HDR10 test
fails before this fix.  The HDR10 test is temporarily suppressed due to
infra issues causing the test to run on different Windows versions.

Bug: 1058580, 1066979
Change-Id: I5a6b91631377a300010d045aaf44a9375d8ffa55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109254
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>
Cr-Commit-Position: refs/heads/master@{#755529}

components/viz/service/display/display_resource_provider.cc

@@ -961,8 +961,7 @@ DisplayResourceProvider::LockSetForExternalUse::~LockSetForExternalUse() {
 ExternalUseClient::ImageContext*
 DisplayResourceProvider::LockSetForExternalUse::LockResource(
     ResourceId id,
-    bool is_video_plane,
-    float sdr_scale_factor) {
+    bool is_video_plane) {
   auto it = resource_provider_->resources_.find(id);
   DCHECK(it != resource_provider_->resources_.end());
 
@@ -973,43 +972,17 @@ DisplayResourceProvider::LockSetForExternalUse::LockResource(
     DCHECK(!base::Contains(resources_, std::make_pair(id, &resource)));
     resources_.emplace_back(id, &resource);
 
-    // Ignore sdr_scale_factor for video planes, if the src color space
-    // is invalid, or if it's already HDR.
-    const gfx::ColorSpace& original_src = resource.transferable.color_space;
-    if (is_video_plane || !original_src.IsValid() || original_src.IsHDR()) {
-      sdr_scale_factor = 1.0f;
-    }
-
-    if (resource.image_context &&
-        resource.image_context->sdr_scale_factor() != sdr_scale_factor) {
-      // Must rebuild the image context with a new color space, which requires
-      // releasing the old image context on the GPU main thread.
-      std::vector<std::unique_ptr<ExternalUseClient::ImageContext>> to_release;
-      to_release.push_back(std::move(resource.image_context));
-      resource_provider_->external_use_client_->ReleaseImageContexts(
-          std::move(to_release));
-    }
-
     if (!resource.image_context) {
-      sk_sp<SkColorSpace> src_color_space;
-      if (!is_video_plane) {
-        if (sdr_scale_factor != 1.0f) {
-          src_color_space = original_src.GetScaledColorSpace(sdr_scale_factor)
-                                .ToSkColorSpace();
-        } else {
-          src_color_space = original_src.ToSkColorSpace();
-        }
-      }
-      // Else the resource |color_space| is handled externally in SkiaRenderer
-      // using a special color filter.
-
+      sk_sp<SkColorSpace> image_color_space;
+      // Video color conversion is handled externally in SkiaRenderer using a
+      // special color filter.
+      if (!is_video_plane)
+        image_color_space = resource.transferable.color_space.ToSkColorSpace();
       resource.image_context =
           resource_provider_->external_use_client_->CreateImageContext(
               resource.transferable.mailbox_holder, resource.transferable.size,
               resource.transferable.format, resource.transferable.ycbcr_info,
-              std::move(src_color_space));
-      // Save the SDR scale, in order to cache the scaled SkColorSpace
-      resource.image_context->set_sdr_scale_factor(sdr_scale_factor);
+              std::move(image_color_space));
     }
     resource.locked_for_external_use = true;
 

components/viz/service/display/display_resource_provider.h

@@ -243,7 +243,7 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
     // There should be at most one instance of this class per
     // |resource_provider|. Both |resource_provider| and |client| outlive this
     // class.
-    LockSetForExternalUse(DisplayResourceProvider* resourcqe_provider,
+    LockSetForExternalUse(DisplayResourceProvider* resource_provider,
                           ExternalUseClient* client);
     ~LockSetForExternalUse();
 
@@ -254,8 +254,7 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
     // Lock a resource for external use. The return value was created by
     // |client| at some point in the past.
     ExternalUseClient::ImageContext* LockResource(ResourceId resource_id,
-                                                  bool is_video_plane = false,
-                                                  float sdr_scale_factor = 1.f);
+                                                  bool is_video_plane);
 
     // Unlock all locked resources with a |sync_token|.  The |sync_token| should
     // be waited on before reusing the resource's backing to ensure that any

components/viz/service/display/display_resource_provider_unittest.cc

@@ -289,7 +289,7 @@ TEST_P(DisplayResourceProviderTest, LockForExternalUse) {
                       Return(ByMove(std::move(owned_image_context)))));
 
   ExternalUseClient::ImageContext* locked_image_context =
-      lock_set.LockResource(parent_id);
+      lock_set.LockResource(parent_id, /*is_video_plane=*/false);
   EXPECT_EQ(image_context, locked_image_context);
   ASSERT_EQ(holder.mailbox, mailbox);
   ASSERT_TRUE(holder.sync_token.HasData());

components/viz/service/display/external_use_client.h

@@ -52,18 +52,6 @@ class VIZ_SERVICE_EXPORT ExternalUseClient {
     ResourceFormat resource_format() const { return resource_format_; }
     sk_sp<SkColorSpace> color_space() const { return color_space_; }
 
-    // NOTE: This is metadata that should be set to match any factor baked into
-    // the |color_space| of this context. It is theoretically possible that the
-    // context's image could be updated using SkImage::reinterpretColorSpace
-    // if just the scale factor changes across frames. However, to respect the
-    // thread safety rules, a new SDR scale factor currently requires making a
-    // new ImageContext if its image has already been created.
-    float sdr_scale_factor() const { return sdr_scale_factor_; }
-    void set_sdr_scale_factor(float sdr_scale_factor) {
-      DCHECK(!image_);
-      sdr_scale_factor_ = sdr_scale_factor;
-    }
-
     SkAlphaType alpha_type() const { return alpha_type_; }
     void set_alpha_type(SkAlphaType alpha_type) {
       DCHECK(!image_);
@@ -90,11 +78,6 @@ class VIZ_SERVICE_EXPORT ExternalUseClient {
     const gfx::Size size_;
     const ResourceFormat resource_format_;
     const sk_sp<SkColorSpace> color_space_;
-    // Records the SDR white level scaling factor applied to |color_space_| when
-    // the ImageContext was made. Since this is already in the SkColorSpace,
-    // this is only used by the DisplayResourceProvider to determine if a cached
-    // ImageContext matches the requested white level.
-    float sdr_scale_factor_ = 1.0f;
 
     SkAlphaType alpha_type_ = kPremul_SkAlphaType;
     GrSurfaceOrigin origin_ = kTopLeft_GrSurfaceOrigin;

components/viz/service/display/skia_output_surface.h

@@ -83,7 +83,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurface : public OutputSurface,
   virtual void MakePromiseSkImage(
       ExternalUseClient::ImageContext* image_context) = 0;
 
-  // Make a promise SkImage from the given |contexts| and the |yuv_color_space|.
+  // Make a promise SkImage from the given |contexts| and |image_color_space|.
   // For YUV format, at least three resource contexts should be provided.
   // contexts[0] contains pixels from y panel, contexts[1] contains pixels
   // from u panel, contexts[2] contains pixels from v panel. For NV12 format,
@@ -92,8 +92,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurface : public OutputSurface,
   // has_alpha is true, the last item in contexts contains alpha panel.
   virtual sk_sp<SkImage> MakePromiseSkImageFromYUV(
       const std::vector<ExternalUseClient::ImageContext*>& contexts,
-      SkYUVColorSpace yuv_color_space,
-      sk_sp<SkColorSpace> dst_color_space,
+      sk_sp<SkColorSpace> image_color_space,
       bool has_alpha) = 0;
 
   // Called if SwapBuffers() will be skipped.

components/viz/service/display/skia_renderer.cc

@@ -559,13 +559,9 @@ SkiaRenderer::ScopedSkImageBuilder::ScopedSkImageBuilder(
     lock_.emplace(resource_provider, resource_id, alpha_type, origin);
     sk_image_ = lock_->sk_image();
   } else {
-    float sdr_white_level =
-        skia_renderer->current_frame()->display_color_spaces.GetSDRWhiteLevel();
-    float sdr_scale_factor =
-        sdr_white_level / gfx::ColorSpace::kDefaultSDRWhiteLevel;
     auto* image_context =
         skia_renderer->lock_set_for_external_use_->LockResource(
-            resource_id, /*video_plane=*/false, sdr_scale_factor);
+            resource_id, /*is_video_plane=*/false);
     // |ImageContext::image| provides thread safety: (a) this ImageContext is
     // only accessed by GPU thread after |image| is set and (b) the fields of
     // ImageContext that are accessed by both compositor and GPU thread are no
@@ -585,8 +581,7 @@ class SkiaRenderer::ScopedYUVSkImageBuilder {
  public:
   ScopedYUVSkImageBuilder(SkiaRenderer* skia_renderer,
                           const YUVVideoDrawQuad* quad,
-                          sk_sp<SkColorSpace> dst_color_space,
-                          bool has_color_conversion_filter) {
+                          sk_sp<SkColorSpace> dst_color_space) {
     DCHECK(skia_renderer->is_using_ddl());
     DCHECK(IsTextureResource(skia_renderer->resource_provider_,
                              quad->y_plane_resource_id()));
@@ -598,14 +593,6 @@ class SkiaRenderer::ScopedYUVSkImageBuilder {
            IsTextureResource(skia_renderer->resource_provider_,
                              quad->a_plane_resource_id()));
 
-    SkYUVColorSpace yuv_color_space;
-    if (has_color_conversion_filter) {
-      yuv_color_space = kIdentity_SkYUVColorSpace;
-    } else {
-      yuv_color_space = kRec601_SkYUVColorSpace;
-      quad->video_color_space.ToSkYUVColorSpace(&yuv_color_space);
-    }
-
     const bool is_i420 =
         quad->u_plane_resource_id() != quad->v_plane_resource_id();
     const bool has_alpha = quad->a_plane_resource_id() != kInvalidResourceId;
@@ -615,25 +602,26 @@ class SkiaRenderer::ScopedYUVSkImageBuilder {
     // Skia API ignores the color space information on the individual planes.
     // Dropping them here avoids some LOG spam.
     auto* y_context = skia_renderer->lock_set_for_external_use_->LockResource(
-        quad->y_plane_resource_id(), true /* is_video_plane */);
+        quad->y_plane_resource_id(), /*is_video_plane=*/true);
     contexts.push_back(std::move(y_context));
     auto* u_context = skia_renderer->lock_set_for_external_use_->LockResource(
-        quad->u_plane_resource_id(), true /* is_video_plane */);
+        quad->u_plane_resource_id(), /*is_video_plane=*/true);
     contexts.push_back(std::move(u_context));
     if (is_i420) {
       auto* v_context = skia_renderer->lock_set_for_external_use_->LockResource(
-          quad->v_plane_resource_id(), true /* is_video_plane */);
+          quad->v_plane_resource_id(), /*is_video_plane=*/true);
       contexts.push_back(std::move(v_context));
     }
 
     if (has_alpha) {
       auto* a_context = skia_renderer->lock_set_for_external_use_->LockResource(
-          quad->a_plane_resource_id(), true /* is_video_plane */);
+          quad->a_plane_resource_id(), /*is_video_plane=*/true);
       contexts.push_back(std::move(a_context));
     }
 
+    // Note: YUV to RGB and color conversion is handled by a color filter.
     sk_image_ = skia_renderer->skia_output_surface_->MakePromiseSkImageFromYUV(
-        std::move(contexts), yuv_color_space, dst_color_space, has_alpha);
+        std::move(contexts), dst_color_space, has_alpha);
     LOG_IF(ERROR, !sk_image_) << "Failed to create the promise sk yuva image.";
   }
 
@@ -1660,6 +1648,7 @@ void SkiaRenderer::FlushBatchedQuads() {
   SkPaint paint;
   paint.setFilterQuality(batched_quad_state_.filter_quality);
   paint.setBlendMode(batched_quad_state_.blend_mode);
+
   current_canvas_->experimental_DrawEdgeAAImageSet(
       &batched_quads_.front(), batched_quads_.size(),
       batched_draw_regions_.data(), &batched_cdt_matrices_.front(), &paint,
@@ -1694,30 +1683,14 @@ void SkiaRenderer::DrawColoredQuad(SkColor color,
   // PrepareCanvasForRPDQ will have updated params->opacity and blend_mode to
   // account for the layer applying those effects.
   color = SkColorSetA(color, params->opacity * SkColorGetA(color));
+
   const SkPoint* draw_region =
       params->draw_region.has_value() ? params->draw_region->points : nullptr;
 
-  SkColor4f color4f = SkColor4f::FromColor(color);
-  float sdr_white_level =
-      current_frame()->display_color_spaces.GetSDRWhiteLevel();
-  if (sdr_white_level != gfx::ColorSpace::kDefaultSDRWhiteLevel) {
-    // SkColor is always sRGB. Use skcms to linearize, scale, and re-encode
-    const float scale_factor =
-        sdr_white_level / gfx::ColorSpace::kDefaultSDRWhiteLevel;
-    const auto* srgb_to_linear = skcms_sRGB_TransferFunction();
-    const auto* linear_to_srgb = skcms_sRGB_Inverse_TransferFunction();
-
-    for (int c = 0; c < 3; ++c) {
-      color4f[c] = skcms_TransferFunction_eval(
-          linear_to_srgb, scale_factor * skcms_TransferFunction_eval(
-                                             srgb_to_linear, color4f[c]));
-    }
-  }
-
   current_canvas_->experimental_DrawEdgeAAQuad(
       gfx::RectFToSkRect(params->visible_rect), draw_region,
-      static_cast<SkCanvas::QuadAAFlags>(params->aa_flags), color4f,
-      params->blend_mode);
+      static_cast<SkCanvas::QuadAAFlags>(params->aa_flags),
+      SkColor4f::FromColor(color), params->blend_mode);
 }
 
 void SkiaRenderer::DrawSingleImage(const SkImage* image,
@@ -1791,6 +1764,7 @@ void SkiaRenderer::DrawDebugBorderQuad(const DebugBorderDrawQuad* quad,
   paint.setStyle(SkPaint::kStroke_Style);
   paint.setStrokeJoin(SkPaint::kMiter_Join);
   paint.setStrokeWidth(quad->width);
+
   current_canvas_->drawPath(path, paint);
 }
 
@@ -2103,13 +2077,8 @@ void SkiaRenderer::DrawYUVVideoQuad(const YUVVideoDrawQuad* quad,
   }
 #endif
 
-  sk_sp<SkColorFilter> color_filter =
-      GetColorFilter(src_color_space, dst_color_space, quad->resource_offset,
-                     quad->resource_multiplier);
-
   DCHECK(resource_provider_);
-  ScopedYUVSkImageBuilder builder(
-      this, quad, frame_color_space.ToSkColorSpace(), !!color_filter);
+  ScopedYUVSkImageBuilder builder(this, quad, dst_color_space.ToSkColorSpace());
   const SkImage* image = builder.sk_image();
   if (!image)
     return;
@@ -2117,13 +2086,17 @@ void SkiaRenderer::DrawYUVVideoQuad(const YUVVideoDrawQuad* quad,
   params->vis_tex_coords = cc::MathUtil::ScaleRectProportional(
       quad->ya_tex_coord_rect, gfx::RectF(quad->rect), params->visible_rect);
 
-  SkPaint paint = params->paint();
-  if (color_filter)
-    paint.setColorFilter(color_filter);
-
   // Use provided, unclipped texture coordinates as the content area, which will
   // force coord clamping unless the geometry was clipped, or they span the
   // entire YUV image.
+  SkPaint paint = params->paint();
+
+  sk_sp<SkColorFilter> color_filter =
+      GetColorFilter(src_color_space, dst_color_space, quad->resource_offset,
+                     quad->resource_multiplier);
+  DCHECK(!paint.getColorFilter());
+  paint.setColorFilter(color_filter);
+
   DrawSingleImage(image, quad->ya_tex_coord_rect, rpdq_params, &paint, params);
 }
 
@@ -2202,14 +2175,10 @@ sk_sp<SkColorFilter> SkiaRenderer::GetColorFilter(const gfx::ColorSpace& src,
                                                   const gfx::ColorSpace& dst,
                                                   float resource_offset,
                                                   float resource_multiplier) {
-  gfx::ColorSpace adjusted_src = src;
-  float sdr_white_level =
-      current_frame()->display_color_spaces.GetSDRWhiteLevel();
-  if (src.IsValid() && !src.IsHDR() &&
-      sdr_white_level != gfx::ColorSpace::kDefaultSDRWhiteLevel) {
-    adjusted_src = src.GetScaledColorSpace(
-        sdr_white_level / gfx::ColorSpace::kDefaultSDRWhiteLevel);
-  }
+  // If the input color space is PQ, and it did not specify a white level,
+  // override it with the frame's white level.
+  gfx::ColorSpace adjusted_src = src.GetWithPQSDRWhiteLevel(
+      current_frame()->display_color_spaces.GetSDRWhiteLevel());
 
   sk_sp<SkRuntimeEffect>& effect = color_filter_cache_[dst][adjusted_src];
   if (!effect) {

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -313,8 +313,7 @@ void SkiaOutputSurfaceImpl::MakePromiseSkImage(ImageContext* image_context) {
 
 sk_sp<SkImage> SkiaOutputSurfaceImpl::MakePromiseSkImageFromYUV(
     const std::vector<ImageContext*>& contexts,
-    SkYUVColorSpace yuv_color_space,
-    sk_sp<SkColorSpace> dst_color_space,
+    sk_sp<SkColorSpace> image_color_space,
     bool has_alpha) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(current_paint_);
@@ -348,10 +347,11 @@ sk_sp<SkImage> SkiaOutputSurfaceImpl::MakePromiseSkImageFromYUV(
     texture_contexts[i] = context;
   }
 
+  // Note: YUV to RGB conversion is handled by a color filter in SkiaRenderer.
   auto image = current_paint_->recorder()->makeYUVAPromiseTexture(
-      yuv_color_space, formats, yuva_sizes, indices, yuva_sizes[0].width(),
-      yuva_sizes[0].height(), kTopLeft_GrSurfaceOrigin, dst_color_space,
-      Fulfill, DoNothing, DoNothing, texture_contexts);
+      kIdentity_SkYUVColorSpace, formats, yuva_sizes, indices,
+      yuva_sizes[0].width(), yuva_sizes[0].height(), kTopLeft_GrSurfaceOrigin,
+      image_color_space, Fulfill, DoNothing, DoNothing, texture_contexts);
   DCHECK(image);
   return image;
 }

components/viz/service/display_embedder/skia_output_surface_impl.h

@@ -93,8 +93,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceImpl : public SkiaOutputSurface {
   SkCanvas* BeginPaintCurrentFrame() override;
   sk_sp<SkImage> MakePromiseSkImageFromYUV(
       const std::vector<ImageContext*>& contexts,
-      SkYUVColorSpace yuv_color_space,
-      sk_sp<SkColorSpace> dst_color_space,
+      sk_sp<SkColorSpace> image_color_space,
       bool has_alpha) override;
   void SwapBuffersSkipped() override;
   void ScheduleOutputSurfaceAsOverlay(

components/viz/test/fake_skia_output_surface.cc

@@ -164,8 +164,7 @@ void FakeSkiaOutputSurface::MakePromiseSkImage(ImageContext* image_context) {
 
 sk_sp<SkImage> FakeSkiaOutputSurface::MakePromiseSkImageFromYUV(
     const std::vector<ImageContext*>& contexts,
-    SkYUVColorSpace yuv_color_space,
-    sk_sp<SkColorSpace> dst_color_space,
+    sk_sp<SkColorSpace> image_color_space,
     bool has_alpha) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   NOTIMPLEMENTED();

components/viz/test/fake_skia_output_surface.h

@@ -70,8 +70,7 @@ class FakeSkiaOutputSurface : public SkiaOutputSurface {
   SkCanvas* BeginPaintCurrentFrame() override;
   sk_sp<SkImage> MakePromiseSkImageFromYUV(
       const std::vector<ImageContext*>& contexts,
-      SkYUVColorSpace yuv_color_space,
-      sk_sp<SkColorSpace> dst_color_space,
+      sk_sp<SkColorSpace> image_color_space,
       bool has_alpha) override;
   void SwapBuffersSkipped() override {}
   SkCanvas* BeginPaintRenderPass(const RenderPassId& id,

content/test/gpu/gpu_tests/pixel_integration_test.py

@@ -78,6 +78,7 @@ class PixelIntegrationTest(
     if sys.platform.startswith('win'):
       pages += namespace.DirectCompositionPages(cls.test_base_name)
       pages += namespace.LowLatencySwapChainPages(cls.test_base_name)
+      pages += namespace.HdrTestPages(cls.test_base_name)
     for p in pages:
       yield(p.name,
            skia_gold_integration_test_base.GPU_RELATIVE_PATH + p.url,

content/test/gpu/gpu_tests/pixel_test_pages.py

@@ -1976,3 +1976,20 @@ class PixelTestPages(object):
         tolerance=tolerance_dc,
         expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS),
       ]
+
+  @staticmethod
+  def HdrTestPages(base_name):
+    return [
+      PixelTestPage(
+        'pixel_canvas2d.html',
+        base_name + '_Canvas2DRedBoxScrgbLinear',
+        test_rect=[0, 0, 300, 300],
+        browser_args=['--force-color-profile=scrgb-linear']),
+
+      PixelTestPage(
+        'pixel_canvas2d.html',
+        base_name + '_Canvas2DRedBoxHdr10',
+        test_rect=[0, 0, 300, 300],
+        browser_args=['--force-color-profile=hdr10']),
+    ]
+

content/test/gpu/gpu_tests/skia_gold_integration_test_base.py

@@ -102,12 +102,16 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
   def _AddDefaultArgs(browser_args):
     if not browser_args:
       browser_args = []
+    force_color_profile_arg = [
+        arg for arg in browser_args if arg.startswith('--force-color-profile=')
+    ]
+    if not force_color_profile_arg:
+      browser_args = browser_args + [
+          '--force-color-profile=srgb',
+          '--ensure-forced-color-profile',
+      ]
     # All tests receive the following options.
-    return [
-      '--force-color-profile=srgb',
-      '--ensure-forced-color-profile',
-      '--enable-gpu-benchmarking',
-      '--test-type=gpu'] + browser_args
+    return browser_args + ['--enable-gpu-benchmarking', '--test-type=gpu']
 
   @classmethod
   def StopBrowser(cls):

content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt

@@ -210,3 +210,8 @@ crbug.com/1028975 [ win ] Pixel_PrecisionRoundedCorner [ RetryOnFailure ]
 
 # VP9 appears to not recover correctly after GPU process crashes on Windows.
 crbug.com/1033982 [ win nvidia ] Pixel_Video_Context_Loss_VP9 [ RetryOnFailure ]
+
+# HDR rendering with PQ color space appears to be broken on Windows RS2.
+# TODO(sunnyps): Revert this temporary suppression after ensuring pixel tests
+# always run on Windows RS3 or above.
+crbug.com/1066979 [ win ] Pixel_Canvas2DRedBoxHdr10 [ Skip ]

ui/gfx/color_space.cc

@@ -428,7 +428,12 @@ std::string ColorSpace::ToString() const {
     PRINT_ENUM_CASE(TransferID, IEC61966_2_1_HDR)
     PRINT_ENUM_CASE(TransferID, LINEAR_HDR)
     case TransferID::SMPTEST2084:
-      ss << "PQ (SDR white point " << transfer_params_[0] << ")";
+      ss << "PQ (SDR white point ";
+      if (transfer_params_[0] == 0.f)
+        ss << "default " << kDefaultSDRWhiteLevel;
+      else
+        ss << transfer_params_[0];
+      ss << " nits)";
       break;
     case TransferID::CUSTOM: {
       skcms_TransferFunction fn;
@@ -607,11 +612,11 @@ sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace() const {
       transfer_fn = SkNamedTransferFn::kHLG;
       break;
     case TransferID::SMPTEST2084:
-      transfer_fn = SkNamedTransferFn::kPQ;
+      GetPQTransferFunction(&transfer_fn);
       break;
     default:
       if (!GetTransferFunction(&transfer_fn)) {
-        DLOG(ERROR) << "Failed to transfer function for SkColorSpace";
+        DLOG(ERROR) << "Failed to get transfer function for SkColorSpace";
         return nullptr;
       }
       break;
@@ -940,6 +945,23 @@ bool ColorSpace::GetTransferFunction(skcms_TransferFunction* fn) const {
   }
 }
 
+void ColorSpace::GetPQTransferFunction(skcms_TransferFunction* fn) const {
+  DCHECK_EQ(transfer_, TransferID::SMPTEST2084);
+  const float sdr_white_level =
+      transfer_params_[0] == 0.0f ? kDefaultSDRWhiteLevel : transfer_params_[0];
+  // The generic PQ transfer function produces normalized luminance values i.e.
+  // the range 0-1 represents 0-10000 nits for the reference display, but we
+  // want to map 1.0 to |sdr_white_level| nits so we need to scale accordingly.
+  const float w = 10000.0f / sdr_white_level;
+  // Distribute scaling factor W by scaling A and B with X ^ (1/F):
+  // ((A + Bx^C) / (D + Ex^C))^F * W = ((A + Bx^C) / (D + Ex^C) * W^(1/F))^F
+  // See https://crbug.com/1058580#c32 for discussion.
+  *fn = SkNamedTransferFn::kPQ;
+  const float ws = powf(w, 1 / fn->f);
+  fn->a = ws * fn->a;
+  fn->b = ws * fn->b;
+}
+
 bool ColorSpace::GetInverseTransferFunction(skcms_TransferFunction* fn) const {
   if (!GetTransferFunction(fn))
     return false;
@@ -950,7 +972,7 @@ bool ColorSpace::GetInverseTransferFunction(skcms_TransferFunction* fn) const {
 bool ColorSpace::GetPQSDRWhiteLevel(float* sdr_white_level) const {
   if (transfer_ != TransferID::SMPTEST2084)
     return false;
-  if (transfer_params_[0] == 0.f)
+  if (transfer_params_[0] == 0.0f)
     *sdr_white_level = kDefaultSDRWhiteLevel;
   else
     *sdr_white_level = transfer_params_[0];

ui/gfx/color_space.h

@@ -330,6 +330,8 @@ class COLOR_SPACE_EXPORT ColorSpace {
   static bool GetTransferFunction(TransferID, skcms_TransferFunction* fn);
   static size_t TransferParamCount(TransferID);
 
+  void GetPQTransferFunction(skcms_TransferFunction* fn) const;
+
   void SetCustomTransferFunction(const skcms_TransferFunction& fn);
   void SetCustomPrimaries(const skcms_Matrix3x3& to_XYZD50);