diff --git a/gm/showmiplevels.cpp b/gm/showmiplevels.cpp index a4fd16404f..0394b57318 100644 --- a/gm/showmiplevels.cpp +++ b/gm/showmiplevels.cpp @@ -216,6 +216,46 @@ 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 */ @@ -283,7 +323,7 @@ protected: for (auto ctype : ctypes) { SkBitmap bm; - orig.copyTo(&bm, ctype); + copy_to(&bm, ctype, orig); drawLevels(canvas, bm); canvas->translate(orig.width()/2 + 8.0f, 0); } diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index e7c75b2cb6..00b2f8e503 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -681,6 +681,7 @@ 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; @@ -688,16 +689,6 @@ 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; } @@ -774,7 +765,13 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) if (!src->requestLock(&srcUnlocker)) { return false; } - const SkPixmap& srcPM = srcUnlocker.pixmap(); + 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; diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp index b882146643..98e5b7aced 100644 --- a/src/core/SkConfig8888.cpp +++ b/src/core/SkConfig8888.cpp @@ -12,6 +12,7 @@ #include "SkConfig8888.h" #include "SkColorPriv.h" #include "SkDither.h" +#include "SkImageInfoPriv.h" #include "SkMathPriv.h" #include "SkPM4fPriv.h" #include "SkRasterPipeline.h" @@ -173,9 +174,7 @@ static void memcpy32_row(uint32_t* dst, const uint32_t* src, int count) { } bool SkSrcPixelInfo::convertPixelsTo(SkDstPixelInfo* dst, int width, int height) const { - if (width <= 0 || height <= 0) { - return false; - } + SkASSERT(width > 0 && height > 0); if (!is_32bit_colortype(fColorType) || !is_32bit_colortype(dst->fColorType)) { return false; @@ -237,34 +236,6 @@ 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; @@ -393,38 +364,14 @@ 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) { - if (srcInfo.dimensions() != dstInfo.dimensions()) { - 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; - } - } + SkASSERT(dstInfo.dimensions() == srcInfo.dimensions()); + SkASSERT(SkImageInfoValidConversion(dstInfo, srcInfo)); const int width = srcInfo.width(); const int height = srcInfo.height(); // Do the easiest one first : both configs are equal - if ((srcInfo == dstInfo) && !ctable) { + if (srcInfo == dstInfo && kIndex_8_SkColorType != srcInfo.colorType()) { size_t bytes = width * srcInfo.bytesPerPixel(); for (int y = 0; y < height; ++y) { memcpy(dstPixels, srcPixels, bytes); @@ -491,10 +438,6 @@ 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)) { @@ -517,9 +460,7 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t const SkPMColor* table = nullptr; if (kIndex_8_SkColorType == srcInfo.colorType()) { - if (nullptr == ctable) { - return false; - } + SkASSERT(ctable); table = ctable->readColors(); } diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp index db796b308d..0453c7b38f 100644 --- a/src/core/SkImageInfo.cpp +++ b/src/core/SkImageInfo.cpp @@ -98,17 +98,13 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType, #include "SkReadPixelsRec.h" bool SkReadPixelsRec::trim(int srcWidth, int srcHeight) { - switch (fInfo.colorType()) { - case kUnknown_SkColorType: - case kIndex_8_SkColorType: - return false; - default: - break; + if (kIndex_8_SkColorType == fInfo.colorType()) { + return false; } if (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()) { return false; } - if (0 == fInfo.width() || 0 == fInfo.height()) { + if (0 >= fInfo.width() || 0 >= fInfo.height()) { return false; } diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h new file mode 100644 index 0000000000..72138aebe4 --- /dev/null +++ b/src/core/SkImageInfoPriv.h @@ -0,0 +1,77 @@ +/* + * 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; +} diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp index 3e89af65ed..1e24b93a60 100644 --- a/src/core/SkPixmap.cpp +++ b/src/core/SkPixmap.cpp @@ -10,6 +10,7 @@ #include "SkColorPriv.h" #include "SkConfig8888.h" #include "SkData.h" +#include "SkImageInfoPriv.h" #include "SkHalf.h" #include "SkMask.h" #include "SkNx.h" @@ -84,15 +85,13 @@ 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 (kUnknown_SkColorType == requestedDstInfo.colorType()) { + if (!SkImageInfoValidConversion(requestedDstInfo, fInfo)) { 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())) { diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 258713c537..1dec0f6371 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -26,6 +26,7 @@ #include "SkImageCacherator.h" #include "SkImageFilter.h" #include "SkImageFilterCache.h" +#include "SkImageInfoPriv.h" #include "SkImage_Base.h" #include "SkLatticeIter.h" #include "SkMaskFilter.h" @@ -194,6 +195,10 @@ 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); } @@ -201,6 +206,10 @@ 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); } diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index faccf8dd8b..b56b1bab15 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -24,6 +24,7 @@ #include "SkGrPriv.h" #include "SkImage_Gpu.h" #include "SkImageCacherator.h" +#include "SkImageInfoPriv.h" #include "SkMipMap.h" #include "SkPixelRef.h" @@ -124,6 +125,10 @@ 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) { diff --git a/tests/SpecialImageTest.cpp b/tests/SpecialImageTest.cpp index b0915f073f..33f394811a 100644 --- a/tests/SpecialImageTest.cpp +++ b/tests/SpecialImageTest.cpp @@ -88,7 +88,7 @@ static void test_image(const sk_sp& img, skiatest::Reporter* rep // Test that draw restricts itself to the subset SkImageFilter::OutputProperties outProps(img->getColorSpace()); sk_sp surf(img->makeSurface(outProps, SkISize::Make(kFullSize, kFullSize), - kOpaque_SkAlphaType)); + kPremul_SkAlphaType)); SkCanvas* canvas = surf->getCanvas(); @@ -96,7 +96,7 @@ static void test_image(const sk_sp& img, skiatest::Reporter* rep img->draw(canvas, SkIntToScalar(kPad), SkIntToScalar(kPad), nullptr); SkBitmap bm; - bm.allocN32Pixels(kFullSize, kFullSize, true); + bm.allocN32Pixels(kFullSize, kFullSize, false); bool result = canvas->readPixels(bm.info(), bm.getPixels(), bm.rowBytes(), 0, 0); SkASSERT_RELEASE(result);