From d4236574cfe4ec333d20e7a30f37e084168bb050 Mon Sep 17 00:00:00 2001 From: "djsollen@google.com" Date: Tue, 13 Aug 2013 14:29:06 +0000 Subject: [PATCH] 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 --- gm/canvasstate.cpp | 113 +++++++++++++++++++++++++++++++++++ gyp/gmslides.gypi | 1 + include/core/SkCanvas.h | 6 ++ src/core/SkPictureRecord.cpp | 13 +++- 4 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 gm/canvasstate.cpp diff --git a/gm/canvasstate.cpp b/gm/canvasstate.cpp new file mode 100644 index 0000000000..1a26bf0df7 --- /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 87c46e776f..bc0fc65415 100644 --- 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 b4853f7f33..a368182cb4 100644 --- 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 915736373d..2c2d334caf 100644 --- 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).