Get OffsetImageFilter really working with SkSpecialImages

src/effects/SkOffsetImageFilter.cpp

Index: src/effects/SkOffsetImageFilter.cpp
diff --git a/src/effects/SkOffsetImageFilter.cpp b/src/effects/SkOffsetImageFilter.cpp
index ddc8be8f95ae6aa258d6f1f358e2d25033eec394..e4891f93034361871578d7da4719caced5786b1e 100644
--- a/src/effects/SkOffsetImageFilter.cpp
+++ b/src/effects/SkOffsetImageFilter.cpp
@@ -13,53 +13,73 @@
 #include "SkWriteBuffer.h"
 #include "SkMatrix.h"
 #include "SkPaint.h"
+#include "SkSpecialImage.h"
+#include "SkSpecialSurface.h"
 
-bool SkOffsetImageFilter::onFilterImageDeprecated(Proxy* proxy, const SkBitmap& source,
-                                                  const Context& ctx,
-                                                  SkBitmap* result,
-                                                  SkIPoint* offset) const {
-    SkBitmap src = source;
+SkSpecialImage* SkOffsetImageFilter::onFilterImage(SkSpecialImage* srcIn,
+                                                   const Context& ctx,
+                                                   SkIPoint* offset) const {
     SkIPoint srcOffset = SkIPoint::Make(0, 0);
-    if (!cropRectIsSet()) {
-        if (!this->filterInputDeprecated(0, proxy, source, ctx, &src, &srcOffset)) {
-            return false;
-        }
+    SkAutoTUnref<SkSpecialImage> src(this->filterInput(0, srcIn, ctx, &srcOffset));
+    if (!src) {
+        return nullptr;
+    }
 
-        SkVector vec;
-        ctx.ctm().mapVectors(&vec, &fOffset, 1);
+    SkVector vec;
+    ctx.ctm().mapVectors(&vec, &fOffset, 1);
 
+    if (!this->cropRectIsSet()) {
         offset->fX = srcOffset.fX + SkScalarRoundToInt(vec.fX);
         offset->fY = srcOffset.fY + SkScalarRoundToInt(vec.fY);
-        *result = src;
+        return src.release();
     } else {
-        if (!this->filterInputDeprecated(0, proxy, source, ctx, &src, &srcOffset)) {
-            return false;
-        }
-
         SkIRect bounds;
-        SkIRect srcBounds = src.bounds();
+        SkIRect srcBounds = SkIRect::MakeWH(srcIn->width(), srcIn->height());
         srcBounds.offset(srcOffset);
         if (!this->applyCropRect(ctx, srcBounds, &bounds)) {
             return false;
         }
 
-        SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
-        if (nullptr == device.get()) {
-            return false;
+        SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
+                                                kPremul_SkAlphaType);
+        // TODO: need to plumb MSAA sampleCount down to here!

[Stephen White, 2016/02/18 17:56:18]

I don't think we should (except for the original primitive drawn by saveLayer,
which doesn't go through this path), but I might be missing a special case.

Maybe we should just remove this comment and keep an eye on the GM results?

[robertphillips, 2016/02/19 19:37:59]

Done. I agree that all the draws performed by the image filters shouldn't need
MSAA.

+        SkAutoTUnref<SkSpecialSurface> surf(src->newSurface(info));
+        if (!surf) {
+            return nullptr;
         }
-        SkCanvas canvas(device);
+
+        SkCanvas* canvas = surf->getCanvas();
+
+        canvas->clear(0x0);
+
         SkPaint paint;
         paint.setXfermodeMode(SkXfermode::kSrc_Mode);
-        canvas.translate(SkIntToScalar(srcOffset.fX - bounds.fLeft),
-                         SkIntToScalar(srcOffset.fY - bounds.fTop));
-        SkVector vec;
-        ctx.ctm().mapVectors(&vec, &fOffset, 1);
-        canvas.drawBitmap(src, vec.x(), vec.y(), &paint);
-        *result = device->accessBitmap(false);
+        canvas->translate(SkIntToScalar(srcOffset.fX - bounds.fLeft),
+                          SkIntToScalar(srcOffset.fY - bounds.fTop));
+
+        src->draw(canvas, vec.x(), vec.y(), &paint);
+
         offset->fX = bounds.fLeft;
         offset->fY = bounds.fTop;
+        return surf->newImageSnapshot();
     }
-    return true;
+}
+
+bool SkOffsetImageFilter::onFilterImageDeprecated(Proxy* proxy, const SkBitmap& source,

[Stephen White, 2016/02/18 17:56:18]

Can we not simply remove this function now? When does this get called?

[robertphillips, 2016/02/19 19:37:59]

It can still be called when the offset image filter is the root of the DAG
(e.g., from within SkCanvas::internalDrawDevice).

[Stephen White, 2016/02/19 19:53:55]

Could we also switch over the callsites (in SkCanvas and SkGpuDevice) to use
filterImage() in this patch? Then we should be able to remove the legacy path
from each filter as we go. Would that work?

[robertphillips, 2016/02/22 15:21:58]

I think that should probably be its own patch. We could land the SkImageFilter.*
changes, then that patch, then a revised version of the SkOffsetImageFilter.*
files.

[Stephen White, 2016/02/22 15:42:38]

OK, just to clarify: you're saying

1) Land the SkImageFilter.* changes from this patch
2) Land the infrastructure patch (SkCanvas/SkGpuDevice)
3) Land the (updated) SkOffsetImageFilter changes from this patch
*) Convert the remaining filters

?

If so, I'm good with that.

+                                                  const Context& ctx,
+                                                  SkBitmap* result,
+                                                  SkIPoint* offset) const {
+    SkAutoTUnref<SkSpecialImage> specialSrc(SkSpecialImage::internal_fromBM(proxy, source));
+    if (!specialSrc) {
+        return false;
+    }
+
+    SkAutoTUnref<SkSpecialImage> tmp(this->onFilterImage(specialSrc, ctx, offset));
+    if (!tmp) {
+        return false;
+    }
+
+    return tmp->internal_getBM(result);
 }
 
 void SkOffsetImageFilter::computeFastBounds(const SkRect& src, SkRect* dst) const {