Adding option in SkPicture to record device-space bounds of draw commands.

src/core/SkPicturePlayback.cpp

Index: src/core/SkPicturePlayback.cpp
===================================================================
--- src/core/SkPicturePlayback.cpp	(revision 8043)
+++ src/core/SkPicturePlayback.cpp	(working copy)
@@ -124,6 +124,12 @@
         }
     }
 
+#if SK_RECORD_BOUNDS_IN_PICTURE
+    fHasRecordedBounds = true;
+#else
+    fHasRecordedBounds = true;

[reed1, 2013/03/08 20:23:23]

do we want false here?

[Justin Novosad, 2013/03/08 22:35:42]

Done.

+#endif
+
 #ifdef SK_DEBUG_SIZE
     int overall = fPlayback->size(&overallBytes);
     bitmaps = fPlayback->bitmaps(&bitmapBytes);
@@ -176,7 +182,7 @@
 
     fBoundingHierarchy = src.fBoundingHierarchy;
     fStateTree = src.fStateTree;
-
+    fHasRecordedBounds = src.fHasRecordedBounds;
     SkSafeRef(fBoundingHierarchy);
     SkSafeRef(fStateTree);
 
@@ -270,6 +276,7 @@
     fFactoryPlayback = NULL;
     fBoundingHierarchy = NULL;
     fStateTree = NULL;
+    fHasRecordedBounds = false;
 }
 
 SkPicturePlayback::~SkPicturePlayback() {
@@ -586,7 +593,7 @@
 SkPicturePlayback::SkPicturePlayback(SkStream* stream, const SkPictInfo& info,
                                      SkPicture::InstallPixelRefProc proc) {
     this->init();
-
+    fHasRecordedBounds = info.fFlags & SkPictInfo::kHasRecordedBounds_Flag;

[reed1, 2013/03/08 20:23:23]

SkToBool(...)

[Justin Novosad, 2013/03/08 22:35:42]

Done.

     for (;;) {
         uint32_t tag = stream->readU32();
         if (PICT_EOF_TAG == tag) {
@@ -650,6 +657,21 @@
     return (DrawType) op;
 }
 
+static inline bool get_intial_clip_bounds(const SkCanvas& canvas, SkIRect* bounds, bool* isSet) {

[reed1, 2013/03/08 20:23:23]

Maybe this is the best way, but this treats both isSet and bounds as
input/output params, which is unexpected in general. Of note, we call this
sometimes with an uninitialized bounds, and just happen to know that we are
passing the right value in isSet so that bounds won't be read-from. Is there a
cleaner way to organize this? Perhaps diff variants of this that don't overload
the params so much?

getClipBounds returns its answer in local coordinates, yet you are rounding it
to IRect (which we typically use for device-bounds).

[Justin Novosad, 2013/03/08 22:35:42]

Done.

+    SkASSERT(NULL != bounds && NULL != isSet);
+    if (!(*isSet)) {
+        SkRect clipBounds;
+        *isSet = true;
+        if (canvas.getClipBounds(&clipBounds)) {
+            clipBounds.roundOut(bounds);
+        } else {
+            bounds->setEmpty();
+            return false;
+        }
+    }
+    return !bounds->isEmpty();
+}
+
 void SkPicturePlayback::draw(SkCanvas& canvas) {
 #ifdef ENABLE_TIME_DRAW
     SkAutoTime  at("SkPicture::draw", 50);
@@ -671,21 +693,30 @@
     TextContainer text;
     SkTDArray<void*> results;
 
+    bool initialClipBoundsSet = false;
+    SkIRect initialClipBounds;  // only computed if needed
     if (NULL != fStateTree && NULL != fBoundingHierarchy) {
-        SkRect clipBounds;
-        if (canvas.getClipBounds(&clipBounds)) {
-            SkIRect query;
-            clipBounds.roundOut(&query);
-            fBoundingHierarchy->search(query, &results);
+        if (get_intial_clip_bounds(canvas, &initialClipBounds, &initialClipBoundsSet)) {
+            fBoundingHierarchy->search(initialClipBounds, &results);
             if (results.count() == 0) {
                 return;
             }
             SkTQSort<SkPictureStateTree::Draw>(
                 reinterpret_cast<SkPictureStateTree::Draw**>(results.begin()),
                 reinterpret_cast<SkPictureStateTree::Draw**>(results.end()-1));
+        } else {
+            return;
         }
     }
 
+    if (fHasRecordedBounds) {
+        // TODO(junov): temporarily disable quickReject testing on 'canvas'
+        // because it is redundant with early bounds check.
+        if (!get_intial_clip_bounds(canvas, &initialClipBounds, &initialClipBoundsSet)) {
+            return;
+        }
+    }
+
     SkPictureStateTree::Iterator it = (NULL == fStateTree) ?
         SkPictureStateTree::Iterator() :
         fStateTree->getIterator(results, &canvas);
@@ -715,23 +746,44 @@
         size_t curOffset = reader.offset();
         uint32_t size;
         DrawType op = read_op_and_size(&reader, &size);
-        if (NOOP == op) {
-            // NOOPs are to be ignored - do not propagate them any further
-            reader.setOffset(curOffset+size);
-            continue;
+        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) {
+            if (NOOP == op) {
+                // NOOPs are to be ignored - do not propagate them any further
+                skipTo = curOffset + size;
+            } else if (fHasRecordedBounds && SkPictureRecord::canRecordBounds(op)) {
+                const SkIRect& clippedBounds = reader.skipT<SkIRect>();
+                // recording canvas device bounds are in the same coordinate space
+                // as local clip bounds at start of playback.
+                SkASSERT(initialClipBoundsSet);
+                if (!SkIRect::Intersects(clippedBounds, initialClipBounds)) {
+                    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);