deprecate odd variants of SkCanvas::readPixels

Bug: skia:6513
Change-Id: I51179a85f0912d3f899c368c30a943d346dd1d05
Reviewed-on: https://skia-review.googlesource.com/13589
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Mike Reed <reed@google.com>

18 files changed, 49 insertions, 41 deletions

diff --git a/bench/ReadPixBench.cpp b/bench/ReadPixBench.cpp
index ad9f747e7d..2efb19cc9b 100644
--- a/bench/ReadPixBench.cpp
+++ b/bench/ReadPixBench.cpp
@@ -42,12 +42,13 @@ protected:
 
         SkBitmap bitmap;
 
-        bitmap.setInfo(SkImageInfo::MakeN32Premul(kWindowSize, kWindowSize));
+        bitmap.allocPixels(SkImageInfo::MakeN32Premul(kWindowSize, kWindowSize));
 
         for (int i = 0; i < loops; i++) {
             for (int x = 0; x < kNumStepsX; ++x) {
                 for (int y = 0; y < kNumStepsY; ++y) {
-                    canvas->readPixels(&bitmap, x * offX, y * offY);
+                    canvas->readPixels(bitmap.info(), bitmap.getPixels(), bitmap.rowBytes(),
+                                       x * offX, y * offY);
                 }
             }
         }
diff --git a/bench/WritePixelsBench.cpp b/bench/WritePixelsBench.cpp
index 820bb5902b..5d6787e25c 100644
--- a/bench/WritePixelsBench.cpp
+++ b/bench/WritePixelsBench.cpp
@@ -52,7 +52,7 @@ protected:
 
         SkBitmap bmp;
         bmp.allocN32Pixels(size.width(), size.height());
