Consolidate image filter evaluation into SkBaseDevice

This CL does several things:
1. SkDevice subclasses that implement drawSpecial() now assert that the
provided SkPaint does not have an SkImageFilter.
2. The SkGpuDevice implementation of drawSpecial() is simplified to go
through the draw_texture_producer code path. This removes duplicate logic
for paint and texture handling, and also allows simple drawSpecials to
actually use the texture op.
3. SkCanvas' drawInternalDevice and onDrawImage now define the
skif::Mappings that define the coord transforms for filter eval and
final draw, before calling the new drawFilteredImage on SkDevice.
  - This is implemented in SkBaseDevice, but subclasses can extend it.
  - I had originally put it as a static function in SkCanvas, but that
    meant I had to forward declare more types in SkCanvas.h than I liked.
  - A lot of the skif::Context properties are dependent on the dst device
    anyways, so having a drawFilteredImage on device seems okay, now that
    filter eval is separated from how to draw a special image.
  - Once drawSpecial can take a matrix transform, and once image filters
know how to handle non-(0,0) source origins, this will simplify further.
4. Swapped the behavior of SkDevice::getRelativeTransform. I originally
wrote it so that it would be the matrix mapping from arg to caller.
After finally using it here (its original purpose), I realized it reads
better to be the matrix from caller to arg.
  - Previously this was never called, so compatibility isn't a problem.

Bug: skia:9558
Change-Id: If855903110947a11ae89b1e50cf5848a3c5dbecd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308768
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

Author: lhkbob

src/core/SkBitmapDevice.cpp

