Make ICCProfile no longer store a ColorSpace directly

Make ICCProfile store the primaries and transfer function that it
read or computed directly, rather than storing a ColorSpace object.
Construct the ColorSpace object when requested, rather than storing it.

Bug: 766736
Change-Id: I3b0958545a1bfd889a9b88fb8689c6382d26c781
Reviewed-on: https://chromium-review.googlesource.com/752746
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fredrik Hubinette <hubbe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515084}

ui/gfx/icc_profile.cc

@@ -54,17 +54,6 @@ class ICCProfileCache {
     if (!icc_profile->id_)
       icc_profile->id_ = next_unused_id_++;
 
-    // Ensure that GetColorSpace() point back to this ICCProfile.
-    gfx::ColorSpace& color_space = icc_profile->color_space_;
-    color_space.icc_profile_id_ = icc_profile->id_;
-
-    // Ensure that the GetParametricColorSpace() point back to this ICCProfile
-    // only if the parametric version is accurate.
-    if (color_space.primaries_ != ColorSpace::PrimaryID::ICC_BASED &&
-        color_space.transfer_ != ColorSpace::TransferID::ICC_BASED) {
-      icc_profile->parametric_color_space_.icc_profile_id_ = icc_profile->id_;
-    }
-
     Entry entry;
     entry.icc_profile = *icc_profile;
     id_to_icc_profile_mru_.Put(icc_profile->id_, entry);
@@ -201,76 +190,59 @@ static base::LazyInstance<ICCProfileCache>::DestructorAtExit g_cache =
 
 }  // namespace
 
