Implement SkColorSpace struct traits in skia.mojom.ImageInfo.

Completes the skia mojom struct traits implementation for ImageInfo by
using Skia's built-in SkColorSpace::serialize() functionality. This
allows for exact SkColorSpaces to be efficiently transmitted, alongside
things like SkBitmaps, through mojo message pipes.

Later work to improve color space management in Chromium will depend on
this change. (See crbugs for examples.)

Bug: 758057, 809385
Change-Id: I1193f81c727d8663370fd6fd802edd9dd397abff
Reviewed-on: https://chromium-review.googlesource.com/c/1357633Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615611}

services/viz/public/cpp/compositing/struct_traits_unittest.cc

@@ -238,6 +238,8 @@ TEST_F(StructTraitsTest, CopyOutputRequest_BitmapRequest) {
           [](const base::Closure& quit_closure, const gfx::Rect& expected_rect,
              std::unique_ptr<CopyOutputResult> result) {
             EXPECT_EQ(expected_rect, result->rect());
+            // Note: CopyOutputResult plumbing for bitmap requests is tested in
+            // StructTraitsTest.CopyOutputResult_Bitmap.
             quit_closure.Run();
           },
           run_loop.QuitClosure(), result_rect)));
@@ -264,7 +266,8 @@ TEST_F(StructTraitsTest, CopyOutputRequest_BitmapRequest) {
   EXPECT_EQ(result_rect, output->result_selection());
 
   SkBitmap bitmap;
-  bitmap.allocN32Pixels(result_rect.width(), result_rect.height());
+  bitmap.allocPixels(SkImageInfo::MakeN32Premul(
+      result_rect.width(), result_rect.height(), SkColorSpace::MakeSRGB()));
   output->SendResult(
       std::make_unique<CopyOutputSkBitmapResult>(result_rect, bitmap));
   // If the CopyOutputRequest callback is called, this ends. Otherwise, the test
@@ -312,6 +315,8 @@ TEST_F(StructTraitsTest, CopyOutputRequest_TextureRequest) {
           [](const base::Closure& quit_closure, const gfx::Rect& expected_rect,
              std::unique_ptr<CopyOutputResult> result) {
             EXPECT_EQ(expected_rect, result->rect());
+            // Note: CopyOutputResult plumbing for texture requests is tested in
+            // StructTraitsTest.CopyOutputResult_Texture.
             quit_closure.Run();
           },
           run_loop_for_result.QuitClosure(), result_rect)));
