From 12e946b4bfdf598bffb276776ea6e25439e25265 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Mon, 17 Apr 2017 10:53:29 -0400 Subject: [PATCH] deprecate odd variants of SkCanvas::readPixels Bug: skia:6513 Change-Id: I51179a85f0912d3f899c368c30a943d346dd1d05 Reviewed-on: https://skia-review.googlesource.com/13589 Reviewed-by: Florin Malita Reviewed-by: Matt Sarett Commit-Queue: Mike Reed --- bench/ReadPixBench.cpp | 5 +++-- bench/WritePixelsBench.cpp | 2 +- bench/nanobench.cpp | 4 ++-- dm/DMSrcSink.cpp | 4 ++-- gm/path_stroke_with_zero_length.cpp | 2 +- include/core/SkCanvas.h | 5 ++++- public.bzl | 1 + samplecode/SampleApp.cpp | 2 +- src/core/SkCanvas.cpp | 11 +++++++++++ tests/BlurTest.cpp | 5 +---- tests/DrawPathTest.cpp | 4 ++-- tests/ImageNewShaderTest.cpp | 11 ++++++----- tests/PremulAlphaRoundTripTest.cpp | 4 ++-- tests/ReadPixelsTest.cpp | 17 +++++------------ tests/ResourceCacheTest.cpp | 2 +- tests/WritePixelsTest.cpp | 3 ++- tools/skiaserve/Request.cpp | 4 ++-- tools/skpbench/skpbench.cpp | 4 ++-- 18 files changed, 49 insertions(+), 41 deletions(-) diff --git a/bench/ReadPixBench.cpp b/bench/ReadPixBench.cpp index ad9f747..2efb19c 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 820bb59..5d6787e 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 2f271db..ca8c183 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 e8c33a4..3a19751 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 0f6d200..f21d624 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 399eb21..ddbab07 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 01d70f7..2a4603b 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 ce97024..feace00 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 f8e427c..1b96f49 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 744e202..985917f 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 87d51b1..2434bdf 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 4091db0..37e1e30 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 b1310e3..7719ad8 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 10462c9..697d3ee 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 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 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 } else { REPORTER_ASSERT(reporter, !success); } +#endif } } } diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp index fcf3fe8..0736938 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 cace6b1..bf2e64e 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 7c2b893..12f3596 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 569c204..20ba8b4 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]), -- 2.7.4