Bilinear optimization for 1D convolution.

src/gpu/effects/GrConvolutionEffect.cpp

Index: src/gpu/effects/GrConvolutionEffect.cpp
diff --git a/src/gpu/effects/GrConvolutionEffect.cpp b/src/gpu/effects/GrConvolutionEffect.cpp
index 72640824d2591f4b60e02186aab5d0add0f1d161..a39171818b35e4ace0a618ec80ab97be327a1d30 100644
--- a/src/gpu/effects/GrConvolutionEffect.cpp
+++ b/src/gpu/effects/GrConvolutionEffect.cpp
@@ -13,9 +13,73 @@
 // For brevity
 typedef GrGLProgramDataManager::UniformHandle UniformHandle;
 
+/**
+ * Base class with shared functionality for GrGLBoundedConvolutionEffect and
+ * GrGLBilerpConvolutionEffect.
+ */
 class GrGLConvolutionEffect : public GrGLFragmentProcessor {
 public:
     GrGLConvolutionEffect(const GrProcessor&);
+    static inline void GenKey(const GrProcessor&, const GrGLSLCaps&, GrProcessorKeyBuilder*);
+
+protected:
+    int radius() const { return fRadius; }
+    int width() const { return Gr1DKernelEffect::WidthFromRadius(fRadius); }
+    Gr1DKernelEffect::Direction direction() const { return fDirection; }
+    void getImageIncrement(const GrConvolutionEffect&, float (*)[2]) const;
+
+private:
+    int fRadius;
+    Gr1DKernelEffect::Direction fDirection;
+
+    typedef GrGLFragmentProcessor INHERITED;
+};
+
+GrGLConvolutionEffect::GrGLConvolutionEffect(const GrProcessor& processor) {
+    const GrConvolutionEffect& c = processor.cast<GrConvolutionEffect>();
+    fRadius = c.radius();
+    fDirection = c.direction();
+}
+
+void GrGLConvolutionEffect::GenKey(const GrProcessor& processor,
+                                   const GrGLSLCaps&,
+                                   GrProcessorKeyBuilder* b) {
+    const GrConvolutionEffect& conv = processor.cast<GrConvolutionEffect>();
+    uint32_t key = conv.radius();
+    key <<= 2;
+    if (conv.useBounds()) {
+        key |= 0x2;
+        key |= GrConvolutionEffect::kY_Direction == conv.direction() ? 0x1 : 0x0;
+    }
+    b->add32(key);
+}
+
+void GrGLConvolutionEffect::getImageIncrement(const GrConvolutionEffect& conv,
+                                              float (*imageIncrement)[2]) const {
+    GrTexture& texture = *conv.texture(0);
+    (*imageIncrement)[0] = (*imageIncrement)[1] = 0;
+    float ySign = texture.origin() != kTopLeft_GrSurfaceOrigin ? 1.0f : -1.0f;
+    switch (conv.direction()) {
+        case Gr1DKernelEffect::kX_Direction:
+            (*imageIncrement)[0] = 1.0f / texture.width();
+            break;
+        case Gr1DKernelEffect::kY_Direction:
+            (*imageIncrement)[1] = ySign / texture.height();
+            break;
+        default:
+            SkFAIL("Unknown filter direction.");
+    }
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
+/**
+ * Applies a convolution effect which restricts samples to the provided bounds
+ * using shader logic.
+ */
+class GrGLBoundedConvolutionEffect : public GrGLConvolutionEffect {
+public:
+    GrGLBoundedConvolutionEffect(const GrProcessor& processor) : INHERITED(processor) {}
 
     virtual void emitCode(GrGLFPBuilder*,
                           const GrFragmentProcessor&,
@@ -26,47 +90,29 @@ public:
 
     void setData(const GrGLProgramDataManager& pdman, const GrProcessor&) override;
 
-    static inline void GenKey(const GrProcessor&, const GrGLSLCaps&, GrProcessorKeyBuilder*);
-
 private:
-    int width() const { return Gr1DKernelEffect::WidthFromRadius(fRadius); }
-    bool useBounds() const { return fUseBounds; }
-    Gr1DKernelEffect::Direction direction() const { return fDirection; }
-
-    int                 fRadius;
-    bool                fUseBounds;
-    Gr1DKernelEffect::Direction    fDirection;
     UniformHandle       fKernelUni;
     UniformHandle       fImageIncrementUni;
     UniformHandle       fBoundsUni;
 
-    typedef GrGLFragmentProcessor INHERITED;
+    typedef GrGLConvolutionEffect INHERITED;
 };
 
-GrGLConvolutionEffect::GrGLConvolutionEffect(const GrProcessor& processor) {
-    const GrConvolutionEffect& c = processor.cast<GrConvolutionEffect>();
-    fRadius = c.radius();
-    fUseBounds = c.useBounds();
-    fDirection = c.direction();
-}
+void GrGLBoundedConvolutionEffect::emitCode(GrGLFPBuilder* builder,
+                                            const GrFragmentProcessor& processor,
+                                            const char* outputColor,
+                                            const char* inputColor,
+                                            const TransformedCoordsArray& coords,
+                                            const TextureSamplerArray& samplers) {
+    fImageIncrementUni =
+        builder->addUniform(GrGLProgramBuilder::kFragment_Visibility, kVec2f_GrSLType,
+                            kDefault_GrSLPrecision, "ImageIncrement");
 
-void GrGLConvolutionEffect::emitCode(GrGLFPBuilder* builder,
-                                     const GrFragmentProcessor&,
-                                     const char* outputColor,
-                                     const char* inputColor,
-                                     const TransformedCoordsArray& coords,
-                                     const TextureSamplerArray& samplers) {
-    fImageIncrementUni = builder->addUniform(GrGLProgramBuilder::kFragment_Visibility,
-                                             kVec2f_GrSLType, kDefault_GrSLPrecision,
-                                             "ImageIncrement");
-    if (this->useBounds()) {
-        fBoundsUni = builder->addUniform(GrGLProgramBuilder::kFragment_Visibility,
-                                         kVec2f_GrSLType, kDefault_GrSLPrecision,
-                                         "Bounds");
-    }
-    fKernelUni = builder->addUniformArray(GrGLProgramBuilder::kFragment_Visibility,
-                                          kFloat_GrSLType, kDefault_GrSLPrecision,
-                                          "Kernel", this->width());
+    fBoundsUni = builder->addUniform(GrGLProgramBuilder::kFragment_Visibility, kVec2f_GrSLType,
+                                     kDefault_GrSLPrecision, "Bounds");
+
+    fKernelUni = builder->addUniformArray(GrGLProgramBuilder::kFragment_Visibility, kFloat_GrSLType,
+                                          kDefault_GrSLPrecision, "Kernel", this->width());
 
     GrGLFragmentBuilder* fsBuilder = builder->getFragmentShaderBuilder();
     SkString coords2D = fsBuilder->ensureFSCoords2D(coords, 0);
@@ -77,7 +123,8 @@ void GrGLConvolutionEffect::emitCode(GrGLFPBuilder* builder,
     const GrGLShaderVar& kernel = builder->getUniformVariable(fKernelUni);
     const char* imgInc = builder->getUniformCStr(fImageIncrementUni);
 
-    fsBuilder->codeAppendf("\t\tvec2 coord = %s - %d.0 * %s;\n", coords2D.c_str(), fRadius, imgInc);
+    fsBuilder->codeAppendf("\t\tvec2 coord = %s - %d.0 * %s;\n", coords2D.c_str(), this->radius(),
+                           imgInc);
 
     // Manually unroll loop because some drivers don't; yields 20-30% speedup.
     for (int i = 0; i < width; i++) {
@@ -87,12 +134,10 @@ void GrGLConvolutionEffect::emitCode(GrGLFPBuilder* builder,
         kernel.appendArrayAccess(index.c_str(), &kernelIndex);
         fsBuilder->codeAppendf("\t\t%s += ", outputColor);
         fsBuilder->appendTextureLookup(samplers[0], "coord");
-        if (this->useBounds()) {
-            const char* bounds = builder->getUniformCStr(fBoundsUni);
-            const char* component = this->direction() == Gr1DKernelEffect::kY_Direction ? "y" : "x";
-            fsBuilder->codeAppendf(" * float(coord.%s >= %s.x && coord.%s <= %s.y)",
-                component, bounds, component, bounds);
-        }
+        const char* bounds = builder->getUniformCStr(fBoundsUni);
+        const char* component = this->direction() == Gr1DKernelEffect::kY_Direction ? "y" : "x";
+        fsBuilder->codeAppendf(" * float(coord.%s >= %s.x && coord.%s <= %s.y)", component, bounds,
+                               component, bounds);
         fsBuilder->codeAppendf(" * %s;\n", kernelIndex.c_str());
         fsBuilder->codeAppendf("\t\tcoord += %s;\n", imgInc);
     }
@@ -102,47 +147,155 @@ void GrGLConvolutionEffect::emitCode(GrGLFPBuilder* builder,
     fsBuilder->codeAppend(modulate.c_str());
 }
 
-void GrGLConvolutionEffect::setData(const GrGLProgramDataManager& pdman,
-                                    const GrProcessor& processor) {
+void GrGLBoundedConvolutionEffect::setData(const GrGLProgramDataManager& pdman,
+                                           const GrProcessor& processor) {
     const GrConvolutionEffect& conv = processor.cast<GrConvolutionEffect>();
-    GrTexture& texture = *conv.texture(0);
+
     // the code we generated was for a specific kernel radius
-    SkASSERT(conv.radius() == fRadius);
-    float imageIncrement[2] = { 0 };
-    float ySign = texture.origin() != kTopLeft_GrSurfaceOrigin ? 1.0f : -1.0f;
-    switch (conv.direction()) {
-        case Gr1DKernelEffect::kX_Direction:
-            imageIncrement[0] = 1.0f / texture.width();
-            break;
-        case Gr1DKernelEffect::kY_Direction:
-            imageIncrement[1] = ySign / texture.height();
-            break;
-        default:
-            SkFAIL("Unknown filter direction.");
-    }
+    SkASSERT(conv.radius() == this->radius());
+
+    // the code we generated was for a specific bounding mode.
+    SkASSERT(conv.useBounds() == true);
+
+    GrTexture& texture = *conv.texture(0);
+    float imageIncrement[2];
+    getImageIncrement(conv, &imageIncrement);
     pdman.set2fv(fImageIncrementUni, 1, imageIncrement);
-    if (conv.useBounds()) {
-        const float* bounds = conv.bounds();
-        if (Gr1DKernelEffect::kY_Direction == conv.direction() &&
-            texture.origin() != kTopLeft_GrSurfaceOrigin) {
-            pdman.set2f(fBoundsUni, 1.0f - bounds[1], 1.0f - bounds[0]);
-        } else {
-            pdman.set2f(fBoundsUni, bounds[0], bounds[1]);
-        }
+    const float* bounds = conv.bounds();
+    if (Gr1DKernelEffect::kY_Direction == conv.direction() &&
+        texture.origin() != kTopLeft_GrSurfaceOrigin) {
+        pdman.set2f(fBoundsUni, 1.0f - bounds[1], 1.0f - bounds[0]);
+    } else {
+        pdman.set2f(fBoundsUni, bounds[0], bounds[1]);
     }
     pdman.set1fv(fKernelUni, this->width(), conv.kernel());
 }
 
-void GrGLConvolutionEffect::GenKey(const GrProcessor& processor, const GrGLSLCaps&,
-                                   GrProcessorKeyBuilder* b) {
+///////////////////////////////////////////////////////////////////////////////
+
+/**
+ * Applies a convolution effect which applies the convolution using a bilinear
+ * sampling optimization to use half as many samples.
+ */
+class GrGLBilerpConvolutionEffect : public GrGLConvolutionEffect {

[Stephen White, 2015/06/26 21:28:31]

<bikeshed> is this really a bilinear fetch? Or essentially linear in 1D? (i.e.,
is the non-incrementing coord the same in both fetches?) If so maybe it should
be GrGLLinearConvolutionEffect GrGLLerpConvolutionEffect or something.
</bikeshed>

[ericrk, 2015/06/29 17:33:21]

yup - that's better :D

+public:
+    GrGLBilerpConvolutionEffect(const GrProcessor& processor) : INHERITED(processor) {}
+
+    virtual void emitCode(GrGLFPBuilder*,
+                          const GrFragmentProcessor&,
+                          const char* outputColor,
+                          const char* inputColor,
+                          const TransformedCoordsArray&,
+                          const TextureSamplerArray&) override;
+
+    void setData(const GrGLProgramDataManager& pdman, const GrProcessor&) override;
+
+private:
+    int bilerpSampleCount() const;
+
+    // Bounded uniforms
+    UniformHandle fSampleWeightUni;
+    UniformHandle fSampleOffsetUni;
+
+    typedef GrGLConvolutionEffect INHERITED;
+};
+
+void GrGLBilerpConvolutionEffect::emitCode(GrGLFPBuilder* builder,
+                                           const GrFragmentProcessor& processor,
+                                           const char* outputColor,
+                                           const char* inputColor,
+                                           const TransformedCoordsArray& coords,
+                                           const TextureSamplerArray& samplers) {
+    int sampleCount = bilerpSampleCount();
+
+    fSampleOffsetUni =

[Stephen White, 2015/06/26 21:28:31]

How many uniform values does this add up to? You may have to increase the number
of downsample passes for this optimization to stay under the PS2.0 limits. Or
check that we have PS >= 3.0 before using it.

[ericrk, 2015/06/29 17:33:21]

I think we're safe (assuming we keep the limit of sigma <= 4, which we are for
now).

Looks like PS2.0 gives 32 uniform slots (up to vec4 each).

Max sigma is 4 == kernel radius of 12 == kernel width of 25

for kernel width 25, linear optimization requires ((25+1)/2) = 13 samples.

So max sampleCount is 13. We use two uniform arrays of sampleCount size, which
is still only 26 uniforms, so I think this is safe.

This actually only uses 2 more uniform slots than the previous shader.

Let me know if this makes sense.

[Stephen White, 2015/06/29 19:04:06]

Yep; makes sense. Thanks!

+        builder->addUniformArray(GrGLProgramBuilder::kFragment_Visibility, kVec2f_GrSLType,
+                                 kDefault_GrSLPrecision, "SampleOffset", sampleCount);
+    fSampleWeightUni =
+        builder->addUniformArray(GrGLProgramBuilder::kFragment_Visibility, kFloat_GrSLType,
+                                 kDefault_GrSLPrecision, "SampleWeight", sampleCount);
+
+    GrGLFragmentBuilder* fsBuilder = builder->getFragmentShaderBuilder();
+    SkString coords2D = fsBuilder->ensureFSCoords2D(coords, 0);
+
+    fsBuilder->codeAppendf("\t\t%s = vec4(0, 0, 0, 0);\n", outputColor);

[bsalomon, 2015/06/26 21:07:33]

You don't really need the \t's anymore (we now "pretty" print the shaders).

[ericrk, 2015/06/26 21:28:22]

removed the \t\ts throughout the file

+
+    const GrGLShaderVar& kernel = builder->getUniformVariable(fSampleWeightUni);
+    const GrGLShaderVar& imgInc = builder->getUniformVariable(fSampleOffsetUni);
+
+    fsBuilder->codeAppendf("\t\tvec2 coord; \n");
+
+    // Manually unroll loop because some drivers don't; yields 20-30% speedup.
+    for (int i = 0; i < sampleCount; i++) {
+        SkString index;
+        SkString weightIndex;
+        SkString offsetIndex;
+        index.appendS32(i);
+        kernel.appendArrayAccess(index.c_str(), &weightIndex);
+        imgInc.appendArrayAccess(index.c_str(), &offsetIndex);
+        fsBuilder->codeAppendf("\t\tcoord = %s + %s;\n", coords2D.c_str(), offsetIndex.c_str());
+        fsBuilder->codeAppendf("\t\t%s += ", outputColor);
+        fsBuilder->appendTextureLookup(samplers[0], "coord");
+        fsBuilder->codeAppendf(" * %s;\n", weightIndex.c_str());
+    }
+
+    SkString modulate;
+    GrGLSLMulVarBy4f(&modulate, outputColor, inputColor);
+    fsBuilder->codeAppend(modulate.c_str());
+}
+
+void GrGLBilerpConvolutionEffect::setData(const GrGLProgramDataManager& pdman,
+                                          const GrProcessor& processor) {
     const GrConvolutionEffect& conv = processor.cast<GrConvolutionEffect>();
-    uint32_t key = conv.radius();
-    key <<= 2;
-    if (conv.useBounds()) {
-        key |= 0x2;
-        key |= GrConvolutionEffect::kY_Direction == conv.direction() ? 0x1 : 0x0;
+
+    // the code we generated was for a specific kernel radius
+    SkASSERT(conv.radius() == this->radius());
+
+    // the code we generated was for a specific bounding mode.
+    SkASSERT(conv.useBounds() == false);

[bsalomon, 2015/06/26 21:07:34]

!conv.useBounds()?

[ericrk, 2015/06/26 21:28:22]

yup

+
+    int sampleCount = bilerpSampleCount();
+    SkAutoTArray<float> imageIncrements(sampleCount * 2);  // X and Y floats per sample.
+    SkAutoTArray<float> kernel(sampleCount);
+
+    float baseImageIncrement[2];
+    getImageIncrement(conv, &baseImageIncrement);
+
+    for (int i = 0; i < sampleCount; i++) {
+        int sampleIndex1 = i * 2;
+        int sampleIndex2 = sampleIndex1 + 1;
+
+        // If we have an odd number of samples in our filter, the last sample won't use
+        // the bilinear optimization (it will be pixel aligned).
+        if (sampleIndex2 >= this->width()) {
+            sampleIndex2 = sampleIndex1;
+        }
+
+        float kernelWeight1 = conv.kernel()[sampleIndex1];
+        float kernelWeight2 = conv.kernel()[sampleIndex2];
+
+        float totalKernalWeight =

[Stephen White, 2015/06/26 21:28:31]

Nit: kernal -> kernel?

+            (sampleIndex1 == sampleIndex2) ? kernelWeight1 : (kernelWeight1 + kernelWeight2);
+
+        float sampleRatio =
+            (sampleIndex1 == sampleIndex2) ? 0 : kernelWeight2 / (kernelWeight1 + kernelWeight2);
+
+        imageIncrements[i * 2] = (-this->radius() + i * 2 + sampleRatio) * baseImageIncrement[0];
+        imageIncrements[i * 2 + 1] =
+            (-this->radius() + i * 2 + sampleRatio) * baseImageIncrement[1];
+
+        kernel[i] = totalKernalWeight;
     }
-    b->add32(key);
+    pdman.set2fv(fSampleOffsetUni, sampleCount, imageIncrements.get());
+    pdman.set1fv(fSampleWeightUni, sampleCount, kernel.get());
+}
+
+int GrGLBilerpConvolutionEffect::bilerpSampleCount() const {
+    // We use a bilinear optimization to only sample once for each two pixel aligned
+    // samples in the kernel. If we have an odd number of samples, we will have to
+    // skip this optimization for the last sample. Because of this we always round
+    // up our sample count (by adding 1 before dividing).
+    return (this->width() + 1) / 2;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -153,7 +306,12 @@ GrConvolutionEffect::GrConvolutionEffect(GrTexture* texture,
                                          const float* kernel,
                                          bool useBounds,
                                          float bounds[2])
-    : Gr1DKernelEffect(texture, direction, radius), fUseBounds(useBounds) {
+    : Gr1DKernelEffect(texture,
+                       direction,
+                       radius,
+                       useBounds ? GrTextureParams::FilterMode::kNone_FilterMode
+                                 : GrTextureParams::FilterMode::kBilerp_FilterMode)
+    , fUseBounds(useBounds) {
     this->initClassID<GrConvolutionEffect>();
     SkASSERT(radius <= kMaxKernelRadius);
     SkASSERT(kernel);
@@ -170,7 +328,12 @@ GrConvolutionEffect::GrConvolutionEffect(GrTexture* texture,
                                          float gaussianSigma,
                                          bool useBounds,
                                          float bounds[2])
-    : Gr1DKernelEffect(texture, direction, radius), fUseBounds(useBounds) {
+    : Gr1DKernelEffect(texture,
+                       direction,
+                       radius,
+                       useBounds ? GrTextureParams::FilterMode::kNone_FilterMode
+                                 : GrTextureParams::FilterMode::kBilerp_FilterMode)
+    , fUseBounds(useBounds) {
     this->initClassID<GrConvolutionEffect>();
     SkASSERT(radius <= kMaxKernelRadius);
     int width = this->width();
@@ -201,7 +364,14 @@ void GrConvolutionEffect::getGLProcessorKey(const GrGLSLCaps& caps,
 }
 
 GrGLFragmentProcessor* GrConvolutionEffect::createGLInstance() const  {
-    return SkNEW_ARGS(GrGLConvolutionEffect, (*this));
+    // We support a bilinear optimization which (when feasible) uses half the number
+    // of samples to apply the kernel. This is not always applicable, as the
+    // bilinear sampling optimization does not support bounded sampling.
+    if (this->useBounds()) {
+        return SkNEW_ARGS(GrGLBoundedConvolutionEffect, (*this));
+    } else {
+        return SkNEW_ARGS(GrGLBilerpConvolutionEffect, (*this));
+    }
 }
 
 bool GrConvolutionEffect::onIsEqual(const GrFragmentProcessor& sBase) const {