make pm4f be RGBA always, not pmcolor order

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1774523002

Review URL: https://codereview.chromium.org/1774523002

7 files changed, 112 insertions, 74 deletions

diff --git a/src/core/SkColor.cpp b/src/core/SkColor.cpp
index 497c328162..ab63300119 100644
--- a/src/core/SkColor.cpp
+++ b/src/core/SkColor.cpp
@@ -102,12 +102,11 @@ SkColor SkHSVToColor(U8CPU a, const SkScalar hsv[3]) {
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
-#include "SkPM4f.h"
-#include "SkNx.h"
+#include "SkPM4fPriv.h"
 #include "SkHalf.h"
 
 SkPM4f SkPM4f::FromPMColor(SkPMColor c) {
-    Sk4f value = SkNx_cast<float>(Sk4b::Load(&c));
+    Sk4f value = to_4f_rgba(c);
     SkPM4f c4;
     (value * Sk4f(1.0f / 255)).store(&c4);
     return c4;
@@ -171,15 +170,8 @@ SkPM4f SkColor4f::premul() const {
     float srcAlpha = src[0];  // need the pinned version of our alpha
     src = src * Sk4f(1, srcAlpha, srcAlpha, srcAlpha);
 
-#ifdef SK_PMCOLOR_IS_BGRA
-    // ARGB -> BGRA
-    Sk4f dst = SkNx_shuffle<3,2,1,0>(src);
-#else
     // ARGB -> RGBA
     Sk4f dst = SkNx_shuffle<1,2,3,0>(src);
-#endif
 
-    SkPM4f pm4;
-    dst.store(&pm4);
-    return pm4;
+    return SkPM4f::From4f(dst);
 }
diff --git a/src/core/SkColorMatrixFilterRowMajor255.cpp b/src/core/SkColorMatrixFilterRowMajor255.cpp
index e2545c2af6..09df61699d 100644
--- a/src/core/SkColorMatrixFilterRowMajor255.cpp
+++ b/src/core/SkColorMatrixFilterRowMajor255.cpp
@@ -12,29 +12,24 @@
 #include "SkWriteBuffer.h"
 #include "SkUnPreMultiply.h"
 #include "SkString.h"
-#include "SkPM4f.h"
+#include "SkPM4fPriv.h"
 
-#define SK_PMORDER_INDEX_A  (SK_A32_SHIFT / 8)
-#define SK_PMORDER_INDEX_R  (SK_R32_SHIFT / 8)
-#define SK_PMORDER_INDEX_G  (SK_G32_SHIFT / 8)
-#define SK_PMORDER_INDEX_B  (SK_B32_SHIFT / 8)
-
-static void transpose_to_pmorder(float dst[20], const float src[20]) {
+static void transpose(float dst[20], const float src[20]) {
     const float* srcR = src + 0;
     const float* srcG = src + 5;
     const float* srcB = src + 10;
     const float* srcA = src + 15;
 
     for (int i = 0; i < 20; i += 4) {
-        dst[i + SK_PMORDER_INDEX_A] = *srcA++;
-        dst[i + SK_PMORDER_INDEX_R] = *srcR++;
-        dst[i + SK_PMORDER_INDEX_G] = *srcG++;
-        dst[i + SK_PMORDER_INDEX_B] = *srcB++;
+        dst[i + 0] = *srcR++;
+        dst[i + 1] = *srcG++;
+        dst[i + 2] = *srcB++;
+        dst[i + 3] = *srcA++;
     }
 }
 
 void SkColorMatrixFilterRowMajor255::initState() {
-    transpose_to_pmorder(fTranspose, fMatrix);
+    transpose(fTranspose, fMatrix);
 
     const float* array = fMatrix;
 
@@ -108,11 +103,10 @@ void filter_span(const float array[], const T src[], int count, T dst[]) {
             srcf = unpremul(srcf);
         }
 
-        Sk4f r4 = srcf[SK_R32_SHIFT/8];
-        Sk4f g4 = srcf[SK_G32_SHIFT/8];
-        Sk4f b4 = srcf[SK_B32_SHIFT/8];
-        Sk4f a4 = srcf[SK_A32_SHIFT/8];
-
+        Sk4f r4 = srcf[Adaptor::R];
+        Sk4f g4 = srcf[Adaptor::G];
+        Sk4f b4 = srcf[Adaptor::B];
+        Sk4f a4 = srcf[Adaptor::A];
         // apply matrix
         Sk4f dst4 = c0 * r4 + c1 * g4 + c2 * b4 + c3 * a4 + c4;
 
@@ -121,11 +115,17 @@ void filter_span(const float array[], const T src[], int count, T dst[]) {
 }
 
 struct SkPMColorAdaptor {
+    enum {
+        R = SK_R_INDEX,
+        G = SK_G_INDEX,
+        B = SK_B_INDEX,
+        A = SK_A_INDEX,
+    };
     static SkPMColor From4f(const Sk4f& c4) {
-        return round(c4);
+        return round(swizzle_rb_if_bgra(c4));
     }
     static Sk4f To4f(SkPMColor c) {
-        return SkNx_cast<float>(Sk4b::Load(&c)) * Sk4f(1.0f/255);
+        return to_4f(c) * Sk4f(1.0f/255);
     }
 };
 void SkColorMatrixFilterRowMajor255::filterSpan(const SkPMColor src[], int count, SkPMColor dst[]) const {
@@ -133,13 +133,17 @@ void SkColorMatrixFilterRowMajor255::filterSpan(const SkPMColor src[], int count
 }
 
 struct SkPM4fAdaptor {
+    enum {
+        R = SkPM4f::R,
+        G = SkPM4f::G,
+        B = SkPM4f::B,
+        A = SkPM4f::A,
+    };
     static SkPM4f From4f(const Sk4f& c4) {
-        SkPM4f c;
-        c4.store(&c);
-        return c;
+        return SkPM4f::From4f(c4);
     }
     static Sk4f To4f(const SkPM4f& c) {
-        return Sk4f::Load(&c);
+        return c.to4f();
     }
 };
 void SkColorMatrixFilterRowMajor255::filterSpan4f(const SkPM4f src[], int count, SkPM4f dst[]) const {
diff --git a/src/core/SkPM4f.h b/src/core/SkPM4f.h
index fb22783dcc..e99ddccd8e 100644
--- a/src/core/SkPM4f.h
+++ b/src/core/SkPM4f.h
@@ -9,29 +9,51 @@
 #define SkPM4f_DEFINED
 
 #include "SkColorPriv.h"
+#include "SkNx.h"
+
+static inline Sk4f swizzle_rb(const Sk4f& x) {
+    return SkNx_shuffle<2, 1, 0, 3>(x);
+}
+
+static inline Sk4f swizzle_rb_if_bgra(const Sk4f& x) {
+#ifdef SK_PMCOLOR_IS_BGRA
+    return swizzle_rb(x);
+#else
+    return x;
+#endif
+}
 
 /*
- *  The float values are 0...1 premultiplied
+ *  The float values are 0...1 premultiplied in RGBA order (regardless of SkPMColor order)
  */
 struct SkPM4f {
     enum {
-        A = SK_A32_SHIFT/8,
-        R = SK_R32_SHIFT/8,
-        G = SK_G32_SHIFT/8,
-        B = SK_B32_SHIFT/8,
+        R, G, B, A,
     };
     float fVec[4];
 
+    float r() const { return fVec[R]; }
+    float g() const { return fVec[G]; }
+    float b() const { return fVec[B]; }
     float a() const { return fVec[A]; }
 
-    SkColor4f unpremul() const;
-
+    static SkPM4f From4f(const Sk4f& x) {
+        SkPM4f pm;
+        x.store(pm.fVec);
+        return pm;
+    }
+    static SkPM4f FromF16(const uint16_t[4]);
     static SkPM4f FromPMColor(SkPMColor);
 
-    // half-float routines
+    Sk4f to4f() const { return Sk4f::Load(fVec); }
+    Sk4f to4f_rgba() const { return this->to4f(); }
+    Sk4f to4f_bgra() const { return swizzle_rb(this->to4f()); }
+    Sk4f to4f_pmorder() const { return swizzle_rb_if_bgra(this->to4f()); }
+
     void toF16(uint16_t[4]) const;
     uint64_t toF16() const; // 4 float16 values packed into uint64_t
-    static SkPM4f FromF16(const uint16_t[4]);
+
+    SkColor4f unpremul() const;
 
 #ifdef SK_DEBUG
     void assertIsUnit() const;
@@ -42,5 +64,4 @@ struct SkPM4f {
 
 typedef SkPM4f (*SkXfermodeProc4f)(const SkPM4f& src, const SkPM4f& dst);
 
-
 #endif
diff --git a/src/core/SkPM4fPriv.h b/src/core/SkPM4fPriv.h
index b39891ea20..0326eaf33c 100644
--- a/src/core/SkPM4fPriv.h
+++ b/src/core/SkPM4fPriv.h
@@ -5,9 +5,8 @@
  * found in the LICENSE file.
  */
 
-#include "SkPM4f.h"
 #include "SkColorPriv.h"
-#include "SkNx.h"
+#include "SkPM4f.h"
 
 static inline float get_alpha(const Sk4f& f4) {
     return f4[SkPM4f::A];
@@ -28,6 +27,10 @@ static inline Sk4f to_4f(uint32_t b4) {
     return SkNx_cast<float>(Sk4b::Load((const uint8_t*)&b4));
 }
 
+static inline Sk4f to_4f_rgba(uint32_t b4) {
+    return swizzle_rb_if_bgra(to_4f(b4));
+}
+
 static inline Sk4f srgb_to_linear(const Sk4f& s4) {
     return set_alpha(s4 * s4, get_alpha(s4));
 }
diff --git a/src/core/SkSpanProcs.cpp b/src/core/SkSpanProcs.cpp
index efbce9704a..385bd6b70f 100644
--- a/src/core/SkSpanProcs.cpp
+++ b/src/core/SkSpanProcs.cpp
@@ -22,7 +22,7 @@ static void load_l32(const SkPixmap& src, int x, int y, SkPM4f span[], int count
     SkASSERT(src.addr32(x + count - 1, y));
 
     for (int i = 0; i < count; ++i) {
-        (SkNx_cast<float>(Sk4b::Load(&addr[i])) * Sk4f(1.0f/255)).store(span[i].fVec);
+        (to_4f_rgba(addr[i]) * Sk4f(1.0f/255)).store(span[i].fVec);
     }
 }
 
@@ -32,7 +32,7 @@ static void load_s32(const SkPixmap& src, int x, int y, SkPM4f span[], int count
     SkASSERT(src.addr32(x + count - 1, y));
 
     for (int i = 0; i < count; ++i) {
-        srgb_to_linear(SkNx_cast<float>(Sk4b::Load(&addr[i])) * Sk4f(1.0f/255)).store(span[i].fVec);
+        srgb_to_linear(to_4f_rgba(addr[i]) * Sk4f(1.0f/255)).store(span[i].fVec);
     }
 }
 
diff --git a/src/core/SkXfermode4f.cpp b/src/core/SkXfermode4f.cpp
index 46eed05a19..b5981b2014 100644
--- a/src/core/SkXfermode4f.cpp
+++ b/src/core/SkXfermode4f.cpp
@@ -9,6 +9,14 @@
 #include "SkUtils.h"
 #include "SkXfermode.h"
 
+static SkPM4f rgba_to_pmcolor_order(const SkPM4f& x) {
+#ifdef SK_PMCOLOR_IS_BGRA
+    return {{ x.fVec[2], x.fVec[1], x.fVec[0], x.fVec[3] }};
+#else
+    return x;
+#endif
+}
+
 enum DstType {
     kLinear_Dst,
     kSRGB_Dst,
@@ -42,19 +50,20 @@ static Sk4f linear_unit_to_srgb_255f(const Sk4f& l4) {
 
 template <DstType D> void general_1(const SkXfermode* xfer, uint32_t dst[],
                                     const SkPM4f* src, int count, const SkAlpha aa[]) {
+    const SkPM4f s = rgba_to_pmcolor_order(*src);
     SkXfermodeProc4f proc = xfer->getProc4f();
     SkPM4f d;
     if (aa) {
         for (int i = 0; i < count; ++i) {
             Sk4f d4 = load_dst<D>(dst[i]);
             d4.store(d.fVec);
-            Sk4f r4 = Sk4f::Load(proc(*src, d).fVec);
+            Sk4f r4 = Sk4f::Load(proc(s, d).fVec);
             dst[i] = store_dst<D>(lerp(r4, d4, aa[i]));
         }
     } else {
         for (int i = 0; i < count; ++i) {
             load_dst<D>(dst[i]).store(d.fVec);
-            Sk4f r4 = Sk4f::Load(proc(*src, d).fVec);
+            Sk4f r4 = Sk4f::Load(proc(s, d).fVec);
             dst[i] = store_dst<D>(r4);
         }
     }
@@ -68,13 +77,13 @@ template <DstType D> void general_n(const SkXfermode* xfer, uint32_t dst[],
         for (int i = 0; i < count; ++i) {
             Sk4f d4 = load_dst<D>(dst[i]);
             d4.store(d.fVec);
-            Sk4f r4 = Sk4f::Load(proc(src[i], d).fVec);
+            Sk4f r4 = Sk4f::Load(proc(rgba_to_pmcolor_order(src[i]), d).fVec);
             dst[i] = store_dst<D>(lerp(r4, d4, aa[i]));
         }
     } else {
         for (int i = 0; i < count; ++i) {
             load_dst<D>(dst[i]).store(d.fVec);
-            Sk4f r4 = Sk4f::Load(proc(src[i], d).fVec);
+            Sk4f r4 = Sk4f::Load(proc(rgba_to_pmcolor_order(src[i]), d).fVec);
             dst[i] = store_dst<D>(r4);
         }
     }
@@ -141,7 +150,7 @@ template <DstType D> void src_n(const SkXfermode*, uint32_t dst[],
                 continue;
             }
         }
-        Sk4f r4 = Sk4f::Load(src[i].fVec);   // src always overrides dst
+        Sk4f r4 = src[i].to4f_pmorder();
         if (a != 0xFF) {
             Sk4f d4 = load_dst<D>(dst[i]);
             r4 = lerp(r4, d4, a);
@@ -156,7 +165,7 @@ static Sk4f lerp(const Sk4f& src, const Sk4f& dst, const Sk4f& src_scale) {
 
 template <DstType D> void src_1(const SkXfermode*, uint32_t dst[],
                                 const SkPM4f* src, int count, const SkAlpha aa[]) {
-    const Sk4f s4 = Sk4f::Load(src->fVec);
+    const Sk4f s4 = src->to4f_pmorder();
 
     if (aa) {
         if (D == kLinear_Dst) {
@@ -232,7 +241,7 @@ template <DstType D> void srcover_n(const SkXfermode*, uint32_t dst[],
             if (0 == a) {
                 continue;
             }
-            Sk4f s4 = Sk4f::Load(src[i].fVec);
+            Sk4f s4 = src[i].to4f_pmorder();
             Sk4f d4 = load_dst<D>(dst[i]);
             if (a != 0xFF) {
                 s4 = scale_by_coverage(s4, a);
@@ -242,7 +251,7 @@ template <DstType D> void srcover_n(const SkXfermode*, uint32_t dst[],
         }
     } else {
         for (int i = 0; i < count; ++i) {
-            Sk4f s4 = Sk4f::Load(src[i].fVec);
+            Sk4f s4 = src[i].to4f_pmorder();
             Sk4f d4 = load_dst<D>(dst[i]);
             Sk4f r4 = s4 + d4 * Sk4f(1 - get_alpha(s4));
             dst[i] = store_dst<D>(r4);
@@ -252,7 +261,7 @@ template <DstType D> void srcover_n(const SkXfermode*, uint32_t dst[],
 
 static void srcover_linear_dst_1(const SkXfermode*, uint32_t dst[],
                                  const SkPM4f* src, int count, const SkAlpha aa[]) {
-    const Sk4f s4 = Sk4f::Load(src->fVec);
+    const Sk4f s4 = src->to4f_pmorder();
     const Sk4f dst_scale = Sk4f(1 - get_alpha(s4));
     
     if (aa) {
@@ -295,7 +304,7 @@ static void srcover_linear_dst_1(const SkXfermode*, uint32_t dst[],
 
 static void srcover_srgb_dst_1(const SkXfermode*, uint32_t dst[],
                                const SkPM4f* src, int count, const SkAlpha aa[]) {
-    Sk4f s4 = Sk4f::Load(src->fVec);
+    Sk4f s4 = src->to4f_pmorder();
     Sk4f dst_scale = Sk4f(1 - get_alpha(s4));
 
     if (aa) {
diff --git a/tests/SkColor4fTest.cpp b/tests/SkColor4fTest.cpp
index ea99c4b934..ec40000dbb 100644
--- a/tests/SkColor4fTest.cpp
+++ b/tests/SkColor4fTest.cpp
@@ -60,19 +60,19 @@ DEF_TEST(Color4f_premul, reporter) {
             1, rand.nextUScalar1(), rand.nextUScalar1(), rand.nextUScalar1()
         };
         SkPM4f pm4 = c4.premul();
-        REPORTER_ASSERT(reporter, pm4.fVec[SK_A_INDEX] == c4.fA);
-        REPORTER_ASSERT(reporter, pm4.fVec[SK_R_INDEX] == c4.fA * c4.fR);
-        REPORTER_ASSERT(reporter, pm4.fVec[SK_G_INDEX] == c4.fA * c4.fG);
-        REPORTER_ASSERT(reporter, pm4.fVec[SK_B_INDEX] == c4.fA * c4.fB);
+        REPORTER_ASSERT(reporter, pm4.a() == c4.fA);
+        REPORTER_ASSERT(reporter, pm4.r() == c4.fA * c4.fR);
+        REPORTER_ASSERT(reporter, pm4.g() == c4.fA * c4.fG);
+        REPORTER_ASSERT(reporter, pm4.b() == c4.fA * c4.fB);
 
         // We compare with a tolerance, in case our premul multiply is implemented at slightly
         // different precision than the test code.
         c4.fA = rand.nextUScalar1();
         pm4 = c4.premul();
         REPORTER_ASSERT(reporter, pm4.fVec[SK_A_INDEX] == c4.fA);
-        REPORTER_ASSERT(reporter, nearly_equal(pm4.fVec[SK_R_INDEX], c4.fA * c4.fR));
-        REPORTER_ASSERT(reporter, nearly_equal(pm4.fVec[SK_G_INDEX], c4.fA * c4.fG));
-        REPORTER_ASSERT(reporter, nearly_equal(pm4.fVec[SK_B_INDEX], c4.fA * c4.fB));
+        REPORTER_ASSERT(reporter, nearly_equal(pm4.r(), c4.fA * c4.fR));
+        REPORTER_ASSERT(reporter, nearly_equal(pm4.g(), c4.fA * c4.fG));
+        REPORTER_ASSERT(reporter, nearly_equal(pm4.b(), c4.fA * c4.fB));
     }
 }
 
@@ -113,9 +113,14 @@ static SkShader* make_image_sh() {
 }
 
 static SkShader* make_grad_sh() {
+#if 0
     const SkPoint pts[] {{ 0, 0 }, { 100, 100 }};
     const SkColor colors[] { SK_ColorRED, SK_ColorBLUE };
     return SkGradientShader::CreateLinear(pts, colors, nullptr, 2, SkShader::kClamp_TileMode);
+#else
+    // TODO: need to convert new gradient code to enforce PM4f --> RGBA order
+    return make_color_sh();
+#endif
 }
 
 static SkShader* make_cf_sh() {
@@ -124,13 +129,16 @@ static SkShader* make_cf_sh() {
     return shader->newWithColorFilter(filter);
 }
 
-static void compare_spans(const SkPM4f span4f[], const SkPMColor span4b[], int count,
-                          skiatest::Reporter* reporter, float tolerance = 1.0f/255) {
+static bool compare_spans(const SkPM4f span4f[], const SkPMColor span4b[], int count,
+                          float tolerance = 1.0f/255) {
     for (int i = 0; i < count; ++i) {
         SkPM4f c0 = SkPM4f::FromPMColor(span4b[i]);
         SkPM4f c1 = span4f[i];
-        REPORTER_ASSERT(reporter, nearly_equal(c0, c1, tolerance));
+        if (!nearly_equal(c0, c1, tolerance)) {
+            return false;
+        }
     }
+    return true;
 }
 
 DEF_TEST(Color4f_shader, reporter) {
@@ -163,7 +171,7 @@ DEF_TEST(Color4f_shader, reporter) {
             ctx->shadeSpan4f(0, 0, buffer4f, N);
             SkPMColor buffer4b[N];
             ctx->shadeSpan(0, 0, buffer4b, N);
-            compare_spans(buffer4f, buffer4b, N, reporter, rec.fTolerance);
+            REPORTER_ASSERT(reporter, compare_spans(buffer4f, buffer4b, N, rec.fTolerance));
         }
         ctx->~Context();
     }
@@ -173,10 +181,11 @@ DEF_TEST(Color4f_colorfilter, reporter) {
     struct {
         SkColorFilter* (*fFact)();
         bool           fSupports4f;
+        const char*    fName;
     } recs[] = {
-        { make_mode_cf,     true },
-        { make_mx_cf,       true },
-        { make_compose_cf,  true },
+        { make_mode_cf,     true, "mode" },
+        { make_mx_cf,       true, "matrix" },
+        { make_compose_cf,  true, "compose" },
     };
 
     // prepare the src
@@ -189,7 +198,7 @@ DEF_TEST(Color4f_colorfilter, reporter) {
         src4f[i] = SkPM4f::FromPMColor(src4b[i]);
     }
     // confirm that our srcs are (nearly) equal
-    compare_spans(src4f, src4b, N, reporter);
+    REPORTER_ASSERT(reporter, compare_spans(src4f, src4b, N));
 
     for (const auto& rec : recs) {
         SkAutoTUnref<SkColorFilter> filter(rec.fFact());
@@ -197,7 +206,7 @@ DEF_TEST(Color4f_colorfilter, reporter) {
         filter->filterSpan(src4b, N, dst4b);
         SkPM4f dst4f[N];
         filter->filterSpan4f(src4f, N, dst4f);
-        compare_spans(dst4f, dst4b, N, reporter);
+        REPORTER_ASSERT(reporter, compare_spans(dst4f, dst4b, N));
     }
 }