Fix saveLayer() with a pixel-moving filter vs SkBBoxHierarchyRecord / SkRecordDraw

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 "SkRecordDraw.h"
 #include "SkTSort.h"
 
@@ skipping 142 matching lines @@
         this->trackBounds(op);
     }
 
 private:
     struct SaveBounds {
         int controlOps;        // Number of control ops in this Save block, including the Save.
         SkIRect bounds;        // Bounds of everything in the block.
         const SkPaint* paint;  // Unowned.  If set, adjusts the bounds of all ops in this block.
     };
 
+    static bool PaintMayAffectTransparentBlack(const SkPaint* paint) {
+        // FIXME: this is very conservative
+        return paint && (paint->getImageFilter() || paint->getColorFilter());
+    }
+
     template <typename T> void updateCTM(const T&) { /* most ops don't change the CTM */ }
     void updateCTM(const Restore& op)   { fCTM = &op.matrix; }
     void updateCTM(const SetMatrix& op) { fCTM = &op.matrix; }
 
     template <typename T> void updateClipBounds(const T&) { /* most ops don't change the clip */ }
     // Each of these devBounds fields is the state of the device bounds after the op.
     // So Restore's devBounds are those bounds saved by its paired Save or SaveLayer.
     void updateClipBounds(const Restore& op)    { fCurrentClipBounds = op.devBounds; }
     void updateClipBounds(const ClipPath& op)   { fCurrentClipBounds = op.devBounds; }
     void updateClipBounds(const ClipRRect& op)  { fCurrentClipBounds = op.devBounds; }
     void updateClipBounds(const ClipRect& op)   { fCurrentClipBounds = op.devBounds; }
     void updateClipBounds(const ClipRegion& op) { fCurrentClipBounds = op.devBounds; }
     void updateClipBounds(const SaveLayer& op)  {
-        if (op.bounds) {
+        // Intersect the clip with the SaveLayer bounds, unless the paint affects transparent black.
+        if (op.bounds && !PaintMayAffectTransparentBlack(op.paint)) {

[mtklein, 2014/08/22 15:45:58]

Just reading again:

"Since the bounds we pass to saveLayer are in the pre-filtering
coordinate space, they aren't correct for determining the actual
device pixels touched by the saveLayer in this case."

Does the fact that I'm running bounds through adjustAndMap() (adjust for paint
affects, map back to indentity space) mean I can logically drop this part of the
change?

             fCurrentClipBounds.intersect(this->adjustAndMap(*op.bounds, op.paint));
         }
     }
 
     // The bounds of these ops must be calculated when we hit the Restore
     // from the bounds of the ops in the same Save block.
     void trackBounds(const Save&)          { this->pushSaveBlock(NULL); }
     // TODO: bounds of SaveLayer may be more complicated?
     void trackBounds(const SaveLayer& op)  { this->pushSaveBlock(op.paint); }
     void trackBounds(const Restore&) { fBounds[fCurrentOp] = this->popSaveBlock(); }
@@ skipping 14 matching lines @@
         // Starting a new Save block.  Push a new entry to represent that.
         SaveBounds sb = { 0, SkIRect::MakeEmpty(), paint };
         fSaveStack.push(sb);
         this->pushControl();
     }
 
     SkIRect popSaveBlock() {
         // We're done the Save block.  Apply the block's bounds to all control ops inside it.
         SaveBounds sb;
         fSaveStack.pop(&sb);
+
+        // If the paint affects transparent black, we can't trust any of our calculated bounds.
+        const SkIRect& bounds =
+            PaintMayAffectTransparentBlack(sb.paint) ? fCurrentClipBounds : sb.bounds;
+
         while (sb.controlOps --> 0) {
-            this->popControl(sb.bounds);
+            this->popControl(bounds);
         }
 
         // This whole Save block may be part another Save block.
-        this->updateSaveBounds(sb.bounds);
+        this->updateSaveBounds(bounds);
 
         // If called from a real Restore (not a phony one for balance), it'll need the bounds.
-        return sb.bounds;
+        return bounds;
     }
 
     void pushControl() {
         fControlIndices.push(fCurrentOp);
         if (!fSaveStack.isEmpty()) {
             fSaveStack.top().controlOps++;
         }
     }
 
     void popControl(const SkIRect& bounds) {
@@ skipping 160 matching lines @@
     // Used to track the bounds of Save/Restore blocks and the control ops inside them.
     SkTDArray<SaveBounds> fSaveStack;
     SkTDArray<unsigned>   fControlIndices;
 };
 
 }  // namespace SkRecords
 
 void SkRecordFillBounds(const SkRecord& record, SkBBoxHierarchy* bbh) {
     SkRecords::FillBounds(record, bbh);
 }