correctly plumb through explicit rowbytes for allocPixels

BUG=skia:
R=caryclark@google.com

Author: reed@google.com

Review URL: https://codereview.chromium.org/357073003

diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h
index ce35308..2db81bb 100644 (file)
--- a/include/core/SkMallocPixelRef.h
+++ b/include/core/SkMallocPixelRef.h
@@ -78,6 +78,7 @@ public:
     class PRFactory : public SkPixelRefFactory {
     public:
         virtual SkPixelRef* create(const SkImageInfo&,
+                                   size_t rowBytes,
                                    SkColorTable*) SK_OVERRIDE;
     };
 

diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index 339cea1..842f493 100644 (file)
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -374,7 +374,7 @@ public:
      *  the pixelref will ref() the colortable.
      *  On failure return NULL.
      */
-    virtual SkPixelRef* create(const SkImageInfo&, SkColorTable*) = 0;
+    virtual SkPixelRef* create(const SkImageInfo&, size_t rowBytes, SkColorTable*) = 0;
 };
 
 #endif

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 82e0b4c..316878b 100644 (file)
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -177,7 +177,7 @@ bool SkBitmap::setInfo(const SkImageInfo& origInfo, size_t rowBytes) {
         rowBytes = 0;
     } else if (0 == rowBytes) {
         rowBytes = (size_t)mrb;
-    } else if (rowBytes < info.minRowBytes()) {
+    } else if (!info.validRowBytes(rowBytes)) {
         return reset_return_false(this);
     }
 
@@ -339,19 +339,18 @@ bool SkBitmap::allocPixels(const SkImageInfo& requestedInfo, size_t rowBytes) {
     if (kIndex_8_SkColorType == requestedInfo.colorType()) {
         return reset_return_false(this);
     }
-    if (!this->setInfo(requestedInfo)) {
+    if (!this->setInfo(requestedInfo, rowBytes)) {
         return reset_return_false(this);
     }
     
     // setInfo may have corrected info (e.g. 565 is always opaque).
     const SkImageInfo& correctedInfo = this->info();
-    if (!correctedInfo.validRowBytes(rowBytes)) {
-        return reset_return_false(this);
-    }
-    
+    // setInfo may have computed a valid rowbytes if 0 were passed in
+    rowBytes = this->rowBytes();
+
     SkMallocPixelRef::PRFactory defaultFactory;
     
-    SkPixelRef* pr = defaultFactory.create(correctedInfo, NULL);
+    SkPixelRef* pr = defaultFactory.create(correctedInfo, rowBytes, NULL);
     if (NULL == pr) {
         return reset_return_false(this);
     }
@@ -382,7 +381,7 @@ bool SkBitmap::allocPixels(const SkImageInfo& requestedInfo, SkPixelRefFactory*
         factory = &defaultFactory;
     }
 
-    SkPixelRef* pr = factory->create(correctedInfo, ctable);
+    SkPixelRef* pr = factory->create(correctedInfo, correctedInfo.minRowBytes(), ctable);
     if (NULL == pr) {
         return reset_return_false(this);
     }

diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index 12cb05b..d51fb7c 100644 (file)
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -240,7 +240,7 @@ SkMallocPixelRef::SkMallocPixelRef(SkReadBuffer& buffer)
 
 ///////////////////////////////////////////////////////////////////////////////
 
-SkPixelRef* SkMallocPixelRef::PRFactory::create(const SkImageInfo& info,
+SkPixelRef* SkMallocPixelRef::PRFactory::create(const SkImageInfo& info, size_t rowBytes,
                                                 SkColorTable* ctable) {
-    return SkMallocPixelRef::NewAllocate(info, info.minRowBytes(), ctable);
+    return SkMallocPixelRef::NewAllocate(info, rowBytes, ctable);
 }

diff --git a/tests/BitmapTest.cpp b/tests/BitmapTest.cpp
index ef560aa..42ed884 100644 (file)
--- a/tests/BitmapTest.cpp
+++ b/tests/BitmapTest.cpp
@@ -9,6 +9,42 @@
 
 #include "Test.h"
 
+static void test_allocpixels(skiatest::Reporter* reporter) {
+    const int width = 10;
+    const int height = 10;
+    const SkImageInfo info = SkImageInfo::MakeN32Premul(width, height);
+    const size_t explicitRowBytes = info.minRowBytes() + 24;
+
+    SkBitmap bm;
+    bm.setInfo(info);
+    REPORTER_ASSERT(reporter, info.minRowBytes() == bm.rowBytes());
+    bm.allocPixels();
+    REPORTER_ASSERT(reporter, info.minRowBytes() == bm.rowBytes());
+    bm.reset();
+    bm.allocPixels(info);
+    REPORTER_ASSERT(reporter, info.minRowBytes() == bm.rowBytes());
+
+    bm.setInfo(info, explicitRowBytes);
+    REPORTER_ASSERT(reporter, explicitRowBytes == bm.rowBytes());
+    bm.allocPixels();
+    REPORTER_ASSERT(reporter, explicitRowBytes == bm.rowBytes());
+    bm.reset();
+    bm.allocPixels(info, explicitRowBytes);
+    REPORTER_ASSERT(reporter, explicitRowBytes == bm.rowBytes());
+
+    bm.reset();
+    bm.setInfo(info, 0);
+    REPORTER_ASSERT(reporter, info.minRowBytes() == bm.rowBytes());
+    bm.reset();
+    bm.allocPixels(info, 0);
+    REPORTER_ASSERT(reporter, info.minRowBytes() == bm.rowBytes());
+
+    bm.reset();
+    bool success = bm.setInfo(info, info.minRowBytes() - 1);   // invalid for 32bit
+    REPORTER_ASSERT(reporter, !success);
+    REPORTER_ASSERT(reporter, bm.isNull());
+}
+
 static void test_bigwidth(skiatest::Reporter* reporter) {
     SkBitmap bm;
     int width = 1 << 29;    // *4 will be the high-bit of 32bit int
@@ -46,4 +82,5 @@ DEF_TEST(Bitmap, reporter) {
     }
 
     test_bigwidth(reporter);
+    test_allocpixels(reporter);
 }