calculate cull rects for SkMiniPictures

This is the other half of https://skia-review.googlesource.com/c/7874/,
adding the cull-shrinking feature of SkBigPictures to SkMiniPictures.

Like SkBigPictures, shrink only when we're asked to build an R-tree.
(We don't actually build a tree for one rect, of course.)

We could do unconditionally, but SkPictureImageFilter uses the cull rect
as its crop.  It's unclear to me what this image filter unit test I've
changed here was intending... had it had two draws we would have shrunk
its cull, but because it was hitting the 1-draw SkMiniPicture path it
kept the larger user-supplied cull.

As the test doesn't appear to have been written with cull shrinking in
mind, I've removed its SkRTreeFactory to keep that feature explicitly
disabled there.

BUG=skia:5974

Change-Id: I4118d2e85f2a69adef2e7a7fa9b9b8c17607a94f
Reviewed-on: https://skia-review.googlesource.com/12624
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>

5 files changed, 63 insertions, 8 deletions

diff --git a/include/private/SkMiniRecorder.h b/include/private/SkMiniRecorder.h
index 06b35ca714..fd1e8f624d 100644
--- a/include/private/SkMiniRecorder.h
+++ b/include/private/SkMiniRecorder.h
@@ -25,11 +25,12 @@ public:
     bool drawTextBlob(const SkTextBlob*, SkScalar x, SkScalar y, const SkPaint&);
 
     // Detach anything we've recorded as a picture, resetting this SkMiniRecorder.
-    sk_sp<SkPicture> detachAsPicture(const SkRect& cull);
+    // If cull is nullptr we'll calculate it.
+    sk_sp<SkPicture> detachAsPicture(const SkRect* cull);
 
     // Flush anything we've recorded to the canvas, resetting this SkMiniRecorder.
     // This is logically the same as but rather more efficient than:
-    //    sk_sp<SkPicture> pic(this->detachAsPicture(SkRect::MakeEmpty()));
+    //    sk_sp<SkPicture> pic(this->detachAsPicture(nullptr));
     //    pic->playback(canvas);
     void flushAndReset(SkCanvas*);
 
diff --git a/src/core/SkMiniRecorder.cpp b/src/core/SkMiniRecorder.cpp
index 51ff5acbbc..b5d21a9efc 100644
--- a/src/core/SkMiniRecorder.cpp
+++ b/src/core/SkMiniRecorder.cpp
@@ -27,10 +27,27 @@ public:
     bool   willPlayBackBitmaps()  const override { return false; }
 };
 
