Add UMAs for numerical approximation of table-based transfer functions.

third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp

Index: third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp b/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
index 54c4afb4c80e9349d99d459a68b7e3b1c2ef8735..c8276632bec50fd240745f39541c4b84da466a67 100644
--- a/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
@@ -8,6 +8,7 @@
 #include "platform/graphics/BitmapImageMetrics.h"
 #include "third_party/skia/include/core/SkICC.h"
 #include "ui/gfx/icc_profile.h"
+#include "ui/gfx/skia_color_space_util.h"
 #include "wtf/SpinLock.h"
 
 namespace blink {
@@ -47,6 +48,70 @@ void ColorBehavior::setGlobalTargetColorProfile(
       bool isNumericalTransferFnResult = skICC->isNumericalTransferFn(&fn);
       UMA_HISTOGRAM_BOOLEAN("Blink.ColorSpace.Destination.Numerical",
                             isNumericalTransferFnResult);
+
+      // Analyze the numerical approximation of table-based transfer functions.
+      if (!isNumericalTransferFnResult) {
+        SkICC::Tables tables;
+        bool rawTransferFnResult = skICC->rawTransferFnData(&tables);
+        UMA_HISTOGRAM_BOOLEAN("Blink.ColorSpace.Destination.RawData",
+                              rawTransferFnResult);
+        if (rawTransferFnResult) {
+          SkICC::Channel* channels[3] = {&tables.fRed, &tables.fGreen,
+                                         &tables.fBlue};
+          for (size_t c = 0; c < 3; ++c) {
+            SkICC::Channel* channel = channels[c];
+            UMA_HISTOGRAM_CUSTOM_COUNTS(
+                "Blink.ColorSpace.Destination.TableSize", channel->fCount, 0,
+                1023, 32);

[Ilya Sherman, 2017/02/22 22:48:48]

Just to double-check: Are you taking into account that buckets are exponentially
sized?  It looks like you might want linearly-spaced buckets, which
UMA_HISTOGRAM_CUSTOM_COUNTS will not provide (but UMA_HISTOGRAM_LINEAR will).

[ccameron, 2017/02/23 00:47:33]

Oh, I didn't know that. Thanks!

+            DCHECK_GE(channel->fCount, 2);
+            const float* data = reinterpret_cast<const float*>(
+                tables.fStorage->bytes() + channel->fOffset);
+            std::vector<float> x;
+            std::vector<float> t;
+            float tMin = data[0];
+            float tMax = data[0];
+            for (int i = 0; i < channel->fCount; ++i) {
+              float xi = i / (channel->fCount - 1.f);
+              float ti = data[i];
+              x.push_back(xi);
+              t.push_back(ti);
+              tMin = std::min(tMin, ti);
+              tMax = std::max(tMax, ti);
+            }
+            // Record the range of the table-based transfer function. If it is
+            // found that almost all ranges are from 0 to 1, then we will bake
+            // the assumption that the range is 0 to 1 into the numerical
+            // approximation code (which will improve stability).
+            UMA_HISTOGRAM_CUSTOM_COUNTS("Blink.ColorSpace.Destination.tMin",
+                                        static_cast<int>(tMin * 255), 0, 255,
+                                        256);

[Ilya Sherman, 2017/02/22 22:48:48]

Please don't pre-allocate space for 256 buckets -- across many histograms, that
adds up to significant allocated-but-unused memory.  Instead, you could use a
sparse histogram (UMA_HISTOGRAM_SPARSE_SLOWLY), with custom clamping code. 
(Actually, if you want to go the extra mile, it would be great to have a wrapper
for sparse histograms that includes clamping values.)

[ccameron, 2017/02/23 00:47:33]

Actually, with exponentially-sized buckets, having 16 buckets for the range of 0
through 127 is good enough for error.

For tMin/tMax, I need even less precision (a value of 32 is huge), so I've
changed this to an 8-bucket with a max of 32. I also changed TMax to
OneMinusTMax.

+            UMA_HISTOGRAM_CUSTOM_COUNTS("Blink.ColorSpace.Destination.tMax",
+                                        static_cast<int>(tMax * 255), 0, 255,
+                                        256);
+            bool nonlinearFitConverged = false;
+            float error = 0;
+            gfx::SkApproximateTransferFn(x.data(), t.data(), x.size(), &fn,
+                                         &error, &nonlinearFitConverged);
+
+            // Record if the numerical fit converged, or if it didn't.
+            UMA_HISTOGRAM_BOOLEAN(
+                "Blink.ColorSpace.Destination.NonlinearFitConverged",
+                nonlinearFitConverged);
+
+            // Record the accuracy of the fit, separating out by nonlinear and
+            // linear fits.
+            if (nonlinearFitConverged) {
+              UMA_HISTOGRAM_CUSTOM_COUNTS(
+                  "Blink.ColorSpace.Destination.NonlinearFitError",
+                  static_cast<int>(error * 255), 0, 255, 256);
+            } else {
+              UMA_HISTOGRAM_CUSTOM_COUNTS(
+                  "Blink.ColorSpace.Destination.LinearFitError",
+                  static_cast<int>(error * 255), 0, 255, 256);
+            }
+          }
+        }
+      }
     }
   }