Ensure raster color space contains sRGB

Color artifacts can occur if the color space used for rasterization does
not contain the full range of sRGB values. In this case, default to
using sRGB for rasterization.

Bug: 1073962
Change-Id: Id8cbc0b5e917cf6160f8b34b46f231427144a442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438196
Commit-Queue: Sean Gilhuly <sgilhuly@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814367}

cc/BUILD.gn

@@ -813,6 +813,7 @@ cc_test("cc_unittests") {
     "//media",
     "//mojo/core/embedder",
     "//mojo/public/cpp/bindings",
+    "//skia:skcms",
     "//testing/gmock",
     "//testing/gtest",
     "//ui/events:events_base",

cc/trees/layer_tree_host_impl.cc

@@ -1793,6 +1793,11 @@ gfx::ColorSpace LayerTreeHostImpl::GetRasterColorSpace(
   if (result.IsHDR() && content_color_usage != gfx::ContentColorUsage::kHDR)
     return gfx::ColorSpace::CreateDisplayP3D65();
 
+  // The raster color space should contain sRGB to avoid artifacts during
+  // rasterization.
+  if (!result.Contains(srgb))
+    return srgb;
+
   return result;
 }
 

cc/trees/layer_tree_host_pixeltest_tiles.cc

@@ -48,6 +48,18 @@ class LayerTreeHostTilesPixelTest
     target->RequestCopyOfOutput(CreateCopyOutputRequest());
   }
 
+  void WillPrepareTilesOnThread(LayerTreeHostImpl* host_impl) override {
+    // Issue a GL finish before preparing tiles to ensure resources become
+    // available for use in a timely manner. Needed for the one-copy path.
+    viz::RasterContextProvider* context_provider =
+        host_impl->layer_tree_frame_sink()->worker_context_provider();
+    if (!context_provider)
+      return;
+
+    viz::RasterContextProvider::ScopedRasterContextLock lock(context_provider);
+    lock.RasterInterface()->Finish();
+  }
+
   base::FilePath ref_file_;
   std::unique_ptr<SkBitmap> result_bitmap_;
   bool use_partial_raster_ = false;
@@ -130,21 +142,82 @@ class LayerTreeHostTilesTestPartialInvalidation
     }
   }
 
-  void WillPrepareTilesOnThread(LayerTreeHostImpl* host_impl) override {
-    // Issue a GL finish before preparing tiles to ensure resources become
-    // available for use in a timely manner. Needed for the one-copy path.
-    viz::RasterContextProvider* context_provider =
-        host_impl->layer_tree_frame_sink()->worker_context_provider();
-    if (!context_provider)
-      return;
+ protected:
+  BlueYellowClient client_;
+  scoped_refptr<PictureLayer> picture_layer_;
+};
 
