Support for color-spaces with multi-stop (texture) gradients

src/effects/gradients/SkGradientShader.cpp

Index: src/effects/gradients/SkGradientShader.cpp
diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index 3a28da8099501708cf9ced29befa7154207293c7..61cee69a4cbd368e651184096ef05da07291f9c2 100644
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -7,6 +7,7 @@
 
 #include "Sk4fLinearGradient.h"
 #include "SkGradientShaderPriv.h"
+#include "SkHalf.h"
 #include "SkLinearGradient.h"
 #include "SkRadialGradient.h"
 #include "SkTwoPointConicalGradient.h"
@@ -544,6 +545,61 @@ void SkGradientShaderBase::GradientShaderCache::initCache32(GradientShaderCache*
     }
 }
 
+void SkGradientShaderBase::initLinearBitmap(SkBitmap* bitmap) const {

[Brian Osman, 2016/09/20 17:29:53]

This is essentially a condensed version of the logic from Build32bitCache
(above), that can construct either an F16 or sRGB format bitmap containing the
gradient strip, interpolated in premul or not.

[f(malita), 2016/09/21 02:51:35]

Kinda feels like we're reinventing a specialized version of Sk4fLinearGradient
here... I wonder what it would take to just set up a context and invoke that
code instead?

(not necessarily in this iteration, just a thought)

Also, is the legacy Build32bitCache going to be used going forward?  I think
we're going to switch SW gradients to the analytical path sooner or later, but
maybe the GPU multi-stop impl will still use it for L32?  If so, do you think we
could merge these two at some point?

[Brian Osman, 2016/09/21 13:24:18]

That sounds like a good idea. Almost looks like I could just make a
LinearGradient4fContext on the stack and invoke it? I'm going to try that
experiment before landing this.
The GPU is going to need *some* way to build textures. Yes, if I can create a
non-gamma-correct context to shade the texture span, I think we could make all
the handling much more consistent. (It's trickier for me to do the L32 case
right now, because this code is using the 4f colors, which are already linear).

+    const bool interpInPremul = SkToBool(fGradFlags &
+                                         SkGradientShader::kInterpolateColorsInPremul_Flag);
+    bitmap->lockPixels();
+    SkHalf* pixelsF16 = reinterpret_cast<SkHalf*>(bitmap->getPixels());
+    uint32_t* pixelsS32 = reinterpret_cast<uint32_t*>(bitmap->getPixels());
+
+    typedef std::function<void(const Sk4f&, int)> pixelWriteFn_t;
+
+    pixelWriteFn_t writeF16Pixel = [&](const Sk4f& x, int index) {
+        Sk4h c = SkFloatToHalf_finite_ftz(x);
+        pixelsF16[4*index+0] = c[0];
+        pixelsF16[4*index+1] = c[1];
+        pixelsF16[4*index+2] = c[2];
+        pixelsF16[4*index+3] = c[3];
+    };
+    pixelWriteFn_t writeS32Pixel = [&](const Sk4f& c, int index) {
+        pixelsS32[index] = Sk4f_toS32(c);
+    };
+
+    pixelWriteFn_t writeSizedPixel =
+        (kRGBA_F16_SkColorType == bitmap->colorType()) ? writeF16Pixel : writeS32Pixel;
+    pixelWriteFn_t writeUnpremulPixel = [&](const Sk4f& c, int index) {
+        writeSizedPixel(Sk4f(c[0] * c[3], c[1] * c[3], c[2] * c[3], c[3]), index);

[f(malita), 2016/09/21 02:51:35]

nit: c * Sk4f(c[3], c[3], c[3], 1)?

[Brian Osman, 2016/09/21 13:24:18]

Duh! I was sure there was a clearer way to write this, but my brain was not
working that day.

+    };
+
+    pixelWriteFn_t writePixel = interpInPremul ? writeSizedPixel : writeUnpremulPixel;
+
+    int prevIndex = 0;
+    for (int i = 1; i < fColorCount; i++) {
+        int nextIndex = (fColorCount == 2) ? (kCache32Count - 1)

[f(malita), 2016/09/21 02:51:35]

wouldn't SkFixedToFFFF(fRecs[i].fPos) >> kCache32Shift work for fColorCount == 2
also?

[Brian Osman, 2016/09/21 13:24:18]

I don't think so. The constructor never initializes the contents of fRecs unless
fColorCount > 2. (All the other code that uses fRecs is similarly guarded).

[f(malita), 2016/09/22 14:05:39]

Acknowledged.

+            : SkFixedToFFFF(fRecs[i].fPos) >> kCache32Shift;
+        SkASSERT(nextIndex < kCache32Count);
+
+        if (nextIndex > prevIndex) {
+            Sk4f c0 = Sk4f::Load(fOrigColors4f[i - 1].vec());
+            Sk4f c1 = Sk4f::Load(fOrigColors4f[i].vec());
+            if (interpInPremul) {
+                c0 = Sk4f(c0[0] * c0[3], c0[1] * c0[3], c0[2] * c0[3], c0[3]);

[f(malita), 2016/09/21 02:51:35]

nit: c0 * Sk4f(c0[3], c0[3], c0[3], 1)?

[Brian Osman, 2016/09/21 13:24:18]

Done.

+                c1 = Sk4f(c1[0] * c1[3], c1[1] * c1[3], c1[2] * c1[3], c1[3]);

[f(malita), 2016/09/21 02:51:35]

nit: ditto

[Brian Osman, 2016/09/21 13:24:18]

Done.

+            }
+
+            Sk4f step = Sk4f(1.0f / static_cast<float>(nextIndex - prevIndex));
+            Sk4f delta = (c1 - c0) * step;
+
+            for (int curIndex = prevIndex; curIndex <= nextIndex; ++curIndex) {
+                writePixel(c0, curIndex);
+                c0 += delta;
+            }
+        }
+        prevIndex = nextIndex;
+    }

[f(malita), 2016/09/21 02:51:35]

SkASSERT(prevIndex == kCache32Count - 1)?

[Brian Osman, 2016/09/21 13:24:18]

Done.

+    bitmap->unlockPixels();
+}
+
 /*
  *  The gradient holds a cache for the most recent value of alpha. Successive
  *  callers with the same alpha value will share the same cache.
@@ -570,13 +626,13 @@ SK_DECLARE_STATIC_MUTEX(gGradientCacheMutex);
  *  colors and positions. Note: we don't try to flatten the fMapper, so if one
  *  is present, we skip the cache for now.
  */
