[go: nahoru, domu]

Make YUVA tone mapping functional

Make YUVA PQ and HLG images be tone mapped. Add a gfx::ColorSpace
method IsPQOrHLG, since that comes up often.

In ColorConversionSkFilterCache::ConvertImage, change the way that
we avoid color conversion. We wish for no color conversion to happen,
because we are doing it manually in our own shader. Previously, we
set the source image's color space to the destination's color space.
This doesn't work on YUV images (the results are not right). Perhaps
it should work. In the mean time, it's just as easy to set the
destination's color space to the source, which does work.

Bug: 1286088
Change-Id: I01884391a6e5201fe3babceee90706d7b3adcdb8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3490056
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975414}
diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index 86e24b4..091ec28 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn
@@ -465,6 +465,7 @@
     "//services/metrics/public/cpp:ukm_builders",
     "//services/metrics/public/mojom",
     "//services/tracing/public/cpp:cpp",
+    "//skia:skcms",
     "//ui/base:features",
     "//ui/events:events_base",
     "//ui/gfx",
diff --git a/cc/paint/image_transfer_cache_entry.cc b/cc/paint/image_transfer_cache_entry.cc
index 6f1ef12..35ee71e 100644
--- a/cc/paint/image_transfer_cache_entry.cc
+++ b/cc/paint/image_transfer_cache_entry.cc
@@ -528,7 +528,6 @@
         return false;
       }
 
-      DCHECK(!target_color_params);
       sk_sp<SkImage> plane = SkImage::MakeFromRaster(pixmap, nullptr, nullptr);
       if (!plane) {
         DLOG(ERROR) << "Failed to create image from plane pixmap";
diff --git a/cc/tiles/gpu_image_decode_cache.cc b/cc/tiles/gpu_image_decode_cache.cc
index e9090ca..73fa44a 100644
--- a/cc/tiles/gpu_image_decode_cache.cc
+++ b/cc/tiles/gpu_image_decode_cache.cc
@@ -2115,7 +2115,7 @@
   DCHECK_GT(image_data->decode.ref_count, 0u);
   DCHECK_GT(image_data->upload.ref_count, 0u);
 
-  sk_sp<SkColorSpace> color_space =
+  sk_sp<SkColorSpace> target_color_space =
       SupportsColorSpaceConversion() &&
               draw_image.target_color_space().IsValid()
           ? draw_image.target_color_space().ToSkColorSpace()
@@ -2126,12 +2126,17 @@
   // have happened at decode time.
   sk_sp<SkColorSpace> decoded_target_colorspace =
       ColorSpaceForImageDecode(draw_image, image_data->mode);
-  if (color_space && SkColorSpace::Equals(color_space.get(),
-                                          decoded_target_colorspace.get())) {
-    color_space = nullptr;
+  if (target_color_space &&
+      SkColorSpace::Equals(target_color_space.get(),
+                           decoded_target_colorspace.get())) {
+    target_color_space = nullptr;
   }
 
   absl::optional<TargetColorParams> target_color_params;
+  if (target_color_space) {
+    target_color_params = draw_image.target_color_params();
+    target_color_params->color_space = gfx::ColorSpace(*target_color_space);
+  }
 
   // Will be nullptr for non-HDR images or when we're using the default level.
   const bool needs_adjusted_color_space =
@@ -2143,16 +2148,15 @@
     DCHECK(use_transfer_cache_);
     if (image_data->decode.do_hardware_accelerated_decode()) {
       UploadImageIfNecessary_TransferCache_HardwareDecode(
-          draw_image, image_data, color_space);
+          draw_image, image_data, target_color_space);
     } else if (image_data->yuva_pixmap_info.has_value()) {
+      const bool needs_tone_mapping =
+          decoded_target_colorspace &&
+          gfx::ColorSpace(*decoded_target_colorspace).IsPQOrHLG();
       UploadImageIfNecessary_TransferCache_SoftwareDecode_YUVA(
           draw_image, image_data, decoded_target_colorspace,
-          target_color_params);
+          needs_tone_mapping ? target_color_params : absl::nullopt);
     } else {
-      if (color_space) {
-        target_color_params = draw_image.target_color_params();
-        target_color_params->color_space = gfx::ColorSpace(*color_space);
-      }
       UploadImageIfNecessary_TransferCache_SoftwareDecode_RGBA(
           draw_image, image_data, needs_adjusted_color_space,
           decoded_target_colorspace, target_color_params);
@@ -2167,11 +2171,12 @@
     if (image_data->yuva_pixmap_info.has_value()) {
       UploadImageIfNecessary_GpuCpu_YUVA(
           draw_image, image_data, uploaded_image, image_needs_mips,
-          decoded_target_colorspace, color_space);
+          decoded_target_colorspace, target_color_space);
     } else {
       UploadImageIfNecessary_GpuCpu_RGBA(
           draw_image, image_data, uploaded_image, image_needs_mips,
-          needs_adjusted_color_space, decoded_target_colorspace, color_space);
+          needs_adjusted_color_space, decoded_target_colorspace,
+          target_color_space);
     }
   }
 }
