color-correct patch

Key work is to correctly convert SkColor corners into linear floats,
then interpolate, then (correctly) convert back to SkColors.

Bug: skia:6659
Change-Id: Iaf0ab842d7a4f8f3481e609903cec83814e5a749
Reviewed-on: https://skia-review.googlesource.com/17533
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

6 files changed, 145 insertions, 46 deletions

diff --git a/bench/PatchBench.cpp b/bench/PatchBench.cpp
index 4ca4bc8f5e..d88dd14a2b 100644
--- a/bench/PatchBench.cpp
+++ b/bench/PatchBench.cpp
@@ -327,16 +327,20 @@ DEF_BENCH( return new LoopPatchBench(SkVector::Make(3.0f, 3.0f),
 #include "SkPatchUtils.h"
 
 class PatchUtilsBench : public Benchmark {
+    SkString    fName;
+    const bool  fLinearInterp;
 public:
-    const char* onGetName() override {
-        return "patchutils";
+    PatchUtilsBench(bool linearInterp) : fLinearInterp(linearInterp) {
+        fName.printf("patchutils_%s", linearInterp ? "linear" : "legacy");
     }
 
+    const char* onGetName() override { return fName.c_str(); }
+
     bool isSuitableFor(Backend backend) override {
         return backend == kNonRendering_Backend;
     }
 
-    void onDraw(int, SkCanvas*) override {
+    void onDraw(int loops, SkCanvas*) override {
         const SkColor colors[] = { 0xFF000000, 0xFF00FF00, 0xFF0000FF, 0xFFFF0000 };
         const SkPoint pts[] = {
             { 0, 0 }, { 10, 0 }, { 20, 0 }, { 30, 0 },
@@ -347,9 +351,10 @@ public:
             { 0, 0 }, { 10, 0 }, { 10, 10 }, { 0, 10 },
         };
 
-        for (int i = 0; i < 100; ++i) {
-            SkPatchUtils::MakeVertices(pts, colors, tex, 20, 20);
+        for (int i = 0; i < 100*loops; ++i) {
+            SkPatchUtils::MakeVertices(pts, colors, tex, 20, 20, fLinearInterp);
         }
     }
 };
-DEF_BENCH( return new PatchUtilsBench; )
+DEF_BENCH( return new PatchUtilsBench(false); )
+DEF_BENCH( return new PatchUtilsBench(true); )
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 216d8fa952..3c7e6477d6 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -2644,10 +2644,12 @@ void SkCanvas::onDrawPatch(const SkPoint cubics[12], const SkColor colors[4],
         return;
     }
 
+    const bool interpColorsLinearly = (this->imageInfo().colorSpace() != nullptr);
+
     LOOPER_BEGIN(paint, SkDrawFilter::kPath_Type, nullptr)
 
     while (iter.next()) {
-        iter.fDevice->drawPatch(cubics, colors, texCoords, bmode, paint);
+        iter.fDevice->drawPatch(cubics, colors, texCoords, bmode, interpColorsLinearly, paint);
     }
 
     LOOPER_END
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index 0b72eeb9e8..e210e3695e 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -125,9 +125,11 @@ void SkBaseDevice::drawDRRect(const SkRRect& outer,
 }
 
 void SkBaseDevice::drawPatch(const SkPoint cubics[12], const SkColor colors[4],
-                             const SkPoint texCoords[4], SkBlendMode bmode, const SkPaint& paint) {
+                             const SkPoint texCoords[4], SkBlendMode bmode,
+                             bool interpColorsLinearly, const SkPaint& paint) {
     SkISize lod = SkPatchUtils::GetLevelOfDetail(cubics, &this->ctm());
-    auto vertices = SkPatchUtils::MakeVertices(cubics, colors, texCoords, lod.width(), lod.height());
+    auto vertices = SkPatchUtils::MakeVertices(cubics, colors, texCoords, lod.width(), lod.height(),
+                                               interpColorsLinearly);
     if (vertices) {
         this->drawVertices(vertices.get(), bmode, paint);
     }
diff --git a/src/core/SkDevice.h b/src/core/SkDevice.h
index 17ea6f71f7..82c1897452 100644
--- a/src/core/SkDevice.h
+++ b/src/core/SkDevice.h
@@ -242,7 +242,8 @@ protected:
                               const SkPaint& paint, SkDrawFilter* drawFilter);
     // default implementation calls drawVertices
     virtual void drawPatch(const SkPoint cubics[12], const SkColor colors[4],
-                           const SkPoint texCoords[4], SkBlendMode, const SkPaint& paint);
+                           const SkPoint texCoords[4], SkBlendMode, bool interpColorsLinearly,
+                           const SkPaint& paint);
 
     // default implementation calls drawPath
     virtual void drawAtlas(const SkImage* atlas, const SkRSXform[], const SkRect[],
diff --git a/src/utils/SkPatchUtils.cpp b/src/utils/SkPatchUtils.cpp
index 119f734a24..3dc7ba9f16 100644
--- a/src/utils/SkPatchUtils.cpp
+++ b/src/utils/SkPatchUtils.cpp
@@ -8,7 +8,9 @@
 #include "SkPatchUtils.h"
 
 #include "SkColorPriv.h"
+#include "SkColorSpace_Base.h"
 #include "SkGeometry.h"
+#include "SkPM4f.h"
 
 namespace {
     enum CubicCtrlPts {
@@ -140,12 +142,19 @@ static SkScalar approx_arc_length(SkPoint* points, int count) {
 }
 
 static SkScalar bilerp(SkScalar tx, SkScalar ty, SkScalar c00, SkScalar c10, SkScalar c01,
-                      SkScalar c11) {
+                       SkScalar c11) {
     SkScalar a = c00 * (1.f - tx) + c10 * tx;
     SkScalar b = c01 * (1.f - tx) + c11 * tx;
     return a * (1.f - ty) + b * ty;
 }
 
+static Sk4f bilerp(SkScalar tx, SkScalar ty,
+                   const Sk4f& c00, const Sk4f& c10, const Sk4f& c01, const Sk4f& c11) {
+    Sk4f a = c00 * (1.f - tx) + c10 * tx;
+    Sk4f b = c01 * (1.f - tx) + c11 * tx;
+    return a * (1.f - ty) + b * ty;
+}
+
 SkISize SkPatchUtils::GetLevelOfDetail(const SkPoint cubics[12], const SkMatrix* matrix) {
 
     // Approximate length of each cubic.
@@ -201,8 +210,88 @@ void SkPatchUtils::GetRightCubic(const SkPoint cubics[12], SkPoint points[4]) {
     points[3] = cubics[kRightP3_CubicCtrlPts];
 }
 
+#include "SkPM4fPriv.h"
+#include "SkColorSpace_Base.h"
+#include "SkColorSpaceXform.h"
+
+struct SkRGBAf {
+    float fVec[4];
+
+    static SkRGBAf From4f(const Sk4f& x) {
+        SkRGBAf c;
+        x.store(c.fVec);
+        return c;
+    }
+
+    static SkRGBAf FromBGRA32(SkColor c) {
+        return From4f(swizzle_rb(SkNx_cast<float>(Sk4b::Load(&c)) * (1/255.0f)));
+    }
+
+    Sk4f to4f() const {
+        return Sk4f::Load(fVec);
+    }
+
+    SkColor toBGRA32() const {
+        SkColor color;
+        SkNx_cast<uint8_t>(swizzle_rb(this->to4f()) * Sk4f(255) + Sk4f(0.5f)).store(&color);
+        return color;
+    }
+
+    SkRGBAf premul() const {
+        float a = fVec[3];
+        return From4f(this->to4f() * Sk4f(a, a, a, 1));
+    }
+
+    SkRGBAf unpremul() const {
+        float a = fVec[3];
+        float inv = a ? 1/a : 0;
+        return From4f(this->to4f() * Sk4f(inv, inv, inv, 1));
+    }
+};
+
+static void skcolor_to_linear(SkRGBAf dst[], const SkColor src[], int count, SkColorSpace* cs,
+                              bool doPremul) {
+    if (cs) {
+        auto srcCS = SkColorSpace::MakeSRGB();
+        auto dstCS = as_CSB(cs)->makeLinearGamma();
+        auto op = doPremul ? SkColorSpaceXform::kPremul_AlphaOp
+                           : SkColorSpaceXform::kPreserve_AlphaOp;
+        SkColorSpaceXform::Apply(dstCS.get(), SkColorSpaceXform::kRGBA_F32_ColorFormat,  dst,
+                                 srcCS.get(), SkColorSpaceXform::kBGRA_8888_ColorFormat, src,
+                                 count, op);
+    } else {
+        for (int i = 0; i < count; ++i) {
+            dst[i] = SkRGBAf::FromBGRA32(src[i]);
+            if (doPremul) {
+                dst[i] = dst[i].premul();
+            }
+        }
+    }
+}
+
+static void linear_to_skcolor(SkColor dst[], const SkRGBAf src[], int count, SkColorSpace* cs) {
+    if (cs) {
+        auto srcCS = as_CSB(cs)->makeLinearGamma();
+        auto dstCS = SkColorSpace::MakeSRGB();
+        SkColorSpaceXform::Apply(dstCS.get(), SkColorSpaceXform::kBGRA_8888_ColorFormat, dst,
+                                 srcCS.get(), SkColorSpaceXform::kRGBA_F32_ColorFormat,  src,
+                                 count, SkColorSpaceXform::kPreserve_AlphaOp);
+    } else {
+        for (int i = 0; i < count; ++i) {
+            dst[i] = src[i].toBGRA32();
+        }
+    }
+}
+
+static void unpremul(SkRGBAf array[], int count) {
+    for (int i = 0; i < count; ++i) {
+        array[i] = array[i].unpremul();
+    }
+}
+
 sk_sp<SkVertices> SkPatchUtils::MakeVertices(const SkPoint cubics[12], const SkColor srcColors[4],
-                                            const SkPoint srcTexCoords[4], int lodX, int lodY) {
+                                             const SkPoint srcTexCoords[4], int lodX, int lodY,
+                                             bool interpColorsLinearly) {
     if (lodX < 1 || lodY < 1 || nullptr == cubics) {
         return nullptr;
     }
@@ -237,24 +326,35 @@ sk_sp<SkVertices> SkPatchUtils::MakeVertices(const SkPoint cubics[12], const SkC
         flags |= SkVertices::kHasColors_BuilderFlag;
     }
 
+    char allocStorage[2048];
+    SkArenaAlloc alloc(allocStorage, sizeof(allocStorage));
+    SkRGBAf* cornerColors = srcColors ? alloc.makeArray<SkRGBAf>(4) : nullptr;
+    SkRGBAf* tmpColors = srcColors ? alloc.makeArray<SkRGBAf>(vertexCount) : nullptr;
+    auto convertCS = interpColorsLinearly ? SkColorSpace::MakeSRGB() : nullptr;
+
     SkVertices::Builder builder(SkVertices::kTriangles_VertexMode, vertexCount, indexCount, flags);
     SkPoint* pos = builder.positions();
     SkPoint* texs = builder.texCoords();
-    SkColor* colors = builder.colors();
     uint16_t* indices = builder.indices();
     bool is_opaque = false;
 
-    // if colors is not null then create array for colors
-    SkPMColor colorsPM[kNumCorners];
-    if (srcColors) {
+    /*
+     *  1. Should we offer this as a runtime choice, as we do in gradients?
+     *  2. Since drawing the vertices wants premul, shoudl we extend SkVertices to store
+     *     premul colors (as floats, w/ a colorspace)?
+     */
+    bool doPremul = true;
+    if (cornerColors) {
         SkColor c = ~0;
-        // premultiply colors to avoid color bleeding.
         for (int i = 0; i < kNumCorners; i++) {
-            colorsPM[i] = SkPreMultiplyColor(srcColors[i]);
             c &= srcColors[i];
         }
-        srcColors = colorsPM;
         is_opaque = (SkColorGetA(c) == 0xFF);
+        if (is_opaque) {
+            doPremul = false;   // no need
+        }
+
+        skcolor_to_linear(cornerColors, srcColors, kNumCorners, convertCS.get(), doPremul);
     }
 
     SkPoint pts[kNumPtsCubic];
@@ -297,33 +397,14 @@ sk_sp<SkVertices> SkPatchUtils::MakeVertices(const SkPoint cubics[12], const SkC
                                               + u * fBottom.getCtrlPoints()[3].y()));
             pos[dataIndex] = s0 + s1 - s2;
 
-            if (colors) {
-                uint8_t a = 0xFF;
-                // We do the opaque check for speed, and to ensure that opaque stays opaque,
-                // in case we lose precision in the bilerp.
-                if (!is_opaque) {
-                    a = uint8_t(bilerp(u, v,
-                                       SkScalar(SkColorGetA(colorsPM[kTopLeft_Corner])),
-                                       SkScalar(SkColorGetA(colorsPM[kTopRight_Corner])),
-                                       SkScalar(SkColorGetA(colorsPM[kBottomLeft_Corner])),
-                                       SkScalar(SkColorGetA(colorsPM[kBottomRight_Corner]))));
+            if (cornerColors) {
+                bilerp(u, v, cornerColors[kTopLeft_Corner].to4f(),
+                             cornerColors[kTopRight_Corner].to4f(),
+                             cornerColors[kBottomLeft_Corner].to4f(),
+                             cornerColors[kBottomRight_Corner].to4f()).store(tmpColors[dataIndex].fVec);
+                if (is_opaque) {
+                    tmpColors[dataIndex].fVec[3] = 1;
                 }
-                uint8_t r = uint8_t(bilerp(u, v,
-                                           SkScalar(SkColorGetR(colorsPM[kTopLeft_Corner])),
-                                           SkScalar(SkColorGetR(colorsPM[kTopRight_Corner])),
-                                           SkScalar(SkColorGetR(colorsPM[kBottomLeft_Corner])),
-                                           SkScalar(SkColorGetR(colorsPM[kBottomRight_Corner]))));
-                uint8_t g = uint8_t(bilerp(u, v,
-                                           SkScalar(SkColorGetG(colorsPM[kTopLeft_Corner])),
-                                           SkScalar(SkColorGetG(colorsPM[kTopRight_Corner])),
-                                           SkScalar(SkColorGetG(colorsPM[kBottomLeft_Corner])),
-                                           SkScalar(SkColorGetG(colorsPM[kBottomRight_Corner]))));
-                uint8_t b = uint8_t(bilerp(u, v,
-                                           SkScalar(SkColorGetB(colorsPM[kTopLeft_Corner])),
-                                           SkScalar(SkColorGetB(colorsPM[kTopRight_Corner])),
-                                           SkScalar(SkColorGetB(colorsPM[kBottomLeft_Corner])),
-                                           SkScalar(SkColorGetB(colorsPM[kBottomRight_Corner]))));
-                colors[dataIndex] = SkPackARGB32(a,r,g,b);
             }
 
             if (texs) {
@@ -351,5 +432,12 @@ sk_sp<SkVertices> SkPatchUtils::MakeVertices(const SkPoint cubics[12], const SkC
         }
         u = SkScalarClampMax(u + 1.f / lodX, 1);
     }
+
+    if (tmpColors) {
+        if (doPremul) {
+            unpremul(tmpColors, vertexCount);
+        }
+        linear_to_skcolor(builder.colors(), tmpColors, vertexCount, convertCS.get());
+    }
     return builder.detach();
 }
diff --git a/src/utils/SkPatchUtils.h b/src/utils/SkPatchUtils.h
index 6d6e3491b6..9627f74720 100644
--- a/src/utils/SkPatchUtils.h
+++ b/src/utils/SkPatchUtils.h
@@ -48,7 +48,8 @@ public:
     static SkISize GetLevelOfDetail(const SkPoint cubics[12], const SkMatrix* matrix);
 
     static sk_sp<SkVertices> MakeVertices(const SkPoint cubics[12], const SkColor colors[4],
-                                          const SkPoint texCoords[4], int lodX, int lodY);
+                                          const SkPoint texCoords[4], int lodX, int lodY,
+                                          bool interpColorsLinearly);
 };
 
 #endif