Implement SkRecord::willPlaybackBitmaps, cache in SkPicture

src/core/SkPicture.cpp

Index: src/core/SkPicture.cpp
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 75160b8be4bf6afd93ceda34378b5013dca17186..e2359ce71b93f7e389fff5c5345c1dc76c509b7c 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -19,6 +19,7 @@
 #include "SkDrawPictureCallback.h"
 #include "SkPaintPriv.h"
 #include "SkPicture.h"
+#include "SkRecordAnalysis.h"
 #include "SkRegion.h"
 #include "SkStream.h"
 #include "SkTDArray.h"
@@ -132,7 +133,8 @@ static void validateMatrix(const SkMatrix* matrix) {
 // fRecord OK
 SkPicture::SkPicture()
     : fWidth(0)
-    , fHeight(0) {
+    , fHeight(0)
+    , fRecordWillPlayBackBitmaps(false) {
     this->needsNewGenID();
 }
 
@@ -141,7 +143,8 @@ SkPicture::SkPicture(int width, int height,
                      const SkPictureRecord& record,
                      bool deepCopyOps)
     : fWidth(width)
-    , fHeight(height) {
+    , fHeight(height)
+    , fRecordWillPlayBackBitmaps(false) {
     this->needsNewGenID();
 
     SkPictInfo info;
@@ -170,6 +173,7 @@ SkPicture::SkPicture(const SkPicture& src) : INHERITED() {
     this->needsNewGenID();
     fWidth = src.fWidth;
     fHeight = src.fHeight;
+    fRecordWillPlayBackBitmaps = src.fRecordWillPlayBackBitmaps;
 
     if (NULL != src.fPlayback.get()) {
         fPlayback.reset(SkNEW_ARGS(SkPicturePlayback, (*src.fPlayback)));
@@ -202,6 +206,7 @@ void SkPicture::clone(SkPicture* pictures, int count) const {
         clone->needsNewGenID();
         clone->fWidth = fWidth;
         clone->fHeight = fHeight;
+        clone->fRecordWillPlayBackBitmaps = fRecordWillPlayBackBitmaps;
         clone->fPlayback.reset(NULL);
 
         /*  We want to copy the src's playback. However, if that hasn't been built
@@ -379,7 +384,8 @@ bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer& buffer, SkPictInfo* pInfo
 SkPicture::SkPicture(SkPicturePlayback* playback, int width, int height)
     : fPlayback(playback)
     , fWidth(width)
-    , fHeight(height) {
+    , fHeight(height)
+    , fRecordWillPlayBackBitmaps(false) {
     this->needsNewGenID();
 }
 
@@ -519,8 +525,11 @@ bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **rea
 }
 #endif
 
-// fRecord TODO
+// fRecord OK
 bool SkPicture::willPlayBackBitmaps() const {
+    if (fRecord.get()) {
+        return fRecordWillPlayBackBitmaps;
+    }
     if (!fPlayback.get()) {
         return false;
     }
@@ -561,6 +570,7 @@ uint32_t SkPicture::uniqueID() const {
 SkPicture::SkPicture(int width, int height, SkRecord* record)
     : fWidth(width)
     , fHeight(height)
-    , fRecord(record) {
+    , fRecord(record)
+    , fRecordWillPlayBackBitmaps(record ? SkRecordWillPlaybackBitmaps(*record) : false) {

[mtklein, 2014/07/02 15:53:08]

You can assume record != NULL here.  If you'd like, add an SkASSERT?

[tomhudson, 2014/07/02 19:21:34]

Since this is in the initializer, by the time we hit the assert we'd already
have tried to dereference NULL?

[mtklein, 2014/07/02 19:23:03]

Yeah, so doesn't make much of a difference if we guard record or not.  I'd
rather not so it doesn't look optional.

     this->needsNewGenID();
 }