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

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 43 matching lines @@
 SkPictureRecord::~SkPictureRecord() {
     SkSafeUnref(fBitmapHeap);
     SkSafeUnref(fPathHeap);
     SkSafeUnref(fBoundingHierarchy);
     SkSafeUnref(fStateTree);
     fFlattenableHeap.setBitmapStorage(NULL);
     fPictureRefs.unrefAll();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 

[robertphillips, 2013/03/07 18:37:40]

Do you want to also make this one static inline?

[reed1, 2013/03/07 20:36:44]

Maybe its time to create a static uint8_t array that holds a bitfield of
attributes for each op-code, so we don't have to executed any switch-statements
when trying to classify an draw-op.

+bool SkPictureRecord::canRecordBounds(DrawType op) {

[Justin Novosad, 2013/03/07 20:25:47]

It needs to be accessible from SkPictureBitmap.

+    SkASSERT(op != DRAW_POS_TEXT_TOP_BOTTOM);  // Should never be used when encoding bounds
+    SkASSERT(op != DRAW_POS_TEXT_H_TOP_BOTTOM);
+    switch (op) {
+        case DRAW_BITMAP:
+        case DRAW_BITMAP_MATRIX:
+        case DRAW_BITMAP_NINE:
+        case DRAW_BITMAP_RECT_TO_RECT:
+        case DRAW_OVAL:
+        case DRAW_PATH:
+        case DRAW_POINTS:
+        case DRAW_POS_TEXT:
+        case DRAW_POS_TEXT_H:
+        case DRAW_RECT:
+        case DRAW_RRECT:
+        case DRAW_TEXT:
+        case DRAW_TEXT_ON_PATH:
+        case DRAW_VERTICES:
+            return true;
+        default:
+            return false;
+    }
+}
+
+static inline bool modifiesState(DrawType op) {
+    switch (op) {
+        case CLIP_PATH:
+        case CLIP_REGION:
+        case CLIP_RECT:
+        case CLIP_RRECT:
+        case CONCAT:
+        case DRAW_PICTURE:
+        case RESTORE:
+        case ROTATE:
+        case SAVE:
+        case SAVE_LAYER:
+        case SCALE:
+        case SET_MATRIX:
+        case SKEW:
+        case TRANSLATE:
+            return true;
+        default:
+            return false;
+    }
+}
+
 // 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) {
+static inline uint32_t getPaintOffset(DrawType op, uint32_t opSize, bool recordBounds) {
     // 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
         0,  // CONCAT - no paint
         1,  // DRAW_BITMAP - right after op code
         1,  // DRAW_BITMAP_MATRIX - right after op code
@@ skipping 30 matching lines @@
 
     SkASSERT(sizeof(gPaintOffsets) == LAST_DRAWTYPE_ENUM + 1);
     SkASSERT((unsigned)op <= (unsigned)LAST_DRAWTYPE_ENUM);
 
     int overflow = 0;
     if (0 != (opSize & ~MASK_24) || opSize == MASK_24) {
         // This op's size overflows so an extra uint32_t will be written
         // after the op code
         overflow = sizeof(uint32_t);
     }
+    if (recordBounds && SkPictureRecord::canRecordBounds(op)) {
+        overflow += sizeof(SkIRect);
+    }
 
     if (SAVE_LAYER == op) {
         static const uint32_t kSaveLayerNoBoundsPaintOffset = 2 * kUInt32Size;
         static const uint32_t kSaveLayerWithBoundsPaintOffset = 2 * kUInt32Size + sizeof(SkRect);
 
         if (kSaveLayerNoBoundsSize == opSize) {
             return kSaveLayerNoBoundsPaintOffset + overflow;
         } else {
             SkASSERT(kSaveLayerWithBoundsSize == opSize);
             return kSaveLayerWithBoundsPaintOffset + overflow;
@@ skipping 10 matching lines @@
 }
 
 int SkPictureRecord::save(SaveFlags flags) {
     // record the offset to us, making it non-positive to distinguish a save
     // from a clip entry.
     fRestoreOffsetStack.push(-(int32_t)fWriter.size());
 
     // op + flags
     uint32_t size = 2 * kUInt32Size;
     uint32_t initialOffset = this->addDraw(SAVE, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addInt(flags);
 
     validate(initialOffset, size);
     return this->INHERITED::save(flags);
 }
 
 int SkPictureRecord::saveLayer(const SkRect* bounds, const SkPaint* paint,
                                SaveFlags flags) {
     // record the offset to us, making it non-positive to distinguish a save
     // from a clip entry.
     fRestoreOffsetStack.push(-(int32_t)fWriter.size());
 
     // op + bool for 'bounds'
     uint32_t size = 2 * kUInt32Size;
     if (NULL != bounds) {
         size += sizeof(*bounds); // + rect
     }
     // + paint index + flags
     size += 2 * kUInt32Size;
 
     SkASSERT(kSaveLayerNoBoundsSize == size || kSaveLayerWithBoundsSize == size);
 
     uint32_t initialOffset = this->addDraw(SAVE_LAYER, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addRectPtr(bounds);
-    SkASSERT(initialOffset+getPaintOffset(SAVE_LAYER, size) == fWriter.size());
+    SkASSERT(initialOffset + getPaintOffset(SAVE_LAYER, size, false) == fWriter.size());
     addPaintPtr(paint);
     addInt(flags);
 
     if (kNoSavedLayerIndex == fFirstSavedLayerIndex) {
         fFirstSavedLayerIndex = fRestoreOffsetStack.count();
     }
 
     validate(initialOffset, size);
     /*  Don't actually call saveLayer, because that will try to allocate an
         offscreen device (potentially very big) which we don't actually need
@@ skipping 44 matching lines @@
 
 /*
  * Restore has just been called (but not recorded), look back at the
  * matching save* and see if we are in the configuration:
  *   SAVE_LAYER
  *       DRAW_BITMAP|DRAW_BITMAP_MATRIX|DRAW_BITMAP_NINE|DRAW_BITMAP_RECT_TO_RECT
  *   RESTORE
  * where the saveLayer's color can be moved into the drawBitmap*'s paint
  */
 static bool remove_save_layer1(SkWriter32* writer, int32_t offset,
-                               SkPaintDictionary* paintDict) {
+                               SkPaintDictionary* paintDict, bool recordBounds) {
 
 #ifdef SK_IGNORE_PICTURE_RECORD_SAVE_LAYER_OPT
     return false;
 #endif
 
     int32_t restoreOffset = (int32_t)writer->size();
 
     // back up to the save block
     // TODO: add a stack to track save*/restore offsets rather than searching backwards
     while (offset > 0) {
@@ skipping 27 matching lines @@
     if (DRAW_BITMAP != op && DRAW_BITMAP_MATRIX != op &&
         DRAW_BITMAP_NINE != op && DRAW_BITMAP_RECT_TO_RECT != op) {
         return false;       // not a match
     }
 
     offset = dbmOffset + dbmSize;
     if (offset < restoreOffset) {
         return false;       // something else between the dbm* and the restore
     }
 
-    uint32_t dbmPaintOffset = getPaintOffset(op, dbmSize);
-    uint32_t slPaintOffset = getPaintOffset(SAVE_LAYER, saveLayerSize);
+    uint32_t dbmPaintOffset = getPaintOffset(op, dbmSize, recordBounds);
+    uint32_t slPaintOffset = getPaintOffset(SAVE_LAYER, saveLayerSize, false);
 
     // we have a match, now we need to get the paints involved
     int dbmPaintId = *((int32_t*)writer->peek32(dbmOffset+dbmPaintOffset));
     int saveLayerPaintId = *((int32_t*)writer->peek32(saveLayerOffset+slPaintOffset));
 
     if (0 == saveLayerPaintId) {
         // In this case the saveLayer/restore isn't needed at all - just kill the saveLayer
         // and signal the caller (by returning true) to not add the RESTORE op
         uint32_t* ptr = writer->peek32(saveLayerOffset);
         *ptr = (*ptr & MASK_24) | (NOOP << 24);
@@ skipping 113 matching lines @@
     if (fRestoreOffsetStack.count() == 0) {
         return;
     }
 
     if (fRestoreOffsetStack.count() == fFirstSavedLayerIndex) {
         fFirstSavedLayerIndex = kNoSavedLayerIndex;
     }
 
     uint32_t initialOffset, size;
     if (!collapseSaveClipRestore(&fWriter, fRestoreOffsetStack.top()) &&
-        !remove_save_layer1(&fWriter, fRestoreOffsetStack.top(), &fPaints)) {
+        !remove_save_layer1(&fWriter, fRestoreOffsetStack.top(), &fPaints,
+        this->recordBounds())) {
         fillRestoreOffsetPlaceholdersForCurrentStackLevel((uint32_t)fWriter.size());
         // op
         size = 1 * kUInt32Size;
         initialOffset = this->addDraw(RESTORE, &size);
     } else {
         size = 0;
         initialOffset = fWriter.size();
     }
+    SkASSERT(kInvalidOffset != initialOffset);
 
     fRestoreOffsetStack.pop();
 
     validate(initialOffset, size);
     return this->INHERITED::restore();
 }
 
+uint32_t SkPictureRecord::addDraw(DrawType drawType, uint32_t* size,
+                                  const SkPaint* paint, const SkRect* localBounds) {
+    bool writeBounds = this->recordBounds() && canRecordBounds(drawType);
+    SkIRect clippedDevBounds;  // Bounds of the clipped draw in device space
+    SkIRect devClipBounds;  // Bounds of the current clip in device space
+    if (!modifiesState(drawType) && !this->getClipDeviceBounds(&devClipBounds)) {
+        // Optimize-out calls that are clipped-out.  State modifying commands
+        // can not be optimized-out because they may have an impact on future calls
+        // if the clip is ever expanded. For example, with a union clip op. 
+        return kInvalidOffset;
+    }

[robertphillips, 2013/03/07 18:37:40]

mv clippedDevBounds down to here?

+    if (writeBounds) {
+        SkASSERT(!modifiesState(drawType)); // Ensure devClipBounds is initialized
+        if (NULL == localBounds || (NULL != paint && !paint->canComputeFastBounds())) {
+            clippedDevBounds = devClipBounds;
+        } else {
+            SkRect paintBounds;
+            SkRect rectifiedBounds = *localBounds;
+            rectifiedBounds.sort();
+            if (NULL != paint) {
+                if (drawType == DRAW_POINTS) {
+                    paintBounds = paint->computeFastStrokeBounds(rectifiedBounds, &paintBounds);
+                } else {
+                    paintBounds = paint->computeFastBounds(rectifiedBounds, &paintBounds);
+                }
+            } else {
+                paintBounds = *localBounds;
+            }
+            if (paintBounds.isEmpty()) {
+                // Area affected by draw op is empty even after accounting
+                // for stroke width.
+                return kInvalidOffset;
+            }
+            SkRect devBounds;
+            this->getTotalMatrix().mapRect(&devBounds, paintBounds);
+            devBounds.roundOut(&clippedDevBounds);
+            if (!clippedDevBounds.intersect(devClipBounds)) {
+                // Draw lies outside of clip. The clip can only get further
+                // restricted at playback time, so it is safe to reject during
+                // record stage.
+                return kInvalidOffset;
+            }
+        }
+        *size += sizeof(SkIRect);
+    }
+
+    uint32_t offset = fWriter.size();
+
+    this->predrawNotify();
+
+    #ifdef SK_DEBUG_TRACE
+        SkDebugf("add %s\n", DrawTypeToString(drawType));
+    #endif
+    SkASSERT(0 != *size);
+    SkASSERT(((uint8_t) drawType) == drawType);
+    if (0 != (*size & ~MASK_24) || *size == MASK_24) {
+        fWriter.writeInt(PACK_8_24(drawType, MASK_24));
+        *size += 1;
+        fWriter.writeInt(*size);
+    } else {
+        fWriter.writeInt(PACK_8_24(drawType, *size));
+    }
+
+    if (writeBounds) {
+        fWriter.write(&clippedDevBounds, sizeof(clippedDevBounds));
+    }
+
+    return offset;
+}
+
 bool SkPictureRecord::translate(SkScalar dx, SkScalar dy) {
     // op + dx + dy
     uint32_t size = 1 * kUInt32Size + 2 * sizeof(SkScalar);
     uint32_t initialOffset = this->addDraw(TRANSLATE, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addScalar(dx);
     addScalar(dy);
     validate(initialOffset, size);
     return this->INHERITED::translate(dx, dy);
 }
 
 bool SkPictureRecord::scale(SkScalar sx, SkScalar sy) {
     // op + sx + sy
     uint32_t size = 1 * kUInt32Size + 2 * sizeof(SkScalar);
     uint32_t initialOffset = this->addDraw(SCALE, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addScalar(sx);
     addScalar(sy);
     validate(initialOffset, size);
     return this->INHERITED::scale(sx, sy);
 }
 
 bool SkPictureRecord::rotate(SkScalar degrees) {
     // op + degrees
     uint32_t size = 1 * kUInt32Size + sizeof(SkScalar);
     uint32_t initialOffset = this->addDraw(ROTATE, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addScalar(degrees);
     validate(initialOffset, size);
     return this->INHERITED::rotate(degrees);
 }
 
 bool SkPictureRecord::skew(SkScalar sx, SkScalar sy) {
     // op + sx + sy
     uint32_t size = 1 * kUInt32Size + 2 * sizeof(SkScalar);
     uint32_t initialOffset = this->addDraw(SKEW, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addScalar(sx);
     addScalar(sy);
     validate(initialOffset, size);
     return this->INHERITED::skew(sx, sy);
 }
 
 bool SkPictureRecord::concat(const SkMatrix& matrix) {
     validate(fWriter.size(), 0);
     // op + matrix index
     uint32_t size = 2 * kUInt32Size;
     uint32_t initialOffset = this->addDraw(CONCAT, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addMatrix(matrix);
     validate(initialOffset, size);
     return this->INHERITED::concat(matrix);
 }
 
 void SkPictureRecord::setMatrix(const SkMatrix& matrix) {
     validate(fWriter.size(), 0);
     // op + matrix index
     uint32_t size = 2 * kUInt32Size;
     uint32_t initialOffset = this->addDraw(SET_MATRIX, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addMatrix(matrix);
     validate(initialOffset, size);
     this->INHERITED::setMatrix(matrix);
 }
 
 static bool regionOpExpands(SkRegion::Op op) {
     switch (op) {
         case SkRegion::kUnion_Op:
         case SkRegion::kXOR_Op:
         case SkRegion::kReverseDifference_Op:
@@ skipping 61 matching lines @@
 
 bool SkPictureRecord::clipRect(const SkRect& rect, SkRegion::Op op, bool doAA) {
     // id + rect + clip params
     uint32_t size = 1 * kUInt32Size + sizeof(rect) + 1 * kUInt32Size;
     // recordRestoreOffsetPlaceholder doesn't always write an offset
     if (!fRestoreOffsetStack.isEmpty()) {
         // + restore offset
         size += kUInt32Size;
     }
     uint32_t initialOffset = this->addDraw(CLIP_RECT, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addRect(rect);
     addInt(ClipParams_pack(op, doAA));
     recordRestoreOffsetPlaceholder(op);
 
     validate(initialOffset, size);
     return this->INHERITED::clipRect(rect, op, doAA);
 }
 
 bool SkPictureRecord::clipRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA) {
     if (rrect.isRect()) {
         return this->SkPictureRecord::clipRect(rrect.getBounds(), op, doAA);
     }
 
     // op + rrect + clip params
     uint32_t size = 1 * kUInt32Size + SkRRect::kSizeInMemory + 1 * kUInt32Size;
     // recordRestoreOffsetPlaceholder doesn't always write an offset
     if (!fRestoreOffsetStack.isEmpty()) {
         // + restore offset
         size += kUInt32Size;
     }
     uint32_t initialOffset = this->addDraw(CLIP_RRECT, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addRRect(rrect);
     addInt(ClipParams_pack(op, doAA));
     recordRestoreOffsetPlaceholder(op);
 
     validate(initialOffset, size);
 
     if (fRecordFlags & SkPicture::kUsePathBoundsForClip_RecordingFlag) {
         return this->INHERITED::clipRect(rrect.getBounds(), op, doAA);
     } else {
         return this->INHERITED::clipRRect(rrect, op, doAA);
     }
 }
 
 bool SkPictureRecord::clipPath(const SkPath& path, SkRegion::Op op, bool doAA) {
 
     SkRect r;
     if (!path.isInverseFillType() && path.isRect(&r)) {
         return this->clipRect(r, op, doAA);
     }
 
     // op + path index + clip params
     uint32_t size = 3 * kUInt32Size;
     // recordRestoreOffsetPlaceholder doesn't always write an offset
     if (!fRestoreOffsetStack.isEmpty()) {
         // + restore offset
         size += kUInt32Size;
     }
     uint32_t initialOffset = this->addDraw(CLIP_PATH, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addPath(path);
     addInt(ClipParams_pack(op, doAA));
     recordRestoreOffsetPlaceholder(op);
 
     validate(initialOffset, size);
 
     if (fRecordFlags & SkPicture::kUsePathBoundsForClip_RecordingFlag) {
         return this->INHERITED::clipRect(path.getBounds(), op, doAA);
     } else {
         return this->INHERITED::clipPath(path, op, doAA);
     }
 }
 
 bool SkPictureRecord::clipRegion(const SkRegion& region, SkRegion::Op op) {
     // op + region index + clip params
     uint32_t size = 3 * kUInt32Size;
     // recordRestoreOffsetPlaceholder doesn't always write an offset
     if (!fRestoreOffsetStack.isEmpty()) {
         // + restore offset
         size += kUInt32Size;
     }
     uint32_t initialOffset = this->addDraw(CLIP_REGION, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addRegion(region);
     addInt(ClipParams_pack(op, false));
     recordRestoreOffsetPlaceholder(op);
 
     validate(initialOffset, size);
     return this->INHERITED::clipRegion(region, op);
 }
 
 void SkPictureRecord::clear(SkColor color) {
     // op + color
     uint32_t size = 2 * kUInt32Size;
     uint32_t initialOffset = this->addDraw(DRAW_CLEAR, &size);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
     addInt(color);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawPaint(const SkPaint& paint) {
     // op + paint index
     uint32_t size = 2 * kUInt32Size;
     uint32_t initialOffset = this->addDraw(DRAW_PAINT, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_PAINT, size) == fWriter.size());
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_PAINT, size, false) == fWriter.size());
     addPaint(paint);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawPoints(PointMode mode, size_t count, const SkPoint pts[],
                                  const SkPaint& paint) {
     // op + paint index + mode + count + point data
     uint32_t size = 4 * kUInt32Size + count * sizeof(SkPoint);
-    uint32_t initialOffset = this->addDraw(DRAW_POINTS, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_POINTS, size) == fWriter.size());
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        SkRect bbox;
+        bbox.set(pts, count);
+        initialOffset = this->addDraw(DRAW_POINTS, &size, &paint, &bbox);
+    } else {
+        initialOffset = this->addDraw(DRAW_POINTS, &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_POINTS, size, this->recordBounds()) \
+        == fWriter.size());
     addPaint(paint);
     addInt(mode);
     addInt(count);
     fWriter.writeMul4(pts, count * sizeof(SkPoint));
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawOval(const SkRect& oval, const SkPaint& paint) {
     // op + paint index + rect
     uint32_t size = 2 * kUInt32Size + sizeof(oval);
-    uint32_t initialOffset = this->addDraw(DRAW_OVAL, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_OVAL, size) == fWriter.size());
+    uint32_t initialOffset = this->addDraw(DRAW_OVAL, &size, &paint, &oval);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_OVAL, size, this->recordBounds()) \
+        == fWriter.size());
     addPaint(paint);
     addRect(oval);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawRect(const SkRect& rect, const SkPaint& paint) {
     // op + paint index + rect
     uint32_t size = 2 * kUInt32Size + sizeof(rect);
-    uint32_t initialOffset = this->addDraw(DRAW_RECT, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_RECT, size) == fWriter.size());
+    uint32_t initialOffset = this->addDraw(DRAW_RECT, &size, &paint, &rect);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_RECT, size, this->recordBounds()) \
+        == fWriter.size());
     addPaint(paint);
     addRect(rect);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
-    uint32_t initialOffset, size;
     if (rrect.isRect()) {
-        // op + paint index + rect
-        size = 2 * kUInt32Size + sizeof(SkRect);
-        initialOffset = this->addDraw(DRAW_RECT, &size);
-        SkASSERT(initialOffset+getPaintOffset(DRAW_RECT, size) == fWriter.size());
-        addPaint(paint);
-        addRect(rrect.getBounds());
+        this->drawRect(rrect.getBounds(), paint);
     } else if (rrect.isOval()) {
-        // op + paint index + rect
-        size = 2 * kUInt32Size + sizeof(SkRect);
-        initialOffset = this->addDraw(DRAW_OVAL, &size);
-        SkASSERT(initialOffset+getPaintOffset(DRAW_OVAL, size) == fWriter.size());
-        addPaint(paint);
-        addRect(rrect.getBounds());
+        this->drawOval(rrect.getBounds(), paint);
     } else {
         // op + paint index + rrect
-        size = 2 * kUInt32Size + SkRRect::kSizeInMemory;
-        initialOffset = this->addDraw(DRAW_RRECT, &size);
-        SkASSERT(initialOffset+getPaintOffset(DRAW_RRECT, size) == fWriter.size());
+        uint32_t size = 2 * kUInt32Size + SkRRect::kSizeInMemory;
+        uint32_t initialOffset = this->addDraw(DRAW_RRECT, &size, &paint, &rrect.getBounds());
+        if (kInvalidOffset == initialOffset) {
+            return;
+        }
+        SkASSERT(initialOffset + getPaintOffset(DRAW_RRECT, size, this->recordBounds()) \
+            == fWriter.size());
         addPaint(paint);
         addRRect(rrect);
+        validate(initialOffset, size);
     }
-    validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawPath(const SkPath& path, const SkPaint& paint) {
     // op + paint index + path index
     uint32_t size = 3 * kUInt32Size;
-    uint32_t initialOffset = this->addDraw(DRAW_PATH, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_PATH, size) == fWriter.size());
+    const SkRect* bounds = path.isInverseFillType() ? NULL : &path.getBounds();
+    uint32_t initialOffset = this->addDraw(DRAW_PATH, &size, &paint, bounds);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_PATH, size, this->recordBounds()) \
+        == fWriter.size());
     addPaint(paint);
     addPath(path);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawBitmap(const SkBitmap& bitmap, SkScalar left, SkScalar top,
                         const SkPaint* paint = NULL) {
     // op + paint index + bitmap index + left + top
     uint32_t size = 3 * kUInt32Size + 2 * sizeof(SkScalar);
-    uint32_t initialOffset = this->addDraw(DRAW_BITMAP, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_BITMAP, size) == fWriter.size());
+    SkRect bounds = SkRect::MakeXYWH(left, top, bitmap.width(), bitmap.height());
+    uint32_t initialOffset = this->addDraw(DRAW_BITMAP, &size, paint, &bounds);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_BITMAP, size, this->recordBounds()) \
+        == fWriter.size());
     addPaintPtr(paint);
     addBitmap(bitmap);
     addScalar(left);
     addScalar(top);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawBitmapRectToRect(const SkBitmap& bitmap, const SkRect* src,
                             const SkRect& dst, const SkPaint* paint) {
     // id + paint index + bitmap index + bool for 'src'
     uint32_t size = 4 * kUInt32Size;
     if (NULL != src) {
         size += sizeof(*src);   // + rect
     }
     size += sizeof(dst);        // + rect
 
-    uint32_t initialOffset = this->addDraw(DRAW_BITMAP_RECT_TO_RECT, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_BITMAP_RECT_TO_RECT, size) == fWriter.size());
+    uint32_t initialOffset = this->addDraw(DRAW_BITMAP_RECT_TO_RECT, &size, paint, &dst);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_BITMAP_RECT_TO_RECT, size, this->recordBounds()) \
+             == fWriter.size());
     addPaintPtr(paint);
     addBitmap(bitmap);
     addRectPtr(src);  // may be null
     addRect(dst);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawBitmapMatrix(const SkBitmap& bitmap, const SkMatrix& matrix,
                                        const SkPaint* paint) {
     // id + paint index + bitmap index + matrix index
     uint32_t size = 4 * kUInt32Size;
-    uint32_t initialOffset = this->addDraw(DRAW_BITMAP_MATRIX, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_BITMAP_MATRIX, size) == fWriter.size());
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        SkRect bounds = SkRect::MakeWH(bitmap.width(), bitmap.height());
+        matrix.mapRect(&bounds);
+        initialOffset = this->addDraw(DRAW_BITMAP_MATRIX, &size, paint, &bounds);
+    } else {
+        initialOffset = this->addDraw(DRAW_BITMAP_MATRIX, &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_BITMAP_MATRIX, size, this->recordBounds()) \
+        == fWriter.size());
     addPaintPtr(paint);
     addBitmap(bitmap);
     addMatrix(matrix);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawBitmapNine(const SkBitmap& bitmap, const SkIRect& center,
                                      const SkRect& dst, const SkPaint* paint) {
     // op + paint index + bitmap id + center + dst rect
     uint32_t size = 3 * kUInt32Size + sizeof(center) + sizeof(dst);
-    uint32_t initialOffset = this->addDraw(DRAW_BITMAP_NINE, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_BITMAP_NINE, size) == fWriter.size());
+    uint32_t initialOffset = this->addDraw(DRAW_BITMAP_NINE, &size, paint, &dst);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_BITMAP_NINE, size, this->recordBounds()) == \
+        fWriter.size());
     addPaintPtr(paint);
     addBitmap(bitmap);
     addIRect(center);
     addRect(dst);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawSprite(const SkBitmap& bitmap, int left, int top,
-                        const SkPaint* paint = NULL) {
+                                 const SkPaint* paint) {
     // op + paint index + bitmap index + left + top
     uint32_t size = 5 * kUInt32Size;
-    uint32_t initialOffset = this->addDraw(DRAW_SPRITE, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_SPRITE, size) == fWriter.size());
+    uint32_t initialOffset;

[robertphillips, 2013/03/07 18:37:40]

Could you expand on this with a "because ..."

+    // Note: Bounds encoding not supported.

[robertphillips, 2013/03/07 18:37:40]

Move uint32_t down to line 980?

+    initialOffset = this->addDraw(DRAW_SPRITE, &size);
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_SPRITE, size, false) == fWriter.size());
     addPaintPtr(paint);
     addBitmap(bitmap);
     addInt(left);
     addInt(top);
     validate(initialOffset, size);
 }
 
 // Return fontmetrics.fTop,fBottom in topbot[0,1], after they have been
 // tweaked by paint.computeFastBounds().
 //
@@ skipping 14 matching lines @@
     if (!flat.isTopBotWritten()) {
         computeFontMetricsTopBottom(paint, flat.writableTopBot());
         SkASSERT(flat.isTopBotWritten());
     }
     addScalar(flat.topBot()[0] + minY);
     addScalar(flat.topBot()[1] + maxY);
 }
 
 void SkPictureRecord::drawText(const void* text, size_t byteLength, SkScalar x,
                       SkScalar y, const SkPaint& paint) {
-    bool fast = !paint.isVerticalText() && paint.canComputeFastBounds();
+    // Note: The DRAW_TEXT_TOP_BOTTOM optimization is redundant if we are
+    // already recording full bounds.
+    bool fast = !paint.isVerticalText() && paint.canComputeFastBounds() &&

[robertphillips, 2013/03/07 18:37:40]

spaces

+        !this->recordBounds();
 
     // op + paint index + length + 'length' worth of chars + x + y
     uint32_t size = 3 * kUInt32Size + SkAlign4(byteLength) + 2 * sizeof(SkScalar);
     if (fast) {
         size += 2 * sizeof(SkScalar); // + top & bottom
     }
 
     DrawType op = fast ? DRAW_TEXT_TOP_BOTTOM : DRAW_TEXT;
-    uint32_t initialOffset = this->addDraw(op, &size);
-    SkASSERT(initialOffset+getPaintOffset(op, size) == fWriter.size());
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        SkRect bbox;
+        paint.measureText(text, byteLength, &bbox);
+        SkPaint::FontMetrics metrics;
+        paint.getFontMetrics(&metrics);
+
+        // Vertical and aligned text need to be offset
+        if (paint.isVerticalText()) {
+            SkScalar h = bbox.fBottom - bbox.fTop;
+            if (paint.getTextAlign() == SkPaint::kCenter_Align) {
+                bbox.fTop    -= h / 2;
+                bbox.fBottom -= h / 2;
+            }
+            // Pad top and bottom with max extents from FontMetrics
+            bbox.fBottom += metrics.fBottom;
+            bbox.fTop += metrics.fTop;
+        } else {
+            SkScalar w = bbox.fRight - bbox.fLeft;
+            if (paint.getTextAlign() == SkPaint::kCenter_Align) {
+                bbox.fLeft  -= w / 2;
+                bbox.fRight -= w / 2;
+            } else if (paint.getTextAlign() == SkPaint::kRight_Align) {
+                bbox.fLeft  -= w;
+                bbox.fRight -= w;
+            }
+            // Set vertical bounds to max extents from font metrics
+            bbox.fTop = metrics.fTop;
+            bbox.fBottom = metrics.fBottom;
+        }
+
+        // Pad horizontal bounds on each side by half of max vertical extents (this is sort of

[reed1, 2013/03/07 20:36:44]

80-cols in this file

+        // arbitrary, but seems to produce reasonable results, if there were a way of getting max
+        // glyph X-extents to pad by, that may be better here, but FontMetrics fXMin and fXMax seem
+        // incorrect on most platforms (too small in Linux, never even set in Windows).

[reed1, 2013/03/07 20:36:44]

yikes! please file a bug!

[Justin Novosad, 2013/03/08 14:49:14]

This was copy-pasted from SkBBoxRecord (r5423 by rileya).  Not sure what the
history behind this comment is. I feel this should be made more conservative due
to its fuzzy nature. I think I should remove the divide by two.

+        SkScalar pad = (metrics.fBottom - metrics.fTop) / 2;
+        bbox.fLeft  -= pad;
+        bbox.fRight += pad;
+
+        bbox.fLeft += x;
+        bbox.fRight += x;
+        bbox.fTop += y;
+        bbox.fBottom += y;
+        initialOffset = this->addDraw(op, &size, &paint, &bbox);
+    } else {
+        initialOffset = this->addDraw(op, &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(op, size, this->recordBounds()) == fWriter.size());
     const SkFlatData* flatPaintData = addPaint(paint);
     SkASSERT(flatPaintData);
     addText(text, byteLength);
     addScalar(x);
     addScalar(y);
     if (fast) {
         addFontMetricsTopBottom(paint, *flatPaintData, y, y);
     }
     validate(initialOffset, size);
 }
