Cleaning up SkColorSpaceXform a bit

Remove SkColorSpaceXform_base from the inheritance chain. Move some
enums only used within XYZ to be class scoped. Eliminate redundant
context structs, and just use the SkJumper types directly.

SkColorSpaceXform_Base still exists, but just to hold a couple static
functions. Trying to untangle the dst gamma table mess next.

Bug: skia:
Change-Id: I6d2b7807c33e61a0d7df74e334356567d8a2c0e0
Reviewed-on: https://skia-review.googlesource.com/112601
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>

7 files changed, 60 insertions, 86 deletions

diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index 5090e9b636..7c53ed52dc 100644
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -404,10 +404,9 @@ SkColorSpaceXform_XYZ::SkColorSpaceXform_XYZ(SkColorSpace_XYZ* src,
     }
 }
 
-
-bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
-                                    ColorFormat srcColorFormat, const void* src,
-                                    int len, SkAlphaType alphaType) const {
+bool SkColorSpaceXform_XYZ::apply(ColorFormat dstColorFormat, void* dst,
+                                  ColorFormat srcColorFormat, const void* src,
+                                  int len, SkAlphaType alphaType) const {
     if (fColorSpacesAreIdentical && kPremul_SkAlphaType != alphaType) {
         if ((kRGBA_8888_ColorFormat == dstColorFormat &&
              kRGBA_8888_ColorFormat == srcColorFormat) ||
@@ -433,16 +432,16 @@ bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
     SkJumper_MemoryCtx src_ctx = { (void*)src, 0 },
                        dst_ctx = { (void*)dst, 0 };
 
-    LoadTablesContext loadTables;
+    SkJumper_LoadTablesCtx loadTables;
     switch (srcColorFormat) {
         case kRGBA_8888_ColorFormat:
             if (kLinear_SrcGamma == fSrcGamma) {
                 pipeline.append(SkRasterPipeline::load_8888, &src_ctx);
             } else {
-                loadTables.fSrc = src;
-                loadTables.fR = fSrcGammaTables[0];
-                loadTables.fG = fSrcGammaTables[1];
-                loadTables.fB = fSrcGammaTables[2];
+                loadTables.src = src;
+                loadTables.r = fSrcGammaTables[0];
+                loadTables.g = fSrcGammaTables[1];
+                loadTables.b = fSrcGammaTables[2];
                 pipeline.append(SkRasterPipeline::load_tables, &loadTables);
             }
 
@@ -451,10 +450,10 @@ bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
             if (kLinear_SrcGamma == fSrcGamma) {
                 pipeline.append(SkRasterPipeline::load_bgra, &src_ctx);
             } else {
-                loadTables.fSrc = src;
-                loadTables.fR = fSrcGammaTables[2];
-                loadTables.fG = fSrcGammaTables[1];
-                loadTables.fB = fSrcGammaTables[0];
+                loadTables.src = src;
+                loadTables.r = fSrcGammaTables[2];
+                loadTables.g = fSrcGammaTables[1];
+                loadTables.b = fSrcGammaTables[0];
                 pipeline.append(SkRasterPipeline::load_tables, &loadTables);
                 pipeline.append(SkRasterPipeline::swap_rb);
             }
@@ -482,10 +481,10 @@ bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
                     pipeline.append(SkRasterPipeline::from_srgb);
                     break;
                 case kTable_SrcGamma:
-                    loadTables.fSrc = src;
-                    loadTables.fR = fSrcGammaTables[0];
-                    loadTables.fG = fSrcGammaTables[1];
-                    loadTables.fB = fSrcGammaTables[2];
+                    loadTables.src = src;
+                    loadTables.r = fSrcGammaTables[0];
+                    loadTables.g = fSrcGammaTables[1];
+                    loadTables.b = fSrcGammaTables[2];
                     pipeline.append(SkRasterPipeline::load_tables_u16_be, &loadTables);
                     break;
             }
@@ -500,10 +499,10 @@ bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
                     pipeline.append(SkRasterPipeline::from_srgb);
                     break;
                 case kTable_SrcGamma:
-                    loadTables.fSrc = src;
-                    loadTables.fR = fSrcGammaTables[0];
-                    loadTables.fG = fSrcGammaTables[1];
-                    loadTables.fB = fSrcGammaTables[2];
+                    loadTables.src = src;
+                    loadTables.r = fSrcGammaTables[0];
+                    loadTables.g = fSrcGammaTables[1];
+                    loadTables.b = fSrcGammaTables[2];
                     pipeline.append(SkRasterPipeline::load_tables_rgb_u16_be, &loadTables);
                     break;
             }
@@ -531,7 +530,7 @@ bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
         pipeline.append(SkRasterPipeline::premul);
     }
 
-    TablesContext tables;
+    SkJumper_ByteTablesRGBCtx tables;
     float to_2dot2 = 1/2.2f;
     switch (fDstGamma) {
         case kSRGB_DstGamma:
@@ -541,10 +540,10 @@ bool SkColorSpaceXform_XYZ::onApply(ColorFormat dstColorFormat, void* dst,
             pipeline.append(SkRasterPipeline::gamma, &to_2dot2);
             break;
         case kTable_DstGamma:
-            tables.fR = fDstGammaTables[0];
-            tables.fG = fDstGammaTables[1];
-            tables.fB = fDstGammaTables[2];
-            tables.fCount = SkColorSpaceXform_Base::kDstGammaTableSize;
+            tables.r = fDstGammaTables[0];
+            tables.g = fDstGammaTables[1];
+            tables.b = fDstGammaTables[2];
+            tables.n = SkColorSpaceXform_Base::kDstGammaTableSize;
             pipeline.append(SkRasterPipeline::byte_tables_rgb, &tables);
         default:
             break;
@@ -594,14 +593,6 @@ std::unique_ptr<SkColorSpaceXform> SlowIdentityXform(SkColorSpace_XYZ* space) {
     return std::move(xform);
 }
 
-bool SkColorSpaceXform::apply(ColorFormat dstColorFormat, void* dst,
-                              ColorFormat srcColorFormat, const void* src,
-                              int len, SkAlphaType alphaType) const {
-    return ((SkColorSpaceXform_Base*) this)->onApply(dstColorFormat, dst,
-                                                     srcColorFormat, src,
-                                                     len, alphaType);
-}
-
 bool SkColorSpaceXform::Apply(SkColorSpace* dstCS, ColorFormat dstFormat, void* dst,
                               SkColorSpace* srcCS, ColorFormat srcFormat, const void* src,
                               int len, AlphaOp op) {
diff --git a/src/core/SkColorSpaceXform_A2B.cpp b/src/core/SkColorSpaceXform_A2B.cpp
index 9470247a8d..1e57b5d0d3 100644
--- a/src/core/SkColorSpaceXform_A2B.cpp
+++ b/src/core/SkColorSpaceXform_A2B.cpp
@@ -18,8 +18,8 @@
 #include "SkTypes.h"
 #include "../jumper/SkJumper.h"
 
-bool SkColorSpaceXform_A2B::onApply(ColorFormat dstFormat, void* dst, ColorFormat srcFormat,
-                                    const void* src, int count, SkAlphaType alphaType) const {
+bool SkColorSpaceXform_A2B::apply(ColorFormat dstFormat, void* dst, ColorFormat srcFormat,
+                                  const void* src, int count, SkAlphaType alphaType) const {
     SkRasterPipeline_<256> pipeline;
 
     SkJumper_MemoryCtx src_ctx = { (void*)src, 0 },
diff --git a/src/core/SkColorSpaceXform_A2B.h b/src/core/SkColorSpaceXform_A2B.h
index c04915da1b..65fbbfd052 100644
--- a/src/core/SkColorSpaceXform_A2B.h
+++ b/src/core/SkColorSpaceXform_A2B.h
@@ -9,7 +9,7 @@
 #define SkColorSpaceXform_A2B_DEFINED
 
 #include "SkArenaAlloc.h"
-#include "SkColorSpaceXform_Base.h"
+#include "SkColorSpaceXform.h"
 #include "SkRasterPipeline.h"
 
 class SkColorSpace_A2B;
@@ -20,12 +20,12 @@ struct SkTableTransferFn {
     int          fSize;
 };
 
-class SkColorSpaceXform_A2B : public SkColorSpaceXform_Base {
+class SkColorSpaceXform_A2B : public SkColorSpaceXform {
 public:
     SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace, SkColorSpace_XYZ* dstSpace);
 
-    bool onApply(ColorFormat dstFormat, void* dst, ColorFormat srcFormat, const void* src,
-                 int count, SkAlphaType alphaType) const override;
+    bool apply(ColorFormat dstFormat, void* dst, ColorFormat srcFormat, const void* src,
+               int count, SkAlphaType alphaType) const override;
 
 private:
     void addTransferFn(const SkColorSpaceTransferFn& fn, int channelIndex);
diff --git a/src/core/SkColorSpaceXform_Base.h b/src/core/SkColorSpaceXform_Base.h
index f98b77fe2b..f931cde8e0 100644
--- a/src/core/SkColorSpaceXform_Base.h
+++ b/src/core/SkColorSpaceXform_Base.h
@@ -10,11 +10,11 @@
 
 #include "SkColorSpace.h"
 #include "SkColorSpaceXform.h"
-#include "SkResourceCache.h"
+#include "SkTemplates.h"
 
 class SkColorSpace_XYZ;
 
-class SkColorSpaceXform_Base : public SkColorSpaceXform {
+class SkColorSpaceXform_Base {
 public:
     // A somewhat more powerful SkColorSpaceXform::New() that allows tweaking premulBehavior.
     static std::unique_ptr<SkColorSpaceXform> New(SkColorSpace* srcSpace,
@@ -26,37 +26,34 @@ public:
                                     uint8_t* gammaTableStorage,
                                     const SkColorSpace_XYZ* space,
                                     bool gammasAreMatching);
-
-    virtual bool onApply(ColorFormat dstFormat, void* dst, ColorFormat srcFormat, const void* src,
-                         int count, SkAlphaType alphaType) const = 0;
-};
-
-enum SrcGamma {
-    kLinear_SrcGamma,
-    kTable_SrcGamma,
-    kSRGB_SrcGamma,
-};
-
-enum DstGamma {
-    kLinear_DstGamma,
-    kSRGB_DstGamma,
-    k2Dot2_DstGamma,
-    kTable_DstGamma,
 };
 
-class SkColorSpaceXform_XYZ : public SkColorSpaceXform_Base {
+class SkColorSpaceXform_XYZ : public SkColorSpaceXform {
 public:
     SkColorSpaceXform_XYZ(SkColorSpace_XYZ* src, SkColorSpace_XYZ* dst, SkTransferFunctionBehavior);
 
-    bool onApply(ColorFormat dstFormat, void* dst,
-                 ColorFormat srcFormat, const void* src,
-                 int count, SkAlphaType alphaType) const override;
+    bool apply(ColorFormat dstFormat, void* dst,
+               ColorFormat srcFormat, const void* src,
+               int count, SkAlphaType alphaType) const override;
 
     void pretendNotToBeIdentityForTesting() {
         fSrcToDstIsIdentity = false;
     }
 
 private:
+    enum SrcGamma {
+        kLinear_SrcGamma,
+        kTable_SrcGamma,
+        kSRGB_SrcGamma,
+    };
+
+    enum DstGamma {
+        kLinear_DstGamma,
+        kSRGB_DstGamma,
+        k2Dot2_DstGamma,
+        kTable_DstGamma,
+    };
+
     // These tables pointers may point into fSrcStorage/fDstStorage or into pre-baked tables.
     const float*               fSrcGammaTables[3];
     const uint8_t*             fDstGammaTables[3];
@@ -71,21 +68,6 @@ private:
     SkTransferFunctionBehavior fPremulBehavior;
 };
 
-struct LoadTablesContext {
-    const void*  fSrc;
-    const float* fR;
-    const float* fG;
-    const float* fB;
-};
-
-// Must be kept in sync with "Tables" struct in RasterPipeline_opts byte_tables_rgb.
-struct TablesContext {
-    const uint8_t* fR;
-    const uint8_t* fG;
-    const uint8_t* fB;
-    int            fCount;
-};
-
 // For testing.  Bypasses opts for when src and dst color spaces are equal.
 std::unique_ptr<SkColorSpaceXform> SlowIdentityXform(SkColorSpace_XYZ* space);
 
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index a92b751140..3ed3a8aea4 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -9,7 +9,6 @@
 #include "SkBitmap.h"
 #include "SkBitmapProcShader.h"
 #include "SkCanvas.h"
-#include "SkColorSpaceXform_Base.h"
 #include "SkColorSpaceXformPriv.h"
 #include "SkColorTable.h"
 #include "SkConvertPixels.h"
diff --git a/src/jumper/SkJumper.h b/src/jumper/SkJumper.h
index 1429458d4a..e7bf9b2403 100644
--- a/src/jumper/SkJumper.h
+++ b/src/jumper/SkJumper.h
@@ -94,6 +94,11 @@ struct SkJumper_TableCtx {
     int          size;
 };
 
+struct SkJumper_ByteTablesRGBCtx {
+    const uint8_t *r, *g, *b;
+    int n;
+};
+
 // This should line up with the memory layout of SkColorSpaceTransferFn.
 struct SkJumper_ParametricTransferFunction {
     float G, A,B,C,D,E,F;
diff --git a/src/jumper/SkJumper_stages.cpp b/src/jumper/SkJumper_stages.cpp
index 19b6cd694a..5dd6690c11 100644
--- a/src/jumper/SkJumper_stages.cpp
+++ b/src/jumper/SkJumper_stages.cpp
@@ -1504,14 +1504,11 @@ STAGE(byte_tables, const void* ctx) {  // TODO: rename Tables SkJumper_ByteTable
     a = from_byte(gather(tables->a, to_unorm(a, 255)));
 }
 
-STAGE(byte_tables_rgb, const void* ctx) {  // TODO: rename Tables SkJumper_ByteTablesRGBCtx
-    struct Tables { const uint8_t *r, *g, *b; int n; };
-    auto tables = (const Tables*)ctx;
-
-    int scale = tables->n - 1;
-    r = from_byte(gather(tables->r, to_unorm(r, scale)));
-    g = from_byte(gather(tables->g, to_unorm(g, scale)));
-    b = from_byte(gather(tables->b, to_unorm(b, scale)));
+STAGE(byte_tables_rgb, const SkJumper_ByteTablesRGBCtx* ctx) {
+    int scale = ctx->n - 1;
+    r = from_byte(gather(ctx->r, to_unorm(r, scale)));
+    g = from_byte(gather(ctx->g, to_unorm(g, scale)));
+    b = from_byte(gather(ctx->b, to_unorm(b, scale)));
 }
 
 SI F table(F v, const SkJumper_TableCtx* ctx) {