Pixel conversion refactors: use raster pipeline for 565 and gray

I'm trying not to do too much in one CL.  But, in general, I hope
to drop (non-performance important/optimized) special cases and
use the pipeline.

BUG=skia:

CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD

Change-Id: I724d3982f1467f6232371360b860484f13b1ede8
Reviewed-on: https://skia-review.googlesource.com/8271
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Matt Sarett <msarett@google.com>

diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp
index 10bed27..cae2eef 100644 (file)
--- a/src/core/SkConfig8888.cpp
+++ b/src/core/SkConfig8888.cpp
@@ -32,59 +32,44 @@ static inline bool can_memcpy(const SkImageInfo& dstInfo, const SkImageInfo& src
         return false;
     }
 
-    return !dstInfo.colorSpace() || !srcInfo.colorSpace() ||
+    return !dstInfo.colorSpace() ||
            SkColorSpace::Equals(dstInfo.colorSpace(), srcInfo.colorSpace());
 }
 
-// For now disable 565 in the pipeline. Its (higher) quality is so different its too much to
-// rebase (for now)
-//
-//#define PIPELINE_HANDLES_565
-
-static bool is_srgb(const SkImageInfo& info) {
-    return info.colorSpace() && info.colorSpace()->gammaCloseToSRGB();
-}
-
+// Default: Use the pipeline.
 static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_t dstRB,
                                  const SkImageInfo& srcInfo, const void* srcRow, size_t srcRB,
-                                 SkColorTable* ctable) {
-    SkASSERT(srcInfo.width() == dstInfo.width());
-    SkASSERT(srcInfo.height() == dstInfo.height());
-
-    bool src_srgb = is_srgb(srcInfo);
-    const bool dst_srgb = is_srgb(dstInfo);
-    if (!dstInfo.colorSpace()) {
-        src_srgb = false;   // untagged dst means ignore tags on src
-    }
-
+                                 bool isColorAware) {
     SkRasterPipeline pipeline;
-
     switch (srcInfo.colorType()) {
         case kRGBA_8888_SkColorType:
+            pipeline.append(SkRasterPipeline::load_8888, &srcRow);
+            break;
         case kBGRA_8888_SkColorType:
             pipeline.append(SkRasterPipeline::load_8888, &srcRow);
-            if (src_srgb) {
-                pipeline.append_from_srgb(srcInfo.alphaType());
-            }
-            if (kBGRA_8888_SkColorType == srcInfo.colorType()) {
-                pipeline.append(SkRasterPipeline::swap_rb);
-            }
+            pipeline.append(SkRasterPipeline::swap_rb);
             break;
-#ifdef PIPELINE_HANDLES_565
         case kRGB_565_SkColorType:
             pipeline.append(SkRasterPipeline::load_565, &srcRow);
             break;
-#endif
         case kRGBA_F16_SkColorType:
             pipeline.append(SkRasterPipeline::load_f16, &srcRow);
             break;
+        case kGray_8_SkColorType:
+            pipeline.append(SkRasterPipeline::load_g8, &srcRow);
+            break;
         default:
-            return false;   // src colortype unsupported
+            return false;
+    }
+
+    if (isColorAware && srcInfo.gammaCloseToSRGB()) {
+        pipeline.append_from_srgb(srcInfo.alphaType());
     }
 
     float matrix[12];
-    if (!append_gamut_transform(&pipeline, matrix, srcInfo.colorSpace(), dstInfo.colorSpace())) {
-        return false;
+    if (isColorAware) {
+        SkAssertResult(append_gamut_transform(&pipeline, matrix, srcInfo.colorSpace(),
+                                              dstInfo.colorSpace()));
     }
 
     SkAlphaType sat = srcInfo.alphaType();
@@ -95,27 +80,26 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_
         pipeline.append(SkRasterPipeline::premul);
     }
 
+    if (isColorAware && dstInfo.gammaCloseToSRGB()) {
+        pipeline.append(SkRasterPipeline::to_srgb);
+    }
+
     switch (dstInfo.colorType()) {
         case kRGBA_8888_SkColorType:
+            pipeline.append(SkRasterPipeline::store_8888, &dstRow);
+            break;
         case kBGRA_8888_SkColorType:
-            if (kBGRA_8888_SkColorType == dstInfo.colorType()) {
-                pipeline.append(SkRasterPipeline::swap_rb);
-            }
-            if (dst_srgb) {
-                pipeline.append(SkRasterPipeline::to_srgb);
-            }
+            pipeline.append(SkRasterPipeline::swap_rb);
             pipeline.append(SkRasterPipeline::store_8888, &dstRow);
             break;
-#ifdef PIPELINE_HANDLES_565
         case kRGB_565_SkColorType:
             pipeline.append(SkRasterPipeline::store_565, &dstRow);
             break;
-#endif
         case kRGBA_F16_SkColorType:
             pipeline.append(SkRasterPipeline::store_f16, &dstRow);
             break;
         default:
-            return false;   // dst colortype unsupported
+            return false;
     }
 
     auto p = pipeline.compile();
@@ -124,8 +108,8 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_
         p(0,srcInfo.width());
         // The pipeline has pointers to srcRow and dstRow, so we just need to update them in the
         // loop to move between rows of src/dst.
-        srcRow = (const char*)srcRow + srcRB;
-        dstRow = (char*)dstRow + dstRB;
+        dstRow = SkTAddOffset<void>(dstRow, dstRB);
+        srcRow = SkTAddOffset<const void>(srcRow, srcRB);
     }
     return true;
 }
