color: Always use parametric color spaces for raster

ui/gfx/icc_profile.cc

Index: ui/gfx/icc_profile.cc
diff --git a/ui/gfx/icc_profile.cc b/ui/gfx/icc_profile.cc
index de11f29926017565950c7de9501b5b1fd3ba35ad..93904daea571ca31209492a721f341e5e4585186 100644
--- a/ui/gfx/icc_profile.cc
+++ b/ui/gfx/icc_profile.cc
@@ -11,6 +11,7 @@
 #include "base/synchronization/lock.h"
 #include "third_party/skia/include/core/SkICC.h"
 #include "ui/gfx/color_transform.h"
+#include "ui/gfx/skia_color_space_util.h"
 
 namespace gfx {
 
@@ -19,10 +20,9 @@ const uint64_t ICCProfile::test_id_color_spin_ = 2;
 const uint64_t ICCProfile::test_id_generic_rgb_ = 3;
 const uint64_t ICCProfile::test_id_srgb_ = 4;
 const uint64_t ICCProfile::test_id_no_analytic_tr_fn_ = 5;
+const uint64_t ICCProfile::test_id_a2b_only_ = 6;
 
 namespace {
-const size_t kMinProfileLength = 128;
-const size_t kMaxProfileLength = 4 * 1024 * 1024;
 
 // Allow keeping around a maximum of 8 cached ICC profiles. Beware that
 // we will do a linear search thorugh currently-cached ICC profiles,
@@ -71,9 +71,8 @@ ICCProfile ICCProfile::FromData(const void* data, size_t size) {
 ICCProfile ICCProfile::FromDataWithId(const void* data,
                                       size_t size,
                                       uint64_t new_profile_id) {
-  if (!IsValidProfileLength(size)) {
-    if (size != 0)
-      DLOG(ERROR) << "Invalid ICC profile length: " << size << ".";
+  if (!size) {
+    DLOG(ERROR) << "Invalid empty ICC profile.";
     return ICCProfile();
   }
 
@@ -129,9 +128,21 @@ const ColorSpace& ICCProfile::GetColorSpace() const {
   return color_space_;
 }
 
+const ColorSpace& ICCProfile::GetParametricColorSpace() const {
+  // Move this ICC profile to the most recently used end of the cache,
+  // inserting if needed.
+  if (id_) {
+    Cache& cache = g_cache.Get();
+    base::AutoLock lock(cache.lock);
+    auto found = cache.id_to_icc_profile_mru.Get(id_);
+    if (found == cache.id_to_icc_profile_mru.end())
+      found = cache.id_to_icc_profile_mru.Put(id_, *this);
+  }
+  return parametric_color_space_;
+}
+
 // static
 bool ICCProfile::FromId(uint64_t id,
-                        bool only_if_needed,
                         ICCProfile* icc_profile) {
   if (!id)
     return false;
@@ -143,11 +154,7 @@ bool ICCProfile::FromId(uint64_t id,
   if (found == cache.id_to_icc_profile_mru.end())
     return false;
 
-  const ICCProfile& found_icc_profile = found->second;
-  if (found_icc_profile.color_space_is_accurate_ && only_if_needed)
-    return false;
-
-  *icc_profile = found_icc_profile;
+  *icc_profile = found->second;
   return true;
 }
 
@@ -162,47 +169,69 @@ void ICCProfile::ComputeColorSpaceAndCache() {
     auto found = cache.id_to_icc_profile_mru.Get(id_);
     if (found != cache.id_to_icc_profile_mru.end()) {
       color_space_ = found->second.color_space_;
+      parametric_color_space_ = found->second.parametric_color_space_;
       successfully_parsed_by_sk_icc_ =
           found->second.successfully_parsed_by_sk_icc_;
       return;
     }
   }
 
-  color_space_is_accurate_ = true;
-  SkMatrix44 to_XYZD50_matrix;
-  SkColorSpaceTransferFn fn;
   sk_sp<SkICC> sk_icc = SkICC::Make(data_.data(), data_.size());
   if (sk_icc) {
+    bool parametric_color_space_is_accurate = false;
     successfully_parsed_by_sk_icc_ = true;
-    if (!sk_icc->toXYZD50(&to_XYZD50_matrix)) {
-      // Just say that the primaries were the sRGB primaries if we can't
-      // extract them.
-      gfx::ColorSpace::CreateSRGB().GetPrimaryMatrix(&to_XYZD50_matrix);
-      color_space_is_accurate_ = false;
-      DLOG(ERROR) << "Unable to handle ICCProfile primaries.";
+
+    // Populate |parametric_color_space_| as a primary matrix and analytic
+    // transfer function, if possible.
+    SkMatrix44 to_XYZD50_matrix;
+    if (sk_icc->toXYZD50(&to_XYZD50_matrix)) {
+      SkColorSpaceTransferFn fn;
+      // First try to get a numerical transfer function from the profile.
+      if (sk_icc->isNumericalTransferFn(&fn)) {
+        parametric_color_space_is_accurate = true;
+      } else {
+        // If that fails, try to approximate the transfer function.
+        float fn_max_error = 0;
+        bool got_approximate_fn =
+            SkApproximateTransferFn(sk_icc, &fn_max_error, &fn);
+        if (got_approximate_fn) {
+          float kMaxError = 3.f / 256.f;
+          if (fn_max_error < kMaxError) {
+            parametric_color_space_is_accurate = true;
+          } else {
+            DLOG(ERROR) << "ICCProfile transfer function approximation "
+                        << "inexact, error: " << 256.f * fn_max_error << "/256";
+          }
+        } else {
+          // And if that fails, just say that the transfer function was sRGB.
+          DLOG(ERROR) << "Failed to approximate ICCProfile transfer function.";
+          gfx::ColorSpace::CreateSRGB().GetTransferFunction(&fn);
+        }
+      }
+      parametric_color_space_ =
+          gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, fn);
+    } else {
+      DLOG(ERROR) << "Failed to extract ICCProfile primary matrix.";
+      // TODO(ccameron): Get an approximate gamut for rasterization.
+      parametric_color_space_ = gfx::ColorSpace::CreateSRGB();
     }
-    if (!sk_icc->isNumericalTransferFn(&fn)) {
-      // Just say that the transfer function was sRGB if we cannot read it.
-      // TODO(ccameron): Use a least squares approximation of the transfer
-      // function when it is not numerical.
-      gfx::ColorSpace::CreateSRGB().GetTransferFunction(&fn);
-      color_space_is_accurate_ = false;
-      DLOG(ERROR) << "Unable to handle ICCProfile transfer function.";
+
+    // Populate |color_space_|.

[hubbe, 2017/03/14 18:26:46]

Unhelpful comment.
Either remove or say something more useful.

[ccameron, 2017/03/14 21:41:02]

Good point. Done.

+    if (parametric_color_space_is_accurate) {
+      parametric_color_space_.icc_profile_id_ = id_;
+      color_space_ = parametric_color_space_;
+    } else {
+      color_space_ = ColorSpace(ColorSpace::PrimaryID::ICC_BASED,
+                                ColorSpace::TransferID::ICC_BASED);
+      color_space_.icc_profile_id_ = id_;
+      color_space_.icc_profile_sk_color_space_ =
+          SkColorSpace::MakeICC(data_.data(), data_.size());
     }
   } else {
-    successfully_parsed_by_sk_icc_ = false;
-    gfx::ColorSpace::CreateSRGB().GetPrimaryMatrix(&to_XYZD50_matrix);
-    gfx::ColorSpace::CreateSRGB().GetTransferFunction(&fn);
-    color_space_is_accurate_ = false;
     DLOG(ERROR) << "Unable parse ICCProfile.";
+    successfully_parsed_by_sk_icc_ = false;
   }
 
-  // Compute the color space.
-  color_space_ = gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, fn);
-  color_space_.icc_profile_id_ = id_;
-  color_space_.icc_profile_sk_color_space_ =
-      SkColorSpace::MakeICC(data_.data(), data_.size());
-
   // Add to the cache.
   {
     Cache& cache = g_cache.Get();
@@ -211,9 +240,4 @@ void ICCProfile::ComputeColorSpaceAndCache() {
   }
 }
 
-// static
-bool ICCProfile::IsValidProfileLength(size_t length) {
-  return length >= kMinProfileLength && length <= kMaxProfileLength;
-}
-
 }  // namespace gfx