Switch AlphaThresholdFilter over to new onFilterImage interface

src/effects/SkAlphaThresholdFilter.cpp

Index: src/effects/SkAlphaThresholdFilter.cpp
diff --git a/src/effects/SkAlphaThresholdFilter.cpp b/src/effects/SkAlphaThresholdFilter.cpp
index 5a9ad79a9ddef9ee5b1535541c8e08481e1ff1a9..ca5838858ccb9dd79ead9e2130c301d05acc8ed8 100644
--- a/src/effects/SkAlphaThresholdFilter.cpp
+++ b/src/effects/SkAlphaThresholdFilter.cpp
@@ -8,8 +8,8 @@
 #include "SkAlphaThresholdFilter.h"
 
 #include "SkBitmap.h"
-#include "SkDevice.h"
 #include "SkReadBuffer.h"
+#include "SkSpecialImage.h"
 #include "SkWriteBuffer.h"
 #include "SkRegion.h"
 #if SK_SUPPORT_GPU
@@ -28,11 +28,11 @@ public:
 protected:
     void flatten(SkWriteBuffer&) const override;
 
-    bool onFilterImageDeprecated(Proxy*, const SkBitmap& src, const Context&,
-                                 SkBitmap* result, SkIPoint* offset) const override;
+    sk_sp<SkSpecialImage> onFilterImage(SkSpecialImage* source, const Context&,
+                                        SkIPoint* offset) const override;
+
 #if SK_SUPPORT_GPU
-    bool asFragmentProcessor(GrFragmentProcessor**, GrTexture*, const SkMatrix&,
-                             const SkIRect& bounds) const override;
+    GrTexture* createMaskTexture(GrContext*, const SkMatrix&, const SkIRect& bounds) const;

[Stephen White, 2016/04/11 20:48:20]

Should this return an sk_sp<GrTexture> instead?

[robertphillips, 2016/04/12 15:36:39]

Done.

 #endif
 
 private:
@@ -285,52 +285,43 @@ SkAlphaThresholdFilterImpl::SkAlphaThresholdFilterImpl(const SkRegion& region,
 }
 
 #if SK_SUPPORT_GPU
