Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(483)

Unified Diff: ui/gfx/icc_profile.cc

Issue 2742613002: color: Always use parametric color spaces for raster (Closed)
Patch Set: Incorporate review feedback Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ui/gfx/icc_profile.h ('k') | ui/gfx/icc_profile_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/gfx/icc_profile.cc
diff --git a/ui/gfx/icc_profile.cc b/ui/gfx/icc_profile.cc
index de11f29926017565950c7de9501b5b1fd3ba35ad..d2ddfef45f3a6a04c5c27791f32ca0f27f937850 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,71 @@ 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.";
+
+ // If the approximation is accurate, then set |parametric_color_space_| and
+ // |color_space_| to the same value, and link them to |this|. Otherwise, set
+ // them separately, and do not link |parametric_color_space_| to |this|.
+ 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 +242,4 @@ void ICCProfile::ComputeColorSpaceAndCache() {
}
}
-// static
-bool ICCProfile::IsValidProfileLength(size_t length) {
- return length >= kMinProfileLength && length <= kMaxProfileLength;
-}
-
} // namespace gfx
« no previous file with comments | « ui/gfx/icc_profile.h ('k') | ui/gfx/icc_profile_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698