commit 431ac5617759688ee30f3e0d080b417bab1dbeee
author Brian Osman <brianosman@google.com> Wed Oct 10 13:20:38 2018 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Oct 10 18:00:00 2018 +0000
tree abde29f25c87baa576a101b83ab58583c23f95de
parent f00b16233d5641df848602b4b1356a4788cf201b

Use SkSurfaceValidateRasterInfo in some lesser-used canvas factories

The previous version would reject valid configs (like non-N32 8888).
This changes the results of some unit tests:

- Creating a zero-sized canvas via MakeRasterDirect now fails (like
  it would if you tried to create a zero-sized surface).
- The picture image generator can now produce BGRA and RGBA results,
  regardless of N32, and can produce unpremul.

Bug: skia:
Change-Id: Ifd9fe98adf56748f73eaea01118c928b0a13606b
Reviewed-on: https://skia-review.googlesource.com/c/161120
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

src/core/SkCanvas.cpp

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 380195f..701a8a1 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -2802,34 +2802,9 @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
-// TODO: This still disagrees with SkSurfaceValidateRasterInfo
-static bool supported_for_raster_canvas(const SkImageInfo& info) {
-    switch (info.alphaType()) {
-        case kPremul_SkAlphaType:
-        case kOpaque_SkAlphaType:
-            break;
-        default:
-            return false;
-    }
-
-    switch (info.colorType()) {
-        case kAlpha_8_SkColorType:
-        case kRGB_565_SkColorType:
-        case kN32_SkColorType:
-        case kRGBA_F16_SkColorType:
-        case kRGBA_F32_SkColorType:
-        case kRGBA_1010102_SkColorType:
-            break;
-        default:
-            return false;
-    }
-
-    return true;
-}
-
 std::unique_ptr<SkCanvas> SkCanvas::MakeRasterDirect(const SkImageInfo& info, void* pixels,
                                                      size_t rowBytes, const SkSurfaceProps* props) {
-    if (!supported_for_raster_canvas(info)) {
+    if (!SkSurfaceValidateRasterInfo(info, rowBytes)) {
         return nullptr;
     }
 
@@ -2907,7 +2882,7 @@
 std::unique_ptr<SkCanvas>
 SkRasterHandleAllocator::MakeCanvas(std::unique_ptr<SkRasterHandleAllocator> alloc,
                                     const SkImageInfo& info, const Rec* rec) {
-    if (!alloc || !supported_for_raster_canvas(info)) {
+    if (!alloc || !SkSurfaceValidateRasterInfo(info, rec ? rec->fRowBytes : kIgnoreRowBytesValue)) {
         return nullptr;
     }
 

tests/CanvasTest.cpp

diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp
index 2a4c26c..08b6df6 100644
--- a/tests/CanvasTest.cpp
+++ b/tests/CanvasTest.cpp
@@ -625,10 +625,10 @@
     info = SkImageInfo::Make(10, 10, kUnknown_SkColorType, info.alphaType());
     REPORTER_ASSERT(reporter, nullptr == SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes));
 
-    // We should succeed with a zero-sized valid info
+    // We should not succeed with a zero-sized valid info
     info = SkImageInfo::MakeN32Premul(0, 0);
     canvas = SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes);
-    REPORTER_ASSERT(reporter, canvas);
+    REPORTER_ASSERT(reporter, nullptr == canvas);
 }
 
 DEF_TEST(Canvas, reporter) {

tests/ImageGeneratorTest.cpp

diff --git a/tests/ImageGeneratorTest.cpp b/tests/ImageGeneratorTest.cpp
index 735e13e..5c8864e 100644
--- a/tests/ImageGeneratorTest.cpp
+++ b/tests/ImageGeneratorTest.cpp
@@ -99,19 +99,18 @@
     const struct {
         SkColorType fColorType;
         SkAlphaType fAlphaType;
-        bool        fExpectSuccess;
     } recs[] = {
-        { kRGBA_8888_SkColorType, kPremul_SkAlphaType, kRGBA_8888_SkColorType == kN32_SkColorType },
-        { kBGRA_8888_SkColorType, kPremul_SkAlphaType, kBGRA_8888_SkColorType == kN32_SkColorType },
-        { kRGBA_F16_SkColorType,  kPremul_SkAlphaType, true },
-        { kRGBA_F32_SkColorType,  kPremul_SkAlphaType, true },
-        { kRGBA_1010102_SkColorType, kPremul_SkAlphaType, true },
+        { kRGBA_8888_SkColorType, kPremul_SkAlphaType },
+        { kBGRA_8888_SkColorType, kPremul_SkAlphaType },
+        { kRGBA_F16_SkColorType,  kPremul_SkAlphaType },
+        { kRGBA_F32_SkColorType,  kPremul_SkAlphaType },
+        { kRGBA_1010102_SkColorType, kPremul_SkAlphaType },
 
-        { kRGBA_8888_SkColorType, kUnpremul_SkAlphaType, false },
-        { kBGRA_8888_SkColorType, kUnpremul_SkAlphaType, false },
-        { kRGBA_F16_SkColorType,  kUnpremul_SkAlphaType, false },
-        { kRGBA_F32_SkColorType,  kUnpremul_SkAlphaType, false },
-        { kRGBA_1010102_SkColorType, kUnpremul_SkAlphaType, false },
+        { kRGBA_8888_SkColorType, kUnpremul_SkAlphaType },
+        { kBGRA_8888_SkColorType, kUnpremul_SkAlphaType },
+        { kRGBA_F16_SkColorType,  kUnpremul_SkAlphaType },
+        { kRGBA_F32_SkColorType,  kUnpremul_SkAlphaType },
+        { kRGBA_1010102_SkColorType, kUnpremul_SkAlphaType },
     };
 
     auto colorspace = SkColorSpace::MakeSRGB();
@@ -124,8 +123,7 @@
 
     for (const auto& rec : recs) {
         SkImageInfo info = SkImageInfo::Make(100, 100, rec.fColorType, rec.fAlphaType, colorspace);
-        bool success = gen->getPixels(info, storage.get(), info.minRowBytes());
-        REPORTER_ASSERT(reporter, success == rec.fExpectSuccess);
+        REPORTER_ASSERT(reporter, gen->getPixels(info, storage.get(), info.minRowBytes()));
     }
 }