Fixing SkPicture command pattern optimizations to make them work correctly with bound...
authorcommit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Fri, 15 Mar 2013 15:06:03 +0000 (15:06 +0000)
committercommit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Fri, 15 Mar 2013 15:06:03 +0000 (15:06 +0000)
BUG=https://code.google.com/p/chromium/issues/detail?id=180645
TEST=render_pictures -r <skp_dir> --validate --bbh <grid|rtree> --mode tile 256 256

Author: junov@chromium.org

Reviewed By: robertphillips@google.com

Review URL: https://chromiumcodereview.appspot.com/12817011

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

12 files changed:
src/core/SkBBoxHierarchy.h
src/core/SkBBoxHierarchyRecord.cpp
src/core/SkBBoxHierarchyRecord.h
src/core/SkPicturePlayback.cpp
src/core/SkPictureRecord.cpp
src/core/SkPictureRecord.h
src/core/SkPictureStateTree.cpp
src/core/SkPictureStateTree.h
src/core/SkRTree.cpp
src/core/SkRTree.h
src/core/SkTileGrid.cpp
src/core/SkTileGrid.h

index 4c8b2ae256c3e61b033c3a8cd3c4e365470ce4ca..9de785192a04b76aafc74f5a7a04dcc62efa44a3 100644 (file)
 #include "SkTDArray.h"
 #include "SkRefCnt.h"
 
+/**
+ * Interface for a client class that implements utility methods needed
+ * by SkBBoxHierarchy that require intrinsic knowledge of the data 
+ * object type that is stored in the bounding box hierarchy.
+ */
+class SkBBoxHierarchyClient {
+public:
+    virtual ~SkBBoxHierarchyClient() {}
+
+    /**
+     * Implements a rewind stop condition used by rewindInserts
+     * Must returns true if 'data' points to an object that should be re-wound
+     * by rewinfInserts.
+     */
+    virtual bool shouldRewind(void* data) = 0;
+};
+
 /**
  * Interface for a spatial data structure that associates user data pointers with axis-aligned
  * bounding boxes, and allows efficient retrieval of intersections with query rectangles.
@@ -21,6 +38,8 @@ class SkBBoxHierarchy : public SkRefCnt {
 public:
     SK_DECLARE_INST_COUNT(SkBBoxHierarchy)
 
+    SkBBoxHierarchy() : fClient(NULL) {}
+
     /**
      * Insert a data pointer and corresponding bounding box
      * @param data The data pointer, may be NULL
@@ -49,6 +68,19 @@ public:
      */
     virtual int getCount() const = 0;
 
+    /**
+     * Rewinds all the most recently inserted data elements until an element
+     * is encountered for which client->shouldRewind(data) returns false. May
+     * not rewind elements that were inserted prior to the last call to
+     * flushDeferredInserts.
+     */
+    virtual void rewindInserts() = 0;
+
+    void setClient(SkBBoxHierarchyClient* client) { fClient = client; }
+
+protected:
+    SkBBoxHierarchyClient* fClient;
+
 private:
     typedef SkRefCnt INHERITED;
 };
index 16172f343d052179848ceadbe0da92a077080dfc..9c02468ae1c0b97c82ddb87327079ff6f84f416c 100644 (file)
@@ -8,7 +8,6 @@
 
 #include "SkBBoxHierarchyRecord.h"
 #include "SkPictureStateTree.h"
