Fix error revealed by Android unit test

tests/PictureTest.cpp

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBitmapDevice.h"
 #include "SkBlurImageFilter.h"
 #include "SkCanvas.h"
@@ skipping 968 matching lines @@
         SkPictureRecorder recorder2;
 
         SkCanvas* canvas = recorder2.beginRecording(10, 10);
 
         recorder->partialReplay(canvas);
 
         return recorder2.endRecording();
     }
 };
 
+static void create_imbalance(SkCanvas* canvas) {
+    SkRect clipRect = SkRect::MakeWH(2, 2);
+    SkRect drawRect = SkRect::MakeWH(10, 10);
+    canvas->save();
+        canvas->clipRect(clipRect, SkRegion::kReplace_Op);
+        canvas->translate(1.0f, 1.0f);
+        SkPaint p;
+        p.setColor(SK_ColorGREEN);
+        canvas->drawRect(drawRect, p);
+    // no restore
+}
+
+static void check_balance(skiatest::Reporter* reporter, SkPicture* picture) {

[scroggo, 2014/06/10 13:31:25]

Some comments here would be nice. e.g.

// Test that playing an SkPicture into an SkCanvas does not modify the 
// SkCanvas' save count.

[robertphillips, 2014/06/10 17:26:28]

Done.

+    SkBitmap bm;
+    bm.allocN32Pixels(4, 3);
+    SkCanvas canvas(bm);
+
+    int beforeSaveCount = canvas.getSaveCount();
+
+    SkMatrix beforeMatrix = canvas.getTotalMatrix();
+
+    canvas.drawPicture(picture);
+
+    REPORTER_ASSERT(reporter, beforeSaveCount == canvas.getSaveCount());
+    REPORTER_ASSERT(reporter, beforeMatrix == canvas.getTotalMatrix());
+
+    SkRect afterClip;
+
+    canvas.getClipBounds(&afterClip);
+
+    REPORTER_ASSERT(reporter, afterClip.width() == 4);
+    REPORTER_ASSERT(reporter, afterClip.height() == 4);

[scroggo, 2014/06/10 13:31:25]

How did the height end up 4?

[robertphillips, 2014/06/10 17:27:24]

Switch to storing a beforeClip so things are clearer.

+}
+
 // Test out SkPictureRecorder::partialReplay
 DEF_TEST(PictureRecorder_replay, reporter) {
     // check save/saveLayer state
     {
         SkPictureRecorder recorder;
 
         SkCanvas* canvas = recorder.beginRecording(10, 10);
 
         canvas->saveLayer(NULL, NULL);
 
@@ skipping 34 matching lines @@
         canvas->drawBitmapRectToRect(bm, NULL, r);
 
         SkAutoTUnref<SkPicture> final(recorder.endRecording());
         REPORTER_ASSERT(reporter, final->willPlayBackBitmaps());
 
         REPORTER_ASSERT(reporter, copy->uniqueID() != final->uniqueID());
 
         // The snapshot shouldn't pick up any operations added after it was made
         REPORTER_ASSERT(reporter, !copy->willPlayBackBitmaps());
     }
+
+    // Recreate the Android partialReplay test case
+    {
+        SkPictureRecorder recorder;
+
+        SkCanvas* canvas = recorder.beginRecording(4, 3, NULL, 0);
+        create_imbalance(canvas);
+
+        int expectedSaveCount = canvas->getSaveCount();
+
+        SkAutoTUnref<SkPicture> copy(SkPictureRecorderReplayTester::Copy(&recorder));
+        check_balance(reporter, copy);
+
+        REPORTER_ASSERT(reporter, expectedSaveCount = canvas->getSaveCount());
+
+        SkAutoTUnref<SkPicture> final(recorder.endRecording());

[scroggo, 2014/06/10 13:31:25]

What does this test?

[robertphillips, 2014/06/10 17:26:28]

Done - added comment.

+    }
 }
 
 static void test_unbalanced_save_restores(skiatest::Reporter* reporter) {
     SkCanvas testCanvas(100, 100);
     set_canvas_to_save_count_4(&testCanvas);
 
     REPORTER_ASSERT(reporter, 4 == testCanvas.getSaveCount());
 
     SkPaint paint;
     SkRect rect = SkRect::MakeLTRB(-10000000, -10000000, 10000000, 10000000);
@@ skipping 525 matching lines @@
     SkAutoTUnref<SkPicture> picture(recorder.endRecording());
 }
 
 DEF_TEST(Canvas_EmptyBitmap, r) {
     SkBitmap dst;
     dst.allocN32Pixels(10, 10);
     SkCanvas canvas(dst);
 
     test_draw_bitmaps(&canvas);
 }