Revert "Add SkImageInfoValidConversion() and SkImageInfoIsValid"

This reverts commit cf5d6caff7.

Reason for revert: Chrome DEPS roll failing, based on the unit tests, I suspect this is the cause.

Original change's description:
> Add SkImageInfoValidConversion() and SkImageInfoIsValid
> 
> The idea is share these standards for the following:
> SkImage::readPixels()
> SkCanvas::readPixels()
> SkCanvas::writePixels()
> SkBitmap::readPixels()
> SkPixmap::readPixels()
> 
> On the raster side, SkPixmap::readPixels() is the right
> place to check, because all raster calls go through
> there eventually.  Then at lower levels (ex: SkPixelInfo),
> we can assert.
> 
> There's not really a unifying location for gpu calls,
> so I've added this in multiple places.  I haven't really
> dug into the gpu code to SkASSERT() on invalid cases
> that we will have already caught.
> 
> Follow-up work:
> Similar refactor for SkReadPixelRec::trim().
> Code cleanup in SkPixelInfo::CopyPixels()
> 
> BUG=skia:6021
> 
> Change-Id: I91ecce10e46c1a6530f0af24a9eb8226dbecaaa2
> Reviewed-on: https://skia-review.googlesource.com/6887
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
> 

TBR=mtklein@google.com,msarett@google.com,brianosman@google.com,reed@google.com,reviews@skia.org
# Not skipping CQ checks because original CL landed > 1 day ago.
BUG=skia:6021

Change-Id: I63b88e90bdbb3051a14de00ac73a8351ab776d25
Reviewed-on: https://skia-review.googlesource.com/7095
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

gm/showmiplevels.cpp

@@ -216,46 +216,6 @@ DEF_GM( return new ShowMipLevels(256); )
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-static void copy_32_to_g8(void* dst, size_t dstRB, const void* src, const SkImageInfo& srcInfo,
-                          size_t srcRB) {
-    uint8_t* dst8 = (uint8_t*)dst;
-    const uint32_t* src32 = (const uint32_t*)src;
-
-    const int w = srcInfo.width();
-    const int h = srcInfo.height();
-    const bool isBGRA = (kBGRA_8888_SkColorType == srcInfo.colorType());
-
-    for (int y = 0; y < h; ++y) {
-        if (isBGRA) {
-            // BGRA
-            for (int x = 0; x < w; ++x) {
-                uint32_t s = src32[x];
-                dst8[x] = SkComputeLuminance((s >> 16) & 0xFF, (s >> 8) & 0xFF, s & 0xFF);
-            }
-        } else {
-            // RGBA
-            for (int x = 0; x < w; ++x) {
-                uint32_t s = src32[x];
-                dst8[x] = SkComputeLuminance(s & 0xFF, (s >> 8) & 0xFF, (s >> 16) & 0xFF);
-            }
-        }
-        src32 = (const uint32_t*)((const char*)src32 + srcRB);
-        dst8 += dstRB;
-    }
-}
-
-void copy_to(SkBitmap* dst, SkColorType dstColorType, const SkBitmap& src) {
-    if (kGray_8_SkColorType == dstColorType) {
-        SkImageInfo grayInfo = src.info().makeColorType(kGray_8_SkColorType);
-        dst->allocPixels(grayInfo);
-        copy_32_to_g8(dst->getPixels(), dst->rowBytes(), src.getPixels(), src.info(),
-                      src.rowBytes());
-        return;
-    }
-
-    src.copyTo(dst, dstColorType);
-}
-
 /**
  *  Show mip levels that were built, for all supported colortypes
  */
@@ -323,7 +283,7 @@ protected:
 
         for (auto ctype : ctypes) {
             SkBitmap bm;
-            copy_to(&bm, ctype, orig);
+            orig.copyTo(&bm, ctype);
             drawLevels(canvas, bm);
             canvas->translate(orig.width()/2 + 8.0f, 0);
         }

src/core/SkBitmap.cpp