-bool SkAlphaThresholdFilterImpl::asFragmentProcessor(GrFragmentProcessor** fp,
-                                                     GrTexture* texture,
-                                                     const SkMatrix& inMatrix,
-                                                     const SkIRect& bounds) const {
-    if (fp) {
-        GrContext* context = texture->getContext();
-        GrSurfaceDesc maskDesc;
-        if (context->caps()->isConfigRenderable(kAlpha_8_GrPixelConfig, false)) {
-            maskDesc.fConfig = kAlpha_8_GrPixelConfig;
-        } else {
-            maskDesc.fConfig = kRGBA_8888_GrPixelConfig;
-        }
-        maskDesc.fFlags = kRenderTarget_GrSurfaceFlag;
-        // Add one pixel of border to ensure that clamp mode will be all zeros
-        // the outside.
-        maskDesc.fWidth = bounds.width();
-        maskDesc.fHeight = bounds.height();
-        SkAutoTUnref<GrTexture> maskTexture(
-            context->textureProvider()->createApproxTexture(maskDesc));
-        if (!maskTexture) {
-            return false;
-        }
+GrTexture* SkAlphaThresholdFilterImpl::createMaskTexture(GrContext* context,
+                                                         const SkMatrix& inMatrix,
+                                                         const SkIRect& bounds) const {
+    GrSurfaceDesc maskDesc;
+    if (context->caps()->isConfigRenderable(kAlpha_8_GrPixelConfig, false)) {
+        maskDesc.fConfig = kAlpha_8_GrPixelConfig;
+    } else {
+        maskDesc.fConfig = kRGBA_8888_GrPixelConfig;
+    }
+    maskDesc.fFlags = kRenderTarget_GrSurfaceFlag;
+    // Add one pixel of border to ensure that clamp mode will be all zeros
+    // the outside.
+    maskDesc.fWidth = bounds.width();
+    maskDesc.fHeight = bounds.height();
+    SkAutoTUnref<GrTexture> maskTexture(context->textureProvider()->createApproxTexture(maskDesc));
+    if (!maskTexture) {
+        return nullptr;
+    }
 
-        SkAutoTUnref<GrDrawContext> drawContext(
-                                            context->drawContext(maskTexture->asRenderTarget()));
-        if (drawContext) {
-            GrPaint grPaint;
-            grPaint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
-            SkRegion::Iterator iter(fRegion);
-            drawContext->clear(nullptr, 0x0, true);
-
-            GrClip clip(SkRect::Make(SkIRect::MakeWH(bounds.width(), bounds.height())));
-            while (!iter.done()) {
-                SkRect rect = SkRect::Make(iter.rect());
-                drawContext->drawRect(clip, grPaint, inMatrix, rect);
-                iter.next();
-            }
-        }
+    SkAutoTUnref<GrDrawContext> drawContext(context->drawContext(maskTexture->asRenderTarget()));
+    if (!drawContext) {
+        return nullptr;
+    }
+
+    GrPaint grPaint;
+    grPaint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
+    SkRegion::Iterator iter(fRegion);
+    drawContext->clear(nullptr, 0x0, true);

[Stephen White, 2016/04/11 20:48:20]

Add a reference to the "clear may not be necessary" bug, if applicable?

[robertphillips, 2016/04/12 15:36:39]

This one is necessary since we don't know where the rects will be drawn in the
mask.

[Stephen White, 2016/04/12 15:41:40]

Good point. Perhaps we could do a pass unioning all the rects first, and then
clear with that as a mask. Anyway, not important.

 
-        *fp = AlphaThresholdEffect::Create(texture,
-                                           maskTexture,
-                                           fInnerThreshold,
-                                           fOuterThreshold,
-                                           bounds);
+    GrClip clip(SkRect::Make(SkIRect::MakeWH(bounds.width(), bounds.height())));
+    while (!iter.done()) {
+        SkRect rect = SkRect::Make(iter.rect());
+        drawContext->drawRect(clip, grPaint, inMatrix, rect);
+        iter.next();
     }
-    return true;
+
+    return maskTexture.release();
 }
 #endif
 
@@ -341,50 +332,139 @@ void SkAlphaThresholdFilterImpl::flatten(SkWriteBuffer& buffer) const {
     buffer.writeRegion(fRegion);
 }
 
-bool SkAlphaThresholdFilterImpl::onFilterImageDeprecated(Proxy* proxy, const SkBitmap& src,
-                                                         const Context& ctx, SkBitmap* dst,
-                                                         SkIPoint* offset) const {
+sk_sp<SkSpecialImage> SkAlphaThresholdFilterImpl::onFilterImage(SkSpecialImage* source,
+                                                                const Context& ctx,
+                                                                SkIPoint* offset) const {
+    SkIPoint inputOffset = SkIPoint::Make(0, 0);
+    sk_sp<SkSpecialImage> input(this->filterInput(0, source, ctx, &inputOffset));
+    if (!input) {
+        return nullptr;
+    }
+
+    const SkIRect inputBounds = SkIRect::MakeXYWH(inputOffset.x(), inputOffset.y(),
+                                                  input->width(), input->height());
+
+    SkIRect bounds;
+    if (!this->applyCropRect(ctx, inputBounds, &bounds)) {

[Stephen White, 2016/04/11 20:48:20]

Given that there's no create param to add a crop rect, I wouldn't bother with
applying it.

Or, add a test which exercises it.

[robertphillips, 2016/04/12 15:36:39]

applyCropRect also does interesting things with the clip. The GPU path was
applying it and thus doing less work than the raster path.

I have also added a parameter to the Make method and a GM to test it. Although
it is unlikely to be used in practice, I prefer the uniformity/consistency.

[Stephen White, 2016/04/12 15:41:39]

Thanks!

+        return nullptr;
+    }
+
+#if SK_SUPPORT_GPU
+    if (source->isTextureBacked()) {
+        GrContext* context = source->getContext();
+
+        sk_sp<GrTexture> inputTexture(input->asTextureRef(context));
+        if (!inputTexture) {

[Stephen White, 2016/04/11 20:48:20]

Please assert this is non-null instead.

[robertphillips, 2016/04/12 15:36:39]

Done.

+            return nullptr;
+        }
+
+        offset->fX = bounds.left();
+        offset->fY = bounds.top();
+
+        bounds.offset(-inputOffset);
+
+        SkMatrix matrix(ctx.ctm());
+        matrix.postTranslate(SkIntToScalar(-bounds.left()), SkIntToScalar(-bounds.top()));
+
+        sk_sp<GrTexture> maskTexture(this->createMaskTexture(context, matrix, bounds));
+        if (!maskTexture) {
+            return nullptr;
+        }
+
+        // SRGBTODO: handle sRGB here

[Stephen White, 2016/04/11 20:48:20]

Is there a bug for that?

[robertphillips, 2016/04/12 15:36:39]

This is Brian O.'s TODO. I believe he is just tracking them as text strings.

[Stephen White, 2016/04/12 15:41:39]

Acknowledged.

+        sk_sp<GrFragmentProcessor> fp(AlphaThresholdEffect::Create(inputTexture.get(),
+                                                                   maskTexture.get(),
+                                                                   fInnerThreshold,
+                                                                   fOuterThreshold,
+                                                                   bounds));
+        if (!fp) {
+            return nullptr;
+        }
+
+        GrPaint paint;
+        paint.addColorFragmentProcessor(fp.get());
+        paint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
+
+        GrSurfaceDesc desc;
+        desc.fFlags = kRenderTarget_GrSurfaceFlag;
+        desc.fWidth = bounds.width();
+        desc.fHeight = bounds.height();
+        desc.fConfig = kRGBA_8888_GrPixelConfig;
+
+        SkAutoTUnref<GrTexture> dst(context->textureProvider()->createApproxTexture(desc));
+        if (!dst) {
+            return nullptr;
+        }
+
+        SkAutoTUnref<GrDrawContext> drawContext(context->drawContext(dst->asRenderTarget()));
+        if (!drawContext) {
+            return nullptr;
+        }
+
+        SkRect srcRect = SkRect::Make(bounds);
+        SkRect dstRect = SkRect::MakeWH(srcRect.width(), srcRect.height());
+        GrClip clip(dstRect);
+        drawContext->fillRectToRect(clip, paint, SkMatrix::I(), dstRect, srcRect);
+
+        return SkSpecialImage::MakeFromGpu(source->internal_getProxy(),
+                                           SkIRect::MakeWH(bounds.width(), bounds.height()),
+                                           kNeedNewImageUniqueID_SpecialImage,
+                                           dst);

[Stephen White, 2016/04/11 20:48:20]

Kind of a shame the GPU code got so much longer in this version. I guess that's
a side-effect of taking the axe to asFragmentProcessor().

[robertphillips, 2016/04/12 15:36:39]

I've added a SkImageFilter::DrawWithFP helper function that should rein this in
a bit (and be reusable in the Magnifier and MatrixConvolution ImageFilters).

[Stephen White, 2016/04/12 15:41:39]

Acknowledged.

+    }
+#endif
 
-    if (src.colorType() != kN32_SkColorType) {
-        return false;
+    SkBitmap inputBM;
+
+    if (!input->getROPixels(&inputBM)) {
+        return nullptr;
+    }
+
+    if (inputBM.colorType() != kN32_SkColorType) {
+        return nullptr;
+    }
+
+    SkAutoLockPixels inputLock(inputBM);
+
+    if (!inputBM.getPixels() || inputBM.width() <= 0 || inputBM.height() <= 0) {
+        return nullptr;
     }
 
+
     SkMatrix localInverse;
     if (!ctx.ctm().invert(&localInverse)) {
-        return false;
+        return nullptr;
     }
 
-    SkAutoLockPixels alp(src);
-    SkASSERT(src.getPixels());
-    if (!src.getPixels() || src.width() <= 0 || src.height() <= 0) {
-        return false;
-    }
+    SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
+                                            inputBM.alphaType());
 
-    SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(src.width(), src.height()));
-    if (!device) {
-        return false;
+    SkBitmap dst;
+    if (!dst.tryAllocPixels(info)) {
+        return nullptr;
     }
-    *dst = device->accessBitmap(false);
-    SkAutoLockPixels alp_dst(*dst);
+
+    SkAutoLockPixels dstLock(dst);
 
     U8CPU innerThreshold = (U8CPU)(fInnerThreshold * 0xFF);
     U8CPU outerThreshold = (U8CPU)(fOuterThreshold * 0xFF);
-    SkColor* sptr = src.getAddr32(0, 0);
-    SkColor* dptr = dst->getAddr32(0, 0);
-    int width = src.width(), height = src.height();
-    for (int y = 0; y < height; ++y) {
-        for (int x = 0; x < width; ++x) {
-            const SkColor& source = sptr[y * width + x];
-            SkColor output_color(source);
+    SkColor* dptr = dst.getAddr32(0, 0);
+    int dstWidth = dst.width(), dstHeight = dst.height();
+    for (int y = 0; y < dstHeight; ++y) {
+        const SkColor* sptr = inputBM.getAddr32(bounds.fLeft, bounds.fTop+y);
+
+        for (int x = 0; x < dstWidth; ++x) {
+            const SkColor& source = sptr[x];
+            SkColor outputColor(source);
             SkPoint position;
-            localInverse.mapXY((SkScalar)x, (SkScalar)y, &position);
+            localInverse.mapXY((SkScalar)x + bounds.fLeft, (SkScalar)y + bounds.fTop, &position);
             if (fRegion.contains((int32_t)position.x(), (int32_t)position.y())) {
                 if (SkColorGetA(source) < innerThreshold) {
                     U8CPU alpha = SkColorGetA(source);
-                    if (alpha == 0)
+                    if (alpha == 0) {
                         alpha = 1;
+                    }
                     float scale = (float)innerThreshold / alpha;
-                    output_color = SkColorSetARGB(innerThreshold,
+                    outputColor = SkColorSetARGB(innerThreshold,
                                                   (U8CPU)(SkColorGetR(source) * scale),
                                                   (U8CPU)(SkColorGetG(source) * scale),
                                                   (U8CPU)(SkColorGetB(source) * scale));
@@ -392,17 +472,21 @@ bool SkAlphaThresholdFilterImpl::onFilterImageDeprecated(Proxy* proxy, const SkB
             } else {
                 if (SkColorGetA(source) > outerThreshold) {
                     float scale = (float)outerThreshold / SkColorGetA(source);
-                    output_color = SkColorSetARGB(outerThreshold,
+                    outputColor = SkColorSetARGB(outerThreshold,
                                                   (U8CPU)(SkColorGetR(source) * scale),
                                                   (U8CPU)(SkColorGetG(source) * scale),
                                                   (U8CPU)(SkColorGetB(source) * scale));
                 }
             }
-            dptr[y * dst->width() + x] = output_color;
+            dptr[y * dstWidth + x] = outputColor;
         }
     }
 
-    return true;
+    offset->fX = bounds.left();
+    offset->fY = bounds.top();
+    return SkSpecialImage::MakeFromRaster(source->internal_getProxy(),
+                                          SkIRect::MakeWH(bounds.width(), bounds.height()),
+                                          dst);
 }
 
 #ifndef SK_IGNORE_TO_STRING