-void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap) const {
-    // our caller assumes no external alpha, so we ensure that our cache is
-    // built with 0xFF
+void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap,
+                                                  GradientBitmapType bitmapType) const {
+    // our caller assumes no external alpha, so we ensure that our cache is built with 0xFF
     SkAutoTUnref<GradientShaderCache> cache(this->refCache(0xFF, true));
 
-    // build our key: [numColors + colors[] + {positions[]} + flags ]
-    int count = 1 + fColorCount + 1;
+    // build our key: [numColors + colors[] + {positions[]} + flags + colorType ]
+    int count = 1 + fColorCount + 1 + 1;
     if (fColorCount > 2) {
         count += fColorCount - 1;    // fRecs[].fPos
     }
@@ -593,12 +649,13 @@ void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap) const {
         }
     }
     *buffer++ = fGradFlags;
+    *buffer++ = static_cast<int32_t>(bitmapType);
     SkASSERT(buffer - storage.get() == count);
 
     ///////////////////////////////////
 
     static SkGradientBitmapCache* gCache;
-    // each cache cost 1K of RAM, since each bitmap will be 1x256 at 32bpp
+    // each cache cost 1K or 2K of RAM, since each bitmap will be 1x256 at either 32bpp or 64bpp
     static const int MAX_NUM_CACHED_GRADIENT_BITMAPS = 32;
     SkAutoMutexAcquire ama(gGradientCacheMutex);
 
@@ -608,11 +665,35 @@ void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap) const {
     size_t size = count * sizeof(int32_t);
 
     if (!gCache->find(storage.get(), size, bitmap)) {
-        // force our cahce32pixelref to be built
-        (void)cache->getCache32();
-        bitmap->setInfo(SkImageInfo::MakeN32Premul(kCache32Count, 1));
-        bitmap->setPixelRef(cache->getCache32PixelRef());
-
+        if (GradientBitmapType::kLegacy == bitmapType) {
+            // force our cache32pixelref to be built
+            (void)cache->getCache32();
+            bitmap->setInfo(SkImageInfo::MakeN32Premul(kCache32Count, 1));
+            bitmap->setPixelRef(cache->getCache32PixelRef());
+        } else {
+            // For these cases we use the bitmap cache, but not the GradientShaderCache. So just

[Brian Osman, 2016/09/20 17:29:54]

We talked about not using the bitmap cache (or bitmaps at all) in the new code
path, but that would mean adding some other caching mechanism, to avoid
re-building this data every frame. Long term, we could make a larger change
(texture strip atlas takes an explicit key, along with callback function to
construct the strip?), but for now it made sense to leverage the code we already
had.

+            // allocate and populate the bitmap's data directly.
+
+            SkImageInfo info;
+            switch (bitmapType) {
+                case GradientBitmapType::kSRGB:
+                    info = SkImageInfo::Make(kCache32Count, 1, kRGBA_8888_SkColorType,
+                                             kPremul_SkAlphaType,
+                                             SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named));
+                    break;
+                case GradientBitmapType::kHalfFloat:
+                    info = SkImageInfo::Make(kCache32Count, 1, kRGBA_F16_SkColorType,
+                                             kPremul_SkAlphaType,
+                                             SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named)
+                                                ->makeLinearGamma());
+                    break;
+                default:
+                    SkFAIL("Unexpected bitmap type");
+                    return;
+            }
+            bitmap->allocPixels(info);
+            initLinearBitmap(bitmap);

[f(malita), 2016/09/21 02:51:35]

nit: this->

[Brian Osman, 2016/09/21 13:24:18]

Done.

+        }
         gCache->add(storage.get(), size, *bitmap);
     }
 }