@@ -681,7 +681,6 @@ bool SkBitmap::canCopyTo(SkColorType dstCT) const {
         case kRGBA_8888_SkColorType:
         case kBGRA_8888_SkColorType:
             break;
-        case kGray_8_SkColorType:
         case kIndex_8_SkColorType:
             if (!sameConfigs) {
                 return false;
@@ -689,6 +688,16 @@ bool SkBitmap::canCopyTo(SkColorType dstCT) const {
             break;
         case kARGB_4444_SkColorType:
             return sameConfigs || kN32_SkColorType == srcCT || kIndex_8_SkColorType == srcCT;
+        case kGray_8_SkColorType:
+            switch (srcCT) {
+                case kGray_8_SkColorType:
+                case kRGBA_8888_SkColorType:
+                case kBGRA_8888_SkColorType:
+                    return true;
+                default:
+                    break;
+            }
+            return false;
         default:
             return false;
     }
@@ -765,13 +774,7 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc)
     if (!src->requestLock(&srcUnlocker)) {
         return false;
     }
-    SkPixmap srcPM = srcUnlocker.pixmap();
+    const SkPixmap& srcPM = srcUnlocker.pixmap();
-    if (kRGB_565_SkColorType == dstColorType && kOpaque_SkAlphaType != srcPM.alphaType()) {
-        // copyTo() is not strict on alpha type.  Here we set the src to opaque to allow
-        // the call to readPixels() to succeed and preserve this lenient behavior.
-        srcPM = SkPixmap(srcPM.info().makeAlphaType(kOpaque_SkAlphaType), srcPM.addr(),
-                         srcPM.rowBytes(), srcPM.ctable());
-    }
 
     const SkImageInfo dstInfo = srcPM.info().makeColorType(dstColorType);
     SkBitmap tmpDst;

src/core/SkConfig8888.cpp

@@ -12,7 +12,6 @@
 #include "SkConfig8888.h"
 #include "SkColorPriv.h"
 #include "SkDither.h"
-#include "SkImageInfoPriv.h"
 #include "SkMathPriv.h"
 #include "SkPM4fPriv.h"
 #include "SkRasterPipeline.h"
@@ -174,7 +173,9 @@ static void memcpy32_row(uint32_t* dst, const uint32_t* src, int count) {
 }
 
 bool SkSrcPixelInfo::convertPixelsTo(SkDstPixelInfo* dst, int width, int height) const {
-    SkASSERT(width > 0 && height > 0);
+    if (width <= 0 || height <= 0) {
+        return false;
+    }
 
     if (!is_32bit_colortype(fColorType) || !is_32bit_colortype(dst->fColorType)) {
         return false;
@@ -236,6 +237,34 @@ static void copy_g8_to_32(void* dst, size_t dstRB, const void* src, size_t srcRB
     }
 }
 
+static void copy_32_to_g8(void* dst, size_t dstRB, const void* src, size_t srcRB,
+                          const SkImageInfo& srcInfo) {
+    uint8_t* dst8 = (uint8_t*)dst;
+    const uint32_t* src32 = (const uint32_t*)src;
+
+    const int w = srcInfo.width();
+    const int h = srcInfo.height();
+    const bool isBGRA = (kBGRA_8888_SkColorType == srcInfo.colorType());
+
+    for (int y = 0; y < h; ++y) {
+        if (isBGRA) {
+            // BGRA
+            for (int x = 0; x < w; ++x) {
+                uint32_t s = src32[x];
+                dst8[x] = SkComputeLuminance((s >> 16) & 0xFF, (s >> 8) & 0xFF, s & 0xFF);
+            }
+        } else {
+            // RGBA
+            for (int x = 0; x < w; ++x) {
+                uint32_t s = src32[x];
+                dst8[x] = SkComputeLuminance(s & 0xFF, (s >> 8) & 0xFF, (s >> 16) & 0xFF);
+            }
+        }
+        src32 = (const uint32_t*)((const char*)src32 + srcRB);
+        dst8 += dstRB;
+    }
+}
+
 static bool extract_alpha(void* dst, size_t dstRB, const void* src, size_t srcRB,
                           const SkImageInfo& srcInfo, SkColorTable* ctable) {
     uint8_t* SK_RESTRICT dst8 = (uint8_t*)dst;
@@ -364,14 +393,38 @@ static inline void apply_color_xform(const SkImageInfo& dstInfo, void* dstPixels
 bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
                              const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRB,
                              SkColorTable* ctable) {
-    SkASSERT(dstInfo.dimensions() == srcInfo.dimensions());
+    if (srcInfo.dimensions() != dstInfo.dimensions()) {
-    SkASSERT(SkImageInfoValidConversion(dstInfo, srcInfo));
+        return false;
+    }
+
+    if (srcInfo.colorType() == kAlpha_8_SkColorType &&
+        dstInfo.colorType() != kAlpha_8_SkColorType)
+    {
+        return false;   // can't convert from alpha to non-alpha
+    }
+
+    if (dstInfo.colorSpace() &&
+        SkColorSpace_Base::Type::kXYZ != as_CSB(dstInfo.colorSpace())->type())
+    {
+        return false;   // unsupported dst space
+    }
+
+    const bool srcIsF16 = kRGBA_F16_SkColorType == srcInfo.colorType();
+    const bool dstIsF16 = kRGBA_F16_SkColorType == dstInfo.colorType();
+    if (srcIsF16 || dstIsF16) {
+        if (!srcInfo.colorSpace() || !dstInfo.colorSpace() ||
+           (srcIsF16 && !srcInfo.colorSpace()->gammaIsLinear()) ||
+           (dstIsF16 && !dstInfo.colorSpace()->gammaIsLinear()))
+        {
+            return false;
+        }
+    }
 
     const int width = srcInfo.width();
     const int height = srcInfo.height();
 
     // Do the easiest one first : both configs are equal
-    if (srcInfo == dstInfo && kIndex_8_SkColorType != srcInfo.colorType()) {
+    if ((srcInfo == dstInfo) && !ctable) {
         size_t bytes = width * srcInfo.bytesPerPixel();
         for (int y = 0; y < height; ++y) {
             memcpy(dstPixels, srcPixels, bytes);
@@ -438,6 +491,10 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
         copy_g8_to_32(dstPixels, dstRB, srcPixels, srcRB, width, height);
         return true;
     }
+    if (kGray_8_SkColorType == dstInfo.colorType() && 4 == srcInfo.bytesPerPixel()) {
+        copy_32_to_g8(dstPixels, dstRB, srcPixels, srcRB, srcInfo);
+        return true;
+    }
 
     if (kAlpha_8_SkColorType == dstInfo.colorType() &&
         extract_alpha(dstPixels, dstRB, srcPixels, srcRB, srcInfo, ctable)) {
@@ -460,7 +517,9 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
 
         const SkPMColor* table = nullptr;
         if (kIndex_8_SkColorType == srcInfo.colorType()) {
-            SkASSERT(ctable);
+            if (nullptr == ctable) {
+                return false;
+            }
             table = ctable->readColors();
         }
 

src/core/SkImageInfo.cpp

@@ -98,13 +98,17 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType,
 #include "SkReadPixelsRec.h"
 
 bool SkReadPixelsRec::trim(int srcWidth, int srcHeight) {
-    if (kIndex_8_SkColorType == fInfo.colorType()) {
+    switch (fInfo.colorType()) {
-        return false;
+        case kUnknown_SkColorType:
+        case kIndex_8_SkColorType:
+            return false;
+        default:
+            break;
     }
     if (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()) {
         return false;
     }
-    if (0 >= fInfo.width() || 0 >= fInfo.height()) {
+    if (0 == fInfo.width() || 0 == fInfo.height()) {
         return false;
     }
 

src/core/SkImageInfoPriv.h

@@ -1,77 +0,0 @@
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-/**
- *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
- *  colorSpace.  Returns false otherwise.
- */
-static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
-    if (info.width() <= 0 || info.height() <= 0) {
-        return false;
-    }
-
-    if (kUnknown_SkColorType == info.colorType() || kUnknown_SkAlphaType == info.alphaType()) {
-        return false;
-    }
-
-    if (kOpaque_SkAlphaType != info.alphaType() &&
-       (kRGB_565_SkColorType == info.colorType() || kGray_8_SkColorType == info.colorType())) {
-        return false;
-    }
-
-    if (kRGBA_F16_SkColorType == info.colorType() &&
-       (!info.colorSpace() || !info.colorSpace()->gammaIsLinear())) {
-        return false;
-    }
-
-    if (info.colorSpace() &&
-       (!info.colorSpace()->gammaCloseToSRGB() && !info.colorSpace()->gammaIsLinear())) {
-        return false;
-    }
-
-    return true;
-}
-
-/**
- *  Returns true if Skia has defined a pixel conversion from the |src| to the |dst|.
- *  Returns false otherwise.  Some discussion of false cases:
- *      We will not convert to kIndex8 unless |src| is kIndex8.  This is possible only
- *      in some cases and likley inefficient.
- *      We do not convert to kGray8 when the |src| is not kGray8.  We may add this
- *      feature - it just requires some work to convert to luminance while handling color
- *      spaces correctly.  Currently no one is asking for this.
- *      We will not convert from kAlpha8 when the |dst| is not kAlpha8.  This would require
- *      inventing color information.
- *      We will not convert to kOpaque when the |src| is not kOpaque.  This could be
- *      implemented to set all the alpha values to 1, but there is still some ambiguity -
- *      should we use kPremul or kUnpremul color values with the opaque alphas?  Or should
- *      we just use whatever the |src| alpha is?  In the future, we could choose to clearly
- *      define this, but currently no one is asking for this feature.
- */
-static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkImageInfo& src) {
-    if (!SkImageInfoIsValid(dst) || !SkImageInfoIsValid(src)) {
-        return false;
-    }
-
-    if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType()) {
-        return false;
-    }
-
-    if (kGray_8_SkColorType == dst.colorType() && kGray_8_SkColorType != src.colorType()) {
-        return false;
-    }
-
-    if (kAlpha_8_SkColorType != dst.colorType() && kAlpha_8_SkColorType == src.colorType()) {
-        return false;
-    }
-
-    if (kOpaque_SkAlphaType == dst.alphaType() && kOpaque_SkAlphaType != src.alphaType()) {
-        return false;
-    }
-
-    return true;
-}

src/core/SkPixmap.cpp

@@ -10,7 +10,6 @@
 #include "SkColorPriv.h"
 #include "SkConfig8888.h"
 #include "SkData.h"
-#include "SkImageInfoPriv.h"
 #include "SkHalf.h"
 #include "SkMask.h"
 #include "SkNx.h"
@@ -85,13 +84,15 @@ bool SkPixmap::extractSubset(SkPixmap* result, const SkIRect& subset) const {
 
 bool SkPixmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
                           int x, int y) const {
-    if (!SkImageInfoValidConversion(requestedDstInfo, fInfo)) {
+    if (kUnknown_SkColorType == requestedDstInfo.colorType()) {
         return false;
     }
-
     if (nullptr == dstPixels || dstRB < requestedDstInfo.minRowBytes()) {
         return false;
     }
+    if (0 == requestedDstInfo.width() || 0 == requestedDstInfo.height()) {
+        return false;
+    }
 
     SkIRect srcR = SkIRect::MakeXYWH(x, y, requestedDstInfo.width(), requestedDstInfo.height());
     if (!srcR.intersect(0, 0, this->width(), this->height())) {

src/gpu/SkGpuDevice.cpp

@@ -26,7 +26,6 @@
 #include "SkImageCacherator.h"
 #include "SkImageFilter.h"
 #include "SkImageFilterCache.h"
-#include "SkImageInfoPriv.h"
 #include "SkImage_Base.h"
 #include "SkLatticeIter.h"
 #include "SkMaskFilter.h"
@@ -195,10 +194,6 @@ bool SkGpuDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size
                                int x, int y) {
     ASSERT_SINGLE_OWNER
 
-    if (!SkImageInfoValidConversion(dstInfo, this->imageInfo())) {
-        return false;
-    }
-
     return fRenderTargetContext->readPixels(dstInfo, dstPixels, dstRowBytes, x, y);
 }
 
@@ -206,10 +201,6 @@ bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixel
                                 size_t srcRowBytes, int x, int y) {
     ASSERT_SINGLE_OWNER
 
-    if (!SkImageInfoValidConversion(this->imageInfo(), srcInfo)) {
-        return false;
-    }
-
     return fRenderTargetContext->writePixels(srcInfo, srcPixels, srcRowBytes, x, y);
 }
 

src/image/SkImage_Gpu.cpp

@@ -24,7 +24,6 @@
 #include "SkGrPriv.h"
 #include "SkImage_Gpu.h"
 #include "SkImageCacherator.h"
-#include "SkImageInfoPriv.h"
 #include "SkMipMap.h"
 #include "SkPixelRef.h"
 
@@ -125,10 +124,6 @@ static void apply_premul(const SkImageInfo& info, void* pixels, size_t rowBytes)
 
 bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
                                int srcX, int srcY, CachingHint) const {
-    if (!SkImageInfoValidConversion(info, this->onImageInfo())) {
-        return false;
-    }
-
     GrPixelConfig config = SkImageInfo2GrPixelConfig(info, *fTexture->getContext()->caps());
     uint32_t flags = 0;
     if (kUnpremul_SkAlphaType == info.alphaType() && kPremul_SkAlphaType == fAlphaType) {

tests/SpecialImageTest.cpp

@@ -88,7 +88,7 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
     // Test that draw restricts itself to the subset
     SkImageFilter::OutputProperties outProps(img->getColorSpace());
     sk_sp<SkSpecialSurface> surf(img->makeSurface(outProps, SkISize::Make(kFullSize, kFullSize),
-                                                  kPremul_SkAlphaType));
+                                                  kOpaque_SkAlphaType));
 
     SkCanvas* canvas = surf->getCanvas();
 
@@ -96,7 +96,7 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
     img->draw(canvas, SkIntToScalar(kPad), SkIntToScalar(kPad), nullptr);
 
     SkBitmap bm;
-    bm.allocN32Pixels(kFullSize, kFullSize, false);
+    bm.allocN32Pixels(kFullSize, kFullSize, true);
 
     bool result = canvas->readPixels(bm.info(), bm.getPixels(), bm.rowBytes(), 0, 0);
     SkASSERT_RELEASE(result);