Switch SkDisplacementImageFilter over to new onFilterImage interface

src/effects/SkDisplacementMapEffect.cpp

Index: src/effects/SkDisplacementMapEffect.cpp
diff --git a/src/effects/SkDisplacementMapEffect.cpp b/src/effects/SkDisplacementMapEffect.cpp
index d054e58e51f2a416730fd3bafb09d0c0e1a2db25..367eded4c7fe3644963c5e18444694ae884819cd 100644
--- a/src/effects/SkDisplacementMapEffect.cpp
+++ b/src/effects/SkDisplacementMapEffect.cpp
@@ -6,8 +6,10 @@
  */
 
 #include "SkDisplacementMapEffect.h"
-#include "SkDevice.h"
+
+#include "SkBitmap.h"
 #include "SkReadBuffer.h"
+#include "SkSpecialImage.h"
 #include "SkWriteBuffer.h"
 #include "SkUnPreMultiply.h"
 #include "SkColorPriv.h"
@@ -57,13 +59,12 @@ template<> uint32_t getValue<SkDisplacementMapEffect::kA_ChannelSelectorType>(
 template<SkDisplacementMapEffect::ChannelSelectorType typeX,
          SkDisplacementMapEffect::ChannelSelectorType typeY>
 void computeDisplacement(const SkVector& scale, SkBitmap* dst,
-                         SkBitmap* displ, const SkIPoint& offset,
-                         SkBitmap* src,
-                         const SkIRect& bounds)
-{
+                         const SkPixmap& displ, const SkIPoint& offset,
+                         const SkPixmap& src,
+                         const SkIRect& bounds) {
     static const SkScalar Inv8bit = SkScalarInvert(255);
-    const int srcW = src->width();
-    const int srcH = src->height();
+    const int srcW = src.width();
+    const int srcH = src.height();
     const SkVector scaleForColor = SkVector::Make(SkScalarMul(scale.fX, Inv8bit),
                                                   SkScalarMul(scale.fY, Inv8bit));
     const SkVector scaleAdj = SkVector::Make(SK_ScalarHalf - SkScalarMul(scale.fX, SK_ScalarHalf),
@@ -71,8 +72,7 @@ void computeDisplacement(const SkVector& scale, SkBitmap* dst,
     const SkUnPreMultiply::Scale* table = SkUnPreMultiply::GetScaleTable();
     SkPMColor* dstPtr = dst->getAddr32(0, 0);
     for (int y = bounds.top(); y < bounds.bottom(); ++y) {
-        const SkPMColor* displPtr = displ->getAddr32(bounds.left() + offset.fX,
-                                                     y + offset.fY);
+        const SkPMColor* displPtr = displ.addr32(bounds.left() + offset.fX, y + offset.fY);
         for (int x = bounds.left(); x < bounds.right(); ++x, ++displPtr) {
             const SkScalar displX = SkScalarMul(scaleForColor.fX,
                 SkIntToScalar(getValue<typeX>(*displPtr, table))) + scaleAdj.fX;
@@ -82,7 +82,7 @@ void computeDisplacement(const SkVector& scale, SkBitmap* dst,
             const int srcX = x + SkScalarTruncToInt(displX);
             const int srcY = y + SkScalarTruncToInt(displY);
             *dstPtr++ = ((srcX < 0) || (srcX >= srcW) || (srcY < 0) || (srcY >= srcH)) ?
-                      0 : *(src->getAddr32(srcX, srcY));
+                      0 : *(src.addr32(srcX, srcY));
         }
     }
 }
@@ -90,10 +90,9 @@ void computeDisplacement(const SkVector& scale, SkBitmap* dst,
 template<SkDisplacementMapEffect::ChannelSelectorType typeX>
 void computeDisplacement(SkDisplacementMapEffect::ChannelSelectorType yChannelSelector,
                          const SkVector& scale, SkBitmap* dst,
-                         SkBitmap* displ, const SkIPoint& offset,
-                         SkBitmap* src,
-                         const SkIRect& bounds)
-{
+                         const SkPixmap& displ, const SkIPoint& offset,
+                         const SkPixmap& src,
+                         const SkIRect& bounds) {
     switch (yChannelSelector) {
       case SkDisplacementMapEffect::kR_ChannelSelectorType:
         computeDisplacement<typeX, SkDisplacementMapEffect::kR_ChannelSelectorType>(
@@ -120,10 +119,9 @@ void computeDisplacement(SkDisplacementMapEffect::ChannelSelectorType yChannelSe
 void computeDisplacement(SkDisplacementMapEffect::ChannelSelectorType xChannelSelector,
                          SkDisplacementMapEffect::ChannelSelectorType yChannelSelector,
                          const SkVector& scale, SkBitmap* dst,
-                         SkBitmap* displ, const SkIPoint& offset,
-                         SkBitmap* src,
-                         const SkIRect& bounds)
-{
+                         const SkPixmap& displ, const SkIPoint& offset,
+                         const SkPixmap& src,
+                         const SkIRect& bounds) {
     switch (xChannelSelector) {
       case SkDisplacementMapEffect::kR_ChannelSelectorType:
         computeDisplacement<SkDisplacementMapEffect::kR_ChannelSelectorType>(
@@ -185,11 +183,10 @@ SkDisplacementMapEffect::SkDisplacementMapEffect(ChannelSelectorType xChannelSel
                                                  SkScalar scale,
                                                  SkImageFilter* inputs[2],
                                                  const CropRect* cropRect)
-  : INHERITED(2, inputs, cropRect)
-  , fXChannelSelector(xChannelSelector)
-  , fYChannelSelector(yChannelSelector)
-  , fScale(scale)
-{
+    : INHERITED(2, inputs, cropRect)
+    , fXChannelSelector(xChannelSelector)
+    , fYChannelSelector(yChannelSelector)
+    , fScale(scale) {
 }
 
 SkDisplacementMapEffect::~SkDisplacementMapEffect() {
@@ -211,59 +208,190 @@ void SkDisplacementMapEffect::flatten(SkWriteBuffer& buffer) const {
     buffer.writeScalar(fScale);
 }
 
-bool SkDisplacementMapEffect::onFilterImageDeprecated(Proxy* proxy,
-                                                      const SkBitmap& src,
-                                                      const Context& ctx,
-                                                      SkBitmap* dst,
-                                                      SkIPoint* offset) const {
-    SkBitmap displ = src, color = src;
-    SkIPoint colorOffset = SkIPoint::Make(0, 0), displOffset = SkIPoint::Make(0, 0);
-    if (!this->filterInputDeprecated(1, proxy, src, ctx, &color, &colorOffset) ||
-        !this->filterInputDeprecated(0, proxy, src, ctx, &displ, &displOffset)) {
-        return false;
+///////////////////////////////////////////////////////////////////////////////
+
+#if SK_SUPPORT_GPU
+
+class GrDisplacementMapEffect : public GrFragmentProcessor {

[Stephen White, 2016/04/01 19:17:42]

Could you move this code back to where it was for review purposes? You can move
it back in a followup patch.

[robertphillips, 2016/04/01 19:41:43]

Done.

+public:
+    static GrFragmentProcessor* Create(
+                SkDisplacementMapEffect::ChannelSelectorType xChannelSelector,
+                SkDisplacementMapEffect::ChannelSelectorType yChannelSelector, SkVector scale,
+                GrTexture* displacement, const SkMatrix& offsetMatrix, GrTexture* color,
+                const SkISize& colorDimensions) {
+        return new GrDisplacementMapEffect(xChannelSelector, yChannelSelector, scale, displacement,
+                                           offsetMatrix, color, colorDimensions);
+    }
+
+    virtual ~GrDisplacementMapEffect();
+
+    SkDisplacementMapEffect::ChannelSelectorType xChannelSelector() const {
+        return fXChannelSelector;
+    }
+    SkDisplacementMapEffect::ChannelSelectorType yChannelSelector() const {
+        return fYChannelSelector;
+    }
+    const SkVector& scale() const { return fScale; }
+
+    const char* name() const override { return "DisplacementMap"; }
+    const GrTextureDomain& domain() const { return fDomain; }
+
+private:
+    GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
+
+    void onGetGLSLProcessorKey(const GrGLSLCaps& caps, GrProcessorKeyBuilder* b) const override;
+
+    bool onIsEqual(const GrFragmentProcessor&) const override;
+
+    void onComputeInvariantOutput(GrInvariantOutput* inout) const override;
+
+    GrDisplacementMapEffect(SkDisplacementMapEffect::ChannelSelectorType xChannelSelector,
+                            SkDisplacementMapEffect::ChannelSelectorType yChannelSelector,
+                            const SkVector& scale,
+                            GrTexture* displacement, const SkMatrix& offsetMatrix,
+                            GrTexture* color,
+                            const SkISize& colorDimensions);
+
+    GR_DECLARE_FRAGMENT_PROCESSOR_TEST;
+
+    GrCoordTransform            fDisplacementTransform;
+    GrTextureAccess             fDisplacementAccess;
+    GrCoordTransform            fColorTransform;
+    GrTextureDomain             fDomain;
+    GrTextureAccess             fColorAccess;
+    SkDisplacementMapEffect::ChannelSelectorType fXChannelSelector;
+    SkDisplacementMapEffect::ChannelSelectorType fYChannelSelector;
+    SkVector fScale;
+
+    typedef GrFragmentProcessor INHERITED;
+};
+
+#endif
+
+///////////////////////////////////////////////////////////////////////////////
+
+sk_sp<SkSpecialImage> SkDisplacementMapEffect::onFilterImage(SkSpecialImage* source,
+                                                             const Context& ctx,
+                                                             SkIPoint* offset) const {
+    SkIPoint colorOffset = SkIPoint::Make(0, 0);
+    sk_sp<SkSpecialImage> color(this->filterInput(1, source, ctx, &colorOffset));
+    if (!color) {
+        return nullptr;
     }
-    if ((displ.colorType() != kN32_SkColorType) ||
-        (color.colorType() != kN32_SkColorType)) {
-        return false;
+
+    SkIPoint displOffset = SkIPoint::Make(0, 0);
+    sk_sp<SkSpecialImage> displ(this->filterInput(0, source, ctx, &displOffset));
+    if (!displ) {
+        return nullptr;
     }
+
+    const SkIRect srcBounds = SkIRect::MakeXYWH(colorOffset.x(), colorOffset.y(),
+                                                color->width(), color->height());
+
+    // Both paths do bounds checking on color pixel access, we don't need to
+    // pad the color bitmap to bounds here.
     SkIRect bounds;
-    // Since computeDisplacement does bounds checking on color pixel access, we don't need to pad
-    // the color bitmap to bounds here.
-    SkIRect srcBounds = color.bounds();
-    srcBounds.offset(colorOffset);
     if (!this->applyCropRect(ctx, srcBounds, &bounds)) {
-        return false;
+        return nullptr;
     }
+
     SkIRect displBounds;
-    if (!this->applyCropRectDeprecated(ctx, proxy, displ, &displOffset, &displBounds, &displ)) {
-        return false;
+    displ = this->applyCropRect(ctx, displ.get(), &displOffset, &displBounds);
+    if (!displ) {
+        return nullptr;
     }
+
     if (!bounds.intersect(displBounds)) {
-        return false;
-    }
-    SkAutoLockPixels alp_displacement(displ), alp_color(color);
-    if (!displ.getPixels() || !color.getPixels()) {
-        return false;
+        return nullptr;
     }
 
-    SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
-    if (!device) {
-        return false;
-    }
-    *dst = device->accessBitmap(false);
-    SkAutoLockPixels alp_dst(*dst);
+    const SkIRect colorBounds = bounds.makeOffset(-colorOffset.x(), -colorOffset.y());
 
     SkVector scale = SkVector::Make(fScale, fScale);
     ctx.ctm().mapVectors(&scale, 1);
-    SkIRect colorBounds = bounds;
-    colorBounds.offset(-colorOffset);
 
-    computeDisplacement(fXChannelSelector, fYChannelSelector, scale, dst,
-                        &displ, colorOffset - displOffset, &color, colorBounds);
+#if SK_SUPPORT_GPU
+    if (color->peekTexture()) {
+        SkASSERT(displ->peekTexture());
+
+        GrTexture* colorTexture = color->peekTexture();
+        GrTexture* displacement = displ->peekTexture();

[Stephen White, 2016/04/01 19:17:42]

Peeking usually implies that these could be null. Is that the case? If so,
should we be checking for null (or asserting non-null) here?

[robertphillips, 2016/04/01 19:41:43]

Done. The logic is a bit circuitous. If 'source' is backed by a texture then
filterInput should fail (and return null) if either 'color' or 'displ' cannot be
backed by a texture.

[Stephen White, 2016/04/02 02:33:48]

OK, so at this point can we assert that both colorTexture and displacement are
non-null? (Sorry, I missed the assert above before).

As in, SkASSERT(colorTexture && displacement)?

Also could we rename displacement -> displTexture? It's nice to have the names
reflect which are SkSpecialImages and which are textures.

[robertphillips, 2016/04/04 14:24:03]

Done.

+        GrContext* context = colorTexture->getContext();
+
+        GrSurfaceDesc desc;
+        desc.fFlags = kRenderTarget_GrSurfaceFlag;
+        desc.fWidth = bounds.width();
+        desc.fHeight = bounds.height();
+        desc.fConfig = kSkia8888_GrPixelConfig;
+
+        SkAutoTUnref<GrTexture> dst(context->textureProvider()->createApproxTexture(desc));
+        if (!dst) {
+            return nullptr;
+        }
+
+        GrPaint paint;
+        SkMatrix offsetMatrix = GrCoordTransform::MakeDivByTextureWHMatrix(displacement);
+        offsetMatrix.preTranslate(SkIntToScalar(colorOffset.fX - displOffset.fX),
+                                  SkIntToScalar(colorOffset.fY - displOffset.fY));
+
+        paint.addColorFragmentProcessor(
+            GrDisplacementMapEffect::Create(fXChannelSelector,
+                                            fYChannelSelector,
+                                            scale,
+                                            displacement,
+                                            offsetMatrix,
+                                            colorTexture,
+                                            SkISize::Make(color->width(),
+                                                          color->height())))->unref();
+        paint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
+        SkMatrix matrix;
+        matrix.setTranslate(-SkIntToScalar(colorBounds.x()), -SkIntToScalar(colorBounds.y()));
+
+        SkAutoTUnref<GrDrawContext> drawContext(context->drawContext(dst->asRenderTarget()));
+        if (!drawContext) {
+            return nullptr;
+        }
+
+        drawContext->drawRect(GrClip::WideOpen(), paint, matrix, SkRect::Make(colorBounds));
+
+        offset->fX = bounds.left();
+        offset->fY = bounds.top();
+        return SkSpecialImage::MakeFromGpu(source->internal_getProxy(),
+                                           SkIRect::MakeWH(bounds.width(), bounds.height()),
+                                           kNeedNewImageUniqueID_SpecialImage,
+                                           dst);
+    }
+#endif
+
+    SkPixmap colorPixmap, displPixmap;
+
+    if (!color->peekPixels(&colorPixmap) || !displ->peekPixels(&displPixmap)) {
+        return nullptr;
+    }
+
+    if ((colorPixmap.colorType() != kN32_SkColorType) ||
+        (displPixmap.colorType() != kN32_SkColorType)) {
+        return nullptr;
+    }
+
+    SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
+                                            colorPixmap.alphaType());
+
+    SkBitmap dst2;

[Stephen White, 2016/04/01 19:17:42]

Nit: it looks like this could actually be called dst also, without fear of
conflict.

[robertphillips, 2016/04/01 19:41:43]

Done.

+    if (!dst2.tryAllocPixels(info)) {
+        return nullptr;
+    }
+
+    SkAutoLockPixels dstLock(dst2);
+
+    computeDisplacement(fXChannelSelector, fYChannelSelector, scale, &dst2,
+                        displPixmap, colorOffset - displOffset, colorPixmap, colorBounds);
 
     offset->fX = bounds.left();
     offset->fY = bounds.top();
-    return true;
+    return SkSpecialImage::MakeFromRaster(source->internal_getProxy(),
+                                          SkIRect::MakeWH(bounds.width(), bounds.height()),
+                                          dst2);
 }
 
 SkRect SkDisplacementMapEffect::computeFastBounds(const SkRect& src) const {
@@ -326,143 +454,14 @@ private:
 
 ///////////////////////////////////////////////////////////////////////////////
 
-class GrDisplacementMapEffect : public GrFragmentProcessor {
-public:
-    static GrFragmentProcessor* Create(
-            SkDisplacementMapEffect::ChannelSelectorType xChannelSelector,
-            SkDisplacementMapEffect::ChannelSelectorType yChannelSelector, SkVector scale,
-            GrTexture* displacement, const SkMatrix& offsetMatrix, GrTexture* color,
-            const SkISize& colorDimensions) {
-        return new GrDisplacementMapEffect(xChannelSelector, yChannelSelector, scale, displacement,
-                                           offsetMatrix, color, colorDimensions);
-    }
-
-    virtual ~GrDisplacementMapEffect();
-
-    SkDisplacementMapEffect::ChannelSelectorType xChannelSelector() const
-        { return fXChannelSelector; }
-    SkDisplacementMapEffect::ChannelSelectorType yChannelSelector() const
-        { return fYChannelSelector; }
-    const SkVector& scale() const { return fScale; }
-
-    const char* name() const override { return "DisplacementMap"; }
-    const GrTextureDomain& domain() const { return fDomain; }
-
-private:
-    GrGLSLFragmentProcessor* onCreateGLSLInstance() const override {
-        return new GrGLDisplacementMapEffect;
-    }
-
-    void onGetGLSLProcessorKey(const GrGLSLCaps& caps, GrProcessorKeyBuilder* b) const override {
-        GrGLDisplacementMapEffect::GenKey(*this, caps, b);
-    }
-
-    bool onIsEqual(const GrFragmentProcessor&) const override;
-
-    void onComputeInvariantOutput(GrInvariantOutput* inout) const override;
-
-    GrDisplacementMapEffect(SkDisplacementMapEffect::ChannelSelectorType xChannelSelector,
-                            SkDisplacementMapEffect::ChannelSelectorType yChannelSelector,
-                            const SkVector& scale,
-                            GrTexture* displacement, const SkMatrix& offsetMatrix,
-                            GrTexture* color,
-                            const SkISize& colorDimensions);
-
-    GR_DECLARE_FRAGMENT_PROCESSOR_TEST;
-
-    GrCoordTransform            fDisplacementTransform;
-    GrTextureAccess             fDisplacementAccess;
-    GrCoordTransform            fColorTransform;
-    GrTextureDomain             fDomain;
-    GrTextureAccess             fColorAccess;
-    SkDisplacementMapEffect::ChannelSelectorType fXChannelSelector;
-    SkDisplacementMapEffect::ChannelSelectorType fYChannelSelector;
-    SkVector fScale;
-
-    typedef GrFragmentProcessor INHERITED;
-};
-
-bool SkDisplacementMapEffect::filterImageGPUDeprecated(Proxy* proxy, const SkBitmap& src,
-                                                       const Context& ctx,
-                                                       SkBitmap* result, SkIPoint* offset) const {
-    SkBitmap colorBM = src;
-    SkIPoint colorOffset = SkIPoint::Make(0, 0);
-    if (!this->filterInputGPUDeprecated(1, proxy, src, ctx, &colorBM, &colorOffset)) {
-        return false;
-    }
-    SkBitmap displacementBM = src;
-    SkIPoint displacementOffset = SkIPoint::Make(0, 0);
-    if (!this->filterInputGPUDeprecated(0, proxy, src, ctx, &displacementBM, &displacementOffset)) {
-        return false;
-    }
-    SkIRect srcBounds = colorBM.bounds();
-    srcBounds.offset(colorOffset);
-    SkIRect bounds;
-    // Since GrDisplacementMapEffect does bounds checking on color pixel access, we don't need to
-    // pad the color bitmap to bounds here.
-    if (!this->applyCropRect(ctx, srcBounds, &bounds)) {
-        return false;
-    }
-    SkIRect displBounds;
-    if (!this->applyCropRectDeprecated(ctx, proxy, displacementBM,
-                                       &displacementOffset, &displBounds, &displacementBM)) {
-        return false;
-    }
-    if (!bounds.intersect(displBounds)) {
-        return false;
-    }
-    GrTexture* color = colorBM.getTexture();
-    GrTexture* displacement = displacementBM.getTexture();
-    GrContext* context = color->getContext();
-
-    GrSurfaceDesc desc;
-    desc.fFlags = kRenderTarget_GrSurfaceFlag;
-    desc.fWidth = bounds.width();
-    desc.fHeight = bounds.height();
-    desc.fConfig = kSkia8888_GrPixelConfig;
-
-    SkAutoTUnref<GrTexture> dst(context->textureProvider()->createApproxTexture(desc));
-
-    if (!dst) {
-        return false;
-    }
-
-    SkVector scale = SkVector::Make(fScale, fScale);
-    ctx.ctm().mapVectors(&scale, 1);
-
-    GrPaint paint;
-    SkMatrix offsetMatrix = GrCoordTransform::MakeDivByTextureWHMatrix(displacement);
-    offsetMatrix.preTranslate(SkIntToScalar(colorOffset.fX - displacementOffset.fX),
-                              SkIntToScalar(colorOffset.fY - displacementOffset.fY));
-
-    paint.addColorFragmentProcessor(
-        GrDisplacementMapEffect::Create(fXChannelSelector,
-                                        fYChannelSelector,
-                                        scale,
-                                        displacement,
-                                        offsetMatrix,
-                                        color,
-                                        colorBM.dimensions()))->unref();
-    paint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
-    SkIRect colorBounds = bounds;
-    colorBounds.offset(-colorOffset);
-    SkMatrix matrix;
-    matrix.setTranslate(-SkIntToScalar(colorBounds.x()),
-                        -SkIntToScalar(colorBounds.y()));
-
-    SkAutoTUnref<GrDrawContext> drawContext(context->drawContext(dst->asRenderTarget()));
-    if (!drawContext) {
-        return false;
-    }
-
-    drawContext->drawRect(GrClip::WideOpen(), paint, matrix, SkRect::Make(colorBounds));
-    offset->fX = bounds.left();
-    offset->fY = bounds.top();
-    GrWrapTextureInBitmap(dst, bounds.width(), bounds.height(), false, result);
-    return true;
+GrGLSLFragmentProcessor* GrDisplacementMapEffect::onCreateGLSLInstance() const {
+    return new GrGLDisplacementMapEffect;
 }
 
-///////////////////////////////////////////////////////////////////////////////
+void GrDisplacementMapEffect::onGetGLSLProcessorKey(const GrGLSLCaps& caps,
+                                                    GrProcessorKeyBuilder* b) const {
+    GrGLDisplacementMapEffect::GenKey(*this, caps, b);
+}
 
 GrDisplacementMapEffect::GrDisplacementMapEffect(
                              SkDisplacementMapEffect::ChannelSelectorType xChannelSelector,