-        canvas->readPixels(&bmp, 0, 0);
+        canvas->readPixels(bmp, 0, 0);
 
         SkImageInfo info = SkImageInfo::Make(bmp.width(), bmp.height(), fColorType, fAlphaType);
 
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 2f271dbbcf..ca8c183043 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -154,8 +154,8 @@ bool Target::capturePixels(SkBitmap* bmp) {
     if (!canvas) {
         return false;
     }
-    bmp->setInfo(canvas->imageInfo());
-    if (!canvas->readPixels(bmp, 0, 0)) {
+    bmp->allocPixels(canvas->imageInfo());
+    if (!canvas->readPixels(*bmp, 0, 0)) {
         SkDebugf("Can't read canvas pixels.\n");
         return false;
     }
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index e8c33a4c4b..3a19751409 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -1306,7 +1306,7 @@ Error GPUSink::draw(const Src& src, SkBitmap* dst, SkWStream*, SkString* log) co
         canvas->getGrContext()->dumpGpuStats(log);
     }
     dst->allocPixels(info);
-    canvas->readPixels(dst, 0, 0);
+    canvas->readPixels(*dst, 0, 0);
     if (FLAGS_abandonGpuContext) {
         factory.abandonContexts();
     } else if (FLAGS_releaseAndAbandonGpuContext) {
@@ -1869,7 +1869,7 @@ Error ViaCSXform::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkSt
         if (fColorSpin) {
             SkBitmap pixels;
             pixels.allocPixels(canvas->imageInfo());
-            canvas->readPixels(&pixels, 0, 0);
+            canvas->readPixels(pixels, 0, 0);
             for (int y = 0; y < pixels.height(); y++) {
                 for (int x = 0; x < pixels.width(); x++) {
                     uint32_t pixel = *pixels.getAddr32(x, y);
diff --git a/gm/path_stroke_with_zero_length.cpp b/gm/path_stroke_with_zero_length.cpp
index 0f6d200b6f..f21d624bbd 100644
--- a/gm/path_stroke_with_zero_length.cpp
+++ b/gm/path_stroke_with_zero_length.cpp
@@ -151,7 +151,7 @@ private:
         SkScalar pathX = bounds.fLeft - 2;
         SkScalar pathY = bounds.fTop - 2;
         SkMatrix cMatrix = canvas->getTotalMatrix();
-        if (!canvas->readPixels(&offscreen, SkScalarRoundToInt(pathX + cMatrix.getTranslateX()),
+        if (!canvas->readPixels(offscreen, SkScalarRoundToInt(pathX + cMatrix.getTranslateX()),
                 SkScalarRoundToInt(pathY + cMatrix.getTranslateY()))) {
             return;
         }
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index 399eb21111..ddbab07480 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -230,14 +230,16 @@ public:
      */
     bool readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
                     int srcX, int srcY);
+    bool readPixels(const SkPixmap&, int srcX, int srcY);
+    bool readPixels(const SkBitmap&, int srcX, int srcY);
 
+#ifdef SK_SUPPORT_LEGACY_CANVAS_READPIXELS
     /**
      *  Helper for calling readPixels(info, ...). This call will check if bitmap has been allocated.
      *  If not, it will attempt to call allocPixels(). If this fails, it will return false. If not,
      *  it calls through to readPixels(info, ...) and returns its result.
      */
     bool readPixels(SkBitmap* bitmap, int srcX, int srcY);
-
     /**
      *  Helper for allocating pixels and then calling readPixels(info, ...). The bitmap is resized
      *  to the intersection of srcRect and the base-layer bounds. On success, pixels will be
@@ -245,6 +247,7 @@ public:
      *  set to empty.
      */
     bool readPixels(const SkIRect& srcRect, SkBitmap* bitmap);
+#endif
 
     /**
      *  This method affects the pixels in the base-layer, and operates in pixel coordinates,
diff --git a/public.bzl b/public.bzl
index 01d70f7bcd..2a4603b659 100644
--- a/public.bzl
+++ b/public.bzl
@@ -656,6 +656,7 @@ DEFINES_ALL = [
     # Staging flags for API changes
     # Temporarily Disable analytic AA for Google3
     "SK_NO_ANALYTIC_AA",
+    "SK_SUPPORT_LEGACY_CANVAS_READPIXELS",
 ]
 
 ################################################################################
diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index ce97024774..feace008fd 100644
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -1064,7 +1064,7 @@ void SampleWindow::listTitles() {
 static SkBitmap capture_bitmap(SkCanvas* canvas) {
     SkBitmap bm;
     if (bm.tryAllocPixels(canvas->imageInfo())) {
-        canvas->readPixels(&bm, 0, 0);
+        canvas->readPixels(bm, 0, 0);
     }
     return bm;
 }
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index f8e427cd81..1b96f4934e 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -831,6 +831,7 @@ SkBaseDevice* SkCanvas::getTopDevice() const {
     return fMCRec->fTopLayer->fDevice;
 }
 
+#ifdef SK_SUPPORT_LEGACY_CANVAS_READPIXELS
 bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
     bool weAllocated = false;
     if (nullptr == bitmap->pixelRef()) {
@@ -872,6 +873,7 @@ bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
     }
     return true;
 }
+#endif
 
 bool SkCanvas::readPixels(const SkImageInfo& dstInfo, void* dstP, size_t rowBytes, int x, int y) {
     SkBaseDevice* device = this->getDevice();
@@ -882,6 +884,15 @@ bool SkCanvas::readPixels(const SkImageInfo& dstInfo, void* dstP, size_t rowByte
     return device->readPixels(dstInfo, dstP, rowBytes, x, y);
 }
 
+bool SkCanvas::readPixels(const SkPixmap& pm, int x, int y) {
+    return pm.addr() && this->readPixels(pm.info(), pm.writable_addr(), pm.rowBytes(), x, y);
+}
+
+bool SkCanvas::readPixels(const SkBitmap& bm, int x, int y) {
+    SkPixmap pm;
+    return bm.peekPixels(&pm) && this->readPixels(pm, x, y);
+}
+
 bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
     SkAutoPixmapUnlock unlocker;
     if (bitmap.requestLock(&unlocker)) {
diff --git a/tests/BlurTest.cpp b/tests/BlurTest.cpp
index 744e2025b9..985917f1c0 100644
--- a/tests/BlurTest.cpp
+++ b/tests/BlurTest.cpp
@@ -242,10 +242,7 @@ static void blur_path(SkCanvas* canvas, const SkPath& path,
 static void readback(SkCanvas* canvas, int* result, int resultCount) {
     SkBitmap readback;
     readback.allocN32Pixels(resultCount, 30);
-
-    SkIRect readBackRect = { 0, 0, resultCount, 30 };
-
-    canvas->readPixels(readBackRect, &readback);
+    canvas->readPixels(readback, 0, 0);
 
     readback.lockPixels();
     SkPMColor* pixels = (SkPMColor*) readback.getAddr32(0, 15);
diff --git a/tests/DrawPathTest.cpp b/tests/DrawPathTest.cpp
index 87d51b100e..2434bdfccd 100644
--- a/tests/DrawPathTest.cpp
+++ b/tests/DrawPathTest.cpp
@@ -26,7 +26,7 @@ static void test_big_aa_rect(skiatest::Reporter* reporter) {
     int y = SkScalarRoundToInt(r.top());
 
     // check that the pixel in question starts as transparent (by the surface)
-    if (canvas->readPixels(&output, x, y)) {
+    if (canvas->readPixels(output, x, y)) {
         REPORTER_ASSERT(reporter, 0 == pixel[0]);
     } else {
         REPORTER_ASSERT_MESSAGE(reporter, false, "readPixels failed");
@@ -39,7 +39,7 @@ static void test_big_aa_rect(skiatest::Reporter* reporter) {
     canvas->drawRect(r, paint);
 
     // Now check that it is BLACK
-    if (canvas->readPixels(&output, x, y)) {
+    if (canvas->readPixels(output, x, y)) {
         // don't know what swizzling PMColor did, but white should always
         // appear the same.
         REPORTER_ASSERT(reporter, 0xFFFFFFFF == pixel[0]);
diff --git a/tests/ImageNewShaderTest.cpp b/tests/ImageNewShaderTest.cpp
index 4091db0d9b..37e1e301ee 100644
--- a/tests/ImageNewShaderTest.cpp
+++ b/tests/ImageNewShaderTest.cpp
@@ -57,14 +57,14 @@ static void run_shader_test(skiatest::Reporter* reporter, SkSurface* sourceSurfa
     destinationCanvas->clear(SK_ColorTRANSPARENT);
     destinationCanvas->drawPaint(paint);
 
-    SkIRect rect = info.bounds();
-
     SkBitmap bmOrig;
-    sourceSurface->getCanvas()->readPixels(rect, &bmOrig);
+    bmOrig.allocN32Pixels(info.width(), info.height());
+    sourceSurface->getCanvas()->readPixels(bmOrig, 0, 0);
 
 
     SkBitmap bm;
-    destinationCanvas->readPixels(rect, &bm);
+    bm.allocN32Pixels(info.width(), info.height());
+    destinationCanvas->readPixels(bm, 0, 0);
 
     test_bitmap_equality(reporter, bmOrig, bm);
 
@@ -85,7 +85,8 @@ static void run_shader_test(skiatest::Reporter* reporter, SkSurface* sourceSurfa
     destinationCanvas->drawPaint(paintTranslated);
 
     SkBitmap bmt;
-    destinationCanvas->readPixels(rect, &bmt);
+    bmt.allocN32Pixels(info.width(), info.height());
+    destinationCanvas->readPixels(bmt, 0, 0);
 
     //  Test correctness
     {
diff --git a/tests/PremulAlphaRoundTripTest.cpp b/tests/PremulAlphaRoundTripTest.cpp
index b1310e3681..7719ad88ab 100644
--- a/tests/PremulAlphaRoundTripTest.cpp
+++ b/tests/PremulAlphaRoundTripTest.cpp
@@ -76,10 +76,10 @@ static void test_premul_alpha_roundtrip(skiatest::Reporter* reporter, SkSurface*
         readBmp1.eraseColor(0);
         readBmp2.eraseColor(0);
 
-        canvas->readPixels(&readBmp1, 0, 0);
+        canvas->readPixels(readBmp1, 0, 0);
         sk_tool_utils::write_pixels(canvas, readBmp1, 0, 0, gUnpremul[upmaIdx].fColorType,
                                     kUnpremul_SkAlphaType);
-        canvas->readPixels(&readBmp2, 0, 0);
+        canvas->readPixels(readBmp2, 0, 0);
 
         bool success = true;
         for (int y = 0; y < 256 && success; ++y) {
diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp
index 10462c9140..697d3ee99e 100644
--- a/tests/ReadPixelsTest.cpp
+++ b/tests/ReadPixelsTest.cpp
@@ -247,8 +247,7 @@ static bool check_read(skiatest::Reporter* reporter,
 enum BitmapInit {
     kFirstBitmapInit = 0,
 
-    kNoPixels_BitmapInit = kFirstBitmapInit,
-    kTight_BitmapInit,
+    kTight_BitmapInit = kFirstBitmapInit,
     kRowBytes_BitmapInit,
     kRowBytesOdd_BitmapInit,
 
@@ -270,10 +269,7 @@ static void init_bitmap(SkBitmap* bitmap, const SkIRect& rect, BitmapInit init,
                         SkAlphaType at) {
     SkImageInfo info = SkImageInfo::Make(rect.width(), rect.height(), ct, at);
     size_t rowBytes = 0;
-    bool alloc = true;
     switch (init) {
-        case kNoPixels_BitmapInit:
-            alloc = false;
         case kTight_BitmapInit:
             break;
         case kRowBytes_BitmapInit:
@@ -286,12 +282,7 @@ static void init_bitmap(SkBitmap* bitmap, const SkIRect& rect, BitmapInit init,
             SkASSERT(0);
             break;
     }
-
-    if (alloc) {
-        bitmap->allocPixels(info, rowBytes);
-    } else {
-        bitmap->setInfo(info, rowBytes);
-    }
+    bitmap->allocPixels(info, rowBytes);
 }
 
 static const struct {
@@ -370,7 +361,7 @@ static void test_readpixels(skiatest::Reporter* reporter, const sk_sp<SkSurface>
                     fill_dst_bmp_with_init_data(&bmp);
                 }
                 uint32_t idBefore = surface->generationID();
-                bool success = canvas->readPixels(&bmp, srcRect.fLeft, srcRect.fTop);
+                bool success = canvas->readPixels(bmp, srcRect.fLeft, srcRect.fTop);
                 uint32_t idAfter = surface->generationID();
 
                 // we expect to succeed when the read isn't fully clipped
@@ -391,6 +382,7 @@ static void test_readpixels(skiatest::Reporter* reporter, const sk_sp<SkSurface>
                     REPORTER_ASSERT(reporter, bmp.isNull());
                 }
             }
+#ifdef SK_SUPPORT_LEGACY_CANVAS_READPIXELS
             // check the old webkit version of readPixels that clips the
             // bitmap size
             SkBitmap wkbmp;
@@ -406,6 +398,7 @@ static void test_readpixels(skiatest::Reporter* reporter, const sk_sp<SkSurface>
             } else {
                 REPORTER_ASSERT(reporter, !success);
             }
+#endif
         }
     }
 }
diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp
index fcf3fe88f3..07369380af 100644
--- a/tests/ResourceCacheTest.cpp
+++ b/tests/ResourceCacheTest.cpp
@@ -66,7 +66,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ResourceCacheCache, reporter, ctxInfo) {
 
     for (int i = 0; i < 100; ++i) {
         canvas->drawBitmap(src, 0, 0);
-        canvas->readPixels(size, &readback);
+        canvas->readPixels(readback, 0, 0);
 
         // "modify" the src texture
         src.notifyPixelsChanged();
diff --git a/tests/WritePixelsTest.cpp b/tests/WritePixelsTest.cpp
index cace6b196e..bf2e64efc1 100644
--- a/tests/WritePixelsTest.cpp
+++ b/tests/WritePixelsTest.cpp
@@ -191,7 +191,8 @@ static bool check_write(skiatest::Reporter* reporter, SkCanvas* canvas, const Sk
     // At some point this will be unsupported, as we won't allow accessBitmap() to magically call
     // readPixels for the client.
     SkBitmap secretDevBitmap;
-    if (!canvas->readPixels(canvasInfo.bounds(), &secretDevBitmap)) {
+    secretDevBitmap.allocN32Pixels(canvasInfo.width(), canvasInfo.height());
+    if (!canvas->readPixels(secretDevBitmap, 0, 0)) {
         return false;
     }
 
diff --git a/tools/skiaserve/Request.cpp b/tools/skiaserve/Request.cpp
index 7c2b893ef2..12f3596817 100644
--- a/tools/skiaserve/Request.cpp
+++ b/tools/skiaserve/Request.cpp
@@ -46,9 +46,9 @@ Request::~Request() {
 
 SkBitmap* Request::getBitmapFromCanvas(SkCanvas* canvas) {
     SkBitmap* bmp = new SkBitmap();
-    bmp->setInfo(canvas->imageInfo());
-    if (!canvas->readPixels(bmp, 0, 0)) {
+    if (!bmp->tryAllocPixels(canvas->imageInfo()) || !canvas->readPixels(*bmp, 0, 0)) {
         fprintf(stderr, "Can't read pixels\n");
+        delete bmp;
         return nullptr;
     }
     return bmp;
diff --git a/tools/skpbench/skpbench.cpp b/tools/skpbench/skpbench.cpp
index 569c204a72..20ba8b4ec4 100644
--- a/tools/skpbench/skpbench.cpp
+++ b/tools/skpbench/skpbench.cpp
@@ -335,8 +335,8 @@ int main(int argc, char** argv) {
     // Save a proof (if one was requested).
     if (!FLAGS_png.isEmpty()) {
         SkBitmap bmp;
-        bmp.setInfo(info);
-        if (!surface->getCanvas()->readPixels(&bmp, 0, 0)) {
+        bmp.allocPixels(info);
+        if (!surface->getCanvas()->readPixels(bmp, 0, 0)) {
             exitf(ExitErr::kUnavailable, "failed to read canvas pixels for png");
         }
         const SkString &dirname = SkOSPath::Dirname(FLAGS_png[0]),