Revert of Fix colorType/alphaType checks in SkCodec (patchset #5 id:80001 of https...

Reason for revert:
Really bad images in Gold.

Original issue's description:
> Fix colorType/alphaType checks in SkCodec
>
> Make getPixels() and startScanlineDecode() behave
> consistently.
>
> Require that kGray8 decodes are opaque.
>
> Assert that creating the swizzler succeeds.
>
> BUG=skia:4203
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1695473002
>
> Committed: https://skia.googlesource.com/skia/+/c7578b6cdd03b61f076ffc7956efd952d6c301c0

TBR=scroggo@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:4203

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

diff --git a/src/codec/SkBmpMaskCodec.cpp b/src/codec/SkBmpMaskCodec.cpp
index 21d231b15f016987964f0e9fbc0bd084d8059ce5..b603de9a03f81fd47b13f26dc234cb4b636c78fa 100644 (file)
--- a/src/codec/SkBmpMaskCodec.cpp
+++ b/src/codec/SkBmpMaskCodec.cpp
@@ -57,12 +57,25 @@ SkCodec::Result SkBmpMaskCodec::onGetPixels(const SkImageInfo& dstInfo,
     return kSuccess;
 }
 
-SkCodec::Result SkBmpMaskCodec::prepareToDecode(const SkImageInfo& dstInfo,
-        const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
-    // Initialize the mask swizzler
+bool SkBmpMaskCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& options) {
+    // Create the swizzler
     fMaskSwizzler.reset(SkMaskSwizzler::CreateMaskSwizzler(dstInfo, this->getInfo(), fMasks,
             this->bitsPerPixel(), options));
-    SkASSERT(fMaskSwizzler);
+
+    if (nullptr == fMaskSwizzler.get()) {
+        return false;
+    }
+
+    return true;
+}
+
+SkCodec::Result SkBmpMaskCodec::prepareToDecode(const SkImageInfo& dstInfo,
+        const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
+    // Initialize a the mask swizzler
+    if (!this->initializeSwizzler(dstInfo, options)) {
+        SkCodecPrintf("Error: cannot initialize swizzler.\n");
+        return SkCodec::kInvalidConversion;
+    }
 
     return SkCodec::kSuccess;
 }

diff --git a/src/codec/SkBmpMaskCodec.h b/src/codec/SkBmpMaskCodec.h
index 93b3bca5a0626f0476cfafa42ac693ca15480a58..73a8b3799d9c5876daa178b14ccd62b4be33fb81 100644 (file)
--- a/src/codec/SkBmpMaskCodec.h
+++ b/src/codec/SkBmpMaskCodec.h
@@ -44,6 +44,7 @@ protected:
 
 private:
 
+    bool initializeSwizzler(const SkImageInfo& dstInfo, const Options& options);
     SkSampler* getSampler(bool createIfNecessary) override {
         SkASSERT(fMaskSwizzler);
         return fMaskSwizzler;

diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 39d357be8b0089c87a7f36b9f2027c1c9770a0c0..47b10701d332156a63ece8f08f852f3d09dc443f 100644 (file)
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -152,9 +152,9 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
     return true;
 }
 
-void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
+bool SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
     // Get swizzler configuration
-    SkSwizzler::SrcConfig config = SkSwizzler::kUnknown;
+    SkSwizzler::SrcConfig config;
     switch (this->bitsPerPixel()) {
         case 1:
             config = SkSwizzler::kIndex1;
@@ -180,6 +180,7 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op
             break;
         default:
             SkASSERT(false);
+            return false;
     }
 
     // Get a pointer to the color table if it exists
@@ -187,7 +188,11 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op
 
     // Create swizzler
     fSwizzler.reset(SkSwizzler::CreateSwizzler(config, colorPtr, dstInfo, opts));
-    SkASSERT(fSwizzler);
+
+    if (nullptr == fSwizzler.get()) {
+        return false;
+    }
+    return true;
 }
 
 SkCodec::Result SkBmpStandardCodec::prepareToDecode(const SkImageInfo& dstInfo,
@@ -202,8 +207,11 @@ SkCodec::Result SkBmpStandardCodec::prepareToDecode(const SkImageInfo& dstInfo,
     // Copy the color table to the client if necessary
     copy_color_table(dstInfo, this->fColorTable, inputColorPtr, inputColorCount);
 
-    // Initialize a swizzler
-    this->initializeSwizzler(dstInfo, options);
+    // Initialize a swizzler if necessary
+    if (!this->initializeSwizzler(dstInfo, options)) {
+        SkCodecPrintf("Error: cannot initialize swizzler.\n");
+        return SkCodec::kInvalidConversion;
+    }
     return SkCodec::kSuccess;
 }
 

diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h
index 557f6535c95fb882f2ccf02d7a14e06524931390..d3e2ed3e4f0b5c7594d950069e77fc240d417901 100644 (file)
--- a/src/codec/SkBmpStandardCodec.h
+++ b/src/codec/SkBmpStandardCodec.h
@@ -72,7 +72,7 @@ private:
      */
     bool createColorTable(SkAlphaType alphaType, int* colorCount);
 
-    void initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts);
+    bool initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts);
 
     int decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
             const Options& opts) override;

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index bb7b26529b9af8fe7c106818e5514c47617cdf76..0da36bb17182ad298060ee110911d6d288dd925d 100644 (file)
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -162,6 +162,15 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
         ctable = nullptr;
     }
 
+    {
+        SkAlphaType canonical;
+        if (!SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &canonical)
+            || canonical != info.alphaType())
+        {
+            return kInvalidConversion;
+        }
+    }
+
     if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }

diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index 5d66118943b19717c288adbc29bed1b7ed4d4e7e..235c4e24aff3bcbc56787487824170c33f6fd4fa 100644 (file)
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -136,12 +136,7 @@ inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src)
         case kN32_SkColorType:
             return true;
         case kRGB_565_SkColorType:
-            return kOpaque_SkAlphaType == dst.alphaType();
-        case kGray_8_SkColorType:
-            if (kOpaque_SkAlphaType != dst.alphaType()) {
-                return false;
-            }
-            // Fall through
+            return src.alphaType() == kOpaque_SkAlphaType;
         default:
             return dst.colorType() == src.colorType();
     }

diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 785c17e0f12ba7f4287d1061fc8c133db900b27b..a938f5fc48fac46ece1f929597ae5844675d3917 100644 (file)
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -436,16 +436,19 @@ SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColo
     // Initialize color table and copy to the client if necessary
     this->initializeColorTable(dstInfo, inputColorPtr, inputColorCount);
 
-    this->initializeSwizzler(dstInfo, opts);
-    return kSuccess;
+    return this->initializeSwizzler(dstInfo, opts);
 }
 
-void SkGifCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
+SkCodec::Result SkGifCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
     const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
     const SkIRect* frameRect = fFrameIsSubset ? &fFrameRect : nullptr;
     fSwizzler.reset(SkSwizzler::CreateSwizzler(SkSwizzler::kIndex, colorPtr, dstInfo, opts,
             frameRect));
-    SkASSERT(fSwizzler);
+
+    if (nullptr != fSwizzler.get()) {
+        return kSuccess;
+    }
+    return kUnimplemented;
 }
 
 bool SkGifCodec::readRow() {

diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 010a344cf5c305f109dce64ded724d0169ea44e0..91861b299abb8b884097eca33d476591378d72c6 100644 (file)
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -128,7 +128,7 @@ private:
      * @param options  Informs the swizzler if destination memory is zero initialized.
      *                 Contains subset information.
      */
-    void initializeSwizzler(const SkImageInfo& dstInfo,
+    Result initializeSwizzler(const SkImageInfo& dstInfo,
             const Options& options);
 
     SkSampler* getSampler(bool createIfNecessary) override {

diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index e1440af5c3b6c79651361e1e20e61660258c7f1c..f920c98f694da703e10ad011d5e4dc5bf12d7ffa 100644 (file)
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -372,7 +372,6 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options&
     }
 
     fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, nullptr, dstInfo, options));
-    SkASSERT(fSwizzler);
     fStorage.reset(get_row_bytes(fDecoderMgr->dinfo()));
     fSrcRow = fStorage.get();
 }

diff --git a/src/codec/SkWbmpCodec.cpp b/src/codec/SkWbmpCodec.cpp
index 706d5ddd460e4dc7b6764512e0de776f4da0fe41..af1ab46f730c9375cf111b2914081ddcf88d6bfe 100644 (file)
--- a/src/codec/SkWbmpCodec.cpp
+++ b/src/codec/SkWbmpCodec.cpp
@@ -30,19 +30,6 @@ static inline void setup_color_table(SkColorType colorType,
     }
 }
 
-static inline bool valid_color_type(SkColorType colorType, SkAlphaType alphaType) {
-    switch (colorType) {
-        case kN32_SkColorType:
-        case kIndex_8_SkColorType:
-            return true;
-        case kGray_8_SkColorType:
-        case kRGB_565_SkColorType:
-            return kOpaque_SkAlphaType == alphaType;
-        default:
-            return false;
-    }
-}
-
 static bool read_byte(SkStream* stream, uint8_t* data)
 {
     return stream->read(data, 1) == 1;
@@ -97,6 +84,16 @@ bool SkWbmpCodec::onRewind() {
 
 SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info, const SkPMColor* ctable,
         const Options& opts) {
+    // Create the swizzler based on the desired color type
+    switch (info.colorType()) {
+        case kIndex_8_SkColorType:
+        case kN32_SkColorType:
+        case kRGB_565_SkColorType:
+        case kGray_8_SkColorType:
+            break;
+        default:
+            return nullptr;
+    }
     return SkSwizzler::CreateSwizzler(SkSwizzler::kBit, ctable, info, opts);
 }
 
@@ -127,8 +124,7 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
         return kUnimplemented;
     }
 
-    if (!valid_color_type(info.colorType(), info.alphaType()) ||
-            !valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
+    if (!valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
         return kInvalidConversion;
     }
 
@@ -137,7 +133,9 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
 
     // Initialize the swizzler
     SkAutoTDelete<SkSwizzler> swizzler(this->initializeSwizzler(info, ctable, options));
-    SkASSERT(swizzler);
+    if (nullptr == swizzler.get()) {
+        return kInvalidConversion;
+    }
 
     // Perform the decode
     SkISize size = info.dimensions();
@@ -195,8 +193,7 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         return kUnimplemented;
     }
 
-    if (!valid_color_type(dstInfo.colorType(), dstInfo.alphaType()) ||
-            !valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
+    if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
         return kInvalidConversion;
     }
 
@@ -210,7 +207,9 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
 
     // Initialize the swizzler
     fSwizzler.reset(this->initializeSwizzler(dstInfo, get_color_ptr(fColorTable.get()), options));
-    SkASSERT(fSwizzler);
+    if (nullptr == fSwizzler.get()) {
+        return kInvalidConversion;
+    }
 
     fSrcBuffer.reset(fSrcRowBytes);