Revert of 565 support for SIMD xfermodes (patchset #4 id:60001 of https://codereview...
authormtklein <mtklein@google.com>
Wed, 22 Jul 2015 01:34:42 +0000 (18:34 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 22 Jul 2015 01:34:43 +0000 (18:34 -0700)
Reason for revert:
NEON 565 gold images have gone ugly.  This is what I get for writing and testing SSE and just writing NEON.

E.g. colortype_xfermodes, dstreadshuffle, bigbitmaprect, pictures, textbloblooper, aaxfermodes (only Plus)

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
>
> Committed: https://skia.googlesource.com/skia/+/860dcaa2ddfdadc050af4f943a84a9d499315066

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

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

src/core/Sk4px.h
src/core/Sk4pxXfermode.h
src/opts/Sk4px_NEON.h
src/opts/Sk4px_SSE2.h
src/opts/Sk4px_none.h

index 24a21c6..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) {
index ad822ed..fc0b643 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 fProc4(src4, dst4);
-            });
-        } else {
-            Sk4px::MapDstSrcAlpha(n, dst, src, aa,
-                    [&](const Sk4px& dst4, const Sk4px& src4, const Sk4px& alpha) {
-                return fAAProc4(src4, dst4, alpha);
-            });
-        }
-    }
-
 private:
     Proc4 fProc4;
     AAProc4 fAAProc4;