@@ -902,6 +983,7 @@ SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END
 #include "GrInvariantOutput.h"
 #include "GrTextureStripAtlas.h"
 #include "gl/GrGLContext.h"
+#include "glsl/GrGLSLColorSpaceXformHelper.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
 #include "glsl/GrGLSLProgramDataManager.h"
 #include "glsl/GrGLSLUniformHandler.h"
@@ -1113,6 +1195,9 @@ void GrGradientEffect::GLSLProcessor::onSetData(const GrGLSLProgramDataManager&
                 pdman.set1f(fFSYUni, yCoord);
                 fCachedYCoord = yCoord;
             }
+            if (SkToBool(e.fColorSpaceXform)) {
+                pdman.setSkMatrix44(fColorSpaceXformUni, e.fColorSpaceXform->srcToDst());
+            }
             break;
         }
     }
@@ -1150,6 +1235,8 @@ uint32_t GrGradientEffect::GLSLProcessor::GenBaseGradientKey(const GrProcessor&
     }
 #endif
 
+    key |= GrColorSpaceXform::XformKey(e.fColorSpaceXform.get()) << kReservedBits;
+
     return key;
 }
 
@@ -1331,11 +1418,15 @@ void GrGradientEffect::GLSLProcessor::emitColor(GrGLSLFPFragmentBuilder* fragBui
         }
 
         case kTexture_ColorType: {
+            GrGLSLColorSpaceXformHelper colorSpaceHelper(uniformHandler, ge.fColorSpaceXform.get(),

[Brian Osman, 2016/09/20 17:29:54]

I *could* have baked the xform into the texture, and that might make more sense.
This method lets us reuse the cached texture for other destination spaces, but
that's probably not a real common situation. The downside here is that we're
applying the matrix on the GPU every time we draw. So yeah, maybe moving to
destination gamut up front is better.

Of course, in pathological cases like the many_stops GM (200 stops), then we're
actually spending a lot of time on the CPU to prepare everything. Sigh.

+                                                         &fColorSpaceXformUni);
+
             const char* fsyuni = uniformHandler->getUniformCStr(fFSYUni);
 
             fragBuilder->codeAppendf("vec2 coord = vec2(%s, %s);", gradientTValue, fsyuni);
             fragBuilder->codeAppendf("%s = ", outputColor);
-            fragBuilder->appendTextureLookupAndModulate(inputColor, texSamplers[0], "coord");
+            fragBuilder->appendTextureLookupAndModulate(inputColor, texSamplers[0], "coord",
+                                                        kVec2f_GrSLType, &colorSpaceHelper);
             fragBuilder->codeAppend(";");
 
             break;
@@ -1351,12 +1442,12 @@ GrGradientEffect::GrGradientEffect(const CreateArgs& args) {
     fIsOpaque = shader.isOpaque();
 
     fColorType = this->determineColorType(shader);
+    fColorSpaceXform = std::move(args.fColorSpaceXform);
 
     if (kTexture_ColorType != fColorType) {
         SkASSERT(shader.fOrigColors && shader.fOrigColors4f);
         if (args.fGammaCorrect) {
             fColors4f = SkTDArray<SkColor4f>(shader.fOrigColors4f, shader.fColorCount);
-            fColorSpaceXform = std::move(args.fColorSpaceXform);
         } else {
             fColors = SkTDArray<SkColor>(shader.fOrigColors, shader.fColorCount);
         }
@@ -1397,8 +1488,22 @@ GrGradientEffect::GrGradientEffect(const CreateArgs& args) {
             // effect key.
             fPremulType = kBeforeInterp_PremulType;
 
+            SkGradientShaderBase::GradientBitmapType bitmapType =
+                SkGradientShaderBase::GradientBitmapType::kLegacy;
+            if (args.fGammaCorrect) {
+                // Try to use F16 if we can
+                if (args.fContext->caps()->isConfigTexturable(kRGBA_half_GrPixelConfig)) {
+                    bitmapType = SkGradientShaderBase::GradientBitmapType::kHalfFloat;
+                } else if (args.fContext->caps()->isConfigTexturable(kSRGBA_8888_GrPixelConfig)) {
+                    bitmapType = SkGradientShaderBase::GradientBitmapType::kSRGB;
+                } else {
+                    // This should never happen, but just fall back to legacy behavior
+                    SkASSERT("Requesting a gamma-correct gradient FP without F16 or sRGB");
+                }
+            }
+
             SkBitmap bitmap;
-            shader.getGradientTableBitmap(&bitmap);
+            shader.getGradientTableBitmap(&bitmap, bitmapType);
 
             GrTextureStripAtlas::Desc desc;
             desc.fWidth  = bitmap.width();