picture nesting bounds bug

- add a unit test reproducing the bug
  - fix SkRecorder::reset() to call resetCanvas(bounds) instead of
    calling resetCanvas(w,h).  (It was actually calling
    resetCanvas(right,top), even worse...)

In short, because we were calling this old resetCanvas(), SkRecorder,
the SkCanvas* we record into, was presenting bad device bounds,
affecting code like where we query the clip to search an R-tree for ops
to draw.  It was trimmed to only the positive/positive portion of the
actual bounds, so content like in the unit test that's all in negative
space was erroneously clipped out.

I'd like to get rid of these w/h methods altogether but they're still
used by some of our test tools and by Android.

Change-Id: Ie46f611250de4d655c4357823895ff885b4f3d59
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/245599
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>

src/core/SkRecorder.cpp

@@ -58,8 +58,7 @@ void SkRecorder::reset(SkRecord* record, const SkRect& bounds,
     this->forgetRecord();
     fDrawPictureMode = dpm;
     fRecord = record;
-    SkIRect rounded = bounds.roundOut();
-    this->resetCanvas(rounded.right(), rounded.bottom());
+    this->resetCanvas(bounds.roundOut());
     fMiniRecorder = mr;
 }
 

src/core/SkRecorder.h

@@ -40,7 +40,7 @@ private:
 class SkRecorder final : public SkCanvasVirtualEnforcer<SkNoDrawCanvas> {
 public:
     // Does not take ownership of the SkRecord.
-    SkRecorder(SkRecord*, int width, int height, SkMiniRecorder* = nullptr);   // legacy version
+    SkRecorder(SkRecord*, int width, int height, SkMiniRecorder* = nullptr);   // TODO: remove
     SkRecorder(SkRecord*, const SkRect& bounds, SkMiniRecorder* = nullptr);
 
     enum DrawPictureMode {

tests/PictureTest.cpp

@@ -909,3 +909,45 @@ DEF_TEST(Picture_drawsNothing, r) {
         REPORTER_ASSERT(r, pic->cullRect().isEmpty() == c.draws_nothing);
     }
 }
+
+DEF_TEST(Picture_emptyNestedPictureBug, r) {
+    const SkRect bounds = {-5000, -5000, 5000, 5000};
+
+    SkPictureRecorder recorder;
+    SkRTreeFactory factory;
+
+    // These three pictures should all draw the same but due to bugs they don't:
+    //
+    //   1) inner has enough content that it is recoreded as an SkBigPicture,
+    //      and all its content falls outside the positive/positive quadrant,
+    //      and it is recorded with an R-tree so we contract the cullRect to those bounds;
+    //
+    //   2) middle wraps inner,
+    //      and it its recorded with an R-tree so we update middle's cullRect to inner's;
+    //
+    //   3) outer wraps inner,
+    //      and notices that middle contains only one op, drawPicture(inner),
+    //      so it plays middle back during recording rather than ref'ing middle,
+    //      querying middle's R-tree with its SkCanvas' bounds* {0,0, 5000,5000},
+    //      finding nothing to draw.
+    //
+    //  * The bug was that these bounds were not tracked as {-5000,-5000, 5000,5000}.
+    {
+        SkCanvas* canvas = recorder.beginRecording(bounds, &factory);
+        canvas->translate(-100,-100);
+        canvas->drawRect({0,0,50,50}, SkPaint{});
+    }
+    sk_sp<SkPicture> inner = recorder.finishRecordingAsPicture();
+
+    recorder.beginRecording(bounds, &factory)->drawPicture(inner);
+    sk_sp<SkPicture> middle = recorder.finishRecordingAsPicture();
+
+    // This doesn't need &factory to reproduce the bug,
+    // but it's nice to see we come up with the same {-100,-100, -50,-50} bounds.
+    recorder.beginRecording(bounds, &factory)->drawPicture(middle);
+    sk_sp<SkPicture> outer = recorder.finishRecordingAsPicture();
+
+    REPORTER_ASSERT(r, (inner ->cullRect() == SkRect{-100,-100, -50,-50}));
+    REPORTER_ASSERT(r, (middle->cullRect() == SkRect{-100,-100, -50,-50}));
+    REPORTER_ASSERT(r, (outer ->cullRect() == SkRect{-100,-100, -50,-50}));   // Used to fail.
+}