Enable legacy premuls in SkColorSpaceXform

***Will allow for simplified Android framework code, they typically
   want a color correct transform followed by a gamma encoded premul.
***Chrome does the same, so this will make it easier to replace their
   codecs.
***Will decrease code size.  Both types of premuls are moved off the
   fast path here - one is essentially unused in production and the
   other is not "encouraged".
***Will actually make the common case faster: sRGB->sRGB means no
   color xform, just premul in SkSwizzler.

BUG=skia:

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

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

src/codec/SkCodec.cpp

@@ -474,17 +474,7 @@ void SkCodec::fillIncompleteImage(const SkImageInfo& info, void* dst, size_t row
     }
 }
 
-bool SkCodec::initializeColorXform(const SkImageInfo& info) {
+bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo) {
-    // TODO (msarett):
-    // Handle equality checking and legacy behavior for flagged SkColorSpaces.
-    // Until this is implemented, remove any flags on output color spaces.  This
-    // will prevent strange behaviors.  Ex: sRGB != sRGB + flag, but we don't want
-    // this to trigger a color xform.
-    SkImageInfo dstInfo = info;
-    if (dstInfo.colorSpace()) {
-        dstInfo = info.makeColorSpace(as_CSB(dstInfo.colorSpace())->makeWithoutFlags());
-    }
-
     fColorXform = nullptr;
     bool needsPremul = needs_premul(dstInfo, fEncodedInfo);
     if (needs_color_xform(dstInfo, fSrcInfo, needsPremul)) {

src/codec/SkCodecPriv.h

@@ -302,16 +302,23 @@ static inline bool needs_premul(const SkImageInfo& dstInfo, const SkEncodedInfo&
 
 static inline bool needs_color_xform(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo,
                                      bool needsPremul) {
+    // We never perform a color xform in legacy mode.
+    if (!dstInfo.colorSpace()) {
+        return false;
+    }
+
     // F16 is by definition a linear space, so we always must perform a color xform.
     bool isF16 = kRGBA_F16_SkColorType == dstInfo.colorType();
 
     // Need a color xform when dst space does not match the src.
-    bool srcDstNotEqual = !SkColorSpace::Equals(srcInfo.colorSpace(), dstInfo.colorSpace());
+    bool srcDstNotEqual =
+            !SkColorSpace_Base::EqualsIgnoreFlags(srcInfo.colorSpace(), dstInfo.colorSpace());
 
-    // We never perform a color xform in legacy mode.
+    // We provide the option for both legacy premuls and color correct premuls.
-    bool isLegacy = nullptr == dstInfo.colorSpace();
+    bool needsColorCorrectPremul =
+            needsPremul && !as_CSB(dstInfo.colorSpace())->nonLinearBlending();
 
-    return !isLegacy && (needsPremul || isF16 || srcDstNotEqual);
+    return needsColorCorrectPremul || isF16 || srcDstNotEqual;
 }
 
 static inline SkAlphaType select_xform_alpha(SkAlphaType dstAlphaType, SkAlphaType srcAlphaType) {

src/core/SkColorSpace.cpp

@@ -554,6 +554,11 @@ sk_sp<SkColorSpace> SkColorSpace::Deserialize(const void* data, size_t length) {
     }
 }
 
+bool SkColorSpace_Base::EqualsIgnoreFlags(SkColorSpace* src, SkColorSpace* dst) {
+    return SkColorSpace::Equals(as_CSB(src)->makeWithoutFlags().get(),
+                                as_CSB(dst)->makeWithoutFlags().get());
+}
+
 bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
     if (src == dst) {
         return true;

src/core/SkColorSpaceXform.cpp

@@ -312,7 +312,7 @@ std::unique_ptr<SkColorSpaceXform> SkColorSpaceXform::New(SkColorSpace* srcSpace
 
     ColorSpaceMatch csm = kNone_ColorSpaceMatch;
     SkMatrix44 srcToDst(SkMatrix44::kUninitialized_Constructor);
-    if (SkColorSpace::Equals(srcSpace, dstSpace)) {
+    if (SkColorSpace_Base::EqualsIgnoreFlags(srcSpace, dstSpace)) {
         srcToDst.setIdentity();
         csm = kFull_ColorSpaceMatch;
     } else {
@@ -505,26 +505,6 @@ static AI void translate_gamut_1(const Sk4f& rTgTbT, Sk4f& rgba) {
     rgba = rgba + rTgTbT;
 }
 
-static AI void premultiply(Sk4f& dr, Sk4f& dg, Sk4f& db, const Sk4f& da, bool kClamp) {
-    if (kClamp) {
-        dr = Sk4f::Min(dr, 1.0f);
-        dg = Sk4f::Min(dg, 1.0f);
-        db = Sk4f::Min(db, 1.0f);
-    }
-
-    dr = da * dr;
-    dg = da * dg;
-    db = da * db;
-}
-
-static AI void premultiply_1(const Sk4f& a, Sk4f& rgba, bool kClamp) {
-    if (kClamp) {
-        rgba = Sk4f::Min(rgba, 1.0f);
-    }
-
-    rgba = a * rgba;
-}
-
 template <Order kOrder>
 static AI void store_srgb(void* dst, const uint32_t* src, Sk4f& dr, Sk4f& dg, Sk4f& db, Sk4f&,
                           const uint8_t* const[3]) {
@@ -765,8 +745,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
                              const uint8_t* const dstTables[3]) {
     LoadFn load;
     Load1Fn load_1;
-    const bool kLoadAlpha = (kPremul_SkAlphaType == kAlphaType) ||
+    const bool kLoadAlpha = kF16_Linear_DstFormat == kDst && kOpaque_SkAlphaType != kAlphaType;
-                            (kF16_Linear_DstFormat == kDst && kOpaque_SkAlphaType != kAlphaType);
     switch (kSrc) {
         case kRGBA_8888_Linear_SrcFormat:
             if (kLoadAlpha) {
@@ -859,14 +838,6 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
             break;
     }
 
-    // We always clamp before converting to 8888 outputs (because we have to).
-    // In these cases, we also need a clamp before the premul step to make sure
-    // R, G, B are not logically greater than A.
-    const bool kClamp = kRGBA_8888_Linear_DstFormat == kDst ||
-                        kRGBA_8888_SRGB_DstFormat == kDst ||
-                        kRGBA_8888_2Dot2_DstFormat == kDst ||
-                        kRGBA_8888_Table_DstFormat == kDst;
-
     const uint32_t* src = (const uint32_t*) vsrc;
     Sk4f rXgXbX, rYgYbY, rZgZbZ, rTgTbT;
     load_matrix(matrix, rXgXbX, rYgYbY, rZgZbZ, rTgTbT);
@@ -891,10 +862,6 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
                 da = a;
             }
 
-            if (kPremul_SkAlphaType == kAlphaType) {
-                premultiply(dr, dg, db, da, kClamp);
-            }
-
             load(src, r, g, b, a, srcTables);
 
             store(dst, src - 4, dr, dg, db, da, dstTables);
@@ -913,10 +880,6 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
             da = a;
         }
 
-        if (kPremul_SkAlphaType == kAlphaType) {
-            premultiply(dr, dg, db, da, kClamp);
-        }
-
         store(dst, src - 4, dr, dg, db, da, dstTables);
         dst = SkTAddOffset<void>(dst, 4 * sizeOfDstPixel);
     }
@@ -933,10 +896,6 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
             rgba = Sk4f(r[0], g[0], b[0], a[0]);
         }
 
-        if (kPremul_SkAlphaType == kAlphaType) {
-            premultiply_1(a, rgba, kClamp);
-        }
-
         store_1(dst, src, rgba, a, dstTables);
 
         src += 1;
@@ -973,6 +932,7 @@ template <ColorSpaceMatch kCSM>
 SkColorSpaceXform_XYZ<kCSM>
 ::SkColorSpaceXform_XYZ(SkColorSpace_XYZ* srcSpace, const SkMatrix44& srcToDst,
                         SkColorSpace_XYZ* dstSpace)
+    : fLinearBlending(!dstSpace->nonLinearBlending())
 {
     fSrcToDst[ 0] = srcToDst.get(0, 0);
     fSrcToDst[ 1] = srcToDst.get(1, 0);
@@ -1033,10 +993,6 @@ static AI bool apply_set_alpha(void* dst, const void* src, int len, SkAlphaType
             color_xform_RGBA<kSrc, kDst, kOpaque_SkAlphaType, kCSM>
                     (dst, src, len, srcTables, matrix, dstTables);
             return true;
-        case kPremul_SkAlphaType:
-            color_xform_RGBA<kSrc, kDst, kPremul_SkAlphaType, kCSM>
-                    (dst, src, len, srcTables, matrix, dstTables);
-            return true;
         case kUnpremul_SkAlphaType:
             color_xform_RGBA<kSrc, kDst, kUnpremul_SkAlphaType, kCSM>
                     (dst, src, len, srcTables, matrix, dstTables);
@@ -1106,7 +1062,8 @@ bool SkColorSpaceXform_XYZ<kCSM>
 
     if (kRGBA_F32_ColorFormat == dstColorFormat ||
         kRGBA_U16_BE_ColorFormat == srcColorFormat ||
-        kRGB_U16_BE_ColorFormat == srcColorFormat)
+        kRGB_U16_BE_ColorFormat == srcColorFormat ||
+        kPremul_SkAlphaType == alphaType)
     {
         return this->applyPipeline(dstColorFormat, dst, srcColorFormat, src, len, alphaType);
     }
@@ -1258,11 +1215,11 @@ bool SkColorSpaceXform_XYZ<kCSM>
         }
     }
 
-    if (kPremul_SkAlphaType == alphaType) {
+    if (kPremul_SkAlphaType == alphaType && fLinearBlending) {
         pipeline.append(SkRasterPipeline::premul);
     }
 
-    StoreTablesContext storeTables;
+    TablesContext tables;
     switch (fDstGamma) {
         case kSRGB_DstGamma:
             pipeline.append(SkRasterPipeline::to_srgb);
@@ -1270,45 +1227,36 @@ bool SkColorSpaceXform_XYZ<kCSM>
         case k2Dot2_DstGamma:
             pipeline.append(SkRasterPipeline::to_2dot2);
             break;
+        case kTable_DstGamma:
+            tables.fR = fDstGammaTables[0];
+            tables.fG = fDstGammaTables[1];
+            tables.fB = fDstGammaTables[2];
+            tables.fCount = SkColorSpaceXform_Base::kDstGammaTableSize;
+            pipeline.append(SkRasterPipeline::byte_tables_rgb, &tables);
         default:
             break;
     }
 
+    if (kPremul_SkAlphaType == alphaType && !fLinearBlending) {
+        pipeline.append(SkRasterPipeline::premul);
+    }
+
     switch (dstColorFormat) {
         case kRGBA_8888_ColorFormat:
-            if (kTable_DstGamma == fDstGamma) {
+             pipeline.append(SkRasterPipeline::store_8888, &dst);
-                storeTables.fDst = (uint32_t*) dst;
-                storeTables.fR = fDstGammaTables[0];
-                storeTables.fG = fDstGammaTables[1];
-                storeTables.fB = fDstGammaTables[2];
-                storeTables.fCount = SkColorSpaceXform_Base::kDstGammaTableSize;
-                pipeline.append(SkRasterPipeline::store_tables, &storeTables);
-            } else {
-                pipeline.append(SkRasterPipeline::store_8888, &dst);
-            }
             break;
         case kBGRA_8888_ColorFormat:
-            if (kTable_DstGamma == fDstGamma) {
+            pipeline.append(SkRasterPipeline::swap_rb);
-                storeTables.fDst = (uint32_t*) dst;
+            pipeline.append(SkRasterPipeline::store_8888, &dst);
-                storeTables.fR = fDstGammaTables[2];
-                storeTables.fG = fDstGammaTables[1];
-                storeTables.fB = fDstGammaTables[0];
-                storeTables.fCount = SkColorSpaceXform_Base::kDstGammaTableSize;
-                pipeline.append(SkRasterPipeline::swap_rb);
-                pipeline.append(SkRasterPipeline::store_tables, &storeTables);
-            } else {
-                pipeline.append(SkRasterPipeline::swap_rb);
-                pipeline.append(SkRasterPipeline::store_8888, &dst);
-            }
             break;
         case kRGBA_F16_ColorFormat:
-            if (kLinear_DstGamma != fDstGamma) {
+            if (kLinear_DstGamma != fDstGamma || !fLinearBlending) {
                 return false;
             }
             pipeline.append(SkRasterPipeline::store_f16, &dst);
             break;
         case kRGBA_F32_ColorFormat:
-            if (kLinear_DstGamma != fDstGamma) {
+            if (kLinear_DstGamma != fDstGamma || !fLinearBlending) {
                 return false;
             }
             pipeline.append(SkRasterPipeline::store_f32, &dst);

src/core/SkColorSpaceXform_Base.h

@@ -74,6 +74,7 @@ private:
 
     SrcGamma             fSrcGamma;
     DstGamma             fDstGamma;
+    bool                 fLinearBlending;
 
     friend class SkColorSpaceXform;
     friend std::unique_ptr<SkColorSpaceXform> SlowIdentityXform(SkColorSpace_XYZ* space);
@@ -86,8 +87,8 @@ struct LoadTablesContext {
     const float* fB;
 };
 
-struct StoreTablesContext {
+// Must be kept in sync with "Tables" struct in RasterPipeline_opts byte_tables_rgb.
-    uint32_t*      fDst;
+struct TablesContext {
     const uint8_t* fR;
     const uint8_t* fG;
     const uint8_t* fB;

src/core/SkColorSpace_Base.h

@@ -207,6 +207,8 @@ public:
 
     static sk_sp<SkColorSpace> MakeNamed(Named);
 
+    static bool EqualsIgnoreFlags(SkColorSpace* src, SkColorSpace* dst);
+
 protected:
     SkColorSpace_Base(sk_sp<SkData> profileData, uint32_t flags);
 

src/core/SkRasterPipeline.h

@@ -72,7 +72,7 @@
     M(load_8888) M(store_8888)                                   \
     M(load_u16_be) M(load_rgb_u16_be) M(store_u16_be)            \
     M(load_tables_u16_be) M(load_tables_rgb_u16_be)              \
-    M(load_tables) M(store_tables)                               \
+    M(load_tables)                                               \
     M(scale_u8) M(scale_1_float)                                 \
     M(lerp_u8) M(lerp_565) M(lerp_1_float)                       \
     M(dstatop) M(dstin) M(dstout) M(dstover)                     \
@@ -96,7 +96,7 @@
     M(bicubic_n3y) M(bicubic_n1y) M(bicubic_p1y) M(bicubic_p3y)  \
     M(save_xy) M(accumulate)                                     \
     M(linear_gradient_2stops)                                    \
-    M(byte_tables)                                               \
+    M(byte_tables) M(byte_tables_rgb)                            \
     M(shader_adapter)                                            \
     M(rgb_to_hsl)                                                \
     M(hsl_to_rgb)

src/opts/SkRasterPipeline_opts.h

@@ -704,20 +704,6 @@ STAGE_CTX(load_tables_rgb_u16_be, const LoadTablesContext*) {
     a = 1.0f;
 }
 
-STAGE_CTX(store_tables, const StoreTablesContext*) {
-    auto ptr = ctx->fDst + x;
-
-    float scale = ctx->fCount - 1;
-    SkNi ri = SkNf_round(scale, r);
-    SkNi gi = SkNf_round(scale, g);
-    SkNi bi = SkNf_round(scale, b);
-
-    store(tail, ( SkNx_cast<int>(gather(tail, ctx->fR, ri)) << 0
-                | SkNx_cast<int>(gather(tail, ctx->fG, gi)) << 8
-                | SkNx_cast<int>(gather(tail, ctx->fB, bi)) << 16
-                | SkNf_round(255.0f, a)                     << 24), (int*)ptr);
-}
-
 SI SkNf inv(const SkNf& x) { return 1.0f - x; }
 
 RGBA_XFERMODE(clear)    { return 0.0f; }
@@ -1145,6 +1131,16 @@ STAGE_CTX(byte_tables, const void*) {
     a = SkNf_from_byte(gather(tail, tables->a, SkNf_round(255.0f, a)));
 }
 
+STAGE_CTX(byte_tables_rgb, const void*) {
+    struct Tables { const uint8_t *r, *g, *b; int n; };
+    auto tables = (const Tables*)ctx;
+
+    float scale = tables->n - 1;
+    r = SkNf_from_byte(gather(tail, tables->r, SkNf_round(scale, r)));
+    g = SkNf_from_byte(gather(tail, tables->g, SkNf_round(scale, g)));
+    b = SkNf_from_byte(gather(tail, tables->b, SkNf_round(scale, b)));
+}
+
 STAGE_CTX(shader_adapter, SkShader::Context*) {
     SkPM4f buf[N];
     static_assert(sizeof(buf) == sizeof(r) + sizeof(g) + sizeof(b) + sizeof(a), "");

tests/ColorSpaceTest.cpp

@@ -311,6 +311,13 @@ DEF_TEST(ColorSpace_Equals, r) {
     REPORTER_ASSERT(r, !SkColorSpace::Equals(upperRight.get(), adobe.get()));
     REPORTER_ASSERT(r, !SkColorSpace::Equals(z30.get(), rgb4.get()));
     REPORTER_ASSERT(r, !SkColorSpace::Equals(srgb.get(), rgb4.get()));
+
+    sk_sp<SkColorSpace> srgbFlag =
+            SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
+                                  SkColorSpace::kSRGB_Gamut,
+                                  SkColorSpace::kNonLinearBlending_ColorSpaceFlag);
+    REPORTER_ASSERT(r, !SkColorSpace::Equals(srgb.get(), srgbFlag.get()));
+    REPORTER_ASSERT(r, SkColorSpace_Base::EqualsIgnoreFlags(srgb.get(), srgbFlag.get()));
 }
 
 static inline bool matrix_almost_equal(const SkMatrix44& a, const SkMatrix44& b) {