@@ skipping 15 matching lines @@
                 canUseDrawH = false;
                 if (pos[index].fY < minY) {
                     minY = pos[index].fY;
                 } else if (pos[index].fY > maxY) {
                     maxY = pos[index].fY;
                 }
             }
         }
     }
 
-    bool fastBounds = !paint.isVerticalText() && paint.canComputeFastBounds();
+    // Note: The DRAW_TEXT_[H_]TOP_BOTTOM optimization is redundant if we are
+    // already recording full bounds.
+    bool fastBounds = !paint.isVerticalText() && paint.canComputeFastBounds() &&

[robertphillips, 2013/03/07 18:37:40]

spaces

+        !this->recordBounds();
     bool fast = canUseDrawH && fastBounds;
 
     // op + paint index + length + 'length' worth of data + num points
     uint32_t size = 3 * kUInt32Size + SkAlign4(byteLength) + 1 * kUInt32Size;
     if (canUseDrawH) {
         if (fast) {
             size += 2 * sizeof(SkScalar); // + top & bottom
         }
         // + y-pos + actual x-point data
         size += sizeof(SkScalar) + points * sizeof(SkScalar);
     } else {
         // + x&y point data
         size += points * sizeof(SkPoint);
         if (fastBounds) {
             size += 2 * sizeof(SkScalar); // + top & bottom
         }
     }
 
     DrawType op;
     if (fast) {
         op = DRAW_POS_TEXT_H_TOP_BOTTOM;
     } else if (canUseDrawH) {
         op = DRAW_POS_TEXT_H;
     } else if (fastBounds) {
         op = DRAW_POS_TEXT_TOP_BOTTOM;
     } else {
         op = DRAW_POS_TEXT;
     }
