Implement SkPicture::hasText() for SkRecord backend.

Plus, some small tweaks to the existing code surrounding it.  Just proposals,
will undo whatever you don't like.

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

Author: mtklein@chromium.org

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

diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index f1ff19c..06cc6e2 100644 (file)
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -307,18 +307,18 @@ private:
     SkAutoTUnref<SkBBoxHierarchy> fBBH;
 
     struct Analysis {
-        // To get setup to work cleanly, we cast away constness and call init()
-        // instead of trying to set everything during construction.
-        void init(const SkRecord&);
+        Analysis() {}  // Only used by SkPictureData codepath.
+        explicit Analysis(const SkRecord&);
 
         bool suitableForGpuRasterization(const char** reason, int sampleCount) const;
 
         bool        fWillPlaybackBitmaps;
+        bool        fHasText;
         int         fNumPaintWithPathEffectUses;
         int         fNumFastPathDashEffects;
         int         fNumAAConcavePaths;
         int         fNumAAHairlineConcavePaths;
-    } const                       fAnalysis;
+    } fAnalysis;
 };
 
 #endif

diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 1396406..f99dfbf 100644 (file)
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -171,10 +171,17 @@ struct PathCounter {
     int numAAHairlineConcavePaths;
 };
 
-} // namespace
+// 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; }
+};
 
-void SkPicture::Analysis::init(const SkRecord& record) {
+} // namespace
 
+SkPicture::Analysis::Analysis(const SkRecord& record) {
     fWillPlaybackBitmaps = WillPlaybackBitmaps(record);
 
     PathCounter counter;
@@ -182,9 +189,18 @@ void SkPicture::Analysis::init(const SkRecord& record) {
         record.visit<void>(i, counter);
     }
     fNumPaintWithPathEffectUses = counter.numPaintWithPathEffectUses;
-    fNumFastPathDashEffects = counter.numFastPathDashEffects;
-    fNumAAConcavePaths = counter.numAAConcavePaths;
-    fNumAAHairlineConcavePaths = counter.numAAHairlineConcavePaths;
+    fNumFastPathDashEffects     = counter.numFastPathDashEffects;
+    fNumAAConcavePaths          = counter.numAAConcavePaths;
+    fNumAAHairlineConcavePaths  = counter.numAAHairlineConcavePaths;
+
+    fHasText = false;
+    TextHunter text;
+    for (unsigned i = 0; i < record.count(); i++) {
+        if (record.visit<bool>(i, text)) {
+            fHasText = true;
+            break;
+        }
+    }
 }
 
 bool SkPicture::Analysis::suitableForGpuRasterization(const char** reason,
@@ -575,9 +591,16 @@ bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **rea
 }
 #endif
 
-// fRecord TODO
+// fRecord OK
 bool SkPicture::hasText() const {
-    return fData.get() && fData->hasText();
+    if (fRecord.get()) {
+        return fAnalysis.fHasText;
+    }
+    if (fData.get()) {
+        return fData->hasText();
+    }
+    SkFAIL("Unreachable");
+    return false;
 }
 
 // fRecord OK