+// Calculate conservative bounds for each type of draw op that can be its own mini picture.
+// These are fairly easy because we know they can't be affected by any matrix or saveLayers.
+static SkRect adjust_for_paint(SkRect bounds, const SkPaint& paint) {
+    return paint.canComputeFastBounds() ? paint.computeFastBounds(bounds, &bounds)
+                                        : SkRect::MakeLargest();
+}
+static SkRect bounds(const DrawRect& op) {
+    return adjust_for_paint(op.rect, op.paint);
+}
+static SkRect bounds(const DrawPath& op) {
+    return op.path.isInverseFillType() ? SkRect::MakeLargest()
+                                       : adjust_for_paint(op.path.getBounds(), op.paint);
+}
+static SkRect bounds(const DrawTextBlob& op) {
+    return adjust_for_paint(op.blob->bounds().makeOffset(op.x, op.y), op.paint);
+}
+
 template <typename T>
 class SkMiniPicture final : public SkPicture {
 public:
-    SkMiniPicture(SkRect cull, T* op) : fCull(cull) {
+    SkMiniPicture(const SkRect* cull, T* op) : fCull(cull ? *cull : bounds(*op)) {
         memcpy(&fOp, op, sizeof(fOp));  // We take ownership of op's guts.
     }
 
@@ -59,7 +76,7 @@ SkMiniRecorder::~SkMiniRecorder() {
     if (fState != State::kEmpty) {
         // We have internal state pending.
         // Detaching then deleting a picture is an easy way to clean up.
-        (void)this->detachAsPicture(SkRect::MakeEmpty());
+        (void)this->detachAsPicture(nullptr);
     }
     SkASSERT(fState == State::kEmpty);
 }
@@ -84,7 +101,7 @@ bool SkMiniRecorder::drawTextBlob(const SkTextBlob* b, SkScalar x, SkScalar y, c
 #undef TRY_TO_STORE
 
 
-sk_sp<SkPicture> SkMiniRecorder::detachAsPicture(const SkRect& cull) {
+sk_sp<SkPicture> SkMiniRecorder::detachAsPicture(const SkRect* cull) {
 #define CASE(Type)              \
     case State::k##Type:        \
         fState = State::kEmpty; \
diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp
index 701df7d44f..7abb12bfa7 100644
--- a/src/core/SkPictureRecorder.cpp
+++ b/src/core/SkPictureRecorder.cpp
@@ -56,7 +56,9 @@ sk_sp<SkPicture> SkPictureRecorder::finishRecordingAsPicture(uint32_t finishFlag
     fRecorder->restoreToCount(1);  // If we were missing any restores, add them now.
 
     if (fRecord->count() == 0) {
-        return fMiniRecorder.detachAsPicture(fCullRect);
+        auto pic = fMiniRecorder.detachAsPicture(fBBH ? nullptr : &fCullRect);
+        fBBH.reset(nullptr);
+        return pic;
     }
 
     // TODO: delay as much of this work until just before first playback?
diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 56cd5d5250..8a19a226c6 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -244,9 +244,8 @@ public:
                                                                          cropRect));
         }
         {
-            SkRTreeFactory factory;
             SkPictureRecorder recorder;
-            SkCanvas* recordingCanvas = recorder.beginRecording(64, 64, &factory, 0);
+            SkCanvas* recordingCanvas = recorder.beginRecording(64, 64);
 
             SkPaint greenPaint;
             greenPaint.setColor(SK_ColorGREEN);
diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 07cbccc51a..fd4bf4ef78 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -1136,3 +1136,39 @@ DEF_TEST(PictureGpuAnalyzer, r) {
 }
 
 #endif // SK_SUPPORT_GPU
+
+// If we record bounded ops into a picture with a big cull and calculate the
+// bounds of those ops, we should trim down the picture cull to the ops' bounds.
+// If we're not using an SkBBH, we shouldn't change it.
+DEF_TEST(Picture_UpdatedCull_1, r) {
+    // Testing 1 draw exercises SkMiniPicture.
+    SkRTreeFactory factory;
+    SkPictureRecorder recorder;
+
+    auto canvas = recorder.beginRecording(SkRect::MakeLargest(), &factory);
+    canvas->drawRect(SkRect::MakeWH(20,20), SkPaint{});
+    auto pic = recorder.finishRecordingAsPicture();
+    REPORTER_ASSERT(r, pic->cullRect() == SkRect::MakeWH(20,20));
+
+    canvas = recorder.beginRecording(SkRect::MakeLargest());
+    canvas->drawRect(SkRect::MakeWH(20,20), SkPaint{});
+    pic = recorder.finishRecordingAsPicture();
+    REPORTER_ASSERT(r, pic->cullRect() == SkRect::MakeLargest());
+}
+DEF_TEST(Picture_UpdatedCull_2, r) {
+    // Testing >1 draw exercises SkBigPicture.
+    SkRTreeFactory factory;
+    SkPictureRecorder recorder;
+
+    auto canvas = recorder.beginRecording(SkRect::MakeLargest(), &factory);
+    canvas->drawRect(SkRect::MakeWH(20,20), SkPaint{});
+    canvas->drawRect(SkRect::MakeWH(10,40), SkPaint{});
+    auto pic = recorder.finishRecordingAsPicture();
+    REPORTER_ASSERT(r, pic->cullRect() == SkRect::MakeWH(20,40));
+
+    canvas = recorder.beginRecording(SkRect::MakeLargest());
+    canvas->drawRect(SkRect::MakeWH(20,20), SkPaint{});
+    canvas->drawRect(SkRect::MakeWH(10,40), SkPaint{});
+    pic = recorder.finishRecordingAsPicture();
+    REPORTER_ASSERT(r, pic->cullRect() == SkRect::MakeLargest());
+}