Revert of 565 support for SIMD xfermodes (patchset #3 id:40001 of https://codereview...
authormtklein <mtklein@google.com>
Tue, 21 Jul 2015 12:02:40 +0000 (05:02 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 21 Jul 2015 12:02:40 +0000 (05:02 -0700)
Reason for revert:
942930d (included in this roll) introduced a 140 kB sizes regression in
libskia.so. Please investigate and reland if this regression is necessary.

Original issue's description:
> 565 support for SIMD xfermodes
>
> This uses the most basic approach possible:
>   - to load an Sk4px from 565, convert to SkPMColors on the stack serially then load those SkPMColors.
>   - to store an Sk4px to 565, store to SkPMColors on the stack then convert to 565 serially.
>
> Clearly, we can optimize these loads and stores.  That's a TODO.
>
> The code using SkPMFloat is the same idea but a little more long-term viable, as we're only operating on one pixel at a time anyway.  We could probably write 565 <-> SkPMFloat methods, but I'd rather not until it's really compelling.
>
> The speedups are varied but similar across SSE and NEON: a few uninteresting, many 50% faster, some 2x faster, and SoftLight ~4x faster.
>
> This will cause minor GM diffs, but I don't think any layout test changes.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/942930dcaa51f66d82cdaf46ae62efebd16c8cd0

TBR=msarett@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

src/core/Sk4px.h
src/core/Sk4pxXfermode.h

index 7d51fd0..e1d4dc1 100644 (file)
@@ -10,7 +10,6 @@
 
 #include "SkNx.h"
 #include "SkColor.h"
-#include "SkColorPriv.h"
 
 // This file may be included multiple times by .cpp files with different flags, leading
 // to different definitions.  Usually that doesn't matter because it's all inlined, but
@@ -48,14 +47,6 @@ public:
     void store2(SkPMColor[2]) const;
     void store1(SkPMColor[1]) const;
 
-    // Same as above for 565.
-    static Sk4px Load4(const SkPMColor16 src[4]);
-    static Sk4px Load2(const SkPMColor16 src[2]);
-    static Sk4px Load1(const SkPMColor16 src[1]);
-    void store4(SkPMColor16 dst[4]) const;
-    void store2(SkPMColor16 dst[2]) const;
-    void store1(SkPMColor16 dst[1]) const;
-
     // 1, 2, or 4 SkPMColors with 16-bit components.
     // This is most useful as the result of a multiply, e.g. from mulWiden().
     class Wide : public Sk16h {
@@ -108,8 +99,8 @@ public:
 
     // A generic driver that maps fn over a src array into a dst array.
     // fn should take an Sk4px (4 src pixels) and return an Sk4px (4 dst pixels).
-    template <typename Fn, typename Dst>
-    static void MapSrc(int n, Dst* dst, const SkPMColor* src, const Fn& fn) {
+    template <typename Fn>
+    static void MapSrc(int n, SkPMColor* dst, const SkPMColor* src, const Fn& fn) {
         // This looks a bit odd, but it helps loop-invariant hoisting across different calls to fn.
         // Basically, we need to make sure we keep things inside a single loop.
         while (n > 0) {
@@ -138,8 +129,8 @@ public:
     }
 
     // As above, but with dst4' = fn(dst4, src4).
-    template <typename Fn, typename Dst>
-    static void MapDstSrc(int n, Dst* dst, const SkPMColor* src, const Fn& fn) {
+    template <typename Fn>
+    static void MapDstSrc(int n, SkPMColor* dst, const SkPMColor* src, const Fn& fn) {
         while (n > 0) {
             if (n >= 8) {
                 Sk4px dst0 = fn(Load4(dst+0), Load4(src+0)),
@@ -166,8 +157,8 @@ public:
     }
 
     // As above, but with dst4' = fn(dst4, src4, alpha4).
-    template <typename Fn, typename Dst>
-    static void MapDstSrcAlpha(int n, Dst* dst, const SkPMColor* src, const SkAlpha* a,
+    template <typename Fn>
+    static void MapDstSrcAlpha(int n, SkPMColor* dst, const SkPMColor* src, const SkAlpha* a,
                                const Fn& fn) {
         while (n > 0) {
             if (n >= 8) {
@@ -198,39 +189,6 @@ private:
     typedef Sk16b INHERITED;
 };
 
-// TODO: specialize these per-backend
-
-inline Sk4px Sk4px::Load4(const SkPMColor16 src[4]) {
-    SkPMColor src32[4];
-    for (int i = 0; i < 4; i++) { src32[i] = SkPixel16ToPixel32(src[i]); }
-    return Load4(src32);
-}
-inline Sk4px Sk4px::Load2(const SkPMColor16 src[2]) {
-    SkPMColor src32[2];
-    for (int i = 0; i < 2; i++) { src32[i] = SkPixel16ToPixel32(src[i]); }
-    return Load2(src32);
-}
-inline Sk4px Sk4px::Load1(const SkPMColor16 src[1]) {
-    SkPMColor src32 = SkPixel16ToPixel32(src[0]);
-    return Load1(&src32);
-}
-
-inline void Sk4px::store4(SkPMColor16 dst[4]) const {
-    SkPMColor dst32[4];
-    this->store4(dst32);
-    for (int i = 0; i < 4; i++) { dst[i] = SkPixel32ToPixel16(dst32[i]); }
-}
-inline void Sk4px::store2(SkPMColor16 dst[2]) const {
-    SkPMColor dst32[2];
-    this->store2(dst32);
-    for (int i = 0; i < 2; i++) { dst[i] = SkPixel32ToPixel16(dst32[i]); }
-}
-inline void Sk4px::store1(SkPMColor16 dst[1]) const {
-    SkPMColor dst32;
-    this->store1(&dst32);
-    dst[0] = SkPixel32ToPixel16(dst32);
-}
-
 }  // namespace
 
 #ifdef SKNX_NO_SIMD
index e912d1e..0c8dcb5 100644 (file)
@@ -222,19 +222,6 @@ public:
         }
     }
 
-    void xfer16(uint16_t dst[], const SkPMColor src[], int n, const SkAlpha aa[]) const override {
-        if (NULL == aa) {
-            Sk4px::MapDstSrc(n, dst, src, [&](const Sk4px& dst4, const Sk4px& src4) {
-                return ProcType::Xfer(src4, dst4);
-            });
-        } else {
-            Sk4px::MapDstSrcAlpha(n, dst, src, aa,
-                    [&](const Sk4px& dst4, const Sk4px& src4, const Sk4px& alpha) {
-                return xfer_aa<ProcType>(src4, dst4, alpha);
-            });
-        }
-    }
-
 private:
     SkT4pxXfermode(const ProcCoeff& rec) : INHERITED(rec, ProcType::kMode) {}
 
@@ -250,35 +237,19 @@ public:
 
     void xfer32(SkPMColor dst[], const SkPMColor src[], int n, const SkAlpha aa[]) const override {
         for (int i = 0; i < n; i++) {
-            dst[i] = aa ? this->xfer32(dst[i], src[i], aa[i])
-                        : this->xfer32(dst[i], src[i]);
-        }
-    }
-
-    void xfer16(uint16_t dst[], const SkPMColor src[], int n, const SkAlpha aa[]) const override {
-        for (int i = 0; i < n; i++) {
-            SkPMColor dst32 = SkPixel16ToPixel32(dst[i]);
-            dst32 = aa ? this->xfer32(dst32, src[i], aa[i])
-                       : this->xfer32(dst32, src[i]);
-            dst[i] = SkPixel32ToPixel16(dst32);
+            SkPMFloat s(src[i]),
+                      d(dst[i]),
+                      b(ProcType::Xfer(s,d));
+            if (aa) {
+                // We do aa in full float precision before going back down to bytes, because we can!
+                SkPMFloat a = Sk4f(aa[i]) * Sk4f(1.0f/255);
+                b = b*a + d*(Sk4f(1)-a);
+            }
+            dst[i] = b.round();
         }
     }
 
 private:
-    inline SkPMColor xfer32(SkPMColor dst, SkPMColor src) const {
-        return ProcType::Xfer(SkPMFloat(src), SkPMFloat(dst)).round();
-    }
-
-    inline SkPMColor xfer32(SkPMColor dst, SkPMColor src, SkAlpha aa) const {
-        SkPMFloat s(src),
-                  d(dst),
-                  b(ProcType::Xfer(s,d));
-        // We do aa in full float precision before going back down to bytes, because we can!
-        SkPMFloat a = Sk4f(aa) * Sk4f(1.0f/255);
-        b = b*a + d*(Sk4f(1)-a);
-        return b.round();
-    }
-
     SkTPMFloatXfermode(const ProcCoeff& rec) : INHERITED(rec, ProcType::kMode) {}
 
     typedef SkProcCoeffXfermode INHERITED;