-    viz::RasterContextProvider::ScopedRasterContextLock lock(context_provider);
-    lock.RasterInterface()->Finish();
+class PrimaryColorClient : public ContentLayerClient {
+ public:
+  explicit PrimaryColorClient(const gfx::Size& size) : size_(size) {}
+
+  gfx::Rect PaintableRegion() override { return gfx::Rect(size_); }
+  scoped_refptr<DisplayItemList> PaintContentsToDisplayList() override {
+    // When painted, the DisplayItemList should produce blocks of red, green,
+    // and blue to test primary color reproduction.
+    auto display_list = base::MakeRefCounted<DisplayItemList>();
+
+    display_list->StartPaint();
+
+    int w = size_.width() / 3;
+    gfx::Rect red_rect(0, 0, w, size_.height());
+    gfx::Rect green_rect(w, 0, w, size_.height());
+    gfx::Rect blue_rect(w * 2, 0, w, size_.height());
+
+    PaintFlags flags;
+    flags.setStyle(PaintFlags::kFill_Style);
+
+    flags.setColor(SK_ColorRED);
+    display_list->push<DrawRectOp>(gfx::RectToSkRect(red_rect), flags);
+    flags.setColor(SK_ColorGREEN);
+    display_list->push<DrawRectOp>(gfx::RectToSkRect(green_rect), flags);
+    flags.setColor(SK_ColorBLUE);
+    display_list->push<DrawRectOp>(gfx::RectToSkRect(blue_rect), flags);
+
+    display_list->EndPaintOfUnpaired(PaintableRegion());
+    display_list->Finalize();
+    return display_list;
+  }
+
+  bool FillsBoundsCompletely() const override { return true; }
+  size_t GetApproximateUnsharedMemoryUsage() const override { return 0; }
+
+ private:
+  gfx::Size size_;
+};
+
+class LayerTreeHostTilesTestRasterColorSpace
+    : public LayerTreeHostTilesPixelTest {
+ public:
+  LayerTreeHostTilesTestRasterColorSpace()
+      : client_(gfx::Size(150, 150)),
+        picture_layer_(PictureLayer::Create(&client_)) {
+    picture_layer_->SetBounds(gfx::Size(150, 150));
+    picture_layer_->SetIsDrawable(true);
+  }
+
+  void SetColorSpace(const gfx::ColorSpace& color_space) {
+    color_space_ = color_space;
+  }
+
+  void WillBeginTest() override {
+    LayerTreeHostTilesPixelTest::WillBeginTest();
+    DCHECK(color_space_.IsValid());
+    layer_tree_host()->SetDisplayColorSpaces(
+        gfx::DisplayColorSpaces(color_space_));
+  }
+
+  void DidCommitAndDrawFrame() override {
+    if (layer_tree_host()->SourceFrameNumber() == 1) {
+      Finish();
+      DoReadback();
+    }
   }
 
  protected:
-  BlueYellowClient client_;
+  PrimaryColorClient client_;
   scoped_refptr<PictureLayer> picture_layer_;
+  gfx::ColorSpace color_space_;
 };
 
 std::vector<RasterTestConfig> const kTestCases = {
@@ -236,6 +309,49 @@ TEST_P(LayerTreeHostTilesTestPartialInvalidationMultiThread, FullRaster) {
                base::FilePath(FILE_PATH_LITERAL("blue_yellow_flipped.png")));
 }
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         LayerTreeHostTilesTestRasterColorSpace,
+                         ::testing::ValuesIn(kTestCases),
+                         ::testing::PrintToStringParamName());
+
+// These tests verify that no artifacts are introduced when the color space for
+// rasterization doesn't contain the primary colors of sRGB.
+// See crbug.com/1073962 for more details.
+TEST_P(LayerTreeHostTilesTestRasterColorSpace, sRGB) {
+  SetColorSpace(gfx::ColorSpace::CreateSRGB());
+
+  RunPixelTest(picture_layer_,
+               base::FilePath(FILE_PATH_LITERAL("primary_colors.png")));
+}
+
+TEST_P(LayerTreeHostTilesTestRasterColorSpace, GenericRGB) {
+  SetColorSpace(gfx::ColorSpace(gfx::ColorSpace::PrimaryID::APPLE_GENERIC_RGB,
+                                gfx::ColorSpace::TransferID::GAMMA18));
+
+  RunPixelTest(picture_layer_,
+               base::FilePath(FILE_PATH_LITERAL("primary_colors.png")));
+}
+
+TEST_P(LayerTreeHostTilesTestRasterColorSpace, CustomColorSpace) {
+  // Create a color space with a different blue point.
+  SkColorSpacePrimaries primaries;
+  skcms_Matrix3x3 to_XYZD50;
+  primaries.fRX = 0.640f;
+  primaries.fRY = 0.330f;
+  primaries.fGX = 0.300f;
+  primaries.fGY = 0.600f;
+  primaries.fBX = 0.130f;
+  primaries.fBY = 0.080f;
+  primaries.fWX = 0.3127f;
+  primaries.fWY = 0.3290f;
+  primaries.toXYZD50(&to_XYZD50);
+  SetColorSpace(gfx::ColorSpace::CreateCustom(
+      to_XYZD50, gfx::ColorSpace::TransferID::IEC61966_2_1));
+
+  RunPixelTest(picture_layer_,
+               base::FilePath(FILE_PATH_LITERAL("primary_colors.png")));
+}
+
 // This test doesn't work on Vulkan because on our hardware we can't render to
 // RGBA4444 format using either SwiftShader or native Vulkan. See
 // crbug.com/987278 for details

ui/gfx/color_space.cc

@@ -69,6 +69,25 @@ float GetSDRWhiteLevelFromPQSkTransferFunction(
   return sdr_white_level_a;
 }
 