-// static
-ICCProfile::AnalyzeResult ICCProfile::ExtractColorSpaces(
-    const std::vector<char>& data,
-    gfx::ColorSpace* parametric_color_space,
-    float* parametric_tr_fn_max_error,
-    sk_sp<SkColorSpace>* useable_sk_color_space) {
-  // Initialize the output parameters as invalid.
-  *parametric_color_space = gfx::ColorSpace();
-  *parametric_tr_fn_max_error = 0;
-  *useable_sk_color_space = nullptr;
+ICCProfile::AnalyzeResult ICCProfile::Initialize() {
+  // Start out with no parametric data.
 
   // Parse the profile and attempt to create a SkColorSpaceXform out of it.
   sk_sp<SkColorSpace> sk_srgb_color_space = SkColorSpace::MakeSRGB();
-  sk_sp<SkICC> sk_icc = SkICC::Make(data.data(), data.size());
+  sk_sp<SkICC> sk_icc = SkICC::Make(data_.data(), data_.size());
   if (!sk_icc) {
     DLOG(ERROR) << "Failed to parse ICC profile to SkICC.";
     return kICCFailedToParse;
   }
-  sk_sp<SkColorSpace> sk_icc_color_space =
-      SkColorSpace::MakeICC(data.data(), data.size());
-  if (!sk_icc_color_space) {
+  sk_color_space_ = SkColorSpace::MakeICC(data_.data(), data_.size());
+  if (!sk_color_space_) {
     DLOG(ERROR) << "Failed to parse ICC profile to SkColorSpace.";
     return kICCFailedToExtractSkColorSpace;
   }
   std::unique_ptr<SkColorSpaceXform> sk_color_space_xform =
-      SkColorSpaceXform::New(sk_srgb_color_space.get(),
-                             sk_icc_color_space.get());
+      SkColorSpaceXform::New(sk_srgb_color_space.get(), sk_color_space_.get());
   if (!sk_color_space_xform) {
     DLOG(ERROR) << "Parsed ICC profile but can't create SkColorSpaceXform.";
     return kICCFailedToCreateXform;
   }
 
-  // Because this SkColorSpace can be used to construct a transform, mark it
-  // as "useable". Mark the "best approximation" as sRGB to start.
-  *useable_sk_color_space = sk_icc_color_space;
-  *parametric_color_space = ColorSpace::CreateSRGB();
+  // Because this SkColorSpace can be used to construct a transform, we can use
+  // it to create a LUT based color transform, at the very least. If we fail to
+  // get any better approximation, we'll use sRGB as our approximation.
+  ColorSpace::CreateSRGB().GetPrimaryMatrix(&to_XYZD50_);
+  ColorSpace::CreateSRGB().GetTransferFunction(&transfer_fn_);
 
   // If our SkColorSpace representation is sRGB then return that.
-  if (SkColorSpace::Equals(sk_srgb_color_space.get(),
-                           sk_icc_color_space.get())) {
+  if (sk_color_space_->isSRGB())
     return kICCExtractedSRGBColorSpace;
-  }
 
-  // A primary matrix is required for our parametric approximation.
+  // A primary matrix is required for our parametric representations. Use it if
+  // it exists.
   SkMatrix44 to_XYZD50_matrix;
   if (!sk_icc->toXYZD50(&to_XYZD50_matrix)) {
     DLOG(ERROR) << "Failed to extract ICC profile primary matrix.";
     return kICCFailedToExtractMatrix;
   }
+  to_XYZD50_ = to_XYZD50_matrix;
 
-  // Try to directly extract a numerical transfer function.
+  // Try to directly extract a numerical transfer function. Use it if it
+  // exists.
   SkColorSpaceTransferFn exact_tr_fn;
   if (sk_icc->isNumericalTransferFn(&exact_tr_fn)) {
-    *parametric_color_space =
-        gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, exact_tr_fn);
+    transfer_fn_ = exact_tr_fn;
     return kICCExtractedMatrixAndAnalyticTrFn;
   }
 
-  // If we fail to get a transfer function, use the sRGB transfer function,
-  // and return false to indicate that the gfx::ColorSpace isn't accurate, but
-  // we can construct accurate LUT transforms using the underlying
-  // SkColorSpace.
-  *parametric_color_space = gfx::ColorSpace::CreateCustom(
-      to_XYZD50_matrix, ColorSpace::TransferID::IEC61966_2_1);
-
   // Attempt to fit a parametric transfer function to the table data in the
   // profile.
   SkColorSpaceTransferFn approx_tr_fn;
-  if (!SkApproximateTransferFn(sk_icc, parametric_tr_fn_max_error,
-                               &approx_tr_fn)) {
+  if (!SkApproximateTransferFn(sk_icc, &transfer_fn_error_, &approx_tr_fn)) {
     DLOG(ERROR) << "Failed approximate transfer function.";
     return kICCFailedToConvergeToApproximateTrFn;
   }
@@ -278,16 +250,15 @@ ICCProfile::AnalyzeResult ICCProfile::ExtractColorSpaces(
   // If this converged, but has too high error, use the sRGB transfer function
   // from above.
   const float kMaxError = 2.f / 256.f;
-  if (*parametric_tr_fn_max_error >= kMaxError) {
+  if (transfer_fn_error_ >= kMaxError) {
     DLOG(ERROR) << "Failed to accurately approximate transfer function, error: "
-                << 256.f * (*parametric_tr_fn_max_error) << "/256";
+                << 256.f * transfer_fn_error_ << "/256";
     return kICCFailedToApproximateTrFnAccurately;
   };
 
   // If the error is sufficiently low, declare that the approximation is
   // accurate.
-  *parametric_color_space =
-      gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, approx_tr_fn);
+  transfer_fn_ = approx_tr_fn;
   return kICCExtractedMatrixAndApproximatedTrFn;
 }
 
@@ -306,18 +277,6 @@ bool ICCProfile::operator!=(const ICCProfile& other) const {
   return !(*this == other);
 }
 
-bool ICCProfile::IsValid() const {
-  switch (analyze_result_) {
-    case kICCFailedToParse:
-    case kICCFailedToExtractSkColorSpace:
-    case kICCFailedToCreateXform:
-      return false;
-    default:
-      break;
-  }
-  return true;
-}
-
 // static
 ICCProfile ICCProfile::FromData(const void* data, size_t size) {
   return FromDataWithId(data, size, 0);
@@ -346,14 +305,40 @@ const std::vector<char>& ICCProfile::GetData() const {
   return data_;
 }
 
-const ColorSpace& ICCProfile::GetColorSpace() const {
+ColorSpace ICCProfile::GetColorSpace() const {
   g_cache.Get().TouchEntry(*this);
-  return color_space_;
+
+  if (!is_valid_)
+    return ColorSpace();
+
+  gfx::ColorSpace color_space;
+  if (is_parametric_) {
+    color_space = GetParametricColorSpace();
+    color_space.icc_profile_sk_color_space_ = sk_color_space_;
+  } else {
+    color_space.matrix_ = ColorSpace::MatrixID::RGB;
+    color_space.range_ = ColorSpace::RangeID::FULL;
+    color_space.primaries_ = ColorSpace::PrimaryID::ICC_BASED;
+    color_space.transfer_ = ColorSpace::TransferID::ICC_BASED;
+    color_space.icc_profile_id_ = id_;
+    color_space.icc_profile_sk_color_space_ = sk_color_space_;
+  }
+  return color_space;
 }
 
-const ColorSpace& ICCProfile::GetParametricColorSpace() const {
+ColorSpace ICCProfile::GetParametricColorSpace() const {
   g_cache.Get().TouchEntry(*this);
-  return parametric_color_space_;
+
+  if (!is_valid_)
+    return ColorSpace();
+
+  ColorSpace color_space =
+      sk_color_space_->isSRGB()
+          ? ColorSpace::CreateSRGB()
+          : ColorSpace::CreateCustom(to_XYZD50_, transfer_fn_);
+  if (is_parametric_)
+    color_space.icc_profile_id_ = id_;
+  return color_space;
 }
 
 // static
@@ -376,32 +361,29 @@ void ICCProfile::ComputeColorSpaceAndCache() {
     return;
 
   // Parse the ICC profile
-  sk_sp<SkColorSpace> useable_sk_color_space;
-  analyze_result_ =
-      ExtractColorSpaces(data_, &parametric_color_space_,
-                         &parametric_tr_fn_error_, &useable_sk_color_space);
+  analyze_result_ = Initialize();
   switch (analyze_result_) {
     case kICCExtractedSRGBColorSpace:
     case kICCExtractedMatrixAndAnalyticTrFn:
     case kICCExtractedMatrixAndApproximatedTrFn:
       // Successfully and accurately extracted color space.
-      color_space_ = parametric_color_space_;
+      is_valid_ = true;
+      is_parametric_ = true;
       break;
     case kICCFailedToExtractRawTrFn:
     case kICCFailedToExtractMatrix:
     case kICCFailedToConvergeToApproximateTrFn:
     case kICCFailedToApproximateTrFnAccurately:
       // Successfully but extracted a color space, but it isn't accurate enough.
-      color_space_ = ColorSpace(ColorSpace::PrimaryID::ICC_BASED,
-                                ColorSpace::TransferID::ICC_BASED);
-      color_space_.icc_profile_sk_color_space_ = useable_sk_color_space;
+      is_valid_ = true;
+      is_parametric_ = false;
       break;
     case kICCFailedToParse:
     case kICCFailedToExtractSkColorSpace:
     case kICCFailedToCreateXform:
       // Can't even use this color space as a LUT.
-      DCHECK(!parametric_color_space_.IsValid());
-      color_space_ = parametric_color_space_;
+      is_valid_ = false;
+      is_parametric_ = false;
       break;
   }
 
@@ -429,7 +411,7 @@ void ICCProfile::HistogramDisplay(int64_t display_id) const {
   if (nonlinear_fit_converged) {
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "Blink.ColorSpace.Destination.NonlinearFitError",
-        static_cast<int>(parametric_tr_fn_error_ * 255), 0, 127, 16);
+        static_cast<int>(transfer_fn_error_ * 255), 0, 127, 16);
   }
 }
 

ui/gfx/icc_profile.h

@@ -41,7 +41,7 @@ class COLOR_SPACE_EXPORT ICCProfile {
   bool operator!=(const ICCProfile& other) const;
 
   // Returns true if this profile was successfully parsed by SkICC.
-  bool IsValid() const;
+  bool IsValid() const { return is_valid_; }
 
 #if defined(OS_MACOSX)
   static ICCProfile FromCGColorSpace(CGColorSpaceRef cg_color_space);
@@ -52,12 +52,12 @@ class COLOR_SPACE_EXPORT ICCProfile {
 
   // Return a ColorSpace that references this ICCProfile. ColorTransforms
   // created using this ColorSpace will match this ICCProfile precisely.
-  const ColorSpace& GetColorSpace() const;
+  ColorSpace GetColorSpace() const;
 
   // Return a ColorSpace that is the best parametric approximation of this
   // ICCProfile. The resulting ColorSpace will reference this ICCProfile only
   // if the parametric approximation is almost exact.
-  const ColorSpace& GetParametricColorSpace() const;
+  ColorSpace GetParametricColorSpace() const;
 
   const std::vector<char>& GetData() const;
 
@@ -103,12 +103,7 @@ class COLOR_SPACE_EXPORT ICCProfile {
                                    size_t size,
                                    uint64_t id);
 
-  static AnalyzeResult ExtractColorSpaces(
-      const std::vector<char>& data,
-      gfx::ColorSpace* parametric_color_space,
-      float* parametric_tr_fn_max_error,
-      sk_sp<SkColorSpace>* useable_sk_color_space);
-
+  AnalyzeResult Initialize();
   void ComputeColorSpaceAndCache();
 
   // This globally identifies this ICC profile. It is used to look up this ICC
@@ -120,18 +115,27 @@ class COLOR_SPACE_EXPORT ICCProfile {
   // The result of attepting to extract a color space from the color profile.
   AnalyzeResult analyze_result_ = kICCFailedToParse;
 
-  // |color_space| always links back to this ICC profile, and its SkColorSpace
-  // is always equal to the SkColorSpace created from this ICCProfile.
-  gfx::ColorSpace color_space_;
+  // True iff we can create a valid ColorSpace (and ColorTransform) from this
+  // object. The transform may be LUT-based (using an SkColorSpaceXform to
+  // compute the lut).
+  bool is_valid_ = false;
+
+  // True iff |to_XYZD50_| and |transfer_fn_| are accurate representations of
+  // the data in this profile. In this case ColorTransforms created from this
+  // profile will be analytic and not LUT-based.
+  bool is_parametric_ = false;
+
+  // Results of Skia parsing the ICC profile data.
+  sk_sp<SkColorSpace> sk_color_space_;
 
-  // |parametric_color_space_| will only link back to this ICC profile if it
-  // is accurate, and its SkColorSpace will always be parametrically created.
-  gfx::ColorSpace parametric_color_space_;
+  // The best-fit parametric primaries and transfer function.
+  SkMatrix44 to_XYZD50_;
+  SkColorSpaceTransferFn transfer_fn_;
 
   // The L-infinity error of the parametric color space fit. This is undefined
   // unless |analyze_result_| is kICCFailedToApproximateTrFnAccurately or
   // kICCExtractedMatrixAndApproximatedTrFn.
-  float parametric_tr_fn_error_ = -1;
+  float transfer_fn_error_ = 0;
 
   FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, BT709toSRGBICC);
   FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, GetColorSpace);