Culling API

src/core/SkPictureRecord.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkPictureRecord.h"
 #include "SkTSearch.h"
 #include "SkPixelRef.h"
@@ skipping 57 matching lines @@
 // Return the offset of the paint inside a given op's byte stream. A zero
 // return value means there is no paint (and you really shouldn't be calling
 // this method)
 static inline uint32_t getPaintOffset(DrawType op, uint32_t opSize) {
     // These offsets are where the paint would be if the op size doesn't overflow
     static const uint8_t gPaintOffsets[LAST_DRAWTYPE_ENUM + 1] = {
         0,  // UNUSED - no paint
         0,  // CLIP_PATH - no paint
         0,  // CLIP_REGION - no paint
         0,  // CLIP_RECT - no paint
         0,  // CLIP_RRECT - no paint

[robertphillips, 2014/02/25 15:58:41]

move to match other moves?

[f(malita), 2014/02/25 16:50:28]

Will do.

+        0,  // PUSH_CULL - no paint
+        0,  // POP_CULL - no paint
         0,  // CONCAT - no paint
         1,  // DRAW_BITMAP - right after op code
         1,  // DRAW_BITMAP_MATRIX - right after op code
         1,  // DRAW_BITMAP_NINE - right after op code
         1,  // DRAW_BITMAP_RECT_TO_RECT - right after op code
         0,  // DRAW_CLEAR - no paint
         0,  // DRAW_DATA - no paint
         1,  // DRAW_OVAL - right after op code
         1,  // DRAW_PAINT - right after op code
         1,  // DRAW_PATH - right after op code
@@ skipping 1452 matching lines @@
     this->validate(initialOffset, size);
 }
 
 void SkPictureRecord::endCommentGroup() {
     // op/size
     uint32_t size = 1 * kUInt32Size;
     size_t initialOffset = this->addDraw(END_COMMENT_GROUP, &size);
     this->validate(initialOffset, size);
 }
 
+// [op/size] [rect] [skip offset]
+static const uint32_t kPushCullOpSize = 2 * kUInt32Size + sizeof(SkRect);
+void SkPictureRecord::onPushCull(const SkRect& cullRect) {

[robertphillips, 2014/02/25 15:58:41]

Don't we also want to assert that the current cull rect is contained inside the
cull rect on the top of the stack (if there is one)?

[f(malita), 2014/02/25 16:50:28]

At a high level it surely makes sense (and my initial thought was to do exactly
that), but it comes at a cost: either the client (blink) or the API will need to
keep a cull rect stack and intersect against its top.

Here's a common Blink pattern (think ads or pictures strip): one small-size
clipping div with a whole bunch of children, only a few of them visible at a
time. The parent div pushes its cull bounds, but then its children are
blissfully unaware of their parent clipping and would naturally push culls
outside their parent's.

Options:

1) Blink tracks cull rects and only pushes the intersection
2) Skia tracks cull rects and always intersects against the last one
3) do nothing and allow overflowing culls - the end result (as far as playback
skipping is concerned) is still correct, but maybe there are other implications
down the line where this would cause issues

What do you guys think?

I guess we can attain #2 with minimal overhead in SkPictureRecord::onPushCull
(we are tracking cull rects for optimizations, etc), but that wouldn't help with
an assert in the non-virtual entry point.

+    // Skip identical cull rects.
+    if (!fCullOffsetStack.isEmpty()) {
+        const SkRect& prevCull = fWriter.readTAt<SkRect>(fCullOffsetStack.top() - sizeof(SkRect));
+        if (prevCull == cullRect) {
+            // Skipped culls are tracked on the stack, but they point to the previous offset.
+            fCullOffsetStack.push(fCullOffsetStack.top());
+            return;
+        }
+    }
+
+    uint32_t size = kPushCullOpSize;
+    size_t initialOffset = this->addDraw(PUSH_CULL, &size);
+    // PUSH_CULL's size should stay constant (used to rewind).
+    SkASSERT(size == kPushCullOpSize);
+

[robertphillips, 2014/02/25 15:58:41]

this->

+    addRect(cullRect);
+    fCullOffsetStack.push(fWriter.bytesWritten());

[robertphillips, 2014/02/25 15:58:41]

this->

+    addInt(0);
+    this->validate(initialOffset, size);
+}
+
+void SkPictureRecord::onPopCull() {
+    SkASSERT(!fCullOffsetStack.isEmpty());
+
+    uint32_t cullSkipOffset = fCullOffsetStack.top();
+    fCullOffsetStack.pop();
+
+    // Skipped push, do the same for pop.
+    if (!fCullOffsetStack.isEmpty() && cullSkipOffset == fCullOffsetStack.top()) {
+        return;
+    }
+
+    // Collapse empty push/pop pairs.
+    if ((size_t)(cullSkipOffset + kUInt32Size) == fWriter.bytesWritten()) {
+        SkASSERT(fWriter.bytesWritten() >= kPushCullOpSize);

[robertphillips, 2014/02/25 15:58:41]

can we have a peek_op static function for this?

+        SkASSERT(fWriter.readTAt<uint32_t>(
+                     fWriter.bytesWritten() - kPushCullOpSize) >> 24 == PUSH_CULL);
+        fWriter.rewindToOffset(fWriter.bytesWritten() - kPushCullOpSize);
+        return;
+    }
+
+    // op only
+    uint32_t size = kUInt32Size;
+    size_t initialOffset = this->addDraw(POP_CULL, &size);
+
+    // update the cull skip offset to point past this op.
+    fWriter.overwriteTAt<uint32_t>(cullSkipOffset, fWriter.bytesWritten());
+
+    this->validate(initialOffset, size);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 SkSurface* SkPictureRecord::onNewSurface(const SkImageInfo& info) {
     return SkSurface::NewPicture(info.fWidth, info.fHeight);
 }
 
 void SkPictureRecord::addBitmap(const SkBitmap& bitmap) {
     const int index = fBitmapHeap->insert(bitmap);
     // In debug builds, a bad return value from insert() will crash, allowing for debugging. In
     // release builds, the invalid value will be recorded so that the reader will know that there
@@ skipping 235 matching lines @@
 void SkPictureRecord::validateRegions() const {
     int count = fRegions.count();
     SkASSERT((unsigned) count < 0x1000);
     for (int index = 0; index < count; index++) {
         const SkFlatData* region = fRegions[index];
         SkASSERT(region);
 //        region->validate();
     }
 }
 #endif