diff --git a/cc/tiles/gpu_image_decode_cache_unittest.cc b/cc/tiles/gpu_image_decode_cache_unittest.cc
index b96d4f1..3ab5d22 100644
--- a/cc/tiles/gpu_image_decode_cache_unittest.cc
+++ b/cc/tiles/gpu_image_decode_cache_unittest.cc
@@ -3128,7 +3128,13 @@
     EXPECT_TRUE(decoded_draw_image.image());
     EXPECT_TRUE(decoded_draw_image.image()->isTextureBacked());
 
-    if (decodes_to_yuv) {
+    // If `draw_image` is tone mapped, then it will be converted to RGBA
+    // during tone mapping.
+    bool color_converted_to_rgba = use_transfer_cache_ &&
+                                   target_cs.IsPQOrHLG() &&
+                                   cache->SupportsColorSpaceConversion();
+
+    if (decodes_to_yuv && !color_converted_to_rgba) {
       // Skia will flatten a YUV SkImage upon calling makeTextureImage. Thus, we
       // must separately request mips for each plane and compare to the original
       // uploaded planes.
diff --git a/ui/gfx/color_conversion_sk_filter_cache.cc b/ui/gfx/color_conversion_sk_filter_cache.cc
index 2f6b559..14d53702 100644
--- a/ui/gfx/color_conversion_sk_filter_cache.cc
+++ b/ui/gfx/color_conversion_sk_filter_cache.cc
@@ -31,16 +31,6 @@
 
 }  // namespace
 
-bool IsHLGOrPQ(const gfx::ColorSpace& color_space) {
-  switch (color_space.GetTransferID()) {
-    case gfx::ColorSpace::TransferID::HLG:
-    case gfx::ColorSpace::TransferID::PQ:
-      return true;
-    default:
-      return false;
-  }
-}
-
 ColorConversionSkFilterCache::ColorConversionSkFilterCache() = default;
 ColorConversionSkFilterCache::~ColorConversionSkFilterCache() = default;
 
@@ -80,14 +70,14 @@
     float dst_max_luminance_relative) {
   // Set unused parameters to bogus values, so that they do not result in
   // different keys for the same conversion.
-  if (!IsHLGOrPQ(src)) {
+  if (!src.IsPQOrHLG()) {
     // If the source is not HLG or PQ, then `dst_max_luminance_relative` will
     // not be used, so set it to a nonsense value.
     dst_max_luminance_relative = 0;
 
     // If neither source nor destination are HLG or PQ, then
     // `sdr_max_luminance_nits` will not be used, so set it to a nonsense value.
-    if (!IsHLGOrPQ(dst)) {
+    if (!dst.IsPQOrHLG()) {
       sdr_max_luminance_nits = 0;
     }
   }
@@ -155,7 +145,7 @@
   static bool image_tone_mapping_enabled =
       base::FeatureList::IsEnabled(kImageToneMapping);
 
-  SkColorSpace* image_sk_color_space = image->colorSpace();
+  sk_sp<SkColorSpace> image_sk_color_space = image->refColorSpace();
   if (!image_sk_color_space || !image_tone_mapping_enabled)
     return image->makeColorSpace(target_color_space, context);
 
@@ -171,7 +161,7 @@
   SkImageInfo image_info =
       SkImageInfo::Make(image->dimensions(),
                         SkColorInfo(kRGBA_F16_SkColorType, kPremul_SkAlphaType,
-                                    target_color_space));
+                                    image_sk_color_space));
   sk_sp<SkSurface> surface;
   if (context) {
     // TODO(https://crbug.com/1286088): Consider adding mipmap support here.
@@ -193,10 +183,10 @@
   paint.setBlendMode(SkBlendMode::kSrc);
   paint.setColorFilter(filter);
   SkSamplingOptions sampling_options(SkFilterMode::kNearest);
-  surface->getCanvas()->drawImage(
-      image->reinterpretColorSpace(target_color_space),
-      /*x=*/0, /*y=*/0, sampling_options, &paint);
-  return surface->makeImageSnapshot();
+  surface->getCanvas()->drawImage(image,
+                                  /*x=*/0, /*y=*/0, sampling_options, &paint);
+  return surface->makeImageSnapshot()->reinterpretColorSpace(
+      target_color_space);
 }
 
 }  // namespace gfx
diff --git a/ui/gfx/color_space.cc b/ui/gfx/color_space.cc
index 7e72b71..9bb81c1 100644
--- a/ui/gfx/color_space.cc
+++ b/ui/gfx/color_space.cc
@@ -358,6 +358,10 @@
          transfer_ == TransferID::PIECEWISE_HDR;
 }
 
+bool ColorSpace::IsPQOrHLG() const {
+  return transfer_ == TransferID::PQ || transfer_ == TransferID::HLG;
+}
+
 bool ColorSpace::FullRangeEncodedValues() const {
   return transfer_ == TransferID::LINEAR_HDR ||
          transfer_ == TransferID::SRGB_HDR ||
diff --git a/ui/gfx/color_space.h b/ui/gfx/color_space.h
index 158caae7..307d4d21 100644
--- a/ui/gfx/color_space.h
+++ b/ui/gfx/color_space.h
@@ -276,6 +276,9 @@
   // Returns true if the transfer function is an HDR one (SMPTE 2084, HLG, etc).
   bool IsHDR() const;
 
+  // Returns true if the transfer function is PQ or HLG.
+  bool IsPQOrHLG() const;
+
   // Returns true if the encoded values can be outside of the 0.0-1.0 range.
   bool FullRangeEncodedValues() const;