Our SkPicture::Analysis visitors should recurse into nested pictures.

BUG=skia:
R=tomhudson@google.com, mtklein@google.com, reed@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/495793002

include/core/SkPicture.h

@@ -308,6 +308,8 @@ private:
     SkAutoTDelete<SkRecord>       fRecord;
     SkAutoTUnref<SkBBoxHierarchy> fBBH;
 
+    struct PathCounter;
+
     struct Analysis {
         Analysis() {}  // Only used by SkPictureData codepath.
         explicit Analysis(const SkRecord&);

src/core/SkPicture.cpp

@@ -68,9 +68,12 @@ struct BitmapTester {
 
 
     // Main entry for visitor:
+    // If the command is a DrawPicture, recurse.
     // If the command has a bitmap directly, return true.
     // If the command has a paint and the paint has a bitmap, return true.
     // Otherwise, return false.
+    bool operator()(const SkRecords::DrawPicture& op) { return op.picture->willPlayBackBitmaps(); }
+
     template <typename T>
     bool operator()(const T& r) { return CheckBitmap(r); }
 
@@ -112,10 +115,21 @@ bool WillPlaybackBitmaps(const SkRecord& record) {
     return false;
 }
 
+// SkRecord visitor to find recorded text.
+struct TextHunter {
+    // All ops with text have that text as a char array member named "text".
+    SK_CREATE_MEMBER_DETECTOR(text);
+    bool operator()(const SkRecords::DrawPicture& op) { return op.picture->hasText(); }
+    template <typename T> SK_WHEN(HasMember_text<T>,  bool) operator()(const T&) { return true;  }
+    template <typename T> SK_WHEN(!HasMember_text<T>, bool) operator()(const T&) { return false; }
+};
+
+} // namespace
+
 /** SkRecords visitor to determine heuristically whether or not a SkPicture
     will be performant when rasterized on the GPU.
  */
-struct PathCounter {
+struct SkPicture::PathCounter {
     SK_CREATE_MEMBER_DETECTOR(paint);
 
     PathCounter()
@@ -125,6 +139,18 @@ struct PathCounter {
         , numAAHairlineConcavePaths (0) {
     }
 
+    // Recurse into nested pictures.
+    void operator()(const SkRecords::DrawPicture& op) {
+        // If you're not also SkRecord-backed, tough luck.  Get on the bandwagon.
+        if (op.picture->fRecord.get() == NULL) {
+            return;
+        }
+        const SkRecord& nested = *op.picture->fRecord;
+        for (unsigned i = 0; i < nested.count(); i++) {
+            nested.visit<void>(i, *this);
+        }
+    }
+
     void checkPaint(const SkPaint* paint) {
         if (paint && paint->getPathEffect()) {
             numPaintWithPathEffectUses++;
@@ -164,23 +190,12 @@ struct PathCounter {
     template <typename T>
     SK_WHEN(!HasMember_paint<T>, void) operator()(const T& op) { /* do nothing */ }
 
-
     int numPaintWithPathEffectUses;
     int numFastPathDashEffects;
     int numAAConcavePaths;
     int numAAHairlineConcavePaths;
 };
 
-// SkRecord visitor to find recorded text.
-struct TextHunter {
-    // All ops with text have that text as a char array member named "text".
-    SK_CREATE_MEMBER_DETECTOR(text);
-    template <typename T> SK_WHEN(HasMember_text<T>,  bool) operator()(const T&) { return true;  }
-    template <typename T> SK_WHEN(!HasMember_text<T>, bool) operator()(const T&) { return false; }
-};
-
-} // namespace
-
 SkPicture::Analysis::Analysis(const SkRecord& record) {
     fWillPlaybackBitmaps = WillPlaybackBitmaps(record);
 

src/core/SkRecords.h

@@ -14,18 +14,6 @@
 
 namespace SkRecords {
 
-template <typename T>
-class RefBox : SkNoncopyable {
-public:
-    RefBox(const T* obj) : fObj(SkRef(obj)) {}
-    ~RefBox() { fObj->unref(); }
-
-    operator const T*() const { return fObj; }
-
-private:
-    const T* fObj;
-};
-
 // A list of all the types of canvas calls we can record.
 // Each of these is reified into a struct below.
 //
@@ -132,6 +120,18 @@ struct T {                                                                \
     T* operator->() { return ptr; }           \
     const T* operator->() const { return ptr; }
 
+template <typename T>
+class RefBox : SkNoncopyable {
+public:
+    RefBox(T* obj) : fObj(SkRef(obj)) {}
+    ~RefBox() { fObj->unref(); }
+
+    ACT_AS_PTR(fObj);
+
+private:
+    T* fObj;
+};
+
 // An Optional doesn't own the pointer's memory, but may need to destroy non-POD data.
 template <typename T>
 class Optional : SkNoncopyable {
@@ -231,7 +231,9 @@ RECORD2(DrawOval, SkPaint, paint, SkRect, oval);
 RECORD1(DrawPaint, SkPaint, paint);
 RECORD2(DrawPath, SkPaint, paint, SkPath, path);
 //RECORD2(DrawPatch, SkPaint, paint, SkPatch, patch);
-RECORD3(DrawPicture, Optional<SkPaint>, paint, RefBox<SkPicture>, picture, Optional<SkMatrix>, matrix);
+RECORD3(DrawPicture, Optional<SkPaint>, paint,
+                     RefBox<const SkPicture>, picture,
+                     Optional<SkMatrix>, matrix);
 RECORD4(DrawPoints, SkPaint, paint, SkCanvas::PointMode, mode, size_t, count, SkPoint*, pts);
 RECORD4(DrawPosText, SkPaint, paint,
                      PODArray<char>, text,
@@ -251,7 +253,7 @@ RECORD5(DrawText, SkPaint, paint,
                   SkScalar, x,
                   SkScalar, y);
 RECORD4(DrawTextBlob, SkPaint, paint,
-                      RefBox<SkTextBlob>, blob,
+                      RefBox<const SkTextBlob>, blob,
                       SkScalar, x,
                       SkScalar, y);
 RECORD5(DrawTextOnPath, SkPaint, paint,

tests/PictureTest.cpp

@@ -843,6 +843,17 @@ static void test_gpu_veto(skiatest::Reporter* reporter,
     picture.reset(recorder.endRecording());
     // ... but only when applied to drawPoint() calls
     REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL));
+
+    // Nest the previous picture inside a new one.
+    // This doesn't work in the old backend.
+    if (useNewPath) {
+        canvas = GENERATE_CANVAS(recorder, useNewPath);
+        {
+            canvas->drawPicture(picture.get());
+        }
+        picture.reset(recorder.endRecording());
+        REPORTER_ASSERT(reporter, !picture->suitableForGpuRasterization(NULL));
+    }
 }
 
 #undef GENERATE_CANVAS
@@ -1043,6 +1054,17 @@ static void test_has_text(skiatest::Reporter* reporter, bool useNewPath) {
     }
     picture.reset(recorder.endRecording());
     REPORTER_ASSERT(reporter, picture->hasText());
+
+    // Nest the previous picture inside a new one.
+    // This doesn't work in the old backend.
+    if (useNewPath) {
+        canvas = BEGIN_RECORDING;
+        {
+            canvas->drawPicture(picture.get());
+        }
+        picture.reset(recorder.endRecording());
+        REPORTER_ASSERT(reporter, picture->hasText());
+    }
 #undef BEGIN_RECORDING
 }