From 9cd016e9b63f3827580d5b19a187dbf26b8e1436 Mon Sep 17 00:00:00 2001 From: reed Date: Sat, 30 Jan 2016 10:01:06 -0800 Subject: [PATCH] allow the caller to specified raster-surface rowbytes. along the way, simplify how we copy the surface's bitmap BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1643873002 Review URL: https://codereview.chromium.org/1643873002 --- include/core/SkSurface.h | 11 +++++++++-- src/image/SkSurface_Raster.cpp | 26 +++++++++++++++++++++----- tests/SurfaceTest.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h index 57527ed..52097be 100644 --- a/include/core/SkSurface.h +++ b/include/core/SkSurface.h @@ -61,12 +61,19 @@ public: void* context, const SkSurfaceProps* = NULL); /** - * Return a new surface, with the memory for the pixels automatically - * allocated. + * Return a new surface, with the memory for the pixels automatically allocated, but respecting + * the specified rowBytes. If rowBytes==0, then a default value will be chosen. If a non-zero + * rowBytes is specified, then any images snapped off of this surface (via newImageSnapshot()) + * are guaranteed to have the same rowBytes. * * If the requested surface cannot be created, or the request is not a * supported configuration, NULL will be returned. */ + static SkSurface* NewRaster(const SkImageInfo&, size_t rowBytes, const SkSurfaceProps*); + + /** + * Allocate a new surface, automatically computing the rowBytes. + */ static SkSurface* NewRaster(const SkImageInfo&, const SkSurfaceProps* = NULL); /** diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp index a606656..52300c3 100644 --- a/src/image/SkSurface_Raster.cpp +++ b/src/image/SkSurface_Raster.cpp @@ -31,6 +31,7 @@ public: private: SkBitmap fBitmap; + size_t fRowBytes; bool fWeOwnThePixels; typedef SkSurface_Base INHERITED; @@ -88,6 +89,7 @@ SkSurface_Raster::SkSurface_Raster(const SkImageInfo& info, void* pixels, size_t : INHERITED(info, props) { fBitmap.installPixels(info, pixels, rb, nullptr, releaseProc, context); + fRowBytes = 0; // don't need to track the rowbytes fWeOwnThePixels = false; // We are "Direct" } @@ -96,8 +98,9 @@ SkSurface_Raster::SkSurface_Raster(SkPixelRef* pr, const SkSurfaceProps* props) { const SkImageInfo& info = pr->info(); - fBitmap.setInfo(info, info.minRowBytes()); + fBitmap.setInfo(info, pr->rowBytes()); fBitmap.setPixelRef(pr); + fRowBytes = pr->rowBytes(); // we track this, so that subsequent re-allocs will match fWeOwnThePixels = true; } @@ -139,12 +142,17 @@ void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) { if (SkBitmapImageGetPixelRef(this->getCachedImage(kNo_Budgeted)) == fBitmap.pixelRef()) { SkASSERT(fWeOwnThePixels); if (kDiscard_ContentChangeMode == mode) { - fBitmap.setPixelRef(nullptr); fBitmap.allocPixels(); } else { SkBitmap prev(fBitmap); - prev.deepCopyTo(&fBitmap); + fBitmap.allocPixels(); + prev.lockPixels(); + SkASSERT(prev.info() == fBitmap.info()); + SkASSERT(prev.rowBytes() == fBitmap.rowBytes()); + memcpy(fBitmap.getPixels(), prev.getPixels(), fBitmap.getSafeSize()); } + SkASSERT(fBitmap.rowBytes() == fRowBytes); // be sure we always use the same value + // Now fBitmap is a deep copy of itself (and therefore different from // what is being used by the image. Next we update the canvas to use // this as its backend, so we can't modify the image's pixels anymore. @@ -176,14 +184,22 @@ SkSurface* SkSurface::NewRasterDirect(const SkImageInfo& info, void* pixels, siz return NewRasterDirectReleaseProc(info, pixels, rowBytes, nullptr, nullptr, props); } -SkSurface* SkSurface::NewRaster(const SkImageInfo& info, const SkSurfaceProps* props) { +SkSurface* SkSurface::NewRaster(const SkImageInfo& info, size_t rowBytes, + const SkSurfaceProps* props) { if (!SkSurface_Raster::Valid(info)) { return nullptr; } - SkAutoTUnref pr(SkMallocPixelRef::NewZeroed(info, 0, nullptr)); + SkAutoTUnref pr(SkMallocPixelRef::NewZeroed(info, rowBytes, nullptr)); if (nullptr == pr.get()) { return nullptr; } + if (rowBytes) { + SkASSERT(pr->rowBytes() == rowBytes); + } return new SkSurface_Raster(pr, props); } + +SkSurface* SkSurface::NewRaster(const SkImageInfo& info, const SkSurfaceProps* props) { + return NewRaster(info, 0, props); +} diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp index f0d9b17..5b7325e 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -661,3 +661,44 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceNoCanvas_Gpu, reporter, context) { } } #endif + +static void check_rowbytes_remain_consistent(SkSurface* surface, skiatest::Reporter* reporter) { + SkImageInfo info; + size_t rowBytes; + REPORTER_ASSERT(reporter, surface->peekPixels(&info, &rowBytes)); + + SkAutoTUnref image(surface->newImageSnapshot()); + SkImageInfo im_info; + size_t im_rowbytes; + REPORTER_ASSERT(reporter, image->peekPixels(&im_info, &im_rowbytes)); + + REPORTER_ASSERT(reporter, rowBytes == im_rowbytes); + + // trigger a copy-on-write + surface->getCanvas()->drawPaint(SkPaint()); + SkAutoTUnref image2(surface->newImageSnapshot()); + REPORTER_ASSERT(reporter, image->uniqueID() != image2->uniqueID()); + + SkImageInfo im_info2; + size_t im_rowbytes2; + REPORTER_ASSERT(reporter, image2->peekPixels(&im_info2, &im_rowbytes2)); + + REPORTER_ASSERT(reporter, im_rowbytes2 == im_rowbytes); +} + +DEF_TEST(surface_rowbytes, reporter) { + const SkImageInfo info = SkImageInfo::MakeN32Premul(100, 100); + + SkAutoTUnref surf0(SkSurface::NewRaster(info)); + check_rowbytes_remain_consistent(surf0, reporter); + + // specify a larger rowbytes + SkAutoTUnref surf1(SkSurface::NewRaster(info, 500, nullptr)); + check_rowbytes_remain_consistent(surf1, reporter); + + // Try some illegal rowByte values + SkSurface* s = SkSurface::NewRaster(info, 396, nullptr); // needs to be at least 400 + REPORTER_ASSERT(reporter, nullptr == s); + s = SkSurface::NewRaster(info, 1 << 30, nullptr); // allocation to large + REPORTER_ASSERT(reporter, nullptr == s); +} -- 2.7.4