@@ -327,7 +332,7 @@ TEST_F(StructTraitsTest, CopyOutputRequest_TextureRequest) {
 
   base::RunLoop run_loop_for_release;
   output->SendResult(std::make_unique<CopyOutputTextureResult>(
-      result_rect, mailbox, sync_token, gfx::ColorSpace(),
+      result_rect, mailbox, sync_token, gfx::ColorSpace::CreateSRGB(),
       SingleReleaseCallback::Create(base::Bind(
           [](const base::Closure& quit_closure,
              const gpu::SyncToken& expected_sync_token,
@@ -1193,7 +1198,7 @@ TEST_F(StructTraitsTest, CopyOutputResult_Bitmap) {
   SkBitmap bitmap;
   const sk_sp<SkColorSpace> adobe_rgb = SkColorSpace::MakeRGB(
       SkColorSpace::kSRGB_RenderTargetGamma, SkColorSpace::kAdobeRGB_Gamut);
-  bitmap.allocN32Pixels(7, 8, adobe_rgb != nullptr);
+  bitmap.allocPixels(SkImageInfo::MakeN32Premul(7, 8, adobe_rgb));
   bitmap.eraseARGB(123, 213, 77, 33);
   std::unique_ptr<CopyOutputResult> input =
       std::make_unique<CopyOutputSkBitmapResult>(result_rect, bitmap);
@@ -1214,7 +1219,7 @@ TEST_F(StructTraitsTest, CopyOutputResult_Bitmap) {
   // Check that the pixels are the same as the input and the color spaces are
   // equivalent.
   SkBitmap expected_bitmap;
-  expected_bitmap.allocN32Pixels(7, 8, adobe_rgb != nullptr);
+  expected_bitmap.allocPixels(SkImageInfo::MakeN32Premul(7, 8, adobe_rgb));
   expected_bitmap.eraseARGB(123, 213, 77, 33);
   EXPECT_EQ(expected_bitmap.computeByteSize(), out_bitmap.computeByteSize());
   EXPECT_EQ(0, std::memcmp(expected_bitmap.getPixels(), out_bitmap.getPixels(),

skia/public/interfaces/image_info.mojom

@@ -24,17 +24,10 @@ enum AlphaType {
   UNPREMUL,
 };
 
-// Capture of the (simple) gamma of SkColorSpace. Need to eventually
-// serialize the actual colorspace object.
-enum ColorProfileType {
-  LINEAR,
-  SRGB,
-};
-
 struct ImageInfo {
   ColorType color_type;
   AlphaType alpha_type;
-  ColorProfileType profile_type;
+  array<uint8> serialized_color_space;  // Empty means "null" SkColorSpace.
   uint32 width;
   uint32 height;
 };

skia/public/interfaces/image_info_struct_traits.cc

@@ -47,17 +47,6 @@ SkAlphaType MojoAlphaTypeToSk(skia::mojom::AlphaType type) {
   return kUnknown_SkAlphaType;
 }
 
-sk_sp<SkColorSpace> MojoProfileTypeToSk(skia::mojom::ColorProfileType type) {
-  switch (type) {
-    case skia::mojom::ColorProfileType::LINEAR:
-      return nullptr;
-    case skia::mojom::ColorProfileType::SRGB:
-      return SkColorSpace::MakeSRGB();
-  }
-  NOTREACHED();
-  return nullptr;
-}
-
 skia::mojom::ColorType SkColorTypeToMojo(SkColorType type) {
   switch (type) {
     case kUnknown_SkColorType:
@@ -97,12 +86,6 @@ skia::mojom::AlphaType SkAlphaTypeToMojo(SkAlphaType type) {
   return skia::mojom::AlphaType::UNKNOWN;
 }
 
-skia::mojom::ColorProfileType SkColorSpaceToMojo(SkColorSpace* cs) {
-  if (cs && cs->gammaCloseToSRGB())
-    return skia::mojom::ColorProfileType::SRGB;
-  return skia::mojom::ColorProfileType::LINEAR;
-}
-
 }  // namespace
 
 // static
@@ -120,10 +103,26 @@ StructTraits<skia::mojom::ImageInfoDataView, SkImageInfo>::alpha_type(
 }
 
 // static
-skia::mojom::ColorProfileType
-StructTraits<skia::mojom::ImageInfoDataView, SkImageInfo>::profile_type(
-    const SkImageInfo& info) {
-  return SkColorSpaceToMojo(info.colorSpace());
+std::vector<uint8_t>
+StructTraits<skia::mojom::ImageInfoDataView,
+             SkImageInfo>::serialized_color_space(const SkImageInfo& info) {
+  std::vector<uint8_t> serialized_color_space;
+  if (auto* sk_color_space = info.colorSpace()) {
+    serialized_color_space.resize(sk_color_space->writeToMemory(nullptr));
+    // Assumption 1: Since a "null" SkColorSpace is represented as an empty byte
+    // array, the serialization of a non-null SkColorSpace should produce at
+    // least one byte.
+    CHECK_GT(serialized_color_space.size(), 0u);
+    // Assumption 2: Serialized data should be reasonably small, since
+    // SkImageInfo should efficiently pass through mojo message pipes. As of
+    // this writing, the max would be 80 bytes. However, that could change in
+    // the future. So, set an upper-bound of 1 KB here.
+    CHECK_LE(serialized_color_space.size(), 1024u);
+    sk_color_space->writeToMemory(serialized_color_space.data());
+  } else {
+    // Represent the "null" color space as an empty byte vector.
+  }
+  return serialized_color_space;
 }
 
 // static
@@ -142,10 +141,20 @@ uint32_t StructTraits<skia::mojom::ImageInfoDataView, SkImageInfo>::height(
 bool StructTraits<skia::mojom::ImageInfoDataView, SkImageInfo>::Read(
     skia::mojom::ImageInfoDataView data,
     SkImageInfo* info) {
-  *info = SkImageInfo::Make(data.width(), data.height(),
-                            MojoColorTypeToSk(data.color_type()),
-                            MojoAlphaTypeToSk(data.alpha_type()),
-                            MojoProfileTypeToSk(data.profile_type()));
+  mojo::ArrayDataView<uint8_t> serialized_color_space;
+  data.GetSerializedColorSpaceDataView(&serialized_color_space);
+  sk_sp<SkColorSpace> sk_color_space;
+  if (serialized_color_space.size() != 0u) {
+    sk_color_space = SkColorSpace::Deserialize(serialized_color_space.data(),
+                                               serialized_color_space.size());
+    CHECK(sk_color_space);  // Deserialize() returns nullptr on invalid input.
+  } else {
+    // Empty byte array is interpreted as "null."
+  }
+
+  *info = SkImageInfo::Make(
+      data.width(), data.height(), MojoColorTypeToSk(data.color_type()),
+      MojoAlphaTypeToSk(data.alpha_type()), std::move(sk_color_space));
   return true;
 }
 

skia/public/interfaces/image_info_struct_traits.h

@@ -5,6 +5,8 @@
 #ifndef SKIA_PUBLIC_INTERFACES_IMAGE_INFO_STRUCT_TRAITS_H_
 #define SKIA_PUBLIC_INTERFACES_IMAGE_INFO_STRUCT_TRAITS_H_
 
+#include <vector>
+
 #include "skia/public/interfaces/image_info.mojom.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 
@@ -14,7 +16,7 @@ template <>
 struct StructTraits<skia::mojom::ImageInfoDataView, SkImageInfo> {
   static skia::mojom::ColorType color_type(const SkImageInfo& info);
   static skia::mojom::AlphaType alpha_type(const SkImageInfo& info);
-  static skia::mojom::ColorProfileType profile_type(const SkImageInfo& info);
+  static std::vector<uint8_t> serialized_color_space(const SkImageInfo& info);
   static uint32_t width(const SkImageInfo& info);
   static uint32_t height(const SkImageInfo& info);
   static bool Read(skia::mojom::ImageInfoDataView data, SkImageInfo* info);

skia/public/interfaces/test/OWNERS

+per-file struct_traits_unittest.cc=file://ipc/SECURITY_OWNERS
+
 per-file *.mojom=set noparent
 per-file *.mojom=file://ipc/SECURITY_OWNERS

skia/public/interfaces/test/struct_traits_unittest.cc

@@ -52,36 +52,39 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
   DISALLOW_COPY_AND_ASSIGN(StructTraitsTest);
 };
 
-static bool colorspace_srgb_gamma(SkColorSpace* cs) {
-  return cs && cs->gammaCloseToSRGB();
-}
-
 }  // namespace
 
 TEST_F(StructTraitsTest, ImageInfo) {
   SkImageInfo input = SkImageInfo::Make(
       34, 56, SkColorType::kGray_8_SkColorType,
-      SkAlphaType::kUnpremul_SkAlphaType, SkColorSpace::MakeSRGB());
+      SkAlphaType::kUnpremul_SkAlphaType,
+      SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
+                            SkColorSpace::kAdobeRGB_Gamut));
   mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy();
   SkImageInfo output;
   proxy->EchoImageInfo(input, &output);
   EXPECT_EQ(input, output);
+
+  SkImageInfo another_input_with_null_color_space =
+      SkImageInfo::Make(54, 43, SkColorType::kRGBA_8888_SkColorType,
+                        SkAlphaType::kPremul_SkAlphaType, nullptr);
+  proxy->EchoImageInfo(another_input_with_null_color_space, &output);
+  EXPECT_FALSE(output.colorSpace());
+  EXPECT_EQ(another_input_with_null_color_space, output);
 }
 
 TEST_F(StructTraitsTest, Bitmap) {
   SkBitmap input;
-  input.allocN32Pixels(10, 5);
+  input.allocPixels(SkImageInfo::MakeN32Premul(
+      10, 5,
+      SkColorSpace::MakeRGB(SkColorSpace::kLinear_RenderTargetGamma,
+                            SkColorSpace::kRec2020_Gamut)));
   input.eraseColor(SK_ColorYELLOW);
   input.erase(SK_ColorTRANSPARENT, SkIRect::MakeXYWH(0, 1, 2, 3));
   mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy();
   SkBitmap output;
   proxy->EchoBitmap(input, &output);
-  EXPECT_EQ(input.colorType(), output.colorType());
-  EXPECT_EQ(input.alphaType(), output.alphaType());
-  EXPECT_EQ(colorspace_srgb_gamma(input.colorSpace()),
-            colorspace_srgb_gamma(output.colorSpace()));
-  EXPECT_EQ(input.width(), output.width());
-  EXPECT_EQ(input.height(), output.height());
+  EXPECT_EQ(input.info(), output.info());
   EXPECT_EQ(input.rowBytes(), output.rowBytes());
   EXPECT_TRUE(gfx::BitmapsAreEqual(input, output));
 }
@@ -89,7 +92,8 @@ TEST_F(StructTraitsTest, Bitmap) {
 TEST_F(StructTraitsTest, BitmapWithExtraRowBytes) {
   SkBitmap input;
   // Ensure traits work with bitmaps containing additional bytes between rows.
-  SkImageInfo info = SkImageInfo::MakeN32(8, 5, kPremul_SkAlphaType);
+  SkImageInfo info =
+      SkImageInfo::MakeN32(8, 5, kPremul_SkAlphaType, SkColorSpace::MakeSRGB());
   // Any extra bytes on each row must be a multiple of the row's pixel size to
   // keep every row's pixels aligned.
   size_t extra = info.bytesPerPixel();
@@ -99,12 +103,7 @@ TEST_F(StructTraitsTest, BitmapWithExtraRowBytes) {
   mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy();
   SkBitmap output;
   proxy->EchoBitmap(input, &output);
-  EXPECT_EQ(input.colorType(), output.colorType());
-  EXPECT_EQ(input.alphaType(), output.alphaType());
-  EXPECT_EQ(colorspace_srgb_gamma(input.colorSpace()),
-            colorspace_srgb_gamma(output.colorSpace()));
-  EXPECT_EQ(input.width(), output.width());
-  EXPECT_EQ(input.height(), output.height());
+  EXPECT_EQ(input.info(), output.info());
   EXPECT_EQ(input.rowBytes(), output.rowBytes());
   EXPECT_TRUE(gfx::BitmapsAreEqual(input, output));
 }