-#include "SkBBoxHierarchy.h"
 
 SkBBoxHierarchyRecord::SkBBoxHierarchyRecord(uint32_t recordFlags,
                                              SkBBoxHierarchy* h,
@@ -17,6 +16,7 @@ SkBBoxHierarchyRecord::SkBBoxHierarchyRecord(uint32_t recordFlags,
     fStateTree = SkNEW(SkPictureStateTree);
     fBoundingHierarchy = h;
     fBoundingHierarchy->ref();
+    fBoundingHierarchy->setClient(this);
 }
 
 void SkBBoxHierarchyRecord::handleBBox(const SkRect& bounds) {
@@ -103,3 +103,13 @@ bool SkBBoxHierarchyRecord::clipRRect(const SkRRect& rrect,
     fStateTree->appendClip(this->writeStream().size());
     return INHERITED::clipRRect(rrect, op, doAntiAlias);
 }
+
+bool SkBBoxHierarchyRecord::shouldRewind(void* data) {
+    // SkBBoxHierarchy::rewindInserts is called by SkPicture after the
+    // SkPicture has rewound its command stream.  To match that rewind in the
+    // BBH, we rewind all draws that reference commands that were recorded
+    // past the point to which the SkPicture has rewound, which is given by
+    // writeStream().size().
+    SkPictureStateTree::Draw* draw = static_cast<SkPictureStateTree::Draw*>(data);
+    return draw->fOffset >= writeStream().size();
+}
index 854e525961a130c6bc8ba20632415536924b6851..27da3c9188fd975279f0e35df00e6f3b8b76396c 100644 (file)
@@ -9,13 +9,14 @@
 #ifndef SkRTreeCanvas_DEFINED
 #define SkRTreeCanvas_DEFINED
 
+#include "SkBBoxHierarchy.h"
 #include "SkBBoxRecord.h"
 
 /**
  * This records bounding box information into an SkBBoxHierarchy, and clip/transform information
  * into an SkPictureStateTree to allow for efficient culling and correct playback of draws.
  */
-class SkBBoxHierarchyRecord : public SkBBoxRecord {
+class SkBBoxHierarchyRecord : public SkBBoxRecord, public SkBBoxHierarchyClient {
 public:
     /** This will take a ref of h */
     SkBBoxHierarchyRecord(uint32_t recordFlags, SkBBoxHierarchy* h,
@@ -47,6 +48,9 @@ public:
                            SkRegion::Op op = SkRegion::kIntersect_Op,
                            bool doAntiAlias = false) SK_OVERRIDE;
 
+    // Implementation of the SkBBoxHierarchyClient interface
+    virtual bool shouldRewind(void* data) SK_OVERRIDE;
+
 private:
     typedef SkBBoxRecord INHERITED;
 };
index 93e5cf17bf7c8c3f42b8571da27b104eb52c5170..22125ee02b0de4330667cfaa87a0185bd6023260 100644 (file)
@@ -715,23 +715,34 @@ void SkPicturePlayback::draw(SkCanvas& canvas) {
         size_t curOffset = reader.offset();
         uint32_t size;
         DrawType op = read_op_and_size(&reader, &size);
-        if (NOOP == op) {
+        size_t skipTo = 0;
+#ifdef SK_DEVELOPER
+        // TODO: once chunk sizes are in all .skps just use
+        // "curOffset + size"
+        skipTo = this->preDraw(curOffset, op);
+#endif
+        if (0 == skipTo && NOOP == op) {
             // NOOPs are to be ignored - do not propagate them any further
-            reader.setOffset(curOffset+size);
-            continue;
+            skipTo = curOffset + size;
         }
 
-#ifdef SK_DEVELOPER
-        // TODO: once chunk sizes are in all .skps just use "curOffset + size"
-        size_t skipTo = this->preDraw(curOffset, op);
         if (0 != skipTo) {
+            if (it.isValid()) {
+                // If using a bounding box hierarchy, advance the state tree
+                // iterator until at or after skipTo
+                uint32_t adjustedSkipTo;
+                do {
+                    adjustedSkipTo = it.draw();
+                } while (adjustedSkipTo < skipTo);
+                skipTo = adjustedSkipTo;
+            }
             if (kDrawComplete == skipTo) {
                 break;
             }
             reader.setOffset(skipTo);
             continue;
         }
-#endif
+
         switch (op) {
             case CLIP_PATH: {
                 const SkPath& path = getPath(reader);
index 45412fe8812f462901f585058a3e692a185db26e..38bbe33be56012621fda8c3dabc11b58a0f77843 100644 (file)
@@ -502,20 +502,48 @@ static bool collapse_save_clip_restore(SkWriter32* writer, int32_t offset,
 
 typedef bool (*PictureRecordOptProc)(SkWriter32* writer, int32_t offset,
                                      SkPaintDictionary* paintDict);
+enum PictureRecordOptType {
+    kRewind_OptType,  // Optimization rewinds the command stream
+    kCollapseSaveLayer_OptType,  // Optimization eliminates a save/restore pair
+};
 
+struct PictureRecordOpt {
+    PictureRecordOptProc fProc;
+    PictureRecordOptType fType;
+};
 /*
  * A list of the optimizations that are tried upon seeing a restore
  * TODO: add a real API for such optimizations
  *       Add the ability to fire optimizations on any op (not just RESTORE)
  */
-static const PictureRecordOptProc gPictureRecordOpts[] = {
-    collapse_save_clip_restore,
-#ifndef SK_IGNORE_PICTURE_RECORD_SAVE_LAYER_OPT
-    remove_save_layer1,
-    remove_save_layer2,
-#endif
+static const PictureRecordOpt gPictureRecordOpts[] = {
+    { collapse_save_clip_restore, kRewind_OptType },
+    { remove_save_layer1,         kCollapseSaveLayer_OptType },
+    { remove_save_layer2,         kCollapseSaveLayer_OptType }
 };
 
+// This is called after an optimization has been applied to the command stream
+// in order to adjust the contents and state of the bounding box hierarchy and
+// state tree to reflect the optimization.
+static void apply_optimization_to_bbh(PictureRecordOptType opt, SkPictureStateTree* stateTree,
+                                      SkBBoxHierarchy* boundingHierarchy) {
+    switch (opt) {
+    case kCollapseSaveLayer_OptType:
+        stateTree->saveCollapsed();
+        break;
+    case kRewind_OptType:
+        if (NULL != boundingHierarchy) {
+            boundingHierarchy->rewindInserts();
+        }
+        // Note: No need to touch the state tree for this to work correctly.
+        // Unused branches do not burden the playback, and pruning the tree
+        // would be O(N^2), so it is best to leave it alone.
+        break;
+    default:
+        SkASSERT(0);
+    }
+}
+
 void SkPictureRecord::restore() {
     // FIXME: SkDeferredCanvas needs to be refactored to respect
     // save/restore balancing so that the following test can be
@@ -536,10 +564,12 @@ void SkPictureRecord::restore() {
     uint32_t initialOffset, size;
     size_t opt;
     for (opt = 0; opt < SK_ARRAY_COUNT(gPictureRecordOpts); ++opt) {
-        if ((*gPictureRecordOpts[opt])(&fWriter, fRestoreOffsetStack.top(), &fPaints)) {
+        if ((*gPictureRecordOpts[opt].fProc)(&fWriter, fRestoreOffsetStack.top(), &fPaints)) {
             // Some optimization fired so don't add the RESTORE
             size = 0;
             initialOffset = fWriter.size();
+            apply_optimization_to_bbh(gPictureRecordOpts[opt].fType, 
+                                      fStateTree, fBoundingHierarchy);
             break;
         }
     }
index 7a2bb8de470a7e9385c5dfa5beb34b586217d4d7..88ff302b7c82326f028529af8275639f513bd056 100644 (file)
@@ -103,6 +103,7 @@ public:
     void endRecording();
 
 private:
+    void handleOptimization(int opt);
     void recordRestoreOffsetPlaceholder(SkRegion::Op);
     void fillRestoreOffsetPlaceholdersForCurrentStackLevel(
         uint32_t restoreOffset);
index 2abed196588693535f5597d67e103aae14d80d91..9f2db258484e5bd847224d3a6a01561e59f39e17 100644 (file)
@@ -14,6 +14,7 @@ SK_DEFINE_INST_COUNT(SkPictureStateTree)
 SkPictureStateTree::SkPictureStateTree()
     : fAlloc(2048)
     , fRoot(NULL)
+    , fLastRestoredNode(NULL)
     , fStateStack(sizeof(Draw), 16) {
     SkMatrix* identity = static_cast<SkMatrix*>(fAlloc.allocThrow(sizeof(SkMatrix)));
     identity->reset();
@@ -49,7 +50,20 @@ void SkPictureStateTree::appendSaveLayer(uint32_t offset) {
     fCurrentState.fNode->fFlags |= Node::kSaveLayer_Flag;
 }
 
+void SkPictureStateTree::saveCollapsed() {
+    SkASSERT(NULL != fLastRestoredNode);
+    SkASSERT(SkToBool(fLastRestoredNode->fFlags & \
+        (Node::kSaveLayer_Flag | Node::kSave_Flag)));
+    SkASSERT(fLastRestoredNode->fParent == fCurrentState.fNode);
+    // The structure of the tree is not modified here. We just turn off
+    // the save or saveLayer flag to prevent the iterator from making state
+    // changing calls on the playback canvas when traversing a save or
+    // saveLayerNode node.
+    fLastRestoredNode->fFlags = 0;
+}
+
 void SkPictureStateTree::appendRestore() {
+    fLastRestoredNode = fCurrentState.fNode;
     fCurrentState = *static_cast<Draw*>(fStateStack.back());
     fStateStack.pop_back();
 }
index 798e5526b63e07cb40007f76e6ad9dad0123742d..7e137d8fcee3a2ecf7bfadd5c9fcda2751740d5a 100644 (file)
@@ -62,6 +62,13 @@ public:
     void appendTransform(const SkMatrix& trans);
     void appendClip(uint32_t offset);
 
+    /**
+     * Call this immediately after an appendRestore call that is associated
+     * a save or saveLayer that was removed from the command stream
+     * due to a command pattern optimization in SkPicture.
+     */
+    void saveCollapsed();
+
     /**
      * Playback helper
      */
@@ -109,6 +116,10 @@ private:
 
     SkChunkAlloc fAlloc;
     Node* fRoot;
+    // Needed by saveCollapsed() because nodes do not currently store
+    // references to their children.  If they did, we could just retrieve the
+    // last added child.
+    Node* fLastRestoredNode; 
 
     // The currently active state
     Draw fCurrentState;
index 88fc0790820f60c3b8e4aa56b19e08a50d22828a..d7a15d59578e5aa13b90717c6611138a4e9dae3e 100644 (file)
@@ -434,6 +434,14 @@ int SkRTree::validateSubtree(Node* root, SkIRect bounds, bool isRoot) {
     }
 }
 
+void SkRTree::rewindInserts() {
+    SkASSERT(this->isEmpty()); // Currently only supports deferred inserts
+    while (!fDeferredInserts.isEmpty() &&
+           fClient->shouldRewind(fDeferredInserts.top().fChild.data)) {
+        fDeferredInserts.pop();
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 static inline uint32_t get_area(const SkIRect& rect) {
index 0a536677cd2c827feb353f73b1235d934fb295db..2d11f28a5739a21c69da94c999db80562196ced9 100644 (file)
@@ -87,6 +87,8 @@ public:
      */
     virtual int getCount() const { return fCount; }
 
+    virtual void rewindInserts() SK_OVERRIDE;
+
 private:
 
     struct Node;
index 7b901e9b3b8028cc5082f33b10355554aa21394e..641a03179c1248016633cf46851b2373512438c3 100644 (file)
@@ -121,3 +121,12 @@ void SkTileGrid::clear() {
 int SkTileGrid::getCount() const {
     return fInsertionCount;
 }
+
+void SkTileGrid::rewindInserts() {
+    SkASSERT(fClient);
+    for (int i = 0; i < fTileCount; ++i) {
+        while (!fTileData[i].isEmpty() && fClient->shouldRewind(fTileData[i].top())) {
+            fTileData[i].pop();
+        }
+    }
+}
index c83a0fd468ce16a9e6aea5b2819214efd53d667d..3152fa39b34c2232500d08563889f3af3bdc63c4 100644 (file)
@@ -55,6 +55,8 @@ public:
      */
     virtual int getCount() const SK_OVERRIDE;
 
+    virtual void rewindInserts() SK_OVERRIDE;
+
     // Used by search() and in SkTileGridHelper implementations
     enum {
         kTileFinished = -1,