-    uint32_t initialOffset = this->addDraw(op, &size);
-    SkASSERT(initialOffset+getPaintOffset(op, size) == fWriter.size());
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        SkRect bbox;
+        bbox.set(pos, paint.countText(text, byteLength));
+        SkPaint::FontMetrics metrics;
+        paint.getFontMetrics(&metrics);
+        bbox.fTop += metrics.fTop;
+        bbox.fBottom += metrics.fBottom;
+
+        // pad on left and right by half of max vertical glyph extents
+        SkScalar pad = (metrics.fTop - metrics.fBottom) / 2;
+        bbox.fLeft += pad;
+        bbox.fRight -= pad;
+        initialOffset = addDraw(op, &size, &paint, &bbox);
+    } else {
+        initialOffset = this->addDraw(op, &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(op, size, this->recordBounds()) == fWriter.size());
     const SkFlatData* flatPaintData = addPaint(paint);
     SkASSERT(flatPaintData);
     addText(text, byteLength);
     addInt(points);
 
 #ifdef SK_DEBUG_SIZE
     size_t start = fWriter.size();
 #endif
     if (canUseDrawH) {
         if (fast) {
@@ skipping 16 matching lines @@
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawPosTextH(const void* text, size_t byteLength,
                           const SkScalar xpos[], SkScalar constY,
                           const SkPaint& paint) {
     size_t points = paint.countText(text, byteLength);
     if (0 == points)
         return;
 
-    bool fast = !paint.isVerticalText() && paint.canComputeFastBounds();
+    // Note: The DRAW_POS_TEXT_H_TOP_BOTTOM optimization is redundant if we are
+    // already recording full bounds.
+    bool fast = !paint.isVerticalText() && paint.canComputeFastBounds() &&

[robertphillips, 2013/03/07 18:37:40]

spaces

+        !this->recordBounds();
 
     // op + paint index + length + 'length' worth of data + num points
     uint32_t size = 3 * kUInt32Size + SkAlign4(byteLength) + 1 * kUInt32Size;
     if (fast) {
         size += 2 * sizeof(SkScalar); // + top & bottom
     }
     // + y + the actual points
     size += 1 * kUInt32Size + points * sizeof(SkScalar);
 
-    uint32_t initialOffset = this->addDraw(fast ? DRAW_POS_TEXT_H_TOP_BOTTOM : DRAW_POS_TEXT_H,
-                                           &size);
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        size_t numChars = paint.countText(text, byteLength);

[robertphillips, 2013/03/07 18:37:40]

We should probably bail somewhere (or add an assert) if numChar == 0

[Justin Novosad, 2013/03/07 20:25:47]

Actually, I just realized this is redundant with 'points', computed above.

+        SkRect bbox;
+        bbox.fLeft  = xpos[0];

[robertphillips, 2013/03/07 18:37:40]

shouldn't this be 0 rather than numChars-1?

[Justin Novosad, 2013/03/07 20:25:47]

For the algorithm to work, it actually does not matter.  Initializing like this
is more likely to be optimal for perf (assuming we usually write from left to
right) because we will never update bbox in the loop below in the well
conditioned cases.

[robertphillips, 2013/03/07 20:57:37]

Since we have no guarantee of monotonicity, what if xpos[0] is the rightmost?

+        bbox.fRight = xpos[numChars - 1];
+        // if we had a guarantee that these will be monotonically increasing, this could be sped up
+        for (size_t i = 1; i < numChars; ++i) {
+            if (xpos[i] < bbox.fLeft) {
+                bbox.fLeft = xpos[i];
+            }
+            if (xpos[i] > bbox.fRight) {
+                bbox.fRight = xpos[i];
+            }
+        }
+        SkPaint::FontMetrics metrics;

[reed1, 2013/03/07 20:36:44]

Some amount of this code looks awefully similar to drawPosText and drawText. Can
we do refactoring to share some?

+        paint.getFontMetrics(&metrics);
+
+        // pad horizontally by max glyph height
+        SkScalar pad = (metrics.fTop - metrics.fBottom);
+        bbox.fLeft  += pad;
+        bbox.fRight -= pad;
+
+        bbox.fTop    = metrics.fTop + constY;
+        bbox.fBottom = metrics.fBottom + constY;
+        initialOffset = this->addDraw(DRAW_POS_TEXT_H, &size, &paint, &bbox);
+    } else {
+        initialOffset = this->addDraw(fast ? DRAW_POS_TEXT_H_TOP_BOTTOM : DRAW_POS_TEXT_H,
+                                      &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_TEXT_ON_PATH, size, this->recordBounds()) \
+             == fWriter.size());
     const SkFlatData* flatPaintData = addPaint(paint);
     SkASSERT(flatPaintData);
     addText(text, byteLength);
     addInt(points);
 
 #ifdef SK_DEBUG_SIZE
     size_t start = fWriter.size();
 #endif
     if (fast) {
         addFontMetricsTopBottom(paint, *flatPaintData, constY, constY);
     }
     addScalar(constY);
     fWriter.writeMul4(xpos, points * sizeof(SkScalar));
 #ifdef SK_DEBUG_SIZE
     fPointBytes += fWriter.size() - start;
     fPointWrites += points;
 #endif
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawTextOnPath(const void* text, size_t byteLength,
                             const SkPath& path, const SkMatrix* matrix,
                             const SkPaint& paint) {
     // op + paint index + length + 'length' worth of data + path index + matrix index
     uint32_t size = 3 * kUInt32Size + SkAlign4(byteLength) + 2 * kUInt32Size;
-    uint32_t initialOffset = this->addDraw(DRAW_TEXT_ON_PATH, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_TEXT_ON_PATH, size) == fWriter.size());
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        SkRect bbox = path.getBounds();
+        SkPaint::FontMetrics metrics;
+        paint.getFontMetrics(&metrics);
+        // pad out all sides by the max glyph height above baseline
+        SkScalar pad = metrics.fTop;
+        bbox.fLeft += pad;
+        bbox.fRight -= pad;
+        bbox.fTop += pad;
+        bbox.fBottom -= pad;
+        initialOffset = this->addDraw(DRAW_TEXT_ON_PATH, &size, &paint, &bbox);
+    } else {
+        initialOffset = this->addDraw(DRAW_TEXT_ON_PATH, &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_TEXT_ON_PATH, size, this->recordBounds()) \
+        == fWriter.size());
     addPaint(paint);
     addText(text, byteLength);
     addPath(path);
     addMatrixPtr(matrix);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawPicture(SkPicture& picture) {
     // op + picture index
     uint32_t size = 2 * kUInt32Size;
     uint32_t initialOffset = this->addDraw(DRAW_PICTURE, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addPicture(picture);
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawVertices(VertexMode vmode, int vertexCount,
                           const SkPoint vertices[], const SkPoint texs[],
                           const SkColor colors[], SkXfermode*,
                           const uint16_t indices[], int indexCount,
                           const SkPaint& paint) {
     uint32_t flags = 0;
@@ skipping 13 matching lines @@
         size += vertexCount * sizeof(SkPoint);  // + uvs
     }
     if (flags & DRAW_VERTICES_HAS_COLORS) {
         size += vertexCount * sizeof(SkColor);  // + vert colors
     }
     if (flags & DRAW_VERTICES_HAS_INDICES) {
         // + num indices + indices
         size += 1 * kUInt32Size + SkAlign4(indexCount * sizeof(uint16_t));
     }
 
-    uint32_t initialOffset = this->addDraw(DRAW_VERTICES, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_VERTICES, size) == fWriter.size());
+    uint32_t initialOffset;
+    if (this->recordBounds()) {
+        SkRect bbox;
+        bbox.set(vertices, vertexCount);
+        initialOffset = this->addDraw(DRAW_VERTICES, &size, &paint, &bbox);
+    } else {
+        initialOffset = this->addDraw(DRAW_VERTICES, &size);
+    }
+    if (kInvalidOffset == initialOffset) {
+        return;
+    }
+    SkASSERT(initialOffset + getPaintOffset(DRAW_VERTICES, size, this->recordBounds()) \
+        == fWriter.size());
     addPaint(paint);
     addInt(flags);
     addInt(vmode);
     addInt(vertexCount);
     addPoints(vertices, vertexCount);
     if (flags & DRAW_VERTICES_HAS_TEXS) {
         addPoints(texs, vertexCount);
     }
     if (flags & DRAW_VERTICES_HAS_COLORS) {
         fWriter.writeMul4(colors, vertexCount * sizeof(SkColor));
     }
     if (flags & DRAW_VERTICES_HAS_INDICES) {
         addInt(indexCount);
         fWriter.writePad(indices, indexCount * sizeof(uint16_t));
     }
     validate(initialOffset, size);
 }
 
 void SkPictureRecord::drawData(const void* data, size_t length) {
     // op + length + 'length' worth of data
     uint32_t size = 2 * kUInt32Size + SkAlign4(length);
     uint32_t initialOffset = this->addDraw(DRAW_DATA, &size);
+    SkASSERT(kInvalidOffset != initialOffset);
     addInt(length);
     fWriter.writePad(data, length);
     validate(initialOffset, size);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 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
@@ skipping 228 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