Prevent picture recording from over optimizing the culling of clips.

BUG=skia:1496
R=mtklein@google.com, reed@google.com, robertphillips@google.com

Review URL: https://codereview.chromium.org/22875008

git-svn-id: http://skia.googlecode.com/svn/trunk@10689 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/gm/canvasstate.cpp b/gm/canvasstate.cpp
new file mode 100644 (file)
index 0000000..1a26bf0
--- /dev/null
+++ b/gm/canvasstate.cpp
@@ -0,0 +1,113 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm.h"
+#include "SkCanvas.h"
+#include "SkPaint.h"
+#include "SkPath.h"
+#include "SkRect.h"
+
+namespace skiagm {
+
+/*
+ * This GM exercises the flags to SkCanvas::save(). The canvas' save() and
+ * restore actions can be limited to only a portion of the canvas' state through
+ * the use of flags when calling save.
+ */
+class CanvasStateGM : public GM {
+    SkSize  fSize;
+    enum {
+        WIDTH = 150,
+        HEIGHT = 150,
+    };
+
+    SkPaint fFillPaint;
+    SkPaint fStrokePaint;
+
+    SkPath fPath;
+
+    SkRect fOutlineRect;
+    SkRect fFillRect;
+
+
+public:
+    CanvasStateGM() {
+        fSize.set(SkIntToScalar(WIDTH), SkIntToScalar(HEIGHT));
+
+        fFillPaint.setColor(SK_ColorRED);
+        fFillPaint.setStyle(SkPaint::kFill_Style);
+
+        fStrokePaint.setColor(SK_ColorBLUE);
+        fStrokePaint.setStyle(SkPaint::kStroke_Style);
+        fStrokePaint.setStrokeWidth(1);
+
+        fPath.moveTo(25, 25);
+        fPath.lineTo(125, 25);
+        fPath.lineTo(75, 125);
+        fPath.close();
+
+        fOutlineRect = SkRect::MakeXYWH(1, 1, WIDTH-2, HEIGHT-2);
+        fFillRect = SkRect::MakeXYWH(10, 10, WIDTH-20, HEIGHT-20);
+    }
+
+protected:
+    virtual SkString onShortName() SK_OVERRIDE {
+        return SkString("canvas-state");
+    }
+
+    virtual SkISize onISize() SK_OVERRIDE {
+        return SkISize::Make(WIDTH*3, HEIGHT*4);
+    }
+
+    virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE {
+
+        SkCanvas::SaveFlags flags[] = { SkCanvas::kMatrix_SaveFlag,
+                                        SkCanvas::kClip_SaveFlag,
+                                        SkCanvas::kMatrixClip_SaveFlag };
+
+        // columns -- flags
+        // rows -- permutations of setting the clip and matrix
+        for (size_t i = 0; i < SK_ARRAY_COUNT(flags); ++i) {
+            for (int j = 0; j < 2; ++j) {
+                for (int k = 0; k < 2; ++k) {
+                    this->drawTestPattern(i, (2*j)+k, canvas, flags[i], j, k);
+                }
+            }
+        }
+    }
+
+
+    virtual uint32_t onGetFlags() SK_OVERRIDE const { return kSkipPicture_Flag; }
+
+private:
+    void drawTestPattern(int x, int y, SkCanvas* canvas,
+                         SkCanvas::SaveFlags flags, bool doClip, bool doScale) {
+        canvas->save();
+        canvas->translate(x*WIDTH, y*HEIGHT);
+
+        canvas->drawRect(fOutlineRect, fStrokePaint);
+        canvas->save(flags);
+        if(doClip) {
+            canvas->clipPath(fPath);
+        }
+        if (doScale) {
+            canvas->scale(0.5, 0.5);
+        }
+        canvas->restore();
+        canvas->drawRect(fFillRect, fFillPaint);
+
+        canvas->restore();
+    }
+
+    typedef GM INHERITED;
+};
+
+//////////////////////////////////////////////////////////////////////////////
+
+DEF_GM( return SkNEW(CanvasStateGM); )
+
+} // end namespace

diff --git a/gyp/gmslides.gypi b/gyp/gmslides.gypi
index 87c46e7..bc0fc65 100644 (file)
--- a/gyp/gmslides.gypi
+++ b/gyp/gmslides.gypi
@@ -19,6 +19,7 @@
     '../gm/blurs.cpp',
     '../gm/blurquickreject.cpp',
     '../gm/blurrect.cpp',
+    '../gm/canvasstate.cpp',
     '../gm/circles.cpp',
     '../gm/circularclips.cpp',
     '../gm/colorfilterimagefilter.cpp',

diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index b4853f7..a368182 100644 (file)
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -239,6 +239,12 @@ public:
         operate on this copy.
         When the balancing call to restore() is made, the previous matrix, clip,
         and drawFilter are restored.
+        @param flags The flags govern what portion of the Matrix/Clip/drawFilter
+                     state the save (and matching restore) effect. For example,
+                     if only kMatrix is specified, then only the matrix state
+                     will be pushed and popped. Likewise for the clip if kClip
+                     is specified.  However, the drawFilter is always affected
+                     by calls to save/restore.
         @return The value to pass to restoreToCount() to balance this save()
     */
     virtual int save(SaveFlags flags = kMatrixClip_SaveFlag);

diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 9157363..2c2d334 100644 (file)
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -24,6 +24,7 @@ enum {
 // A lot of basic types get stored as a uint32_t: bools, ints, paint indices, etc.
 static int const kUInt32Size = 4;
 
+static const uint32_t kSaveSize = 2 * kUInt32Size;
 static const uint32_t kSaveLayerNoBoundsSize = 4 * kUInt32Size;
 static const uint32_t kSaveLayerWithBoundsSize = 4 * kUInt32Size + sizeof(SkRect);
 
@@ -148,7 +149,7 @@ int SkPictureRecord::save(SaveFlags flags) {
     fRestoreOffsetStack.push(-(int32_t)fWriter.size());
 
     // op + flags
-    uint32_t size = 2 * kUInt32Size;
+    uint32_t size = kSaveSize;
     uint32_t initialOffset = this->addDraw(SAVE, &size);
     addInt(flags);
 
@@ -479,6 +480,16 @@ static bool collapse_save_clip_restore(SkWriter32* writer, int32_t offset,
         return false;
     }
     SkASSERT(SAVE == op);
+    SkASSERT(kSaveSize == opSize);
+
+    // get the save flag (last 4-bytes of the space allocated for the opSize)
+    SkCanvas::SaveFlags saveFlags = (SkCanvas::SaveFlags) *writer->peek32(offset+4);
+    if (SkCanvas::kMatrixClip_SaveFlag != saveFlags) {
+        // This function's optimization is only correct for kMatrixClip style saves.
+        // TODO: set checkMatrix & checkClip booleans here and then check for the
+        // offending operations in the following loop.
+        return false;
+    }
 
     // Walk forward until we get back to either a draw-verb (abort) or we hit
     // our restore (success).