+bool PrimaryIdContainsSRGB(ColorSpace::PrimaryID id) {
+  DCHECK(id != ColorSpace::PrimaryID::INVALID &&
+         id != ColorSpace::PrimaryID::CUSTOM);
+
+  switch (id) {
+    case ColorSpace::PrimaryID::BT709:
+    case ColorSpace::PrimaryID::BT2020:
+    case ColorSpace::PrimaryID::SMPTEST428_1:
+    case ColorSpace::PrimaryID::SMPTEST431_2:
+    case ColorSpace::PrimaryID::SMPTEST432_1:
+    case ColorSpace::PrimaryID::XYZ_D50:
+    case ColorSpace::PrimaryID::ADOBE_RGB:
+    case ColorSpace::PrimaryID::WIDE_GAMUT_COLOR_SPIN:
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 // static
@@ -702,6 +721,39 @@ bool ColorSpace::HasExtendedSkTransferFn() const {
          transfer_ == TransferID::IEC61966_2_1_HDR;
 }
 
+bool ColorSpace::Contains(const ColorSpace& other) const {
+  if (primaries_ == PrimaryID::INVALID ||
+      other.primaries_ == PrimaryID::INVALID)
+    return false;
+
+  // Contains() is commonly used to check if a color space contains sRGB. The
+  // computation can be bypassed for known primary IDs.
+  if (primaries_ != PrimaryID::CUSTOM && other.primaries_ == PrimaryID::BT709)
+    return PrimaryIdContainsSRGB(primaries_);
+
+  // |matrix| is the primary transform matrix from |other| to this color space.
+  skcms_Matrix3x3 other_to_xyz;
+  skcms_Matrix3x3 this_to_xyz;
+  skcms_Matrix3x3 xyz_to_this;
+  other.GetPrimaryMatrix(&other_to_xyz);
+  GetPrimaryMatrix(&this_to_xyz);
+  skcms_Matrix3x3_invert(&this_to_xyz, &xyz_to_this);
+  skcms_Matrix3x3 matrix = skcms_Matrix3x3_concat(&xyz_to_this, &other_to_xyz);
+
+  // Return true iff each primary is in the range [0, 1] after transforming.
+  // Transforming a primary vector by |matrix| always results in a column of
+  // |matrix|. So the multiplication can be skipped, and we can just check if
+  // each value in the matrix is in the range [0, 1].
+  constexpr float epsilon = 0.001f;
+  for (int r = 0; r < 3; r++) {
+    for (int c = 0; c < 3; c++) {
+      if (matrix.vals[r][c] < -epsilon || matrix.vals[r][c] > 1 + epsilon)
+        return false;
+    }
+  }
+  return true;
+}
+
 // static
 void ColorSpace::GetPrimaryMatrix(PrimaryID primary_id,
                                   skcms_Matrix3x3* to_XYZD50) {

ui/gfx/color_space.h

@@ -378,6 +378,9 @@ class COLOR_SPACE_EXPORT ColorSpace {
   // skcms_TransferFunction which is extended to all real values.
   bool HasExtendedSkTransferFn() const;
 
+  // Returns true if each color in |other| can be expressed in this color space.
+  bool Contains(const ColorSpace& other) const;
+
  private:
   // The default bit depth assumed by GetRangeAdjustMatrix().
   static constexpr int kDefaultBitDepth = 8;

ui/gfx/color_space_unittest.cc

@@ -362,5 +362,38 @@ TEST(ColorSpace, LinearHDRWhiteLevel) {
             std::make_tuple(1.f, kCustomSlope, 0.f, 0.f, 0.f, 0.f, 0.f));
 }
 
+TEST(ColorSpace, ExpectationsMatchSRGB) {
+  ColorSpace::PrimaryID primary_ids[] = {
+      ColorSpace::PrimaryID::BT709,
+      ColorSpace::PrimaryID::BT470M,
+      ColorSpace::PrimaryID::BT470BG,
+      ColorSpace::PrimaryID::SMPTE170M,
+      ColorSpace::PrimaryID::SMPTE240M,
+      ColorSpace::PrimaryID::FILM,
+      ColorSpace::PrimaryID::BT2020,
+      ColorSpace::PrimaryID::SMPTEST428_1,
+      ColorSpace::PrimaryID::SMPTEST431_2,
+      ColorSpace::PrimaryID::SMPTEST432_1,
+      ColorSpace::PrimaryID::XYZ_D50,
+      ColorSpace::PrimaryID::ADOBE_RGB,
+      ColorSpace::PrimaryID::APPLE_GENERIC_RGB,
+      ColorSpace::PrimaryID::WIDE_GAMUT_COLOR_SPIN,
+  };
+
+  // Create a custom color space with the sRGB primary matrix.
+  ColorSpace srgb = ColorSpace::CreateSRGB();
+  skcms_Matrix3x3 to_XYZD50;
+  srgb.GetPrimaryMatrix(&to_XYZD50);
+  ColorSpace custom_srgb =
+      ColorSpace::CreateCustom(to_XYZD50, ColorSpace::TransferID::IEC61966_2_1);
+
+  for (auto id : primary_ids) {
+    ColorSpace color_space(id, ColorSpace::TransferID::IEC61966_2_1);
+    // The precomputed results for Contains(sRGB) should match the calculation
+    // performed on a custom color space with sRGB primaries.
+    EXPECT_EQ(color_space.Contains(srgb), color_space.Contains(custom_srgb));
+  }
+}
+
 }  // namespace
 }  // namespace gfx