@@ -590,36 +590,13 @@ void SkBitmapDevice::drawAtlas(const SkImage* atlas, const SkRSXform xform[],
 
 ///////////////////////////////////////////////////////////////////////////////
 
-void SkBitmapDevice::drawSpecial(SkSpecialImage* src, int x, int y, const SkPaint& origPaint) {
+void SkBitmapDevice::drawSpecial(SkSpecialImage* src, int x, int y, const SkPaint& paint) {
     SkASSERT(!src->isTextureBacked());
-    SkASSERT(!origPaint.getMaskFilter());
-
-    sk_sp<SkSpecialImage> filteredImage;
-    SkTCopyOnFirstWrite<SkPaint> paint(origPaint);
-
-    if (SkImageFilter* filter = paint->getImageFilter()) {
-        SkIPoint offset = SkIPoint::Make(0, 0);
-        const SkMatrix matrix = SkMatrix::Concat(
-            SkMatrix::Translate(SkIntToScalar(-x), SkIntToScalar(-y)), this->localToDevice());
-        const SkIRect clipBounds = fRCStack.rc().getBounds().makeOffset(-x, -y);
-        sk_sp<SkImageFilterCache> cache(this->getImageFilterCache());
-        SkImageFilter_Base::Context ctx(matrix, clipBounds, cache.get(), fBitmap.colorType(),
-                                        fBitmap.colorSpace(), src);
-
-        filteredImage = as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset);
-        if (!filteredImage) {
-            return;
-        }
-
-        src = filteredImage.get();
-        paint.writable()->setImageFilter(nullptr);
-        x += offset.x();
-        y += offset.y();
-    }
+    SkASSERT(!paint.getMaskFilter() && !paint.getImageFilter());
 
     SkBitmap resultBM;
     if (src->getROPixels(&resultBM)) {
-        BDDraw(this).drawSprite(resultBM, x, y, *paint);
+        BDDraw(this).drawSprite(resultBM, x, y, paint);
     }
 }
 

src/core/SkCanvas.cpp

@@ -1411,21 +1411,47 @@ void SkCanvas::internalDrawDevice(SkBaseDevice* srcDev, const SkPaint* paint) {
         SkBaseDevice* dstDev = iter.fDevice;
         check_drawdevice_colorspaces(dstDev->imageInfo().colorSpace(),
                                      srcDev->imageInfo().colorSpace());
-        paint = &draw.paint();
+
+        SkTCopyOnFirstWrite<SkPaint> noFilterPaint(*paint);
         SkImageFilter* filter = paint->getImageFilter();
-        // TODO(michaelludwig) - Devices aren't created with complex coordinate systems yet,
-        // so it should always be possible to use the relative origin. Once drawDevice() and
-        // drawSpecial() take an SkMatrix, this can switch to getRelativeTransform() instead.
-        SkIPoint pos = srcDev->getOrigin() - dstDev->getOrigin();
         if (filter) {
-            sk_sp<SkSpecialImage> specialImage = srcDev->snapSpecial();
-            if (specialImage) {
-                check_drawdevice_colorspaces(dstDev->imageInfo().colorSpace(),
-                                             specialImage->getColorSpace());
-                dstDev->drawSpecial(specialImage.get(), pos.x(), pos.y(), *paint);
+            // Check if the image filter was just a color filter (this is the same optimization
+            // we apply in AutoLayerForImageFilter but handles explicitly saved layers).
+            sk_sp<SkColorFilter> cf = image_to_color_filter(*paint);
+            if (cf) {
+                noFilterPaint.writable()->setColorFilter(std::move(cf));
+                filter = nullptr;
             }
+
+            noFilterPaint.writable()->setImageFilter(nullptr);
+        }
+
+        SkASSERT(!noFilterPaint->getImageFilter());
+        if (!filter) {
+            // Can draw the src device's buffer w/o any extra image filter evaluation
+            // (although this draw may include color filter processing extracted from the IF DAG).
+            // TODO (michaelludwig) - Once drawSpecial can take a matrix, drawDevice should take
+            // no extra arguments and internally just use the relative transform from src to dst.
+            SkIPoint pos = srcDev->getOrigin() - dstDev->getOrigin();
+            dstDev->drawDevice(srcDev, pos.x(), pos.y(), *noFilterPaint);
         } else {
-            dstDev->drawDevice(srcDev, pos.x(), pos.y(), *paint);
+            // Use the whole device buffer, presumably it was sized appropriately to match the
+            // desired output size of the destination when the layer was first saved.
+            sk_sp<SkSpecialImage> srcBuffer = srcDev->snapSpecial();
+            if (!srcBuffer) {
+                return;
+            }
+
+            // Evaluate the image filter DAG on the src device's buffer. The filter processes an
+            // image in the src's device space. However, the filter parameters need to respect the
+            // dst's local matrix (this reflects the CTM that was set when the layer was first
+            // saved). We can achieve this by concatenating the dst's local-to-device matrix with
+            // the relative transform from dst to src. Then the final result is drawn to dst using
+            // the relative transform from src to dst.
+            SkMatrix srcToDst = srcDev->getRelativeTransform(*dstDev);
+            SkMatrix dstToSrc = dstDev->getRelativeTransform(*srcDev);
+            skif::Mapping mapping(srcToDst, SkMatrix::Concat(dstToSrc, dstDev->localToDevice()));
+            dstDev->drawFilteredImage(mapping, srcBuffer.get(), filter, *noFilterPaint);
         }
     }
 
@@ -2497,11 +2523,15 @@ void SkCanvas::onDrawImage(const SkImage* image, SkScalar x, SkScalar y, const S
     paint = &realPaint;
 
     sk_sp<SkSpecialImage> special;
+    sk_sp<SkImageFilter> filter;
     bool drawAsSprite = this->canDrawBitmapAsSprite(x, y, image->width(), image->height(),
                                                     *paint);
     if (drawAsSprite && paint->getImageFilter()) {
         special = this->getDevice()->makeSpecial(image);
-        if (!special) {
+        if (special) {
+            filter = paint->refImageFilter();
+            realPaint.setImageFilter(nullptr); // This modifies 'paint' but is not const
+        } else {
             drawAsSprite = false;
         }
     }
@@ -2511,11 +2541,18 @@ void SkCanvas::onDrawImage(const SkImage* image, SkScalar x, SkScalar y, const S
     while (iter.next()) {
         const SkPaint& pnt = draw.paint();
         if (special) {
-            SkPoint pt;
-            iter.fDevice->localToDevice().mapXY(x, y, &pt);
-            iter.fDevice->drawSpecial(special.get(),
-                                      SkScalarRoundToInt(pt.fX),
-                                      SkScalarRoundToInt(pt.fY), pnt);
+            SkASSERT(!pnt.getImageFilter());
+
+            // TODO(michaelludwig) - Many filters could probably be evaluated like this even if the
+            // CTM is not translate-only; the post-transformation of the filtered image by the CTM
+            // will probably look just as good and not require an extra layer.
+            // TODO(michaelludwig) - Once image filter implementations can support source images
+            // with non-(0,0) origins, we can just mark the origin as (x,y) instead of doing a
+            // pre-concat here.
+            SkMatrix layerToDevice = iter.fDevice->localToDevice();
+            layerToDevice.preTranslate(x, y);
+            skif::Mapping mapping(layerToDevice, SkMatrix::Translate(-x, -y));
+            iter.fDevice->drawFilteredImage(mapping, special.get(), filter.get(), pnt);
         } else {
             iter.fDevice->drawImageRect(
                     image, nullptr, SkRect::MakeXYWH(x, y, image->width(), image->height()), pnt,

src/core/SkDevice.cpp

@@ -18,6 +18,7 @@
 #include "src/core/SkDraw.h"
 #include "src/core/SkGlyphRun.h"
 #include "src/core/SkImageFilterCache.h"
+#include "src/core/SkImageFilter_Base.h"
 #include "src/core/SkImagePriv.h"
 #include "src/core/SkLatticeIter.h"
 #include "src/core/SkMarkerStack.h"
@@ -86,10 +87,10 @@ SkIPoint SkBaseDevice::getOrigin() const {
                           SkScalarFloorToInt(fDeviceToGlobal.getTranslateY()));
 }
 
-SkMatrix SkBaseDevice::getRelativeTransform(const SkBaseDevice& inputDevice) const {
-    // To get the transform from the input's space to this space, transform from the input space to
-    // the global space, and then from the global space back to this space.
-    return SkMatrix::Concat(fGlobalToDevice, inputDevice.fDeviceToGlobal);
+SkMatrix SkBaseDevice::getRelativeTransform(const SkBaseDevice& dstDevice) const {
+    // To get the transform from this space to the other device's, transform from our space to
+    // global and then from global to the other device.
+    return SkMatrix::Concat(dstDevice.fGlobalToDevice, fDeviceToGlobal);
 }
 
 bool SkBaseDevice::getLocalToMarker(uint32_t id, SkM44* localToMarker) const {
@@ -324,6 +325,41 @@ sk_sp<SkSpecialImage> SkBaseDevice::snapSpecial() {
     return this->snapSpecial(SkIRect::MakeWH(this->width(), this->height()));
 }
 
+void SkBaseDevice::drawFilteredImage(const skif::Mapping& mapping, SkSpecialImage* src,
+                                     const SkImageFilter* filter, const SkPaint& paint) {
+    SkASSERT(!paint.getImageFilter() && !paint.getMaskFilter());
+    using For = skif::Usage;
+
+    skif::LayerSpace<SkIRect> targetOutput = mapping.deviceToLayer(
+            skif::DeviceSpace<SkIRect>(this->devClipBounds()));
+
+    // FIXME If the saved layer (so src) was created to use F16, should we do all image filtering
+    // in F16 and then only flatten to the destination color encoding at the end?
+    // Currently, this context converts everything to the dst color type ASAP.
+    SkColorType colorType = this->imageInfo().colorType();
+    if (colorType == kUnknown_SkColorType) {
+        colorType = kRGBA_8888_SkColorType;
+    }
+
+    // getImageFilterCache returns a bare image filter cache pointer that must be ref'ed until the
+    // filter's filterImage(ctx) function returns.
+    sk_sp<SkImageFilterCache> cache(this->getImageFilterCache());
+    skif::Context ctx(mapping, targetOutput, cache.get(), colorType, this->imageInfo().colorSpace(),
+                      skif::FilterResult<For::kInput>(sk_ref_sp(src)));
+
+    SkIPoint offset;
+    sk_sp<SkSpecialImage> result = as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset);
+    if (result) {
+        // TODO(michaelludwig) - Eventually drawSpecial will take a matrix and we can just
+        // draw using mapping.deviceMatrix() directly. For now, all devices are relative to each
+        // other by a translation, or its a translation-only sprite draw.
+        SkASSERT(mapping.deviceMatrix().isTranslate());
+        offset.fX += SkScalarRoundToInt(mapping.deviceMatrix().getTranslateX());
+        offset.fY += SkScalarRoundToInt(mapping.deviceMatrix().getTranslateY());
+        this->drawSpecial(result.get(), offset.fX, offset.fY, paint);
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool SkBaseDevice::readPixels(const SkPixmap& pm, int x, int y) {

src/core/SkDevice.h

@@ -22,13 +22,18 @@ class SkBitmap;
 struct SkDrawShadowRec;
 class SkGlyphRun;
 class SkGlyphRunList;
+class SkImageFilter;
 class SkImageFilterCache;
 struct SkIRect;
 class SkMarkerStack;
 class SkMatrix;
 class SkRasterHandleAllocator;
 class SkSpecialImage;
 
+namespace skif {
+    class Mapping;
+} // namespace skif
+
 class SkBaseDevice : public SkRefCnt, public SkMatrixProvider {
 public:
     SkBaseDevice(const SkImageInfo&, const SkSurfaceProps&);
@@ -126,10 +131,10 @@ class SkBaseDevice : public SkRefCnt, public SkMatrixProvider {
      */
     bool isPixelAlignedToGlobal() const;
     /**
-     *  Get the transformation from the input device's to this device's coordinate space. This
-     *  transform can be used to draw the input device into this device, such that once this device
-     *  is drawn to the root device, the net effect will have the input device's content drawn
-     *  transformed by the global CTM.
+     * Get the transformation from this device's coordinate system to the provided device space.
+     * This transform can be used to draw this device into the provided device, such that once
+     * that device is drawn to the root device, the net effect will be that this device's contents
+     * have been transformed by the global CTM.
      */
     SkMatrix getRelativeTransform(const SkBaseDevice&) const;
 
@@ -289,19 +294,33 @@ class SkBaseDevice : public SkRefCnt, public SkMatrixProvider {
                                     const SkPoint dstClips[], const SkMatrix preViewMatrices[],
                                     const SkPaint& paint, SkCanvas::SrcRectConstraint);
 
+    void drawGlyphRunRSXform(const SkFont&, const SkGlyphID[], const SkRSXform[], int count,
+                             SkPoint origin, const SkPaint& paint);
+
+    virtual void drawDrawable(SkDrawable*, const SkMatrix*, SkCanvas*);
+
     /** The SkDevice passed will be an SkDevice which was returned by a call to
         onCreateDevice on this device with kNeverTile_TileExpectation.
      */
     virtual void drawDevice(SkBaseDevice*, int x, int y, const SkPaint&) = 0;
 
-    void drawGlyphRunRSXform(const SkFont&, const SkGlyphID[], const SkRSXform[], int count,
-                             SkPoint origin, const SkPaint& paint);
+    virtual void drawSpecial(SkSpecialImage*, int x, int y, const SkPaint&);
 
-    virtual void drawDrawable(SkDrawable*, const SkMatrix*, SkCanvas*);
+    /**
+     * Evaluate 'filter' and draw the final output into this device using 'paint'. The 'mapping'
+     * defines the parameter-to-layer space transform used to evaluate the image filter on 'src',
+     * and the layer-to-device space transform that is used to draw the result into this device.
+     * Since 'mapping' fully specifies the transform, this draw function ignores the current
+     * local-to-device matrix (i.e. just like drawSpecial and drawDevice).
+     *
+     * The final paint must not have an image filter or mask filter set on it; a shader is ignored.
+     */
+    virtual void drawFilteredImage(const skif::Mapping& mapping, SkSpecialImage* src,
+                                   const SkImageFilter* filter, const SkPaint& paint);
 
-    virtual void drawSpecial(SkSpecialImage*, int x, int y, const SkPaint&);
     virtual sk_sp<SkSpecialImage> makeSpecial(const SkBitmap&);
     virtual sk_sp<SkSpecialImage> makeSpecial(const SkImage*);
+
     // Get a view of the entire device's current contents as an image.
     sk_sp<SkSpecialImage> snapSpecial();
     // Snap the 'subset' contents from this device, possibly as a read-only view. If 'forceCopy'
@@ -502,6 +521,9 @@ class SkNoPixelsDevice : public SkBaseDevice {
     void drawGlyphRunList(const SkGlyphRunList& glyphRunList) override {}
     void drawVertices(const SkVertices*, SkBlendMode, const SkPaint&) override {}
 
+    void drawFilteredImage(const skif::Mapping& mapping, SkSpecialImage* src,
+                           const SkImageFilter* filter, const SkPaint& paint) override {}
+
 private:
     typedef SkBaseDevice INHERITED;
 };

src/core/SkImageFilterTypes.h

@@ -519,6 +519,9 @@ class FilterResult {
     FilterResult(sk_sp<SkSpecialImage> image, const LayerSpace<SkIPoint>& origin)
             : fImage(std::move(image))
             , fOrigin(origin) {}
+    explicit FilterResult(sk_sp<SkSpecialImage> image)
+            : fImage(std::move(image))
+            , fOrigin{{0, 0}} {}
 
     // Allow explicit moves/copies in order to cast from one use type to another, except kInput0
     // and kInput1 can only be cast to kOutput (e.g. as part of a noop image filter).