post apply non-scale transforms after imagefilters have run

src/core/SkCanvas.cpp

Index: src/core/SkCanvas.cpp
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 9d7a3d5c0a034f5d352ac7a07baeed9a82502d4e..53e0d61025ac515fa3d0175d579273eaf470357d 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -196,12 +196,14 @@ struct DeviceCM {
     SkPaint*            fPaint; // may be null (in the future)
     const SkMatrix*     fMatrix;
     SkMatrix            fMatrixStorage;
+    SkMatrix            fStashedMatrix; // original CTM; used by imagefilter in saveLayer
     const bool          fDeviceIsBitmapDevice;
 
     DeviceCM(SkBaseDevice* device, const SkPaint* paint, SkCanvas* canvas,
-             bool conservativeRasterClip, bool deviceIsBitmapDevice)
+             bool conservativeRasterClip, bool deviceIsBitmapDevice, const SkMatrix& stashed)
         : fNext(nullptr)
         , fClip(conservativeRasterClip)
+        , fStashedMatrix(stashed)
         , fDeviceIsBitmapDevice(deviceIsBitmapDevice)
     {
         if (nullptr != device) {
@@ -664,7 +666,8 @@ SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) {
 
     SkASSERT(sizeof(DeviceCM) <= sizeof(fDeviceCMStorage));
     fMCRec->fLayer = (DeviceCM*)fDeviceCMStorage;
-    new (fDeviceCMStorage) DeviceCM(nullptr, nullptr, nullptr, fConservativeRasterClip, false);
+    new (fDeviceCMStorage) DeviceCM(nullptr, nullptr, nullptr, fConservativeRasterClip, false,
+                                    fMCRec->fMatrix);
 
     fMCRec->fTopLayer = fMCRec->fLayer;
 
@@ -1193,6 +1196,43 @@ void SkCanvas::internalSaveLayer(const SaveLayerRec& rec, SaveLayerStrategy stra
     saveLayerFlags &= ~kDontClipToLayer_PrivateSaveLayerFlag;
 #endif
 
+    SkLazyPaint lazyP;
+    SkImageFilter* imageFilter = paint ? paint->getImageFilter() : NULL;
+    SkMatrix stashedMatrix = fMCRec->fMatrix;
+#ifndef SK_SUPPORT_LEGACY_IMAGEFILTER_CTM
+    SkMatrix remainder;
+    SkSize scale;
+    /*
+     *  ImageFilters (so far) do not correctly handle matrices (CTM) that contain rotation/skew/etc.
+     *  but they do handle scaling. To accommodate this, we do the following:
+     *
+     *  1. Stash off the current CTM
+     *  2. Decompose the CTM into SCALE and REMAINDER
+     *  3. Wack the CTM to be just SCALE, and wrap the imagefilter with a MatrixImageFilter that
+     *     contains the REMAINDER
+     *  4. Proceed as usual, allowing the client to draw into the layer (now with a scale-only CTM)
+     *  5. During restore, we process the MatrixImageFilter, which applies REMAINDER to the output
+     *     of the original imagefilter, and draw that (via drawSprite)
+     *  6. Unwack the CTM to its original state (i.e. stashedMatrix)
+     *
+     *  Perhaps in the future we could augment #5 to apply REMAINDER as part of the draw (no longer
+     *  a sprite operation) to avoid the extra buffer/overhead of MatrixImageFilter.
+     */
+    if (imageFilter &&
+        !stashedMatrix.isScaleTranslate() &&
+        stashedMatrix.decomposeScale(&scale, &remainder))
+    {
+        // We will restore the matrix (which we are overwriting here) in restore via fStashedMatrix
+        this->internalSetMatrix(SkMatrix::MakeScale(scale.width(), scale.height()));
+        SkPaint* p = lazyP.set(*paint);
+        p->setImageFilter(SkImageFilter::MakeMatrixFilter(remainder,
+                                                          SkFilterQuality::kLow_SkFilterQuality,

[Stephen White, 2016/04/20 18:39:34]

Blink currently uses High quality in SkiaImageFilterBuilder::buildTransform().
Won't using Low here cause a quality loss?

[reed1, 2016/04/20 19:42:50]

My thinking was that since we decompose the scale, the filter should just see
rotations etc, meaning that there should be minimal advantage to using something
fancier than bilerp (e.g. High or Medium). Additionally, those latter two cache
their labors, which will have zero value here, since we don't have a reliable
key for a saveLayer as src.

Are you ok if we try with Low, and examine the layout results, and see if they
are acceptable?

[Stephen White, 2016/04/20 21:19:32]

OK, let's give it a try.

+                                                          sk_ref_sp(imageFilter)));
+        imageFilter = p->getImageFilter();
+        paint = p;
+    }
+#endif
+
     // do this before we create the layer. We don't call the public save() since
     // that would invoke a possibly overridden virtual
     this->internalSave();
@@ -1200,7 +1240,7 @@ void SkCanvas::internalSaveLayer(const SaveLayerRec& rec, SaveLayerStrategy stra
     fDeviceCMDirty = true;
 
     SkIRect ir;
-    if (!this->clipRectBounds(bounds, saveLayerFlags, &ir, paint ? paint->getImageFilter() : nullptr)) {
+    if (!this->clipRectBounds(bounds, saveLayerFlags, &ir, imageFilter)) {
         return;
     }
 
@@ -1256,8 +1296,8 @@ void SkCanvas::internalSaveLayer(const SaveLayerRec& rec, SaveLayerStrategy stra
         draw_filter_into_device(fMCRec->fTopLayer->fDevice, rec.fBackdrop, device, fMCRec->fMatrix);
     }
 
-    DeviceCM* layer =
-            new DeviceCM(device, paint, this, fConservativeRasterClip, forceSpriteOnRestore);
+    DeviceCM* layer = new DeviceCM(device, paint, this, fConservativeRasterClip,
+                                   forceSpriteOnRestore, stashedMatrix);
     device->unref();
 
     layer->fNext = fMCRec->fTopLayer;
@@ -1302,6 +1342,8 @@ void SkCanvas::internalRestore() {
             const SkIPoint& origin = layer->fDevice->getOrigin();
             this->internalDrawDevice(layer->fDevice, origin.x(), origin.y(),
                                      layer->fPaint, layer->fDeviceIsBitmapDevice);
+            // restore what we smashed in internalSaveLayer
+            fMCRec->fMatrix = layer->fStashedMatrix;
             // reset this, since internalDrawDevice will have set it to true
             fDeviceCMDirty = true;
             delete layer;
@@ -1309,6 +1351,7 @@ void SkCanvas::internalRestore() {
             // we're at the root
             SkASSERT(layer == (void*)fDeviceCMStorage);
             layer->~DeviceCM();
+            // no need to update fMCRec, 'cause we're killing the canvas
         }
     }
 }
@@ -1484,19 +1527,20 @@ void SkCanvas::concat(const SkMatrix& matrix) {
     this->didConcat(matrix);
 }
 
-void SkCanvas::setMatrix(const SkMatrix& matrix) {
-    this->checkForDeferredSave();
+void SkCanvas::internalSetMatrix(const SkMatrix& matrix) {
     fDeviceCMDirty = true;
     fCachedLocalClipBoundsDirty = true;
     fMCRec->fMatrix = matrix;
+}
+
+void SkCanvas::setMatrix(const SkMatrix& matrix) {
+    this->checkForDeferredSave();
+    this->internalSetMatrix(matrix);
     this->didSetMatrix(matrix);
 }
 
 void SkCanvas::resetMatrix() {
-    SkMatrix matrix;
-
-    matrix.reset();
-    this->setMatrix(matrix);
+    this->setMatrix(SkMatrix::I());
 }
 
 //////////////////////////////////////////////////////////////////////////////