@@ -231,19 +215,6 @@ bool SkSrcPixelInfo::convertPixelsTo(SkDstPixelInfo* dst, int width, int height)
     return true;
 }
 
-static void copy_g8_to_32(void* dst, size_t dstRB, const void* src, size_t srcRB, int w, int h) {
-    uint32_t* dst32 = (uint32_t*)dst;
-    const uint8_t* src8 = (const uint8_t*)src;
-
-    for (int y = 0; y < h; ++y) {
-        for (int x = 0; x < w; ++x) {
-            dst32[x] = SkPackARGB32(0xFF, src8[x], src8[x], src8[x]);
-        }
-        dst32 = (uint32_t*)((char*)dst32 + dstRB);
-        src8 += srcRB;
-    }
-}
-
 static bool extract_alpha(void* dst, size_t dstRB, const void* src, size_t srcRB,
                           const SkImageInfo& srcInfo, SkColorTable* ctable) {
     uint8_t* SK_RESTRICT dst8 = (uint8_t*)dst;
@@ -384,7 +355,8 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
         return true;
     }
 
-    const bool isColorAware = srcInfo.colorSpace() && dstInfo.colorSpace();
+    const bool isColorAware = dstInfo.colorSpace();
+    SkASSERT(srcInfo.colorSpace() || !isColorAware);
 
     // Handle fancy alpha swizzling if both are ARGB32
     if (4 == srcInfo.bytesPerPixel() && 4 == dstInfo.bytesPerPixel() && !isColorAware) {
@@ -413,11 +385,6 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
      *  are supported.
      */
 
-    if (kGray_8_SkColorType == srcInfo.colorType() && 4 == dstInfo.bytesPerPixel()) {
-        copy_g8_to_32(dstPixels, dstRB, srcPixels, srcRB, width, height);
-        return true;
-    }
-
     if (kAlpha_8_SkColorType == dstInfo.colorType() &&
         extract_alpha(dstPixels, dstRB, srcPixels, srcRB, srcInfo, ctable)) {
         return true;

diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h
index a3e449a..81d7070 100644 (file)
--- a/src/core/SkImageInfoPriv.h
+++ b/src/core/SkImageInfoPriv.h
@@ -39,10 +39,11 @@ static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
 /**
  *  Returns true if Skia has defined a pixel conversion from the |src| to the |dst|.
  *  Returns false otherwise.  Some discussion of false cases:
- *      We will not convert to kIndex8 when the |src| is not kIndex8.
- *      We do not convert to kGray8 when the |src| is not kGray8.  We may add this
- *      feature - it just requires some work to convert to luminance while handling color
- *      spaces correctly.  Currently no one is asking for this.
+ *      We will not convert to kIndex8 when the |src| is not kIndex8 in the same color space
+ *      (color tables are immutable).
+ *      We do not convert to kGray8 when the |src| is not kGray8 in the same color space.
+ *      We may add this feature - it just requires some work to convert to luminance while
+ *      handling color spaces correctly.  Currently no one is asking for this.
  *      We will not convert from kAlpha8 when the |dst| is not kAlpha8.  This would require
  *      inventing color information.
  *      We will not convert to kOpaque when the |src| is not kOpaque.  This could be
@@ -58,11 +59,15 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm
         return false;
     }
 
-    if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType()) {
+    if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType() &&
+        dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace()))
+    {
         return false;
     }
 
-    if (kGray_8_SkColorType == dst.colorType() && kGray_8_SkColorType != src.colorType()) {
+    if (kGray_8_SkColorType == dst.colorType() && kGray_8_SkColorType != src.colorType() &&
+        dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace()))
+    {
         return false;
     }
 

diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h
index 2dd2c32..e507b28 100644 (file)
--- a/src/core/SkRasterPipeline.h
+++ b/src/core/SkRasterPipeline.h
@@ -66,6 +66,7 @@
     M(from_2dot2) M(to_2dot2)                                    \
     M(constant_color) M(seed_shader) M(store_f32)                \
     M(load_a8)   M(store_a8)                                     \
+    M(load_g8)                                                   \
     M(load_565)  M(store_565)                                    \
     M(load_f16)  M(store_f16)                                    \
     M(load_8888) M(store_8888)                                   \

diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h
index dec81d9..4f01fd5 100644 (file)
--- a/src/opts/SkRasterPipeline_opts.h
+++ b/src/opts/SkRasterPipeline_opts.h
@@ -520,6 +520,12 @@ STAGE_CTX(store_a8, uint8_t**) {
     store(tail, SkNx_cast<uint8_t>(SkNf_round(255.0f, a)), ptr);
 }
 
+STAGE_CTX(load_g8, const uint8_t**) {
+    auto ptr = *ctx + x;
+    r = g = b = SkNf_from_byte(load(tail, ptr));
+    a = 1.0f;
+}
+
 STAGE_CTX(load_565, const uint16_t**) {
     auto ptr = *ctx + x;
     from_565(load(tail, ptr), &r,&g,&b);