SkiaRenderer: Use SkColorSpace instead of gfx::ColorSpace

The conversion from gfx::ColorSpace to SkColorSpace now takes an
SDR white level. Move conversion to be done as early as possible in
the various pipelines, so that it is done in as few places as possible.

ScopedSkImageBuilder takes an argument for the color space to override
for the resulting SkImage. Change this to be an SkColorSpace (this is
just more reasonable, because SkImages take SkColorSpaces).

In several places, RenderPassBacking's color space is accessed as
an SkColorSpace. Add a helper function RenderPassBackingSkColorSpace
to do this conversion.

SkiaOutputDevice::Reshape takes two color space arguments, one as a
SkColorSpace in the SkSurfaceCharacterization, and one as an explicit
gfx::ColorSpace. The gfx::ColorSpace is actually needed, because it
is used with SharedImage (where things like PQ and SCRGB_LINEAR_80_NIT
refer to specific formats, independent of SDR white level). In these
cases, if an SkColorSpace is needed, use the one from the
SkSurfaceCharacterization.

Add the function DirectRenderer::CurrentRenderPassSkColorSpace, which
produces the SkColorSpace for the current render pass (to consolidate
places where the conversion is done). Change the function
reshape_color_space to return an SkColorSpace (this is the only way
that it is currently used).

Add an SDR white level parameter to OutputSurface::ReshapeParams.

Bug: 1286076
Change-Id: I49eb1ca3ece305f2f44c2d9383772bd59935b76d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3628391
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001210}

components/viz/service/display/direct_renderer.cc