@@ -585,10 +608,11 @@ bool SkPicture::willPlayBackBitmaps() const {
     if (fRecord.get()) {
         return fAnalysis.fWillPlaybackBitmaps;
     }
-    if (!fData.get()) {
-        return false;
+    if (fData.get()) {
+        return fData->containsBitmaps();
     }
-    return fData->containsBitmaps();
+    SkFAIL("Unreachable");
+    return false;
 }
 
 // fRecord OK
@@ -617,10 +641,8 @@ SkPicture::SkPicture(int width, int height, SkRecord* record, SkBBoxHierarchy* b
     , fHeight(height)
     , fRecord(record)
     , fBBH(SkSafeRef(bbh))
-    , fAnalysis() {
+    , fAnalysis(*record) {
     // TODO: delay as much of this work until just before first playback?
-
-    const_cast<Analysis*>(&fAnalysis)->init(*record);
     if (fBBH.get()) {
         SkRecordFillBounds(*record, fBBH.get());
     }

diff --git a/src/core/SkRecords.h b/src/core/SkRecords.h
index 01cdbf7..6ec6e9c 100644 (file)
--- a/src/core/SkRecords.h
+++ b/src/core/SkRecords.h
@@ -63,11 +63,11 @@ namespace SkRecords {
     M(DrawPoints)                                                   \
     M(DrawPosText)                                                  \
     M(DrawPosTextH)                                                 \
+    M(DrawText)                                                     \
+    M(DrawTextOnPath)                                               \
     M(DrawRRect)                                                    \
     M(DrawRect)                                                     \
     M(DrawSprite)                                                   \
-    M(DrawText)                                                     \
-    M(DrawTextOnPath)                                               \
     M(DrawVertices)
 
 // Defines SkRecords::Type, an enum of all record types.

diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index e9b38d2..6a93a9d 100644 (file)
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -988,61 +988,62 @@ static void test_gpu_picture_optimization(skiatest::Reporter* reporter,
 
 #endif
 
-static void test_has_text(skiatest::Reporter* reporter) {
+static void test_has_text(skiatest::Reporter* reporter, bool useNewPath) {
     SkPictureRecorder recorder;
-    SkPaint paint;
-    paint.setColor(SK_ColorBLUE);
-    SkPoint point = SkPoint::Make(10, 10);
+#define BEGIN_RECORDING useNewPath ? recorder.EXPERIMENTAL_beginRecording(100, 100) \
+                                   : recorder.             beginRecording(100, 100)
 
-    SkCanvas* canvas = recorder.beginRecording(100, 100);
+    SkCanvas* canvas = BEGIN_RECORDING;
     {
-        canvas->drawRect(SkRect::MakeWH(20, 20), paint);
+        canvas->drawRect(SkRect::MakeWH(20, 20), SkPaint());
     }
     SkAutoTUnref<SkPicture> picture(recorder.endRecording());
     REPORTER_ASSERT(reporter, !picture->hasText());
 
-    canvas = recorder.beginRecording(100, 100);
+    SkPoint point = SkPoint::Make(10, 10);
+    canvas = BEGIN_RECORDING;
     {
-        canvas->drawText("Q", 1, point.fX, point.fY, paint);
+        canvas->drawText("Q", 1, point.fX, point.fY, SkPaint());
     }
     picture.reset(recorder.endRecording());
     REPORTER_ASSERT(reporter, picture->hasText());
 
-    canvas = recorder.beginRecording(100, 100);
+    canvas = BEGIN_RECORDING;
     {
-        canvas->drawPosText("Q", 1, &point, paint);
+        canvas->drawPosText("Q", 1, &point, SkPaint());
     }
     picture.reset(recorder.endRecording());
     REPORTER_ASSERT(reporter, picture->hasText());
 
-    canvas = recorder.beginRecording(100, 100);
+    canvas = BEGIN_RECORDING;
     {
-        canvas->drawPosTextH("Q", 1, &point.fX, point.fY, paint);
+        canvas->drawPosTextH("Q", 1, &point.fX, point.fY, SkPaint());
     }
     picture.reset(recorder.endRecording());
     REPORTER_ASSERT(reporter, picture->hasText());
 
-    canvas = recorder.beginRecording(100, 100);
+    canvas = BEGIN_RECORDING;
     {
         SkPath path;
         path.moveTo(0, 0);
         path.lineTo(50, 50);
 
-        canvas->drawTextOnPathHV("Q", 1, path, point.fX, point.fY, paint);
+        canvas->drawTextOnPathHV("Q", 1, path, point.fX, point.fY, SkPaint());
     }
     picture.reset(recorder.endRecording());
     REPORTER_ASSERT(reporter, picture->hasText());
 
-    canvas = recorder.beginRecording(100, 100);
+    canvas = BEGIN_RECORDING;
     {
         SkPath path;
         path.moveTo(0, 0);
         path.lineTo(50, 50);
 
-        canvas->drawTextOnPath("Q", 1, path, NULL, paint);
+        canvas->drawTextOnPath("Q", 1, path, NULL, SkPaint());
     }
     picture.reset(recorder.endRecording());
     REPORTER_ASSERT(reporter, picture->hasText());
+#undef BEGIN_RECORDING
 }
 
 static void set_canvas_to_save_count_4(SkCanvas* canvas) {
@@ -1688,7 +1689,8 @@ DEF_TEST(Picture, reporter) {
     test_gpu_veto(reporter, false);
     test_gpu_veto(reporter, true);
 #endif
-    test_has_text(reporter);
+    test_has_text(reporter, false);
+    test_has_text(reporter, true);
     test_analysis(reporter, false);
     test_analysis(reporter, true);
     test_gatherpixelrefs(reporter);