Fix filter primitive bounds computations.

src/core/SkRecordDraw.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkLayerInfo.h"
 #include "SkRecordDraw.h"
 #include "SkPatchUtils.h"
@@ skipping 203 matching lines @@
         }
 
         return rect;
     }
 
 private:
     struct SaveBounds {
         int controlOps;        // Number of control ops in this Save block, including the Save.
         Bounds bounds;         // Bounds of everything in the block.
         const SkPaint* paint;  // Unowned.  If set, adjusts the bounds of all ops in this block.
+        SkMatrix ctm;
     };
 
     // Only Restore, SetMatrix, and Concat change the CTM.
     template <typename T> void updateCTM(const T&) {}
     void updateCTM(const Restore& op)   { fCTM = op.matrix; }
     void updateCTM(const SetMatrix& op) { fCTM = op.matrix; }
     void updateCTM(const Concat& op)    { fCTM.preConcat(op.matrix); }
 
     // Most ops don't change the clip.
     template <typename T> void updateClipBounds(const T&) {}
@@ skipping 60 matching lines @@
 
     void pushSaveBlock(const SkPaint* paint) {
         // Starting a new Save block.  Push a new entry to represent that.
         SaveBounds sb;
         sb.controlOps = 0;
         // If the paint affects transparent black, the bound shouldn't be smaller
         // than the current clip bounds.
         sb.bounds =
             PaintMayAffectTransparentBlack(paint) ? fCurrentClipBounds : Bounds::MakeEmpty();
         sb.paint = paint;
+        sb.ctm = this->fCTM;
 
         fSaveStack.push(sb);
         this->pushControl();
     }
 
     static bool PaintMayAffectTransparentBlack(const SkPaint* paint) {
         if (paint) {
             // FIXME: this is very conservative
             if (paint->getImageFilter() || paint->getColorFilter()) {
                 return true;
@@ skipping 242 matching lines @@
                 *rect = paint->computeFastBounds(*rect, rect);
                 return true;
             }
             return false;
         }
         return true;
     }
 
     bool adjustForSaveLayerPaints(SkRect* rect, int savesToIgnore = 0) const {
         for (int i = fSaveStack.count() - 1 - savesToIgnore; i >= 0; i--) {
+            SkMatrix inverse;
+            if (!fSaveStack[i].ctm.invert(&inverse)) {
+                return false;
+            }
+            inverse.mapRect(rect);
             if (!AdjustForPaint(fSaveStack[i].paint, rect)) {
                 return false;
             }
+            fSaveStack[i].ctm.mapRect(rect);

[mtklein, 2015/12/09 16:59:41]

Is it important that we do rect = (M (M^-1 rect)) here?

I'd just have expected to stash away the original rect and restore it by a
simple copy instead of re-mapping.

[mtklein, 2015/12/09 17:00:53]

(Or put (M^-1 rect) into another SkRect instead of doing it in place in rect?)

[Stephen White, 2015/12/09 17:02:57]

The problem is that we don't know the rect at saveLayer time: we're trying to
map some primitive that we're drawing [1..N] saveLayers 
deep.

That said, at a minimum I should probably stash the matrix inverse on 
the save stack as well, to avoid inverting it each time.

[mtklein, 2015/12/09 17:06:04]

So, (*rect at line 577) != (*rect at line 571)?

[Stephen White, 2015/12/09 17:10:52]

Yeah. It may have been munged by AdjustForPaint(). I'm essentially 
attempting to ensure that that munging is done relative to the 
layer's origin, which unfortunately we don't have access to here.

[mtklein, 2015/12/09 17:15:25]

Oh, of course.  That's the whole point of this method.  Duh.

I'm ok to wait until we see the inverts matter on a profile before we stash the
inverse.

         }
         return true;
     }
 
     const int fNumRecords;
 
     // We do not guarantee anything for operations outside of the cull rect
     const SkRect fCullRect;
 
     // Conservative identity-space bounds for each op in the SkRecord.
@@ skipping 221 matching lines @@
 void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, SkRect bounds[],
                            const SkBigPicture::SnapshotArray* pictList, SkLayerInfo* data) {
     SkRecords::CollectLayers visitor(cullRect, record, bounds, pictList, data);
     for (int curOp = 0; curOp < record.count(); curOp++) {
         visitor.setCurrentOp(curOp);
         record.visit<void>(curOp, visitor);
     }
     visitor.cleanUp();
 }