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

diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h
index 57527ed..52097be 100644 (file)
--- 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 (file)
--- 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<SkPixelRef> pr(SkMallocPixelRef::NewZeroed(info, 0, nullptr));
+    SkAutoTUnref<SkPixelRef> 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 (file)
--- 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<SkImage> 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<SkImage> 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<SkSurface> surf0(SkSurface::NewRaster(info));
+    check_rowbytes_remain_consistent(surf0, reporter);
+
+    // specify a larger rowbytes
+    SkAutoTUnref<SkSurface> 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);
+}