@@ -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(fProcF(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 fProcF(SkPMFloat(src), SkPMFloat(dst)).round();
-    }
-
-    inline SkPMColor xfer32(SkPMColor dst, SkPMColor src, SkAlpha aa) const {
-        SkPMFloat s(src),
-                  d(dst),
-                  b(fProcF(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();
-    }
-
     ProcF fProcF;
     typedef SkProcCoeffXfermode INHERITED;
 };
index d411ae1..cd6dea9 100644 (file)
@@ -89,76 +89,5 @@ inline Sk4px Sk4px::zeroAlphas() const {
     return Sk16b(vbicq_u8(this->fVec, (uint8x16_t)vdupq_n_u32(0xFF << SK_A32_SHIFT)));
 }
 
-static inline uint8x16_t widen_to_8888(uint16x4_t v) {
-    // RGB565 format:   |R....|G.....|B....|
-    //           Bit:  16    11      5     0
-
-    // First get each pixel into its own 32-bit lane.
-    //      v ==                       rgb3 rgb2  rgb1 rgb0
-    // spread == 0000 rgb3  0000 rgb2  0000 rgb1  0000 rgb0
-    uint32x4_t spread = vmovl_u16(v);
-
-    // Get each color independently, still in 565 precison but down at bit 0.
-    auto r5 = vshrq_n_u32(spread, 11),
-         g6 = vandq_u32(vdupq_n_u32(63), vshrq_n_u32(spread, 5)),
-         b5 = vandq_u32(vdupq_n_u32(31), spread);
-
-    // Scale 565 precision up to 8-bit each, filling low 323 bits with high bits of each component.
-    auto r8 = vorrq_u32(vshlq_n_u32(r5, 3), vshrq_n_u32(r5, 2)),
-         g8 = vorrq_u32(vshlq_n_u32(g6, 2), vshrq_n_u32(g6, 4)),
-         b8 = vorrq_u32(vshrq_n_u32(b5, 3), vshrq_n_u32(b5, 2));
-
-    // Now put all the 8-bit components into SkPMColor order.
-    return (uint8x16_t)vorrq_u32(vshlq_n_u32(r8, SK_R32_SHIFT),   // TODO: one shift is zero...
-                       vorrq_u32(vshlq_n_u32(g8, SK_G32_SHIFT),
-                       vorrq_u32(vshlq_n_u32(b8, SK_B32_SHIFT),
-                                 vdupq_n_u32(0xFF << SK_A32_SHIFT))));
-}
-
-static inline uint16x4_t narrow_to_565(uint8x16_t w8x16) {
-    uint32x4_t w = (uint32x4_t)w8x16;
-
-    // Extract out top RGB 565 bits of each pixel, with no rounding.
-    auto r5 = vandq_u32(vdupq_n_u32(31), vshrq_n_u32(w, SK_R32_SHIFT + 3)),
-         g6 = vandq_u32(vdupq_n_u32(63), vshrq_n_u32(w, SK_G32_SHIFT + 2)),
-         b5 = vandq_u32(vdupq_n_u32(31), vshrq_n_u32(w, SK_B32_SHIFT + 3));
-
-    // Now put the bits in place in the low 16-bits of each 32-bit lane.
-    auto spread = vorrq_u32(vshlq_n_u32(r5, 11),
-                  vorrq_u32(vshlq_n_u32(g6,  5),
-                            b5));
-
-    // Pack the low 16-bits of our 128-bit register down into a 64-bit register.
-    // spread == 0000 rgb3  0000 rgb2  0000 rgb1  0000 rgb0
-    //      v ==                       rgb3 rgb2  rgb1 rgb0
-    auto v = vmovn_u32(spread);
-    return v;
-}
-
-
-inline Sk4px Sk4px::Load4(const SkPMColor16 src[4]) {
-    return Sk16b(widen_to_8888(vld1_u16(src)));
-}
-inline Sk4px Sk4px::Load2(const SkPMColor16 src[2]) {
-    auto src2 = ((uint32_t)src[0]      )
-              | ((uint32_t)src[1] << 16);
-    return Sk16b(widen_to_8888(vcreate_u16(src2)));
-}
-inline Sk4px Sk4px::Load1(const SkPMColor16 src[1]) {
-    return Sk16b(widen_to_8888(vcreate_u16(src[0])));
-}
-
-inline void Sk4px::store4(SkPMColor16 dst[4]) const {
-    vst1_u16(dst, narrow_to_565(this->fVec));
-}
-inline void Sk4px::store2(SkPMColor16 dst[2]) const {
-    auto v = narrow_to_565(this->fVec);
-    dst[0] = vget_lane_u16(v, 0);
-    dst[1] = vget_lane_u16(v, 1);
-}
-inline void Sk4px::store1(SkPMColor16 dst[1]) const {
-    dst[0] = vget_lane_u16(narrow_to_565(this->fVec), 0);
-}
-
 } // namespace
 
index 5e97abf..3809c5e 100644 (file)
@@ -93,79 +93,4 @@ inline Sk4px Sk4px::zeroAlphas() const {
     return Sk16b(_mm_andnot_si128(_mm_set1_epi32(0xFF << SK_A32_SHIFT), this->fVec));
 }
 
-static inline __m128i widen_low_half_to_8888(__m128i v) {
-    // RGB565 format:   |R....|G.....|B....|
-    //           Bit:  16    11      5     0
-
-    // First get each pixel into its own 32-bit lane.
-    //      v == ____ ____  ____ ____  rgb3 rgb2  rgb1 rgb0
-    // spread == 0000 rgb3  0000 rgb2  0000 rgb1  0000 rgb0
-    auto spread = _mm_unpacklo_epi16(v, _mm_setzero_si128());
-
-    // Get each color independently, still in 565 precison but down at bit 0.
-    auto r5 = _mm_srli_epi32(spread, 11),
-         g6 = _mm_and_si128(_mm_set1_epi32(63), _mm_srli_epi32(spread,  5)),
-         b5 = _mm_and_si128(_mm_set1_epi32(31), spread);
-
-    // Scale 565 precision up to 8-bit each, filling low 323 bits with high bits of each component.
-    auto r8 = _mm_or_si128(_mm_slli_epi32(r5, 3), _mm_srli_epi32(r5, 2)),
-         g8 = _mm_or_si128(_mm_slli_epi32(g6, 2), _mm_srli_epi32(g6, 4)),
-         b8 = _mm_or_si128(_mm_slli_epi32(b5, 3), _mm_srli_epi32(b5, 2));
-
-    // Now put all the 8-bit components into SkPMColor order.
-    return _mm_or_si128(_mm_slli_epi32(r8, SK_R32_SHIFT),   // TODO: one of these shifts is zero...
-           _mm_or_si128(_mm_slli_epi32(g8, SK_G32_SHIFT),
-           _mm_or_si128(_mm_slli_epi32(b8, SK_B32_SHIFT),
-                        _mm_set1_epi32(0xFF << SK_A32_SHIFT))));
-}
-
-static inline __m128i narrow_to_565(__m128i w) {
-    // Extract out top RGB 565 bits of each pixel, with no rounding.
-    auto r5 = _mm_and_si128(_mm_set1_epi32(31), _mm_srli_epi32(w, SK_R32_SHIFT + 3)),
-         g6 = _mm_and_si128(_mm_set1_epi32(63), _mm_srli_epi32(w, SK_G32_SHIFT + 2)),
-         b5 = _mm_and_si128(_mm_set1_epi32(31), _mm_srli_epi32(w, SK_B32_SHIFT + 3));
-
-    // Now put the bits in place in the low 16-bits of each 32-bit lane.
-    auto spread = _mm_or_si128(_mm_slli_epi32(r5, 11),
-                  _mm_or_si128(_mm_slli_epi32(g6,  5),
-                               b5));
-
-    // We want to pack the bottom 16-bits of spread down into the low half of the register, v.
-    // spread == 0000 rgb3  0000 rgb2  0000 rgb1  0000 rgb0
-    //      v == ____ ____  ____ ____  rgb3 rgb2  rgb1 rgb0
-
-    // Ideally now we'd use _mm_packus_epi32(spread, <anything>) to pack v.  But that's from SSE4.
-    // With only SSE2, we need to use _mm_packs_epi32.  That does signed saturation, and
-    // we need to preserve all 16 bits.  So we pretend our data is signed by sign-extending first.
-    // TODO: is it faster to just _mm_shuffle_epi8 this when we have SSSE3?
-    auto signExtended = _mm_srai_epi32(_mm_slli_epi32(spread, 16), 16);
-    auto v = _mm_packs_epi32(signExtended, signExtended);
-    return v;
-}
-
-inline Sk4px Sk4px::Load4(const SkPMColor16 src[4]) {
-    return Sk16b(widen_low_half_to_8888(_mm_loadl_epi64((const __m128i*)src)));
-}
-inline Sk4px Sk4px::Load2(const SkPMColor16 src[2]) {
-    auto src2 = ((uint32_t)src[0]      )
-              | ((uint32_t)src[1] << 16);
-    return Sk16b(widen_low_half_to_8888(_mm_cvtsi32_si128(src2)));
-}
-inline Sk4px Sk4px::Load1(const SkPMColor16 src[1]) {
-    return Sk16b(widen_low_half_to_8888(_mm_insert_epi16(_mm_setzero_si128(), src[0], 0)));
-}
-
-inline void Sk4px::store4(SkPMColor16 dst[4]) const {
-    _mm_storel_epi64((__m128i*)dst, narrow_to_565(this->fVec));
-}
-inline void Sk4px::store2(SkPMColor16 dst[2]) const {
-    uint32_t dst2 = _mm_cvtsi128_si32(narrow_to_565(this->fVec));
-    dst[0] = dst2;
-    dst[1] = dst2 >> 16;
-}
-inline void Sk4px::store1(SkPMColor16 dst[1]) const {
-    uint32_t dst2 = _mm_cvtsi128_si32(narrow_to_565(this->fVec));
-    dst[0] = dst2;
-}
-
 }  // namespace
index 540edb8..ba13e58 100644 (file)
@@ -100,35 +100,4 @@ inline Sk4px Sk4px::zeroColors() const {
                  0,0,0, this->kth<15>());
 }
 
-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