color: Add analytic transfer functions in shaders

ui/gfx/color_transform.cc

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/color_transform.h"
 
 #include <algorithm>
 #include <cmath>
 #include <list>
 #include <memory>
@@ skipping 17 matching lines @@
 using std::log;
 using std::max;
 using std::min;
 using std::pow;
 using std::sqrt;
 
 namespace gfx {
 
 namespace {
 
+std::string Str(float f) {
+  return base::StringPrintf("%f", f);
+}
+
 // Helper for scoped QCMS profiles.
 struct QcmsProfileDeleter {
   void operator()(qcms_profile* p) {
     if (p) {
       qcms_profile_release(p);
     }
   }
 };
 using ScopedQcmsProfile = std::unique_ptr<qcms_profile, QcmsProfileDeleter>;
 
@@ skipping 172 matching lines @@
 
 Transform GetPrimaryTransform(const gfx::ColorSpace& color_space) {
   SkMatrix44 primary_matrix;
   color_space.GetPrimaryMatrix(&primary_matrix);
   return Transform(primary_matrix);
 }
 
 }  // namespace
 
 class ColorTransformMatrix;
+class ColorTransformSkTransferFn;
 class ColorTransformFromLinear;
 class ColorTransformToBT2020CL;
 class ColorTransformFromBT2020CL;
 class ColorTransformNull;
 class QCMSColorTransform;
 
 class ColorTransformStep {
  public:
   ColorTransformStep() {}
   virtual ~ColorTransformStep() {}
   virtual ColorTransformFromLinear* GetFromLinear() { return nullptr; }
   virtual ColorTransformToBT2020CL* GetToBT2020CL() { return nullptr; }
   virtual ColorTransformFromBT2020CL* GetFromBT2020CL() { return nullptr; }
+  virtual ColorTransformSkTransferFn* GetSkTransferFn() { return nullptr; }
   virtual ColorTransformMatrix* GetMatrix() { return nullptr; }
   virtual ColorTransformNull* GetNull() { return nullptr; }
   virtual QCMSColorTransform* GetQCMS() { return nullptr; }
 
   // Join methods, returns true if the |next| transform was successfully
   // assimilated into |this|.
   // If Join() returns true, |next| is no longer needed and can be deleted.
   virtual bool Join(ColorTransformStep* next) { return false; }
 
   // Return true if this is a null transform.
@@ skipping 88 matching lines @@
         m.get(0, 0), m.get(1, 0), m.get(2, 0),   // column 1
         m.get(0, 1), m.get(1, 1), m.get(2, 1),   // column 2
         m.get(0, 2), m.get(1, 2), m.get(2, 2));  // column 3
     SRC("color += vec3(%f, %f, %f);", m.get(0, 3), m.get(1, 3), m.get(2, 3));
   }
 
  private:
   class Transform matrix_;
 };
 
