Revert "Make ICCProfile no longer store a ColorSpace directly"
This reverts commit ac3c37d52e71853c54921c488cc52b7a9ebfb2fb.
Reason for revert: crbug.com/780848, crbug.com/780415
Original change's description:
> 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.
>
> Store an approximate (or guessed) primary matrix and transfer function
> for ICC_BASED ColorSpace objects. When computing the parametric
> approximation of an ICC_BASED ColorSpace object, use these values
> directly, rather than having to look them up from original ICCProfile.
>
> Bug: 766736
> Change-Id: I21d6dfaa38706f814f6c9dd54517806e99366113
> Reviewed-on: https://chromium-review.googlesource.com/745311
> Reviewed-by: Fredrik Hubinette <[email protected]>
> Commit-Queue: ccameron <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#512958}
[email protected],[email protected]
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 766736
Change-Id: Ie36929800ab8f748cfc331b1a6ccda328c6559f8
Reviewed-on: https://chromium-review.googlesource.com/752061
Reviewed-by: ccameron <[email protected]>
Commit-Queue: ccameron <[email protected]>
Cr-Commit-Position: refs/heads/master@{#513624}diff --git a/ui/gfx/color_space.cc b/ui/gfx/color_space.cc
index ca60dae..14588f40 100644
--- a/ui/gfx/color_space.cc
+++ b/ui/gfx/color_space.cc
@@ -51,11 +51,11 @@
range_(other.range_),
icc_profile_id_(other.icc_profile_id_),
icc_profile_sk_color_space_(other.icc_profile_sk_color_space_) {
- if (transfer_ == TransferID::CUSTOM || transfer_ == TransferID::ICC_BASED) {
+ if (transfer_ == TransferID::CUSTOM) {
memcpy(custom_transfer_params_, other.custom_transfer_params_,
sizeof(custom_transfer_params_));
}
- if (primaries_ == PrimaryID::CUSTOM || primaries_ == PrimaryID::ICC_BASED) {
+ if (primaries_ == PrimaryID::CUSTOM) {
memcpy(custom_primary_matrix_, other.custom_primary_matrix_,
sizeof(custom_primary_matrix_));
}
@@ -96,18 +96,13 @@
ColorSpace result(ColorSpace::PrimaryID::CUSTOM,
ColorSpace::TransferID::CUSTOM, ColorSpace::MatrixID::RGB,
ColorSpace::RangeID::FULL);
- result.SetCustomPrimaries(to_XYZD50);
- result.SetCustomTransferFunction(fn);
- return result;
-}
-
-void ColorSpace::SetCustomPrimaries(const SkMatrix44& to_XYZD50) {
for (int row = 0; row < 3; ++row) {
for (int col = 0; col < 3; ++col) {
- custom_primary_matrix_[3 * row + col] = to_XYZD50.get(row, col);
+ result.custom_primary_matrix_[3 * row + col] = to_XYZD50.get(row, col);
}
}
- primaries_ = PrimaryID::CUSTOM;
+ result.SetCustomTransferFunction(fn);
+ return result;
}
void ColorSpace::SetCustomTransferFunction(const SkColorSpaceTransferFn& fn) {
@@ -173,13 +168,13 @@
if (primaries_ != other.primaries_ || transfer_ != other.transfer_ ||
matrix_ != other.matrix_ || range_ != other.range_)
return false;
- if (primaries_ == PrimaryID::CUSTOM || primaries_ == PrimaryID::ICC_BASED) {
+ if (primaries_ == PrimaryID::CUSTOM) {
if (memcmp(custom_primary_matrix_, other.custom_primary_matrix_,
sizeof(custom_primary_matrix_))) {
return false;
}
}
- if (transfer_ == TransferID::CUSTOM || transfer_ == TransferID::ICC_BASED) {
+ if (transfer_ == TransferID::CUSTOM) {
if (memcmp(custom_transfer_params_, other.custom_transfer_params_,
sizeof(custom_transfer_params_))) {
return false;
@@ -207,25 +202,27 @@
transfer_ == TransferID::IEC61966_2_4;
}
+bool ColorSpace::IsParametric() const {
+ return primaries_ != PrimaryID::ICC_BASED &&
+ transfer_ != TransferID::ICC_BASED;
+}
+
ColorSpace ColorSpace::GetParametricApproximation() const {
- ColorSpace result = *this;
- // The parametric approximation will not equal the original color space only
- // when the primaries or transfer function of the original color space are not
- // parametric (that is, they require the original ICC profile to represent
- // them).
- if (result.primaries_ == PrimaryID::ICC_BASED ||
- result.transfer_ == TransferID::ICC_BASED) {
- // Ensure that the result not reference the original ICC profile anymore,
- // because ICC profile represents a different space from the returned
- // approximation.
- result.icc_profile_id_ = 0;
- result.icc_profile_sk_color_space_ = nullptr;
- if (result.primaries_ == PrimaryID::ICC_BASED)
- result.primaries_ = PrimaryID::CUSTOM;
- if (result.transfer_ == TransferID::ICC_BASED)
- result.transfer_ = TransferID::CUSTOM;
+ // If this is parametric already, return it directly.
+ if (IsParametric())
+ return *this;
+
+ // Query the ICC profile, if available, for the parametric approximation.
+ ICCProfile icc_profile;
+ if (icc_profile_id_ && GetICCProfile(&icc_profile)) {
+ return icc_profile.GetParametricColorSpace();
+ } else {
+ DLOG(ERROR)
+ << "Unable to acquire ICC profile for parametric approximation.";
}
- return result;
+
+ // Fall back to sRGB if the ICC profile is no longer cached.
+ return CreateSRGB();
}
bool ColorSpace::operator!=(const ColorSpace& other) const {
@@ -249,7 +246,7 @@
return true;
if (range_ > other.range_)
return false;
- if (primaries_ == PrimaryID::CUSTOM || primaries_ == PrimaryID::ICC_BASED) {
+ if (primaries_ == PrimaryID::CUSTOM) {
int primary_result =
memcmp(custom_primary_matrix_, other.custom_primary_matrix_,
sizeof(custom_primary_matrix_));
@@ -258,7 +255,7 @@
if (primary_result > 0)
return false;
}
- if (transfer_ == TransferID::CUSTOM || transfer_ == TransferID::ICC_BASED) {
+ if (transfer_ == TransferID::CUSTOM) {
int transfer_result =
memcmp(custom_transfer_params_, other.custom_transfer_params_,
sizeof(custom_transfer_params_));
@@ -275,14 +272,14 @@
(static_cast<size_t>(transfer_) << 8) |
(static_cast<size_t>(matrix_) << 16) |
(static_cast<size_t>(range_) << 24);
- if (primaries_ == PrimaryID::CUSTOM || primaries_ == PrimaryID::ICC_BASED) {
+ if (primaries_ == PrimaryID::CUSTOM) {
const uint32_t* params =
reinterpret_cast<const uint32_t*>(custom_primary_matrix_);
result ^= params[0];
result ^= params[4];
result ^= params[8];
}
- if (transfer_ == TransferID::CUSTOM || transfer_ == TransferID::ICC_BASED) {
+ if (transfer_ == TransferID::CUSTOM) {
const uint32_t* params =
reinterpret_cast<const uint32_t*>(custom_transfer_params_);
result ^= params[3];
@@ -317,17 +314,16 @@
PRINT_ENUM_CASE(PrimaryID, APPLE_GENERIC_RGB)
PRINT_ENUM_CASE(PrimaryID, WIDE_GAMUT_COLOR_SPIN)
PRINT_ENUM_CASE(PrimaryID, ICC_BASED)
- PRINT_ENUM_CASE(PrimaryID, CUSTOM)
- }
- if (primaries_ == PrimaryID::ICC_BASED || primaries_ == PrimaryID::CUSTOM) {
- ss << ":[";
- for (size_t i = 0; i < 3; ++i) {
+ case PrimaryID::CUSTOM:
ss << "[";
- for (size_t j = 0; j < 3; ++j)
- ss << custom_primary_matrix_[3 * i + j] << ",";
- ss << "],";
- }
- ss << "]";
+ for (size_t i = 0; i < 3; ++i) {
+ ss << "[";
+ for (size_t j = 0; j < 3; ++j)
+ ss << custom_primary_matrix_[3 * i + j] << ",";
+ ss << "],";
+ }
+ ss << "]";
+ break;
}
ss << ", transfer:";
switch (transfer_) {
@@ -354,13 +350,13 @@
PRINT_ENUM_CASE(TransferID, IEC61966_2_1_HDR)
PRINT_ENUM_CASE(TransferID, LINEAR_HDR)
PRINT_ENUM_CASE(TransferID, ICC_BASED)
- PRINT_ENUM_CASE(TransferID, CUSTOM)
- }
- if (transfer_ == TransferID::ICC_BASED || transfer_ == TransferID::CUSTOM) {
- SkColorSpaceTransferFn fn;
- GetTransferFunction(&fn);
- ss << ":(" << fn.fC << "*x + " << fn.fF << " if x < " << fn.fD << " else (";
- ss << fn.fA << "*x + " << fn.fB << ")**" << fn.fG << " + " << fn.fE << ")";
+ case TransferID::CUSTOM: {
+ SkColorSpaceTransferFn fn;
+ GetTransferFunction(&fn);
+ ss << fn.fC << "*x + " << fn.fF << " if x < " << fn.fD << " else (";
+ ss << fn.fA << "*x + " << fn.fB << ")**" << fn.fG << " + " << fn.fE;
+ break;
+ }
}
ss << ", matrix:";
switch (matrix_) {
@@ -400,15 +396,15 @@
}
ColorSpace ColorSpace::GetRasterColorSpace() const {
+ // Rasterization can only be done into parametric color spaces.
+ if (!IsParametric())
+ return GetParametricApproximation();
// Rasterization doesn't support more than 8 bit unorm values. If the output
// space has an extended range, use Display P3 for the rasterization space,
// to get a somewhat wider color gamut.
if (HasExtendedSkTransferFn())
return CreateDisplayP3D65();
-
- // Rasterization can only be done into parametric color spaces. Return the
- // parametric approximation.
- return GetParametricApproximation();
+ return *this;
}
ColorSpace ColorSpace::GetBlendingColorSpace() const {
@@ -420,6 +416,11 @@
}
sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace() const {
+ // If we got a specific SkColorSpace from the ICCProfile that this color space
+ // was created from, use that.
+ if (icc_profile_sk_color_space_)
+ return icc_profile_sk_color_space_;
+
// Unspecified color spaces correspond to the null SkColorSpace.
if (!IsValid())
return nullptr;
@@ -434,11 +435,6 @@
return nullptr;
}
- // If we got a specific SkColorSpace from the ICCProfile that this color space
- // was created from, use that.
- if (icc_profile_sk_color_space_)
- return icc_profile_sk_color_space_;
-
// Use the named SRGB and linear-SRGB instead of the generic constructors.
if (primaries_ == PrimaryID::BT709) {
if (transfer_ == TransferID::IEC61966_2_1)
@@ -516,14 +512,10 @@
}
// If this was created from an ICC profile, retrieve that exact profile.
+ ICCProfile result;
if (ICCProfile::FromId(icc_profile_id_, icc_profile))
return true;
- if (primaries_ == PrimaryID::ICC_BASED ||
- transfer_ == TransferID::ICC_BASED) {
- return false;
- }
-
// Otherwise, construct an ICC profile based on the best approximated
// primaries and matrix.
SkMatrix44 to_XYZD50_matrix;
@@ -588,11 +580,11 @@
SkColorSpacePrimaries primaries = {0};
switch (primaries_) {
case ColorSpace::PrimaryID::CUSTOM:
- case ColorSpace::PrimaryID::ICC_BASED:
to_XYZD50->set3x3RowMajorf(custom_primary_matrix_);
return;
case ColorSpace::PrimaryID::INVALID:
+ case ColorSpace::PrimaryID::ICC_BASED:
to_XYZD50->setIdentity();
return;
@@ -759,7 +751,6 @@
switch (transfer_) {
case ColorSpace::TransferID::CUSTOM:
- case ColorSpace::TransferID::ICC_BASED:
fn->fA = custom_transfer_params_[0];
fn->fB = custom_transfer_params_[1];
fn->fC = custom_transfer_params_[2];
@@ -827,6 +818,7 @@
case ColorSpace::TransferID::SMPTEST2084:
case ColorSpace::TransferID::SMPTEST2084_NON_HDR:
case ColorSpace::TransferID::INVALID:
+ case ColorSpace::TransferID::ICC_BASED:
break;
}
diff --git a/ui/gfx/color_space.h b/ui/gfx/color_space.h
index a4757ef3..8a514de2 100644
--- a/ui/gfx/color_space.h
+++ b/ui/gfx/color_space.h
@@ -53,8 +53,7 @@
CUSTOM,
// For color spaces defined by an ICC profile which cannot be represented
// parametrically. Any ColorTransform using this color space will use the
- // ICC profile directly to compute a transform LUT. A parametric
- // approximation of these primaries is stored in |custom_primary_matrix_|.
+ // ICC profile directly to compute a transform LUT.
ICC_BASED,
LAST = ICC_BASED,
};
@@ -90,8 +89,7 @@
LINEAR_HDR,
// A parametric transfer function defined by |custom_transfer_params_|.
CUSTOM,
- // See PrimaryID::ICC_BASED. A parametric approximation of the transfer
- // function is stored in |custom_transfer_params_|.
+ // See PrimaryID::ICC_BASED.
ICC_BASED,
LAST = ICC_BASED,
};
@@ -173,6 +171,9 @@
// Returns true if the encoded values can be outside of the 0.0-1.0 range.
bool FullRangeEncodedValues() const;
+ // Returns true if this color space can be represented parametrically.
+ bool IsParametric() const;
+
// Return a parametric approximation of this color space (if it is not already
// parametric).
ColorSpace GetParametricApproximation() const;
@@ -207,7 +208,6 @@
void GetRangeAdjustMatrix(SkMatrix44* matrix) const;
private:
- void SetCustomPrimaries(const SkMatrix44& to_XYZD50);
void SetCustomTransferFunction(const SkColorSpaceTransferFn& fn);
// Returns true if the transfer function is defined by an
@@ -219,12 +219,12 @@
MatrixID matrix_ = MatrixID::INVALID;
RangeID range_ = RangeID::INVALID;
- // Only used if primaries_ is PrimaryID::CUSTOM or PrimaryID::ICC_BASED.
+ // Only used if primaries_ is PrimaryID::CUSTOM.
float custom_primary_matrix_[9] = {0, 0, 0, 0, 0, 0, 0, 0};
- // Only used if transfer_ is TransferID::CUSTOM or PrimaryID::ICC_BASED.
- // This array consists of the A through G entries of the
- // SkColorSpaceTransferFn structure in alphabetical order.
+ // Only used if transfer_ is TransferID::CUSTOM. This array consists of the A
+ // through G entries of the SkColorSpaceTransferFn structure in alphabetical
+ // order.
float custom_transfer_params_[7] = {0, 0, 0, 0, 0, 0, 0};
// This is used to look up the ICCProfile from which this ColorSpace was
diff --git a/ui/gfx/color_transform_unittest.cc b/ui/gfx/color_transform_unittest.cc
index ab9a4bbf..906aded 100644
--- a/ui/gfx/color_transform_unittest.cc
+++ b/ui/gfx/color_transform_unittest.cc
@@ -252,9 +252,8 @@
const float kEpsilon = 2.5f / 255.f;
ICCProfile icc_profile = ICCProfileForTestingNoAnalyticTrFn();
ColorSpace icc_space = icc_profile.GetColorSpace();
- ColorSpace colorspin = ICCProfileForTestingColorSpin()
- .GetColorSpace()
- .GetParametricApproximation();
+ ColorSpace colorspin =
+ ICCProfileForTestingColorSpin().GetParametricColorSpace();
ColorTransform::TriStim input_value(0.25f, 0.5f, 0.75f);
ColorTransform::TriStim transformed_value = input_value;
diff --git a/ui/gfx/icc_profile.cc b/ui/gfx/icc_profile.cc
index f26a8181..75fdec5 100644
--- a/ui/gfx/icc_profile.cc
+++ b/ui/gfx/icc_profile.cc
@@ -54,6 +54,17 @@
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);
@@ -190,69 +201,76 @@
} // namespace
-ICCProfile::AnalyzeResult ICCProfile::Initialize() {
- // Start out with no parametric data.
- primaries_ = gfx::ColorSpace::PrimaryID::INVALID;
- transfer_ = gfx::ColorSpace::TransferID::INVALID;
+// 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;
// 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_color_space_ = SkColorSpace::MakeICC(data_.data(), data_.size());
- if (!sk_color_space_) {
+ sk_sp<SkColorSpace> sk_icc_color_space =
+ SkColorSpace::MakeICC(data.data(), data.size());
+ if (!sk_icc_color_space) {
DLOG(ERROR) << "Failed to parse ICC profile to SkColorSpace.";
return kICCFailedToExtractSkColorSpace;
}
-
- // If our SkColorSpace representation is sRGB then return that.
- if (SkColorSpace::Equals(sk_srgb_color_space.get(), sk_color_space_.get())) {
- primaries_ = gfx::ColorSpace::PrimaryID::BT709;
- transfer_ = gfx::ColorSpace::TransferID::IEC61966_2_1;
- return kICCExtractedSRGBColorSpace;
- }
-
std::unique_ptr<SkColorSpaceXform> sk_color_space_xform =
- SkColorSpaceXform::New(sk_srgb_color_space.get(), sk_color_space_.get());
+ SkColorSpaceXform::New(sk_srgb_color_space.get(),
+ sk_icc_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, 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.
- primaries_ = ColorSpace::PrimaryID::ICC_BASED;
- transfer_ = ColorSpace::TransferID::ICC_BASED;
- ColorSpace::CreateSRGB().GetPrimaryMatrix(&to_XYZD50_);
- ColorSpace::CreateSRGB().GetTransferFunction(&transfer_fn_);
+ // 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();
- // A primary matrix is required for our parametric representations. Use it if
- // it exists.
+ // If our SkColorSpace representation is sRGB then return that.
+ if (SkColorSpace::Equals(sk_srgb_color_space.get(),
+ sk_icc_color_space.get())) {
+ return kICCExtractedSRGBColorSpace;
+ }
+
+ // A primary matrix is required for our parametric approximation.
SkMatrix44 to_XYZD50_matrix;
if (!sk_icc->toXYZD50(&to_XYZD50_matrix)) {
DLOG(ERROR) << "Failed to extract ICC profile primary matrix.";
return kICCFailedToExtractMatrix;
}
- primaries_ = ColorSpace::PrimaryID::CUSTOM;
- to_XYZD50_ = to_XYZD50_matrix;
- // Try to directly extract a numerical transfer function. Use it if it
- // exists.
+ // Try to directly extract a numerical transfer function.
SkColorSpaceTransferFn exact_tr_fn;
if (sk_icc->isNumericalTransferFn(&exact_tr_fn)) {
- transfer_ = ColorSpace::TransferID::CUSTOM;
- transfer_fn_ = exact_tr_fn;
+ *parametric_color_space =
+ gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, 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, &transfer_fn_error_, &approx_tr_fn)) {
+ if (!SkApproximateTransferFn(sk_icc, parametric_tr_fn_max_error,
+ &approx_tr_fn)) {
DLOG(ERROR) << "Failed approximate transfer function.";
return kICCFailedToConvergeToApproximateTrFn;
}
@@ -260,16 +278,16 @@
// If this converged, but has too high error, use the sRGB transfer function
// from above.
const float kMaxError = 2.f / 256.f;
- if (transfer_fn_error_ >= kMaxError) {
+ if (*parametric_tr_fn_max_error >= kMaxError) {
DLOG(ERROR) << "Failed to accurately approximate transfer function, error: "
- << 256.f * transfer_fn_error_ << "/256";
+ << 256.f * (*parametric_tr_fn_max_error) << "/256";
return kICCFailedToApproximateTrFnAccurately;
};
// If the error is sufficiently low, declare that the approximation is
// accurate.
- transfer_ = ColorSpace::TransferID::CUSTOM;
- transfer_fn_ = approx_tr_fn;
+ *parametric_color_space =
+ gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, approx_tr_fn);
return kICCExtractedMatrixAndApproximatedTrFn;
}
@@ -328,28 +346,14 @@
return data_;
}
-ColorSpace ICCProfile::GetColorSpace() const {
+const ColorSpace& ICCProfile::GetColorSpace() const {
g_cache.Get().TouchEntry(*this);
+ return color_space_;
+}
- ColorSpace color_space;
- if (!IsValid())
- return color_space;
- color_space.icc_profile_id_ = id_;
- color_space.icc_profile_sk_color_space_ = sk_color_space_;
- DCHECK(color_space.icc_profile_sk_color_space_);
- color_space.matrix_ = ColorSpace::MatrixID::RGB;
- color_space.range_ = ColorSpace::RangeID::FULL;
- if (primaries_ == ColorSpace::PrimaryID::CUSTOM ||
- primaries_ == ColorSpace::PrimaryID::ICC_BASED) {
- color_space.SetCustomPrimaries(to_XYZD50_);
- }
- color_space.primaries_ = primaries_;
- if (transfer_ == ColorSpace::TransferID::CUSTOM ||
- transfer_ == ColorSpace::TransferID::ICC_BASED) {
- color_space.SetCustomTransferFunction(transfer_fn_);
- }
- color_space.transfer_ = transfer_;
- return color_space;
+const ColorSpace& ICCProfile::GetParametricColorSpace() const {
+ g_cache.Get().TouchEntry(*this);
+ return parametric_color_space_;
}
// static
@@ -372,7 +376,34 @@
return;
// Parse the ICC profile
- analyze_result_ = Initialize();
+ sk_sp<SkColorSpace> useable_sk_color_space;
+ analyze_result_ =
+ ExtractColorSpaces(data_, ¶metric_color_space_,
+ ¶metric_tr_fn_error_, &useable_sk_color_space);
+ switch (analyze_result_) {
+ case kICCExtractedSRGBColorSpace:
+ case kICCExtractedMatrixAndAnalyticTrFn:
+ case kICCExtractedMatrixAndApproximatedTrFn:
+ // Successfully and accurately extracted color space.
+ color_space_ = parametric_color_space_;
+ 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;
+ 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_;
+ break;
+ }
// Add to the cache.
g_cache.Get().InsertAndSetIdIfNeeded(this);
@@ -398,7 +429,7 @@
if (nonlinear_fit_converged) {
UMA_HISTOGRAM_CUSTOM_COUNTS(
"Blink.ColorSpace.Destination.NonlinearFitError",
- static_cast<int>(transfer_fn_error_ * 255), 0, 127, 16);
+ static_cast<int>(parametric_tr_fn_error_ * 255), 0, 127, 16);
}
}
diff --git a/ui/gfx/icc_profile.h b/ui/gfx/icc_profile.h
index 3c00e76..bfbdc4e 100644
--- a/ui/gfx/icc_profile.h
+++ b/ui/gfx/icc_profile.h
@@ -52,7 +52,12 @@
// Return a ColorSpace that references this ICCProfile. ColorTransforms
// created using this ColorSpace will match this ICCProfile precisely.
- ColorSpace GetColorSpace() const;
+ 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;
const std::vector<char>& GetData() const;
@@ -98,7 +103,12 @@
size_t size,
uint64_t id);
- AnalyzeResult Initialize();
+ 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);
+
void ComputeColorSpaceAndCache();
// This globally identifies this ICC profile. It is used to look up this ICC
@@ -110,21 +120,18 @@
// The result of attepting to extract a color space from the color profile.
AnalyzeResult analyze_result_ = kICCFailedToParse;
- // Results of Skia parsing the ICC profile data.
- sk_sp<SkColorSpace> sk_color_space_;
+ // |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_;
- // The best-fit parametric primaries.
- gfx::ColorSpace::PrimaryID primaries_;
- SkMatrix44 to_XYZD50_;
-
- // The best-fit parametric transfer function.
- gfx::ColorSpace::TransferID transfer_;
- SkColorSpaceTransferFn transfer_fn_;
+ // |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 L-infinity error of the parametric color space fit. This is undefined
// unless |analyze_result_| is kICCFailedToApproximateTrFnAccurately or
// kICCExtractedMatrixAndApproximatedTrFn.
- float transfer_fn_error_ = 0;
+ float parametric_tr_fn_error_ = -1;
FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, BT709toSRGBICC);
FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, GetColorSpace);
diff --git a/ui/gfx/icc_profile_unittest.cc b/ui/gfx/icc_profile_unittest.cc
index 186fceb..d9a008d 100644
--- a/ui/gfx/icc_profile_unittest.cc
+++ b/ui/gfx/icc_profile_unittest.cc
@@ -32,12 +32,8 @@
EXPECT_EQ(icc_profile.GetColorSpace().ToSkColorSpace().get(),
sk_color_space.get());
// The parametric generating code should recognize that this is SRGB.
- EXPECT_EQ(icc_profile.GetColorSpace().GetParametricApproximation(),
- ColorSpace::CreateSRGB());
- EXPECT_EQ(icc_profile.GetColorSpace()
- .GetParametricApproximation()
- .ToSkColorSpace()
- .get(),
+ EXPECT_EQ(icc_profile.GetParametricColorSpace(), ColorSpace::CreateSRGB());
+ EXPECT_EQ(icc_profile.GetParametricColorSpace().ToSkColorSpace().get(),
sk_color_space.get());
// The generated color space should recognize that this is SRGB.
EXPECT_EQ(color_space.ToSkColorSpace().get(), sk_color_space.get());
@@ -82,8 +78,7 @@
// This ICC profile has three transfer functions that differ enough that the
// parametric color space is considered inaccurate.
ICCProfile multi_tr_fn = ICCProfileForTestingNoAnalyticTrFn();
- EXPECT_NE(multi_tr_fn.GetColorSpace(),
- multi_tr_fn.GetColorSpace().GetParametricApproximation());
+ EXPECT_NE(multi_tr_fn.GetColorSpace(), multi_tr_fn.GetParametricColorSpace());
ICCProfile multi_tr_fn_color_space;
EXPECT_TRUE(
@@ -91,40 +86,35 @@
EXPECT_EQ(multi_tr_fn_color_space, multi_tr_fn);
ICCProfile multi_tr_fn_parametric_color_space;
- EXPECT_TRUE(
- multi_tr_fn.GetColorSpace().GetParametricApproximation().GetICCProfile(
- &multi_tr_fn_parametric_color_space));
+ EXPECT_TRUE(multi_tr_fn.GetParametricColorSpace().GetICCProfile(
+ &multi_tr_fn_parametric_color_space));
EXPECT_NE(multi_tr_fn_parametric_color_space, multi_tr_fn);
// This ICC profile has a transfer function with T(1) that is greater than 1
// in the approximation, but is still close enough to be considered accurate.
ICCProfile overshoot = ICCProfileForTestingOvershoot();
- EXPECT_EQ(overshoot.GetColorSpace(),
- overshoot.GetColorSpace().GetParametricApproximation());
+ EXPECT_EQ(overshoot.GetColorSpace(), overshoot.GetParametricColorSpace());
ICCProfile overshoot_color_space;
EXPECT_TRUE(overshoot.GetColorSpace().GetICCProfile(&overshoot_color_space));
EXPECT_EQ(overshoot_color_space, overshoot);
ICCProfile overshoot_parametric_color_space;
- EXPECT_TRUE(
- overshoot.GetColorSpace().GetParametricApproximation().GetICCProfile(
- &overshoot_parametric_color_space));
+ EXPECT_TRUE(overshoot.GetParametricColorSpace().GetICCProfile(
+ &overshoot_parametric_color_space));
EXPECT_EQ(overshoot_parametric_color_space, overshoot);
// This ICC profile is precisely represented by the parametric color space.
ICCProfile accurate = ICCProfileForTestingAdobeRGB();
- EXPECT_EQ(accurate.GetColorSpace(),
- accurate.GetColorSpace().GetParametricApproximation());
+ EXPECT_EQ(accurate.GetColorSpace(), accurate.GetParametricColorSpace());
ICCProfile accurate_color_space;
EXPECT_TRUE(accurate.GetColorSpace().GetICCProfile(&accurate_color_space));
EXPECT_EQ(accurate_color_space, accurate);
ICCProfile accurate_parametric_color_space;
- EXPECT_TRUE(
- accurate.GetColorSpace().GetParametricApproximation().GetICCProfile(
- &accurate_parametric_color_space));
+ EXPECT_TRUE(accurate.GetParametricColorSpace().GetICCProfile(
+ &accurate_parametric_color_space));
EXPECT_EQ(accurate_parametric_color_space, accurate);
// This ICC profile has only an A2B representation. We cannot create an
@@ -146,15 +136,12 @@
ICCProfile::FromData(bad_data.data(), bad_data.size());
EXPECT_FALSE(garbage_profile.IsValid());
EXPECT_FALSE(garbage_profile.GetColorSpace().IsValid());
- EXPECT_FALSE(
- garbage_profile.GetColorSpace().GetParametricApproximation().IsValid());
+ EXPECT_FALSE(garbage_profile.GetParametricColorSpace().IsValid());
ICCProfile default_ctor_profile;
EXPECT_FALSE(default_ctor_profile.IsValid());
EXPECT_FALSE(default_ctor_profile.GetColorSpace().IsValid());
- EXPECT_FALSE(default_ctor_profile.GetColorSpace()
- .GetParametricApproximation()
- .IsValid());
+ EXPECT_FALSE(default_ctor_profile.GetParametricColorSpace().IsValid());
}
TEST(ICCProfile, GenericRGB) {