@@ -373,6 +373,7 @@ void DirectRenderer::DrawFrame(
   reshape_params.size = surface_resource_size;
   reshape_params.device_scale_factor = device_scale_factor;
   reshape_params.color_space = frame_color_space;
+  reshape_params.sdr_white_level = CurrentFrameSDRWhiteLevel();
   reshape_params.format = frame_buffer_format;
   reshape_params.use_stencil = use_stencil;
   if (next_frame_needs_full_frame_redraw_ ||
@@ -1003,6 +1004,10 @@ bool DirectRenderer::ShouldApplyRoundedCorner(const DrawQuad* quad) const {
   return false;
 }
 
+float DirectRenderer::CurrentFrameSDRWhiteLevel() const {
+  return current_frame()->display_color_spaces.GetSDRMaxLuminanceNits();
+}
+
 gfx::ColorSpace DirectRenderer::RootRenderPassColorSpace() const {
   return current_frame()->display_color_spaces.GetOutputColorSpace(
       current_frame()->root_render_pass->content_color_usage,

components/viz/service/display/direct_renderer.h

@@ -290,8 +290,16 @@ class VIZ_SERVICE_EXPORT DirectRenderer {
 
   bool ShouldApplyRoundedCorner(const DrawQuad* quad) const;
 
+  float CurrentFrameSDRWhiteLevel() const;
   gfx::ColorSpace RootRenderPassColorSpace() const;
   gfx::ColorSpace CurrentRenderPassColorSpace() const;
+  // Return the SkColorSpace for rendering to the current render pass. Unlike
+  // CurrentRenderPassColorSpace, this color space has the value of
+  // CurrentFrameSDRWhiteLevel incorporated into it.
+  sk_sp<SkColorSpace> CurrentRenderPassSkColorSpace() const {
+    return CurrentRenderPassColorSpace().ToSkColorSpace(
+        CurrentFrameSDRWhiteLevel());
+  }
 
   const raw_ptr<const RendererSettings> settings_;
   // Points to the viz-global singleton.
@@ -354,7 +362,8 @@ class VIZ_SERVICE_EXPORT DirectRenderer {
     return reshape_params_->format;
   }
   gfx::ColorSpace reshape_color_space() const {
-    return reshape_params_ ? reshape_params_->color_space : gfx::ColorSpace();
+    DCHECK(reshape_params_);
+    return reshape_params_->color_space;
   }
 
   // Sets a DelegatedInkPointRendererSkiaForTest to be used for testing only, in

components/viz/service/display/display_resource_provider_skia.cc

@@ -130,7 +130,7 @@ DisplayResourceProviderSkia::LockSetForExternalUse::LockResource(
     ResourceId id,
     bool maybe_concurrent_reads,
     bool is_video_plane,
-    const absl::optional<gfx::ColorSpace>& override_color_space,
+    sk_sp<SkColorSpace> override_color_space,
     bool raw_draw_is_possible) {
   auto it = resource_provider_->resources_.find(id);
   DCHECK(it != resource_provider_->resources_.end());
@@ -154,8 +154,9 @@ DisplayResourceProviderSkia::LockSetForExternalUse::LockResource(
         // is very subtle.
         image_color_space =
             override_color_space
-                .value_or(resource.transferable.color_space.GetAsFullRangeRGB())
-                .ToSkColorSpace();
+                ? override_color_space
+                : resource.transferable.color_space.GetAsFullRangeRGB()
+                      .ToSkColorSpace();
       }
       resource.image_context =
           resource_provider_->external_use_client_->CreateImageContext(

components/viz/service/display/display_resource_provider_skia.h

@@ -51,15 +51,14 @@ class VIZ_SERVICE_EXPORT DisplayResourceProviderSkia
 
     // Lock a resource for external use. The return value was created by
     // |client| at some point in the past. The SkImage color space will be set
-    // to |color_space| if valid, otherwise it will be set to the resource's
-    // color space. If |is_video_plane| is true, the image color space will be
-    // set to nullptr (to avoid LOG spam).
+    // to |override_color_space| if non-nullptr, otherwise it will be set to the
+    // resource's color space. If |is_video_plane| is true, the image color
+    // space will be set to nullptr (to avoid LOG spam).
     ExternalUseClient::ImageContext* LockResource(
         ResourceId resource_id,
         bool maybe_concurrent_reads,
         bool is_video_plane,
-        const absl::optional<gfx::ColorSpace>& override_color_space =
-            absl::nullopt,
+        sk_sp<SkColorSpace> override_color_space = nullptr,
         bool raw_draw_if_possible = false);
 
     // Unlock all locked resources with a |sync_token|.  The |sync_token| should

components/viz/service/display/output_surface.h

@@ -209,14 +209,16 @@ class VIZ_SERVICE_EXPORT OutputSurface {
     gfx::Size size;
     float device_scale_factor = 1.f;
     gfx::ColorSpace color_space;
+    float sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevel;
     gfx::BufferFormat format = gfx::BufferFormat::RGBX_8888;
     bool use_stencil = false;
 
     bool operator==(const ReshapeParams& other) const {
       return size == other.size &&
              device_scale_factor == other.device_scale_factor &&
-             color_space == other.color_space && format == other.format &&
-             use_stencil == other.use_stencil;
+             color_space == other.color_space &&
+             sdr_white_level == other.sdr_white_level &&
+             format == other.format && use_stencil == other.use_stencil;
     }
     bool operator!=(const ReshapeParams& other) const {
       return !(*this == other);

components/viz/service/display/skia_renderer.cc

@@ -575,8 +575,7 @@ class SkiaRenderer::ScopedSkImageBuilder {
                        bool maybe_concurrent_reads,
                        SkAlphaType alpha_type = kPremul_SkAlphaType,
                        GrSurfaceOrigin origin = kTopLeft_GrSurfaceOrigin,
-                       const absl::optional<gfx::ColorSpace>&
-                           override_colorspace = absl::nullopt,
+                       sk_sp<SkColorSpace> override_color_space = nullptr,
                        bool raw_draw_if_possible = false);
 
   ScopedSkImageBuilder(const ScopedSkImageBuilder&) = delete;
@@ -600,7 +599,7 @@ SkiaRenderer::ScopedSkImageBuilder::ScopedSkImageBuilder(
     bool maybe_concurrent_reads,
     SkAlphaType alpha_type,
     GrSurfaceOrigin origin,
-    const absl::optional<gfx::ColorSpace>& override_color_space,
+    sk_sp<SkColorSpace> override_color_space,
     bool raw_draw_if_possible) {
   if (!resource_id)
     return;
@@ -946,7 +945,7 @@ void SkiaRenderer::BindFramebufferToTexture(
   RenderPassBacking& backing = iter->second;
   current_canvas_ = skia_output_surface_->BeginPaintRenderPass(
       render_pass_id, backing.size, backing.format, backing.generate_mipmap,
-      backing.color_space.ToSkColorSpace(), /*is_overlay=*/false,
+      RenderPassBackingSkColorSpace(backing), /*is_overlay=*/false,
       backing.mailbox);
 }
 
@@ -2065,13 +2064,12 @@ void SkiaRenderer::DrawStreamVideoQuad(const StreamVideoDrawQuad* quad,
   TRACE_EVENT0("viz", "SkiaRenderer::DrawStreamVideoQuad");
   DCHECK(!MustFlushBatchedQuads(quad, rpdq_params, *params));
 
-  absl::optional<gfx::ColorSpace> override_color_space;
+  sk_sp<SkColorSpace> override_color_space;
 
   // Force SRGB color space if we don't want real color space from media
   // decoder.
-  if (!use_real_color_space_for_stream_video_) {
-    override_color_space = gfx::ColorSpace::CreateSRGB();
-  }
+  if (!use_real_color_space_for_stream_video_)
+    override_color_space = SkColorSpace::MakeSRGB();
 
   ScopedSkImageBuilder builder(this, quad->resource_id(),
                                /*maybe_concurrent_reads=*/true,
@@ -2111,17 +2109,17 @@ void SkiaRenderer::DrawTextureQuad(const TextureDrawQuad* quad,
   const bool needs_color_conversion_filter =
       quad->is_video_frame && src_color_space.IsHDR();
 
-  absl::optional<gfx::ColorSpace> override_color_space;
+  sk_sp<SkColorSpace> override_color_space;
   if (needs_color_conversion_filter)
-    override_color_space = CurrentRenderPassColorSpace();
+    override_color_space = CurrentRenderPassSkColorSpace();
 
-  // TODO(b/221643955): Some Chrome OS tests rely on the old GLRenderer
-  // behavior of skipping color space conversions if the quad's color space is
-  // invalid. Once these tests are migrated, we can remove the override here
-  // and revert to Skia's default behavior of assuming sRGB on invalid.
+    // TODO(b/221643955): Some Chrome OS tests rely on the old GLRenderer
+    // behavior of skipping color space conversions if the quad's color space is
+    // invalid. Once these tests are migrated, we can remove the override here
+    // and revert to Skia's default behavior of assuming sRGB on invalid.
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   if (!src_color_space.IsValid())
-    override_color_space = CurrentRenderPassColorSpace();
+    override_color_space = CurrentRenderPassSkColorSpace();
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
   ScopedSkImageBuilder builder(
@@ -2252,7 +2250,7 @@ void SkiaRenderer::DrawTextureQuad(const TextureDrawQuad* quad,
     // Skia won't perform color conversion.
     const gfx::ColorSpace dst_color_space = CurrentRenderPassColorSpace();
     DCHECK(SkColorSpace::Equals(image->colorSpace(),
-                                dst_color_space.ToSkColorSpace().get()));
+                                CurrentRenderPassSkColorSpace().get()));
     sk_sp<SkColorFilter> color_filter =
         GetColorSpaceConversionFilter(src_color_space, dst_color_space);
     paint.setColorFilter(color_filter->makeComposed(paint.refColorFilter()));
@@ -2286,7 +2284,7 @@ void SkiaRenderer::DrawTileDrawQuad(const TileDrawQuad* quad,
       this, quad->resource_id(), /*maybe_concurrent_reads=*/false,
       quad->is_premultiplied ? kPremul_SkAlphaType : kUnpremul_SkAlphaType,
       /*origin=*/kTopLeft_GrSurfaceOrigin,
-      /*override_colorspace=*/absl::nullopt, raw_draw_if_possible);
+      /*override_color_space=*/nullptr, raw_draw_if_possible);
 
   params->vis_tex_coords = cc::MathUtil::ScaleRectProportional(
       quad->tex_coord_rect, gfx::RectF(quad->rect), params->visible_rect);
@@ -2354,8 +2352,7 @@ void SkiaRenderer::DrawYUVVideoQuad(const YUVVideoDrawQuad* quad,
   // color space we lie and say the SkImage destination color space is always
   // the same as the rest of the frame. Otherwise the two color space
   // adjustments combined will produce the wrong result.
-  const gfx::ColorSpace& frame_color_space = CurrentRenderPassColorSpace();
-  gfx::ColorSpace dst_color_space = frame_color_space;
+  gfx::ColorSpace dst_color_space = CurrentRenderPassColorSpace();
 
 #if BUILDFLAG(IS_WIN)
   // Force sRGB output on Windows for overlay candidate video quads to match
@@ -2376,12 +2373,11 @@ void SkiaRenderer::DrawYUVVideoQuad(const YUVVideoDrawQuad* quad,
 #endif
 
   DCHECK(resource_provider());
-  // Pass in |frame_color_space| here instead of |dst_color_space| so the color
-  // space transform going from SkImage to SkSurface is identity. The
-  // SkColorFilter already handles color space conversion so this avoids
+  // Pass in |CurrentRenderPassSkColorSpace()| here instead of |dst_color_space|
+  // so the color space transform going from SkImage to SkSurface is identity.
+  // The SkColorFilter already handles color space conversion so this avoids
   // applying the conversion twice.
-  ScopedYUVSkImageBuilder builder(this, quad,
-                                  frame_color_space.ToSkColorSpace());
+  ScopedYUVSkImageBuilder builder(this, quad, CurrentRenderPassSkColorSpace());
   const SkImage* image = builder.sk_image();
   if (!image)
     return;
@@ -2794,7 +2790,7 @@ void SkiaRenderer::DrawRenderPassQuad(const AggregatedRenderPassDrawQuad* quad,
   sk_sp<SkImage> content_image =
       skia_output_surface_->MakePromiseSkImageFromRenderPass(
           quad->render_pass_id, backing.size, backing.format,
-          backing.generate_mipmap, backing.color_space.ToSkColorSpace(),
+          backing.generate_mipmap, RenderPassBackingSkColorSpace(backing),
           backing.mailbox);
   DLOG_IF(ERROR, !content_image)
       << "MakePromiseSkImageFromRenderPass() failed for render pass";
@@ -3084,7 +3080,7 @@ void SkiaRenderer::PrepareRenderPassOverlay(
   ResourceFormat buffer_format{};
   gfx::ColorSpace color_space;
 
-  RenderPassBacking* backing = nullptr;
+  RenderPassBacking* src_quad_backing = nullptr;
   auto bypass = render_pass_bypass_quads_.find(quad->render_pass_id);
   BypassMode bypass_mode = BypassMode::kSkip;
   // When Render Pass has a single quad inside we would draw that directly.
@@ -3103,9 +3099,9 @@ void SkiaRenderer::PrepareRenderPassOverlay(
     DCHECK(render_pass_backings_.end() != it);
     // This function is called after AllocateRenderPassResourceIfNeeded, so
     // there should be backing ready.
-    backing = &it->second;
-    buffer_format = backing->format;
-    color_space = backing->color_space;
+    src_quad_backing = &it->second;
+    buffer_format = src_quad_backing->format;
+    color_space = src_quad_backing->color_space;
   }
 
   // Adjust the overlay |buffer_size| to reduce memory fragmentation. It also
@@ -3149,10 +3145,14 @@ void SkiaRenderer::PrepareRenderPassOverlay(
     in_flight_render_pass_overlay_backings_.push_back(std::move(*it));
     available_render_pass_overlay_backings_.erase(it);
   }
+  const RenderPassBacking& dst_overlay_backing =
+      in_flight_render_pass_overlay_backings_.back();
 
   current_canvas_ = skia_output_surface_->BeginPaintRenderPass(
-      quad->render_pass_id, buffer_size, buffer_format, /*mipmap=*/false,
-      color_space.ToSkColorSpace(), /*is_overlay=*/true, overlay->mailbox);
+      quad->render_pass_id, dst_overlay_backing.size,
+      dst_overlay_backing.format, /*mipmap=*/false,
+      RenderPassBackingSkColorSpace(dst_overlay_backing), /*is_overlay=*/true,
+      overlay->mailbox);
   if (!current_canvas_) {
     DLOG(ERROR)
         << "BeginPaintRenderPass() in PrepareRenderPassOverlay() failed.";
@@ -3182,19 +3182,20 @@ void SkiaRenderer::PrepareRenderPassOverlay(
       NOTREACHED();
     }
   } else {
-    DCHECK(backing);
+    DCHECK(src_quad_backing);
     auto content_image = skia_output_surface_->MakePromiseSkImageFromRenderPass(
-        quad->render_pass_id, backing->size, backing->format,
-        backing->generate_mipmap, backing->color_space.ToSkColorSpace(),
-        backing->mailbox);
+        quad->render_pass_id, src_quad_backing->size, src_quad_backing->format,
+        src_quad_backing->generate_mipmap,
+        RenderPassBackingSkColorSpace(*src_quad_backing),
+        src_quad_backing->mailbox);
     if (!content_image) {
       DLOG(ERROR) << "MakePromiseSkImageFromRenderPass() in "
                      "PrepareRenderPassOverlay() failed.";
       skia_output_surface_->EndPaint(base::NullCallback());
       return;
     }
 
-    if (backing->generate_mipmap)
+    if (src_quad_backing->generate_mipmap)
       params.sampling =
           SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kLinear);
 

components/viz/service/display/skia_renderer.h

@@ -279,6 +279,10 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
   };
   base::flat_map<AggregatedRenderPassId, RenderPassBacking>
       render_pass_backings_;
+  sk_sp<SkColorSpace> RenderPassBackingSkColorSpace(
+      const RenderPassBacking& backing) {
+    return backing.color_space.ToSkColorSpace(CurrentFrameSDRWhiteLevel());
+  }
 
   // Interface used for drawing. Common among different draw modes.
   sk_sp<SkSurface> root_surface_;

components/viz/service/display/software_renderer.cc

@@ -604,8 +604,7 @@ void SoftwareRenderer::DrawUnsupportedQuad(const DrawQuad* quad) {
 void SoftwareRenderer::CopyDrawnRenderPass(
     const copy_output::RenderPassGeometry& geometry,
     std::unique_ptr<CopyOutputRequest> request) {
-  sk_sp<SkColorSpace> color_space =
-      CurrentRenderPassColorSpace().ToSkColorSpace();
+  sk_sp<SkColorSpace> color_space = CurrentRenderPassSkColorSpace();
   DCHECK(color_space);
 
   SkBitmap bitmap;

components/viz/service/display_embedder/skia_output_device_gl.cc

@@ -269,7 +269,7 @@ bool SkiaOutputDeviceGL::Reshape(
                     : kBottomLeft_GrSurfaceOrigin;
   sk_surface_ = SkSurface::MakeFromBackendRenderTarget(
       context_state_->gr_context(), render_target, origin, color_type,
-      color_space.ToSkColorSpace(), &surface_props);
+      characterization.refColorSpace(), &surface_props);
   if (!sk_surface_) {
     LOG(ERROR) << "Couldn't create surface:"
                << "\n  abandoned()="

components/viz/service/display_embedder/skia_output_device_webview.cc

@@ -65,7 +65,7 @@ bool SkiaOutputDeviceWebView::Reshape(
   }
 
   size_ = size;
-  color_space_ = color_space;
+  sk_color_space_ = characterization.refColorSpace();
   InitSkiaSurface(gl_surface_->GetBackingFramebufferObject());
   return !!sk_surface_;
 }
@@ -115,14 +115,13 @@ void SkiaOutputDeviceWebView::InitSkiaSurface(unsigned int fbo) {
   SkSurfaceProps surface_props{0, kUnknown_SkPixelGeometry};
   sk_surface_ = SkSurface::MakeFromBackendRenderTarget(
       context_state_->gr_context(), render_target, origin, color_type,
-      color_space_.ToSkColorSpace(), &surface_props);
+      sk_color_space_, &surface_props);
 
   if (!sk_surface_) {
     LOG(ERROR) << "Couldn't create surface: "
                << context_state_->gr_context()->abandoned() << " " << color_type
                << " " << framebuffer_info.fFBOID << " "
-               << framebuffer_info.fFormat << " " << color_space_.ToString()
-               << " " << size_.ToString();
+               << framebuffer_info.fFormat << " " << size_.ToString();
   }
 }
 

components/viz/service/display_embedder/skia_output_device_webview.h

@@ -54,7 +54,7 @@ class SkiaOutputDeviceWebView : public SkiaOutputDevice {
   sk_sp<SkSurface> sk_surface_;
 
   gfx::Size size_;
-  gfx::ColorSpace color_space_;
+  sk_sp<SkColorSpace> sk_color_space_;
   unsigned int last_frame_buffer_object_ = -1;
 
   base::WeakPtrFactory<SkiaOutputDeviceWebView> weak_ptr_factory_{this};

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -317,7 +317,8 @@ void SkiaOutputSurfaceImpl::Reshape(const ReshapeParams& params) {
   DCHECK(color_type != kUnknown_SkColorType)
       << "SkColorType is invalid for buffer format_index: " << format_index;
 
-  auto sk_color_space = params.color_space.ToSkColorSpace();
+  auto sk_color_space =
+      params.color_space.ToSkColorSpace(params.sdr_white_level);
   characterization_ = CreateSkSurfaceCharacterization(
       params.size, color_type, /*mipmap=*/false, std::move(sk_color_space),
       /*is_root_render_pass=*/true, /*is_overlay=*/false);