cc::PaintOp: Use mojo to serialize gfx::HDRMetadata

Avoid duplicating the code to serialize gfx::HDRMetadata.

Create a separate build target for gfx::HDRMetadata's mojo, to avoid
circular dependencies.

Update and re-enable the paint op test that stresses this path.

Bug: 1446302
Change-Id: Iec34a7d06e536e1f41b2da841f6c81a13b145d37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4593389
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1154106}

cc/paint/BUILD.gn

@@ -130,6 +130,7 @@ cc_component("paint") {
     "//ui/gfx:color_space",
     "//ui/gfx/geometry",
     "//ui/gfx/geometry:geometry_skia",
+    "//ui/gfx/mojom:hdr_metadata",
   ]
 
   deps = [

cc/paint/image_transfer_cache_entry.cc

@@ -29,6 +29,7 @@
 #include "third_party/skia/include/gpu/graphite/Recorder.h"
 #include "ui/gfx/color_conversion_sk_filter_cache.h"
 #include "ui/gfx/hdr_metadata.h"
+#include "ui/gfx/mojom/hdr_metadata.mojom.h"
 
 namespace cc {
 namespace {
@@ -263,19 +264,8 @@ size_t SafeSizeForTargetColorParams(
     // bool for whether or not there is HDR metadata.
     target_color_params_size += PaintOpWriter::SerializedSize<bool>();
     if (auto& hdr_metadata = target_color_params->hdr_metadata) {
-      // The minimum and maximum luminance.
-      target_color_params_size += PaintOpWriter::SerializedSize(
-          hdr_metadata->cta_861_3.max_content_light_level);
-      target_color_params_size += PaintOpWriter::SerializedSize(
-          hdr_metadata->cta_861_3.max_frame_average_light_level);
-      // The x and y coordinates for primaries and white point.
-      target_color_params_size += PaintOpWriter::SerializedSizeOfElements(
-          &hdr_metadata->smpte_st_2086.primaries.fRX, 4 * 2);
-      // The CLL and FALL
-      target_color_params_size += PaintOpWriter::SerializedSize(
-          hdr_metadata->smpte_st_2086.luminance_max);
-      target_color_params_size += PaintOpWriter::SerializedSize(
-          hdr_metadata->smpte_st_2086.luminance_min);
+      target_color_params_size +=
+          PaintOpWriter::SerializedSize(hdr_metadata.value());
     }
   }
   return target_color_params_size;
@@ -295,23 +285,7 @@ void WriteTargetColorParams(
     const bool has_hdr_metadata = !!target_color_params->hdr_metadata;
     writer.Write(has_hdr_metadata);
     if (target_color_params->hdr_metadata) {
-      const auto& hdr_metadata = target_color_params->hdr_metadata;
-
-      const auto& cta_861_3 = hdr_metadata->cta_861_3;
-      writer.Write(cta_861_3.max_content_light_level);
-      writer.Write(cta_861_3.max_frame_average_light_level);
-
-      const auto& smpte_st_2086 = hdr_metadata->smpte_st_2086;
-      writer.Write(smpte_st_2086.primaries.fRX);
-      writer.Write(smpte_st_2086.primaries.fRY);
-      writer.Write(smpte_st_2086.primaries.fGX);
-      writer.Write(smpte_st_2086.primaries.fGY);
-      writer.Write(smpte_st_2086.primaries.fBX);
-      writer.Write(smpte_st_2086.primaries.fBY);
-      writer.Write(smpte_st_2086.primaries.fWX);
-      writer.Write(smpte_st_2086.primaries.fWY);
-      writer.Write(smpte_st_2086.luminance_max);
-      writer.Write(smpte_st_2086.luminance_min);
+      writer.Write(target_color_params->hdr_metadata.value());
     }
   }
 }
@@ -341,29 +315,8 @@ bool ReadTargetColorParams(
   reader.Read(&has_hdr_metadata);
   if (has_hdr_metadata) {
     gfx::HDRMetadata hdr_metadata;
-    unsigned max_content_light_level = 0;
-    unsigned max_frame_average_light_level = 0;
-    reader.Read(&max_content_light_level);
-    reader.Read(&max_frame_average_light_level);
-
-    SkColorSpacePrimaries primaries = SkNamedPrimariesExt::kInvalid;
-    float luminance_max = 0;
-    float luminance_min = 0;
-    reader.Read(&primaries.fRX);
-    reader.Read(&primaries.fRY);
-    reader.Read(&primaries.fGX);
-    reader.Read(&primaries.fGY);
-    reader.Read(&primaries.fBX);
-    reader.Read(&primaries.fBY);
-    reader.Read(&primaries.fWX);
-    reader.Read(&primaries.fWY);
-    reader.Read(&luminance_max);
-    reader.Read(&luminance_min);
-
-    target_color_params->hdr_metadata = gfx::HDRMetadata(
-        gfx::HdrMetadataSmpteSt2086(primaries, luminance_max, luminance_min),
-        gfx::HdrMetadataCta861_3(max_content_light_level,
-                                 max_frame_average_light_level));
+    reader.Read(&hdr_metadata);
+    target_color_params->hdr_metadata = hdr_metadata;
   }
   return true;
 }

cc/paint/oop_pixeltest.cc

@@ -625,37 +625,39 @@ TEST_F(OopPixelTest, DrawImageWithTargetColorSpace) {
   EXPECT_NE(actual.getColor(0, 0), SkColors::kMagenta.toSkColor());
 }
 
-// crbug.com/1429057
-TEST_F(OopPixelTest, DISABLED_DrawHdrImageWithMetadata) {
-  constexpr gfx::Size size(100, 100);
-  constexpr gfx::Rect rect(size);
-  float sdr_luminance = 250.f;
-  const float kPQMaxLuminance = 10000.f;
-
-  const skcms_TransferFunction pq = SkNamedTransferFn::kPQ;
-  skcms_TransferFunction pq_inv;
-  skcms_TransferFunction_invert(&pq, &pq_inv);
-
-  const float image_luminance = 1000.f;
-  const float image_pq_pixel =
-      skcms_TransferFunction_eval(&pq_inv, image_luminance / kPQMaxLuminance);
-
-  // Create `image` with pixel value `image_pq_pixel` and PQ color space.
+TEST_F(OopPixelTest, DrawHdrImageWithMetadata) {
+  constexpr gfx::Size kSize(8, 8);
+  constexpr gfx::Rect kRect(kSize);
+
+  constexpr float kImageNits = 500.f;
+  constexpr float kContentAvgNits = 100;
+
+#if BUILDFLAG(IS_ANDROID)
+  // Allow large quantization error on Android.
+  // TODO(https://crbug.com/1363056): Ensure higher precision for HDR images.
+  constexpr float kEpsilon = 1 / 16.f;
+#else
+  constexpr float kEpsilon = 1 / 32.f;
+#endif
+
+  // Create `image` with `kImageNits` in PQ color space.
   sk_sp<SkImage> image;
   {
+    constexpr float kImagePixelValue = 0.6765848107833876f;
+
     SkBitmap bitmap;
     bitmap.allocPixelsFlags(
-        SkImageInfo::MakeN32Premul(size.width(), size.height(),
+        SkImageInfo::MakeN32Premul(kSize.width(), kSize.height(),
                                    SkColorSpace::MakeSRGB()),
         SkBitmap::kZeroPixels_AllocFlag);
 
     SkCanvas canvas(bitmap, SkSurfaceProps{});
-    SkColor4f color{image_pq_pixel, image_pq_pixel, image_pq_pixel, 1.f};
+    SkColor4f color{kImagePixelValue, kImagePixelValue, kImagePixelValue, 1.f};
     canvas.drawColor(color);
 
     image = SkImages::RasterFromBitmap(bitmap);
     image = image->reinterpretColorSpace(
-        SkColorSpace::MakeRGB(pq, SkNamedGamut::kSRGB));
+        gfx::ColorSpace::CreateHDR10().ToSkColorSpace());
   }
 
   // Create a DisplayItemList drawing `image`.
@@ -669,48 +671,61 @@ TEST_F(OopPixelTest, DISABLED_DrawHdrImageWithMetadata) {
       PaintFlags::FilterQualityToSkSamplingOptions(kDefaultFilterQuality));
   display_item_list->push<DrawImageOp>(paint_image, 0.f, 0.f, sampling,
                                        nullptr);
-  display_item_list->EndPaintOfUnpaired(rect);
+  display_item_list->EndPaintOfUnpaired(kRect);
   display_item_list->Finalize();
-  RasterOptions options(rect.size());
+  RasterOptions options(kSize);
   {
-    options.target_color_params.color_space = gfx::ColorSpace::CreateSRGB();
+    options.target_color_params.color_space =
+        gfx::ColorSpace::CreateSRGBLinear();
     options.target_color_params.enable_tone_mapping = true;
-    options.target_color_params.sdr_max_luminance_nits = sdr_luminance;
-    options.target_color_params.hdr_metadata = gfx::HDRMetadata();
   }
 
-  // The exact value that `image_luminance` is mapped to may change as tone
-  // mapping is tweaked. When
-  constexpr float kCutoff = 0.95f;
-
-  // Draw using image HDR metadata indicating that `image_luminance` is the
+  // Draw using image HDR metadata indicating that `kImageNits` is the
   // maximum luminance. The result should map the image to solid white (up
   // to rounding error).
   {
-    options.target_color_params.hdr_metadata->smpte_st_2086 =
-        gfx::HdrMetadataSmpteSt2086(SkNamedPrimariesExt::kSRGB, image_luminance,
-                                    0.f);
+    constexpr float kContentMaxNits = kImageNits;
+    constexpr float kExpected = 1.0;
 
+    options.target_color_params.hdr_metadata = gfx::HDRMetadata(
+        gfx::HdrMetadataCta861_3(kContentMaxNits, kContentAvgNits));
     auto actual = Raster(display_item_list, options);
     auto color = actual.getColor4f(0, 0);
-    EXPECT_GT(color.fR, kCutoff);
-    EXPECT_GT(color.fG, kCutoff);
-    EXPECT_GT(color.fB, kCutoff);
+    EXPECT_NEAR(color.fR, kExpected, kEpsilon);
+    EXPECT_NEAR(color.fG, kExpected, kEpsilon);
+    EXPECT_NEAR(color.fB, kExpected, kEpsilon);
   }
 
   // Draw using image HDR metadata indicating that 10,000 nits is the maximum
   // luminance. The result should map the image to something darker than solid
   // white.
   {
-    options.target_color_params.hdr_metadata->smpte_st_2086 =
-        gfx::HdrMetadataSmpteSt2086(SkNamedPrimariesExt::kSRGB, kPQMaxLuminance,
-                                    0.f);
+    constexpr float kContentMaxNits = 10000;
+    constexpr float kExpected = 0.7114198123454021f;
+
+    options.target_color_params.hdr_metadata = gfx::HDRMetadata(
+        gfx::HdrMetadataCta861_3(kContentMaxNits, kContentAvgNits));
+    auto actual = Raster(display_item_list, options);
+    auto color = actual.getColor4f(0, 0);
+    EXPECT_NEAR(color.fR, kExpected, kEpsilon);
+    EXPECT_NEAR(color.fG, kExpected, kEpsilon);
+    EXPECT_NEAR(color.fB, kExpected, kEpsilon);
+  }
+
+  // Increase the destination HDR headroom. The result should now be brighter.
+  {
+    constexpr float kContentMaxNits = 10000;
+    constexpr float kExpected = 0.933675419515227f;
+    constexpr float kDstHeadroom = 1.5f;
 
+    options.target_color_params.hdr_max_luminance_relative = kDstHeadroom;
+    options.target_color_params.hdr_metadata = gfx::HDRMetadata(
+        gfx::HdrMetadataCta861_3(kContentMaxNits, kContentAvgNits));
     auto actual = Raster(display_item_list, options);
     auto color = actual.getColor4f(0, 0);
-    EXPECT_LT(color.fR, kCutoff);
-    EXPECT_LT(color.fG, kCutoff);
-    EXPECT_LT(color.fB, kCutoff);
+    EXPECT_NEAR(color.fR, kExpected, kEpsilon);
+    EXPECT_NEAR(color.fG, kExpected, kEpsilon);
+    EXPECT_NEAR(color.fB, kExpected, kEpsilon);
   }
 }
 

cc/paint/paint_op_reader.cc

@@ -43,6 +43,9 @@
 #include "third_party/skia/include/private/SkGainmapInfo.h"
 #include "third_party/skia/include/private/chromium/GrSlug.h"
 #include "third_party/skia/include/private/chromium/SkChromeRemoteGlyphCache.h"
+#include "ui/gfx/hdr_metadata.h"
+#include "ui/gfx/mojom/hdr_metadata.mojom.h"
+#include "ui/gfx/mojom/hdr_metadata_mojom_traits.h"
 
 namespace cc {
 namespace {
@@ -764,6 +767,23 @@ void PaintOpReader::Read(SkHighContrastConfig* config) {
            SkHighContrastConfig::InvertStyle::kLast>(&config->fInvertStyle);
   ReadSimple(&config->fContrast);
 }
+
+void PaintOpReader::Read(gfx::HDRMetadata* hdr_metadata) {
+  size_t size = 0;
+  ReadSize(&size);
+  if (remaining_bytes_ < size) {
+    valid_ = false;
+  }
+  if (!valid_ || size == 0) {
+    return;
+  }
+  uint8_t* scratch = CopyScratchSpace(size);
+  if (!gfx::mojom::HDRMetadata::Deserialize(scratch, size, hdr_metadata)) {
+    SetInvalid(DeserializationError::kHdrMetadataDeserializeFailure);
+  }
+  DidRead(size);
+}
+
 void PaintOpReader::Read(scoped_refptr<SkottieWrapper>* skottie) {
   if (!options_->is_privileged) {
     valid_ = false;

cc/paint/paint_op_reader.h

@@ -22,6 +22,10 @@ namespace gpu {
 struct Mailbox;
 }
 
+namespace gfx {
+struct HDRMetadata;
+}
+
 namespace cc {
 
 class PaintShader;
@@ -84,6 +88,7 @@ class CC_PAINT_EXPORT PaintOpReader {
   void Read(SkYUVAInfo::Subsampling* subsampling);
   void Read(gpu::Mailbox* mailbox);
   void Read(SkHighContrastConfig* config);
+  void Read(gfx::HDRMetadata* hdr_metadata);
 
   void Read(scoped_refptr<SkottieWrapper>* skottie);
 
@@ -196,8 +201,9 @@ class CC_PAINT_EXPORT PaintOpReader {
     kZeroSkColorFilterBytes = 52,
     kInsufficientPixelData = 53,
     kSkGainmapInfoDeserializationFailure = 54,
+    kHdrMetadataDeserializeFailure = 55,
 
-    kMaxValue = kSkGainmapInfoDeserializationFailure
+    kMaxValue = kHdrMetadataDeserializeFailure
   };
 
   template <typename T>

cc/paint/paint_op_writer.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <type_traits>
+#include <vector>
 
 #include "base/bits.h"
 #include "base/notreached.h"
@@ -42,6 +43,7 @@
 #include "third_party/skia/include/private/chromium/SkChromeRemoteGlyphCache.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/geometry/skia_conversions.h"
+#include "ui/gfx/mojom/hdr_metadata.mojom.h"
 
 namespace cc {
 namespace {
@@ -89,6 +91,12 @@ size_t PaintOpWriter::SerializedSize(const SkColorSpace* color_space) {
                                            : 0u);
 }
 
+// static
+size_t PaintOpWriter::SerializedSize(const gfx::HDRMetadata& hdr_metadata) {
+  return SerializedSizeOfBytes(
+      gfx::mojom::HDRMetadata::Serialize(&hdr_metadata).size());
+}
+
 // static
 size_t PaintOpWriter::SerializedSize(const PaintRecord& record) {
   // TODO(khushalsagar): Querying the size of a PaintRecord is not supported.
@@ -488,6 +496,13 @@ void PaintOpWriter::Write(const SkColorSpace* color_space) {
   DidWrite(written);
 }
 
+void PaintOpWriter::Write(const gfx::HDRMetadata& hdr_metadata) {
+  std::vector<uint8_t> bytes =
+      gfx::mojom::HDRMetadata::Serialize(&hdr_metadata);
+  WriteSize(bytes.size());
+  WriteData(bytes.size(), bytes.data());
+}
+
 void PaintOpWriter::Write(const SkGainmapInfo& gainmap_info) {
   Write(gainmap_info.fGainmapRatioMin);
   Write(gainmap_info.fGainmapRatioMax);

cc/paint/paint_op_writer.h

@@ -25,6 +25,10 @@ struct SkRect;
 struct SkIRect;
 class SkRRect;
 
+namespace gfx {
+struct HDRMetadata;
+}
+
 namespace gpu {
 struct Mailbox;
 }
@@ -131,6 +135,7 @@ class CC_PAINT_EXPORT PaintOpWriter {
   }
   static size_t SerializedSize(const SkFlattenable* flattenable);
   static size_t SerializedSize(const SkColorSpace* color_space);
+  static size_t SerializedSize(const gfx::HDRMetadata& hdr_metadata);
   static size_t SerializedSize(const ColorFilter* filter);
   static size_t SerializedSize(const PaintFilter* filter);
 
@@ -215,6 +220,7 @@ class CC_PAINT_EXPORT PaintOpWriter {
              const SkM44& current_ctm);
   void Write(const ColorFilter* filter);
   void Write(const PaintFilter* filter, const SkM44& current_ctm);
+  void Write(const gfx::HDRMetadata& hdr_metadata);
 
   void Write(SkClipOp op) { WriteEnum(op); }
   void Write(PaintCanvas::AnnotationType type) { WriteEnum(type); }

media/mojo/mojom/BUILD.gn

@@ -88,6 +88,7 @@ mojom("mojom") {
     "//services/service_manager/public/mojom",
     "//ui/gfx/geometry/mojom",
     "//ui/gfx/mojom",
+    "//ui/gfx/mojom:hdr_metadata",
     "//ui/gl/mojom",
     "//url/mojom:url_mojom_gurl",
     "//url/mojom:url_mojom_origin",

services/viz/public/mojom/BUILD.gn

@@ -60,6 +60,7 @@ mojom("mojom") {
     "//skia/public/mojom",
     "//ui/gfx/geometry/mojom",
     "//ui/gfx/mojom",
+    "//ui/gfx/mojom:hdr_metadata",
     "//ui/gl/mojom",
     "//ui/latency/mojom",
   ]