+class ColorTransformSkTransferFn : public ColorTransformStep {
+ public:
+  explicit ColorTransformSkTransferFn(const SkColorSpaceTransferFn& fn)
+      : fn_(fn) {}
+  ColorTransformSkTransferFn* GetSkTransferFn() override { return this; }
+
+  bool Join(ColorTransformStep* next_untyped) override {
+    ColorTransformSkTransferFn* next = next_untyped->GetSkTransferFn();
+    if (!next)
+      return false;
+    if (SkTransferFnsApproximatelyCancel(fn_, next->fn_)) {
+      // Set to be the identity.
+      fn_.fA = 1;
+      fn_.fB = 0;
+      fn_.fC = 1;
+      fn_.fD = 0;
+      fn_.fE = 0;
+      fn_.fF = 0;
+      fn_.fG = 1;
+      return true;
+    }
+    return false;
+  }
+
+  bool IsNull() override { return SkTransferFnIsApproximatelyIdentity(fn_); }
+
+  void Transform(ColorTransform::TriStim* colors, size_t num) const override {
+    for (size_t i = 0; i < num; i++) {
+      colors[i].set_x(SkTransferFnEval(fn_, colors[i].x()));
+      colors[i].set_y(SkTransferFnEval(fn_, colors[i].y()));
+      colors[i].set_z(SkTransferFnEval(fn_, colors[i].z()));
+    }
+  }
+
+  bool CanAppendShaderSource() override { return true; }
+
+  void AppendShaderSourceChannel(std::string* result, const char* value) {

[hubbe, 2017/02/16 23:06:38]

Please add some tests for this.
It would also help me review the generated code, as I can see what the code
would look like in the test. :)

[ccameron, 2017/02/16 23:51:58]

Mmh, good point -- this was hit a little bit by the GLRenderer tests, but it
didn't get all of the cases (where some things are 1 and others are 0).

I added a test that goes through a variety of SkColorSpaceTransferFns, to
generate the various combinations of elided operations.

[hubbe, 2017/02/17 00:06:00]

Could I get one of two tests on the form:

string str;
trasnform->AppendShaderSource(&str);
EXPECT_EQ(str, "LITERAL SHADER SOURCE HERE"); ?

[ccameron, 2017/02/19 22:28:07]

I don't feel that that sort of test is particularly helpful -- it will fail
every time we change the shader source, but will give no signal as to whether or
not the shader source is correct.

I added a test similar to this, which verifies that the shader source doesn't
emit an "if" or a "pow" if they aren't needed -- is that closer to what you were
looking for?

We currently have 3 other sets of tests for this
1. GLRenderer tests that verify that all versions of this compile
2. GLRenderer pixel tests that verify that the output pixels did not change
3. Blink pixel layout tests that verify that the output pixels did not change

IMO we need to add more of category [2] tests (and I'm working on that, but it
has lots of unrelated plumbing that is needed first).

[hubbe, 2017/02/20 05:01:02]

That is exactly the point. I don't want a lot of these kind of tests, because
that would be really annoying, but one or two will force people to explicitly
show how their changes affect the generated code, which will make it much easier
for the person reviewing the changes. (Which is likely to be you or me.)

[ccameron, 2017/02/21 22:32:42]

Ah, that's a good idea.

I've added one test which does that.

+    const float kEpsilon = 1.f / 1024.f;
+
+    // Construct the linear segment
+    //   linear = C * x + F
+    // Elide operations that will be close to the identity.
+    std::string linear = value;
+    if (std::abs(fn_.fC - 1.f) > kEpsilon)
+      linear = Str(fn_.fC) + " * " + linear;
+    if (std::abs(fn_.fF) > kEpsilon)
+      linear = linear + "+ " + Str(fn_.fF);
+
+    // Construct the nonlinear segment.
+    //   nonlinear = pow(A * x + B, G) + E
+    // Elide operations (especially the pow) that will be close to the
+    // identity.
+    std::string nonlinear = value;
+    if (std::abs(fn_.fA - 1.f) > kEpsilon)
+      nonlinear = Str(fn_.fA) + " * " + nonlinear;
+    if (std::abs(fn_.fB) > kEpsilon)
+      nonlinear = nonlinear + " + " + Str(fn_.fB);
+    if (std::abs(fn_.fG - 1.f) > kEpsilon)
+      nonlinear = "pow(" + nonlinear + ", " + Str(fn_.fG) + ")";
+    if (std::abs(fn_.fE) > kEpsilon)
+      nonlinear = nonlinear + " + " + Str(fn_.fE);
+
+    // Add both parts, as needed.
+    SRC("// Apply transfer function.");
+    if (fn_.fD > kEpsilon) {
+      SRC("if (%s < %f)", value, fn_.fD);
+      SRC("  %s = %s;", value, linear.c_str());
+      SRC("else");
+      SRC("  %s = %s;", value, nonlinear.c_str());
+    } else {
+      SRC("%s = %s;", value, nonlinear.c_str());
+    }
+  }
+
+  void AppendShaderSource(std::string* result) override {
+    // Append the transfer function for each channel.
+    AppendShaderSourceChannel(result, "color.r");

[hubbe, 2017/02/16 23:06:38]

Is the compiler smart enough to vectorize these?
I just wonder if it would be better to work on all channels simultaneously?

[ccameron, 2017/02/16 23:51:58]

I can't speak to anything but NVIDIA GPUs, but I'm pretty sure they operate the
same.

Since the ~Tesla generation, GPU shader cores are actually scalar units -- when
you do matrix-vector multiplies, they're executed one-float-at-a-time. At least
at a per-pixel level.

The place where GPU shader cores get parallelism is that they're executing the
same scalar program for each pixel. So, the compiler can be super-lazy and not
try to vectorize anything, and just rely on the fact that we're running the same
program on thousands of pixels in parallel to get the parallelism. In
particular, vectorizing in the compiler is a losing game.

In NVIDIA parlance, the parallel execution units are called "warps" of 16 or 32
"threads", where "thread" is sort of a misnomer (it's more like 1 lane of a
SIMD). Within this warp
- if the shader programs diverge (like have an if-statement), the shader will
execute both paths of the if-statement 
- if one of the threads is delayed cause of a texture cache miss, every thread
is delayed

This is all a mental relief for me, cause it means I don't have to think about
"is this vectorize-able".

[hubbe, 2017/02/17 00:06:00]

That's very interesting. It's my impression that some GPUs operate more
efficiently on quads. I'd like to get a second opinion on on this, perhaps
piman@?

+    AppendShaderSourceChannel(result, "color.g");
+    AppendShaderSourceChannel(result, "color.b");
+  }
+
+ private:
+  SkColorSpaceTransferFn fn_;
+};
+
 class ColorTransformFromLinear : public ColorTransformStep {
  public:
-  explicit ColorTransformFromLinear(ColorSpace::TransferID transfer,
-                                    const SkColorSpaceTransferFn& fn,
-                                    bool fn_valid)
-      : transfer_(transfer), fn_(fn), fn_valid_(fn_valid) {
-    if (transfer_ == ColorSpace::TransferID::LINEAR_HDR)
-      transfer_ = ColorSpace::TransferID::LINEAR;
-  }
+  explicit ColorTransformFromLinear(ColorSpace::TransferID transfer)
+      : transfer_(transfer) {}
   ColorTransformFromLinear* GetFromLinear() override { return this; }
   bool IsNull() override { return transfer_ == ColorSpace::TransferID::LINEAR; }
   void Transform(ColorTransform::TriStim* colors, size_t num) const override {
-    if (fn_valid_) {
-      for (size_t i = 0; i < num; i++) {
-        colors[i].set_x(EvalSkTransferFn(fn_, colors[i].x()));
-        colors[i].set_y(EvalSkTransferFn(fn_, colors[i].y()));
-        colors[i].set_z(EvalSkTransferFn(fn_, colors[i].z()));
-      }
-    } else {
-      for (size_t i = 0; i < num; i++) {
-        colors[i].set_x(FromLinear(transfer_, colors[i].x()));
-        colors[i].set_y(FromLinear(transfer_, colors[i].y()));
-        colors[i].set_z(FromLinear(transfer_, colors[i].z()));
-      }
+    for (size_t i = 0; i < num; i++) {
+      colors[i].set_x(FromLinear(transfer_, colors[i].x()));
+      colors[i].set_y(FromLinear(transfer_, colors[i].y()));
+      colors[i].set_z(FromLinear(transfer_, colors[i].z()));
     }
   }
 
  private:
   friend class ColorTransformToLinear;
   ColorSpace::TransferID transfer_;
-  SkColorSpaceTransferFn fn_;
-  bool fn_valid_ = false;
 };
 
 class ColorTransformToLinear : public ColorTransformStep {
  public:
-  explicit ColorTransformToLinear(ColorSpace::TransferID transfer,
-                                  const SkColorSpaceTransferFn& fn,
-                                  bool fn_valid)
-      : transfer_(transfer), fn_(fn), fn_valid_(fn_valid) {
-    if (transfer_ == ColorSpace::TransferID::LINEAR_HDR)
-      transfer_ = ColorSpace::TransferID::LINEAR;
-  }
-
-  static bool IsGamma22(ColorSpace::TransferID transfer) {
-    switch (transfer) {
-      // We don't need to check BT709 here because it's been translated into
-      // SRGB in ColorSpaceToColorSpaceTransform::ColorSpaceToLinear below.
-      case ColorSpace::TransferID::GAMMA22:
-      case ColorSpace::TransferID::IEC61966_2_1:  // SRGB
-        return true;
-
-      default:
-        return false;
-    }
-  }
+  explicit ColorTransformToLinear(ColorSpace::TransferID transfer)
+      : transfer_(transfer) {}
 
   bool Join(ColorTransformStep* next_untyped) override {
     ColorTransformFromLinear* next = next_untyped->GetFromLinear();
     if (!next)
       return false;
-    // TODO(ccameron): Use SkTransferFnsApproximatelyCancel and
-    // SkTransferFnIsApproximatelyIdentity to merge parametric transfer
-    // functions.
-    if (transfer_ == next->transfer_ ||
-        (IsGamma22(transfer_) && IsGamma22(next->transfer_))) {
+    if (transfer_ == next->transfer_) {
       transfer_ = ColorSpace::TransferID::LINEAR;
       return true;
     }
     return false;
   }
 
   bool IsNull() override { return transfer_ == ColorSpace::TransferID::LINEAR; }
 
   // Assumes BT2020 primaries.
   static float Luma(const ColorTransform::TriStim& c) {
     return c.x() * 0.2627f + c.y() * 0.6780f + c.z() * 0.0593f;
   }
 
   static ColorTransform::TriStim ClipToWhite(ColorTransform::TriStim& c) {
     float maximum = max(max(c.x(), c.y()), c.z());
     if (maximum > 1.0f) {
       float l = Luma(c);
       c.Scale(1.0f / maximum);
       ColorTransform::TriStim white(1.0f, 1.0f, 1.0f);
       white.Scale((1.0f - 1.0f / maximum) * l / Luma(white));
       ColorTransform::TriStim black(0.0f, 0.0f, 0.0f);
       c += white - black;
     }
     return c;
   }
 
   void Transform(ColorTransform::TriStim* colors, size_t num) const override {
-    if (fn_valid_) {
-      for (size_t i = 0; i < num; i++) {
-        colors[i].set_x(EvalSkTransferFn(fn_, colors[i].x()));
-        colors[i].set_y(EvalSkTransferFn(fn_, colors[i].y()));
-        colors[i].set_z(EvalSkTransferFn(fn_, colors[i].z()));
-      }
-    } else if (transfer_ == ColorSpace::TransferID::SMPTEST2084_NON_HDR) {
+    if (transfer_ == ColorSpace::TransferID::SMPTEST2084_NON_HDR) {
       for (size_t i = 0; i < num; i++) {
         ColorTransform::TriStim ret(ToLinear(transfer_, colors[i].x()),
                                     ToLinear(transfer_, colors[i].y()),
                                     ToLinear(transfer_, colors[i].z()));
         if (Luma(ret) > 0.0) {
           ColorTransform::TriStim smpte2084(
               ToLinear(ColorSpace::TransferID::SMPTEST2084, colors[i].x()),
               ToLinear(ColorSpace::TransferID::SMPTEST2084, colors[i].y()),
               ToLinear(ColorSpace::TransferID::SMPTEST2084, colors[i].z()));
           smpte2084.Scale(Luma(ret) / Luma(smpte2084));
           ret = ClipToWhite(smpte2084);
         }
         colors[i] = ret;
       }
     } else {
       for (size_t i = 0; i < num; i++) {
         colors[i].set_x(ToLinear(transfer_, colors[i].x()));
         colors[i].set_y(ToLinear(transfer_, colors[i].y()));
         colors[i].set_z(ToLinear(transfer_, colors[i].z()));
       }
     }
   }
 
  private:
   ColorSpace::TransferID transfer_;
-  SkColorSpaceTransferFn fn_;
-  bool fn_valid_ = false;
 };
 
 // BT2020 Constant Luminance is different than most other
 // ways to encode RGB values as YUV. The basic idea is that
 // transfer functions are applied on the Y value instead of
 // on the RGB values. However, running the transfer function
 // on the U and V values doesn't make any sense since they
 // are centered at 0.5. To work around this, the transfer function
 // is applied to the Y, R and B values, and then the U and V
 // values are calculated from that.
@@ skipping 126 matching lines @@
 
   steps_.push_back(
       base::MakeUnique<ColorTransformMatrix>(Invert(GetTransferMatrix(from))));
 
   // If the target color space is not defined, just apply the adjust and
   // tranfer matrices.
   if (!to.IsValid())
     return;
 
   SkColorSpaceTransferFn to_linear_fn;
-  bool to_linear_fn_valid = from.GetTransferFunction(&to_linear_fn);
-  steps_.push_back(base::MakeUnique<ColorTransformToLinear>(
-      from.transfer_, to_linear_fn, to_linear_fn_valid));
+  if (from.GetTransferFunction(&to_linear_fn)) {
+    steps_.push_back(
+        base::MakeUnique<ColorTransformSkTransferFn>(to_linear_fn));
+  } else {
+    steps_.push_back(base::MakeUnique<ColorTransformToLinear>(from.transfer_));
+  }
 
   if (from.matrix_ == ColorSpace::MatrixID::BT2020_CL) {
     // BT2020 CL is a special case.
     steps_.push_back(base::MakeUnique<ColorTransformFromBT2020CL>());
   }
   steps_.push_back(
       base::MakeUnique<ColorTransformMatrix>(GetPrimaryTransform(from)));
 
   steps_.push_back(
       base::MakeUnique<ColorTransformMatrix>(Invert(GetPrimaryTransform(to))));
   if (to.matrix_ == ColorSpace::MatrixID::BT2020_CL) {
     // BT2020 CL is a special case.
     steps_.push_back(base::MakeUnique<ColorTransformToBT2020CL>());
   }
 
   SkColorSpaceTransferFn from_linear_fn;
-  bool from_linear_fn_valid = to.GetInverseTransferFunction(&from_linear_fn);
-  steps_.push_back(base::MakeUnique<ColorTransformFromLinear>(
-      to.transfer_, from_linear_fn, from_linear_fn_valid));
+  if (to.GetInverseTransferFunction(&from_linear_fn)) {
+    steps_.push_back(
+        base::MakeUnique<ColorTransformSkTransferFn>(from_linear_fn));
+  } else {
+    steps_.push_back(base::MakeUnique<ColorTransformFromLinear>(to.transfer_));
+  }
 
   steps_.push_back(
       base::MakeUnique<ColorTransformMatrix>(GetTransferMatrix(to)));
 
   steps_.push_back(
       base::MakeUnique<ColorTransformMatrix>(Invert(GetRangeAdjustMatrix(to))));
 }
 
 class QCMSColorTransform : public ColorTransformStep {
  public:
@@ skipping 154 matching lines @@
     const ColorSpace& to,
     Intent intent) {
   return std::unique_ptr<ColorTransform>(
       new ColorTransformInternal(from, to, intent));
 }
 
 ColorTransform::ColorTransform() {}
 ColorTransform::~ColorTransform() {}
 
 }  // namespace gfx