Detect all named gammas
authormsarett <msarett@google.com>
Mon, 22 Aug 2016 16:44:35 +0000 (09:44 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 22 Aug 2016 16:44:35 +0000 (09:44 -0700)
Our DstColorSpace UMA is showing some named gammas that are
not appropriately detected and placed in named categories.
https://uma.googleplex.com/p/chrome/histograms?endDate=latest&dayCount=1&histograms=Blink.ColorSpace.Destination&fixupData=true&showMax=true&filters=isofficial%2Ceq%2CTrue&implicitFilters=isofficial

This CL should fix that.

I'm not sure (yet) how I feel about this landing permanently.
Seems a little messy.

But it will be interesting to see how this affects the UMA.
My best guess is that we are hitting this case when all
three gammas are "invalid" in different ways.  I'm expecting
to see some profiles end up in the "invalid" category now.

It's also possible that we'll see these cases being absorbed
into sRGB or somewhere else.

BUG=skia:5656
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2261213002

Review-Url: https://codereview.chromium.org/2261213002

src/core/SkColorSpace_ICC.cpp

index e28a746..9ab1da0 100755 (executable)
@@ -716,6 +716,18 @@ static bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
     return true;
 }
 
+static inline SkColorSpace::GammaNamed is_named(const sk_sp<SkGammas>& gammas) {
+    if (gammas->isNamed(0) && gammas->isNamed(1) && gammas->isNamed(2) &&
+        gammas->fRedData.fNamed == gammas->fGreenData.fNamed &&
+        gammas->fRedData.fNamed == gammas->fBlueData.fNamed)
+    {
+        return gammas->fRedData.fNamed;
+    }
+
+    return SkColorSpace::kNonStandard_GammaNamed;
+}
+
+
 static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT, SkColorSpace::GammaNamed* gammaNamed,
                       sk_sp<SkGammas>* gammas, SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
     if (len < 32) {
@@ -849,6 +861,14 @@ static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT, SkColorSpace::GammaNa
         *gammaNamed = SkColorSpace::kInvalid_GammaNamed;
     }
 
+    if (SkColorSpace::kNonStandard_GammaNamed == *gammaNamed) {
+        *gammaNamed = is_named(*gammas);
+        if (SkColorSpace::kNonStandard_GammaNamed != *gammaNamed) {
+            // No need to keep the gammas struct, the enum is enough.
+            *gammas = nullptr;
+        }
+    }
+
     uint32_t offsetToMatrix = read_big_endian_i32(src + 16);
     if (0 != offsetToMatrix && offsetToMatrix < len) {
         if (!load_matrix(toXYZ, src + offsetToMatrix, len - offsetToMatrix)) {
@@ -1042,12 +1062,17 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
                 }
 
                 if (kNonStandard_GammaNamed == gammaNamed) {
-                    return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, gammaNamed,
-                                                                     std::move(gammas), mat,
-                                                                     std::move(data)));
-                } else {
-                    return SkColorSpace_Base::NewRGB(gammaNamed, mat);
+                    // It's possible that we'll initially detect non-matching gammas, only for
+                    // them to evaluate to the same named gamma curve.
+                    gammaNamed = is_named(gammas);
+                    if (kNonStandard_GammaNamed == gammaNamed) {
+                        return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, gammaNamed,
+                                                                         std::move(gammas), mat,
+                                                                         std::move(data)));
+                    }
                 }
+
+                return SkColorSpace_Base::NewRGB(gammaNamed, mat);
             }
 
             // Recognize color profile specified by A2B0 tag.
@@ -1066,9 +1091,9 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
                     return sk_sp<SkColorSpace>(new SkColorSpace_Base(std::move(colorLUT),
                                                                      gammaNamed, std::move(gammas),
                                                                      toXYZ, std::move(data)));
-                } else {
-                    return SkColorSpace_Base::NewRGB(gammaNamed, toXYZ);
                 }
+
+                return SkColorSpace_Base::NewRGB(gammaNamed, toXYZ);
             }
         }
         default: