Make SkImageInfo::validRowBytes consider alignment

SkSurface will already reject a rowBytes that does not align on a pixel
boundary. Push that check into SkImageInfo. This will make SkBitmap,
SkPixmap, SkMallocPixelRef, and SkImage_Raster, which already call
validRowBytes, make the same check. If an SkSurface cannot use a
non-pixel-aligned rowBytes, then an SkCanvas wrapping an SkBitmap should
not either.

Update MallocPixelRefTest to use a rowBytes that is still valid.

Change-Id: I848d94dbeab8b58b92877104dd67ea23a9d19ca8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265599
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Herb Derby <herb@google.com>

RELEASE_NOTES.txt

@@ -9,6 +9,10 @@ Milestone 81
 <Insert new notes here- top is most recent.>
   * Added support for GL_NV_fence extension.
 
+  * Make SkImageInfo::validRowBytes require rowBytes to be pixel aligned. This
+    makes SkBitmap match the behavior of raster SkSurfaces in rejecting
+    non-aligned rowBytes.
+
   * Added support for BC1 RGBA compressed textures
 
   * Added CachingHint to SkImage::makeRasterImage

include/core/SkImageInfo.h

@@ -731,13 +731,19 @@ public:
         return SIZE_MAX == byteSize;
     }
 
-    /** Returns true if rowBytes is smaller than width times pixel size.
+    /** Returns true if rowBytes is valid for this SkImageInfo.
 
-        @param rowBytes  size of pixel row or larger
-        @return          true if rowBytes is large enough to contain pixel row
+        @param rowBytes  size of pixel row including padding
+        @return          true if rowBytes is large enough to contain pixel row and is properly
+                         aligned
     */
     bool validRowBytes(size_t rowBytes) const {
-        return rowBytes >= this->minRowBytes64();
+        if (rowBytes < this->minRowBytes64()) {
+            return false;
+        }
+        int shift = this->shiftPerPixel();
+        size_t alignedRowBytes = rowBytes >> shift << shift;
+        return alignedRowBytes == rowBytes;
     }
 
     /** Creates an empty SkImageInfo with kUnknown_SkColorType, kUnknown_SkAlphaType,

src/image/SkSurface_Raster.cpp

@@ -55,15 +55,7 @@ bool SkSurfaceValidateRasterInfo(const SkImageInfo& info, size_t rowBytes) {
         return true;
     }
 
-    int shift = info.shiftPerPixel();
-
-    uint64_t minRB = (uint64_t)info.width() << shift;
-    if (minRB > rowBytes) {
-        return false;
-    }
-
-    size_t alignedRowBytes = rowBytes >> shift << shift;
-    if (alignedRowBytes != rowBytes) {
+    if (!info.validRowBytes(rowBytes)) {
         return false;
     }
 

tests/BitmapTest.cpp

@@ -92,6 +92,42 @@ static void test_allocpixels(skiatest::Reporter* reporter) {
     bool success = bm.setInfo(info, info.minRowBytes() - 1);   // invalid for 32bit
     REPORTER_ASSERT(reporter, !success);
     REPORTER_ASSERT(reporter, bm.isNull());
+
+    for (SkColorType ct : {
+        kAlpha_8_SkColorType,
+        kRGB_565_SkColorType,
+        kARGB_4444_SkColorType,
+        kRGBA_8888_SkColorType,
+        kBGRA_8888_SkColorType,
+        kRGB_888x_SkColorType,
+        kRGBA_1010102_SkColorType,
+        kRGB_101010x_SkColorType,
+        kGray_8_SkColorType,
+        kRGBA_F16Norm_SkColorType,
+        kRGBA_F16_SkColorType,
+        kRGBA_F32_SkColorType,
+        kR8G8_unorm_SkColorType,
+        kA16_unorm_SkColorType,
+        kR16G16_unorm_SkColorType,
+        kA16_float_SkColorType,
+        kR16G16_float_SkColorType,
+        kR16G16B16A16_unorm_SkColorType,
+    }) {
+        SkImageInfo imageInfo = info.makeColorType(ct);
+        for (int rowBytesPadding = 1; rowBytesPadding <= 17; rowBytesPadding++) {
+            bm.reset();
+            success = bm.setInfo(imageInfo, imageInfo.minRowBytes() + rowBytesPadding);
+            if (rowBytesPadding % imageInfo.bytesPerPixel() == 0) {
+                REPORTER_ASSERT(reporter, success);
+                success = bm.tryAllocPixels();
+                REPORTER_ASSERT(reporter, success);
+            } else {
+                // Not pixel aligned.
+                REPORTER_ASSERT(reporter, !success);
+                REPORTER_ASSERT(reporter, bm.isNull());
+            }
+        }
+    }
 }
 
 static void test_bigwidth(skiatest::Reporter* reporter) {

tests/CanvasTest.cpp

@@ -182,6 +182,10 @@ DEF_TEST(CanvasNewRasterTest, reporter) {
         addr = (const SkPMColor*)((const char*)addr + pmap.rowBytes());
     }
 
+    // unaligned rowBytes
+    REPORTER_ASSERT(reporter, nullptr == SkCanvas::MakeRasterDirect(info, baseAddr,
+                                                                    minRowBytes + 1));
+
     // now try a deliberately bad info
     info = info.makeWH(-1, info.height());
     REPORTER_ASSERT(reporter, nullptr == SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes));

tests/MallocPixelRefTest.cpp

@@ -41,7 +41,7 @@ DEF_TEST(MallocPixelRef, reporter) {
         REPORTER_ASSERT(reporter, nullptr == pr.get());
     }
     {
-        size_t rowBytes = info.minRowBytes() + 2;
+        size_t rowBytes = info.minRowBytes() + info.bytesPerPixel();
         size_t size = info.computeByteSize(rowBytes) - 1;
         sk_sp<SkData> data(SkData::MakeUninitialized(size));
         sk_sp<SkPixelRef> pr(
@@ -49,7 +49,7 @@ DEF_TEST(MallocPixelRef, reporter) {
         // data too small.
         REPORTER_ASSERT(reporter, nullptr == pr.get());
     }
-    size_t rowBytes = info.minRowBytes() + 7;
+    size_t rowBytes = info.minRowBytes() + info.bytesPerPixel();
     size_t size = info.computeByteSize(rowBytes) + 9;
     {
         SkAutoMalloc memory(size);