diff options
26 files changed, 471 insertions, 222 deletions
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index 09c88af320..cd4f111627 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -1881,7 +1881,8 @@ Error GPUSink::onDraw(const Src& src, SkBitmap* dst, SkWStream*, SkString* log, canvas->getGrContext()->contextPriv().dumpGpuStats(log); #endif } - if (info.colorType() == kRGB_565_SkColorType || info.colorType() == kARGB_4444_SkColorType) { + if (info.colorType() == kRGB_565_SkColorType || info.colorType() == kARGB_4444_SkColorType || + info.colorType() == kRGB_888x_SkColorType) { // We don't currently support readbacks into these formats on the GPU backend. Convert to // 32 bit. info = SkImageInfo::Make(size.width(), size.height(), kRGBA_8888_SkColorType, diff --git a/include/gpu/GrTypes.h b/include/gpu/GrTypes.h index 3fec742907..979014cca4 100644 --- a/include/gpu/GrTypes.h +++ b/include/gpu/GrTypes.h @@ -286,58 +286,29 @@ static inline int GrMaskFormatBytesPerPixel(GrMaskFormat format) { } /** - * Pixel configurations. + * Pixel configurations. This type conflates texture formats, CPU pixel formats, and + * premultipliedness. We are moving away from it towards SkColorType and backend API (GL, Vulkan) + * texture formats in the pulbic API. Right now this mostly refers to texture formats as we're + * migrating. */ enum GrPixelConfig { kUnknown_GrPixelConfig, kAlpha_8_GrPixelConfig, kGray_8_GrPixelConfig, kRGB_565_GrPixelConfig, - /** - * Premultiplied - */ kRGBA_4444_GrPixelConfig, - /** - * Premultiplied. Byte order is r,g,b,a. - */ kRGBA_8888_GrPixelConfig, - /** - * Premultiplied. Byte order is b,g,r,a. - */ + kRGB_888_GrPixelConfig, kBGRA_8888_GrPixelConfig, - /** - * Premultiplied and sRGB. Byte order is r,g,b,a. - */ kSRGBA_8888_GrPixelConfig, - /** - * Premultiplied and sRGB. Byte order is b,g,r,a. - */ kSBGRA_8888_GrPixelConfig, - - /** - * Premultiplied. - */ kRGBA_1010102_GrPixelConfig, - - /** - * Byte order is r, g, b, a. This color format is 32 bits per channel - */ kRGBA_float_GrPixelConfig, - /** - * Byte order is r, g. This color format is 32 bits per channel - */ kRG_float_GrPixelConfig, - - /** - * This color format is a single 16 bit float channel - */ kAlpha_half_GrPixelConfig, - - /** - * Byte order is r, g, b, a. This color format is 16 bits per channel - */ kRGBA_half_GrPixelConfig, + /** For internal usage. */ kPrivateConfig1_GrPixelConfig, kPrivateConfig2_GrPixelConfig, kPrivateConfig3_GrPixelConfig, diff --git a/include/private/GrTypesPriv.h b/include/private/GrTypesPriv.h index 69c79ab042..74b2425c14 100644 --- a/include/private/GrTypesPriv.h +++ b/include/private/GrTypesPriv.h @@ -12,6 +12,8 @@ #include "GrSharedEnums.h" #include "GrTypes.h" #include "SkImageInfo.h" +#include "SkImageInfoPriv.h" +#include "SkRefCnt.h" #include "SkWeakRefCnt.h" class GrCaps; @@ -814,6 +816,7 @@ static inline GrSRGBEncoded GrPixelConfigIsSRGBEncoded(GrPixelConfig config) { case kGray_8_as_Red_GrPixelConfig: case kRGB_565_GrPixelConfig: case kRGBA_4444_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kRGBA_8888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kRGBA_1010102_GrPixelConfig: @@ -852,6 +855,7 @@ static inline GrPixelConfig GrPixelConfigSwapRAndB(GrPixelConfig config) { case kGray_8_as_Red_GrPixelConfig: case kRGB_565_GrPixelConfig: case kRGBA_4444_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kRGBA_1010102_GrPixelConfig: case kRGBA_float_GrPixelConfig: case kRG_float_GrPixelConfig: @@ -879,6 +883,7 @@ static inline size_t GrBytesPerPixel(GrPixelConfig config) { case kAlpha_half_as_Red_GrPixelConfig: return 2; case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: // Assuming GPUs store this 4-byte aligned. case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: @@ -900,6 +905,7 @@ static inline size_t GrBytesPerPixel(GrPixelConfig config) { static inline bool GrPixelConfigIsOpaque(GrPixelConfig config) { switch (config) { case kRGB_565_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kGray_8_GrPixelConfig: case kGray_8_as_Lum_GrPixelConfig: case kGray_8_as_Red_GrPixelConfig: @@ -940,6 +946,7 @@ static inline bool GrPixelConfigIsAlphaOnly(GrPixelConfig config) { case kRGB_565_GrPixelConfig: case kRGBA_4444_GrPixelConfig: case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: @@ -971,6 +978,7 @@ static inline bool GrPixelConfigIsFloatingPoint(GrPixelConfig config) { case kRGB_565_GrPixelConfig: case kRGBA_4444_GrPixelConfig: case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: @@ -992,6 +1000,7 @@ static inline bool GrPixelConfigIsUnorm(GrPixelConfig config) { case kRGB_565_GrPixelConfig: case kRGBA_4444_GrPixelConfig: case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: @@ -1024,6 +1033,7 @@ static inline GrSLPrecision GrSLSamplerPrecision(GrPixelConfig config) { case kRGB_565_GrPixelConfig: case kRGBA_4444_GrPixelConfig: case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: @@ -1059,6 +1069,7 @@ enum class GrColorType { kRGB_565, kABGR_4444, // This name differs from SkColorType. kARGB_4444_SkColorType is misnamed. kRGBA_8888, + kRGB_888x, kBGRA_8888, kRGBA_1010102, kGray_8, @@ -1075,6 +1086,7 @@ static inline SkColorType GrColorTypeToSkColorType(GrColorType ct) { case GrColorType::kRGB_565: return kRGB_565_SkColorType; case GrColorType::kABGR_4444: return kARGB_4444_SkColorType; case GrColorType::kRGBA_8888: return kRGBA_8888_SkColorType; + case GrColorType::kRGB_888x: return kRGB_888x_SkColorType; case GrColorType::kBGRA_8888: return kBGRA_8888_SkColorType; case GrColorType::kRGBA_1010102: return kRGBA_1010102_SkColorType; case GrColorType::kGray_8: return kGray_8_SkColorType; @@ -1094,8 +1106,8 @@ static inline GrColorType SkColorTypeToGrColorType(SkColorType ct) { case kRGB_565_SkColorType: return GrColorType::kRGB_565; case kARGB_4444_SkColorType: return GrColorType::kABGR_4444; case kRGBA_8888_SkColorType: return GrColorType::kRGBA_8888; + case kRGB_888x_SkColorType: return GrColorType::kRGB_888x; case kBGRA_8888_SkColorType: return GrColorType::kBGRA_8888; - case kRGB_888x_SkColorType: return GrColorType::kUnknown; case kGray_8_SkColorType: return GrColorType::kGray_8; case kRGBA_F16_SkColorType: return GrColorType::kRGBA_F16; case kRGBA_1010102_SkColorType: return GrColorType::kRGBA_1010102; @@ -1105,6 +1117,35 @@ static inline GrColorType SkColorTypeToGrColorType(SkColorType ct) { return GrColorType::kUnknown; } +static inline uint32_t GrColorTypeComponentFlags(GrColorType ct) { + switch (ct) { + case GrColorType::kUnknown: return 0; + case GrColorType::kAlpha_8: return kAlpha_SkColorTypeComponentFlag; + case GrColorType::kRGB_565: return kRGB_SkColorTypeComponentFlags; + case GrColorType::kABGR_4444: return kRGBA_SkColorTypeComponentFlags; + case GrColorType::kRGBA_8888: return kRGBA_SkColorTypeComponentFlags; + case GrColorType::kRGB_888x: return kRGB_SkColorTypeComponentFlags; + case GrColorType::kBGRA_8888: return kRGBA_SkColorTypeComponentFlags; + case GrColorType::kRGBA_1010102: return kRGBA_SkColorTypeComponentFlags; + case GrColorType::kGray_8: return kGray_SkColorTypeComponentFlag; + case GrColorType::kAlpha_F16: return kAlpha_SkColorTypeComponentFlag; + case GrColorType::kRGBA_F16: return kRGBA_SkColorTypeComponentFlags; + case GrColorType::kRG_F32: return kRed_SkColorTypeComponentFlag | + kGreen_SkColorTypeComponentFlag; + case GrColorType::kRGBA_F32: return kRGBA_SkColorTypeComponentFlags; + } + SK_ABORT("Invalid GrColorType"); + return kUnknown_SkColorType; +} + +static inline bool GrColorTypeIsAlphaOnly(GrColorType ct) { + return kAlpha_SkColorTypeComponentFlag == GrColorTypeComponentFlags(ct); +} + +static inline bool GrColorTypeHasAlpha(GrColorType ct) { + return kAlpha_SkColorTypeComponentFlag & GrColorTypeComponentFlags(ct); +} + static inline int GrColorTypeBytesPerPixel(GrColorType ct) { switch (ct) { case GrColorType::kUnknown: return 0; @@ -1112,6 +1153,7 @@ static inline int GrColorTypeBytesPerPixel(GrColorType ct) { case GrColorType::kRGB_565: return 2; case GrColorType::kABGR_4444: return 2; case GrColorType::kRGBA_8888: return 4; + case GrColorType::kRGB_888x: return 4; case GrColorType::kBGRA_8888: return 4; case GrColorType::kRGBA_1010102: return 4; case GrColorType::kGray_8: return 1; @@ -1124,25 +1166,6 @@ static inline int GrColorTypeBytesPerPixel(GrColorType ct) { return 0; } -static inline int GrColorTypeIsAlphaOnly(GrColorType ct) { - switch (ct) { - case GrColorType::kUnknown: return false; - case GrColorType::kAlpha_8: return true; - case GrColorType::kRGB_565: return false; - case GrColorType::kABGR_4444: return false; - case GrColorType::kRGBA_8888: return false; - case GrColorType::kBGRA_8888: return false; - case GrColorType::kRGBA_1010102: return false; - case GrColorType::kGray_8: return false; - case GrColorType::kAlpha_F16: return true; - case GrColorType::kRGBA_F16: return false; - case GrColorType::kRG_F32: return false; - case GrColorType::kRGBA_F32: return false; - } - SK_ABORT("Invalid GrColorType"); - return false; -} - static inline GrColorType GrPixelConfigToColorTypeAndEncoding(GrPixelConfig config, GrSRGBEncoded* srgbEncoded) { SkASSERT(srgbEncoded); @@ -1164,6 +1187,9 @@ static inline GrColorType GrPixelConfigToColorTypeAndEncoding(GrPixelConfig conf case kRGBA_8888_GrPixelConfig: *srgbEncoded = GrSRGBEncoded::kNo; return GrColorType::kRGBA_8888; + case kRGB_888_GrPixelConfig: + *srgbEncoded = GrSRGBEncoded::kNo; + return GrColorType::kRGB_888x; case kBGRA_8888_GrPixelConfig: *srgbEncoded = GrSRGBEncoded::kNo; return GrColorType::kBGRA_8888; @@ -1238,6 +1264,10 @@ static inline GrPixelConfig GrColorTypeToPixelConfig(GrColorType config, return (GrSRGBEncoded::kYes == srgbEncoded) ? kSRGBA_8888_GrPixelConfig : kRGBA_8888_GrPixelConfig; + case GrColorType::kRGB_888x: + return (GrSRGBEncoded::kYes == srgbEncoded) ? kUnknown_GrPixelConfig + : kRGB_888_GrPixelConfig; + case GrColorType::kBGRA_8888: return (GrSRGBEncoded::kYes == srgbEncoded) ? kSBGRA_8888_GrPixelConfig : kBGRA_8888_GrPixelConfig; diff --git a/src/core/SkImageInfoPriv.h b/include/private/SkImageInfoPriv.h index 04779ff3fb..597906af29 100644 --- a/src/core/SkImageInfoPriv.h +++ b/include/private/SkImageInfoPriv.h @@ -15,10 +15,51 @@ enum class SkDestinationSurfaceColorMode { kGammaAndColorSpaceAware, }; +enum SkColorTypeComponentFlag { + kRed_SkColorTypeComponentFlag = 0x1, + kGreen_SkColorTypeComponentFlag = 0x2, + kBlue_SkColorTypeComponentFlag = 0x4, + kAlpha_SkColorTypeComponentFlag = 0x8, + kGray_SkColorTypeComponentFlag = 0x10, + kRGB_SkColorTypeComponentFlags = kRed_SkColorTypeComponentFlag | + kGreen_SkColorTypeComponentFlag | + kBlue_SkColorTypeComponentFlag, + kRGBA_SkColorTypeComponentFlags = kRGB_SkColorTypeComponentFlags | + kAlpha_SkColorTypeComponentFlag, +}; + +static inline uint32_t SkColorTypeComponentFlags(SkColorType ct) { + switch (ct) { + case kUnknown_SkColorType: return 0; + case kAlpha_8_SkColorType: return kAlpha_SkColorTypeComponentFlag; + case kRGB_565_SkColorType: return kRGB_SkColorTypeComponentFlags; + case kARGB_4444_SkColorType: return kRGBA_SkColorTypeComponentFlags; + case kRGBA_8888_SkColorType: return kRGBA_SkColorTypeComponentFlags; + case kRGB_888x_SkColorType: return kRGB_SkColorTypeComponentFlags; + case kBGRA_8888_SkColorType: return kRGBA_SkColorTypeComponentFlags; + case kRGBA_1010102_SkColorType: return kRGBA_SkColorTypeComponentFlags; + case kRGB_101010x_SkColorType: return kRGB_SkColorTypeComponentFlags; + case kGray_8_SkColorType: return kGray_SkColorTypeComponentFlag; + case kRGBA_F16_SkColorType: return kRGBA_SkColorTypeComponentFlags; + } + return 0; +} + +static inline bool SkColorTypeIsAlphaOnly(SkColorType ct) { + return kAlpha_SkColorTypeComponentFlag == SkColorTypeComponentFlags(ct); +} + static inline bool SkAlphaTypeIsValid(unsigned value) { return value <= kLastEnum_SkAlphaType; } +static inline bool SkColorTypeIsGray(SkColorType ct) { + auto flags = SkColorTypeComponentFlags(ct); + // Currently assuming that a color type has only gray or does not have gray. + SkASSERT(!(kGray_SkColorTypeComponentFlag & flags) || kGray_SkColorTypeComponentFlag == flags); + return kGray_SkColorTypeComponentFlag == flags; +} + static int SkColorTypeShiftPerPixel(SkColorType ct) { switch (ct) { case kUnknown_SkColorType: return 0; @@ -152,8 +193,8 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm return false; } - if (kGray_8_SkColorType == dst.colorType()) { - if (kGray_8_SkColorType != src.colorType()) { + if (SkColorTypeIsGray(dst.colorType())) { + if (!SkColorTypeIsGray(src.colorType())) { return false; } @@ -162,7 +203,7 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm } } - if (kAlpha_8_SkColorType != dst.colorType() && kAlpha_8_SkColorType == src.colorType()) { + if (!SkColorTypeIsAlphaOnly(dst.colorType()) && SkColorTypeIsAlphaOnly(src.colorType())) { return false; } diff --git a/src/core/SkGpuBlurUtils.cpp b/src/core/SkGpuBlurUtils.cpp index b42bb04782..fe614bd0df 100644 --- a/src/core/SkGpuBlurUtils.cpp +++ b/src/core/SkGpuBlurUtils.cpp @@ -233,10 +233,10 @@ sk_sp<GrRenderTargetContext> GaussianBlur(GrContext* context, } SkASSERT(kBGRA_8888_GrPixelConfig == config || kRGBA_8888_GrPixelConfig == config || - kRGBA_4444_GrPixelConfig == config || kRGB_565_GrPixelConfig == config || - kSRGBA_8888_GrPixelConfig == config || kSBGRA_8888_GrPixelConfig == config || - kRGBA_half_GrPixelConfig == config || kAlpha_8_GrPixelConfig == config || - kRGBA_1010102_GrPixelConfig == config); + kRGB_888_GrPixelConfig == config || kRGBA_4444_GrPixelConfig == config || + kRGB_565_GrPixelConfig == config || kSRGBA_8888_GrPixelConfig == config || + kSBGRA_8888_GrPixelConfig == config || kRGBA_half_GrPixelConfig == config || + kAlpha_8_GrPixelConfig == config || kRGBA_1010102_GrPixelConfig == config); const int width = dstBounds.width(); const int height = dstBounds.height(); diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp index f9933736e3..fd5e8087ba 100644 --- a/src/core/SkImageInfo.cpp +++ b/src/core/SkImageInfo.cpp @@ -45,16 +45,7 @@ enum Stored_SkColorType { }; bool SkColorTypeIsAlwaysOpaque(SkColorType ct) { - switch (ct) { - case kRGB_565_SkColorType: - case kRGB_888x_SkColorType: - case kRGB_101010x_SkColorType: - case kGray_8_SkColorType: - return true; - default: - break; - } - return false; + return !(kAlpha_SkColorTypeComponentFlag & SkColorTypeComponentFlags(ct)); } /////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp index 1d25c1a791..0507a3fdfb 100644 --- a/src/gpu/GrCaps.cpp +++ b/src/gpu/GrCaps.cpp @@ -24,6 +24,7 @@ static const char* pixel_config_name(GrPixelConfig config) { case kRGB_565_GrPixelConfig: return "RGB565"; case kRGBA_4444_GrPixelConfig: return "RGBA444"; case kRGBA_8888_GrPixelConfig: return "RGBA8888"; + case kRGB_888_GrPixelConfig: return "RGB888"; case kBGRA_8888_GrPixelConfig: return "BGRA8888"; case kSRGBA_8888_GrPixelConfig: return "SRGBA8888"; case kSBGRA_8888_GrPixelConfig: return "SBGRA8888"; diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 626b3f6ecd..3baf0b9737 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -358,6 +358,7 @@ static bool valid_premul_config(GrPixelConfig config) { case kRGB_565_GrPixelConfig: return false; case kRGBA_4444_GrPixelConfig: return true; case kRGBA_8888_GrPixelConfig: return true; + case kRGB_888_GrPixelConfig: return false; case kBGRA_8888_GrPixelConfig: return true; case kSRGBA_8888_GrPixelConfig: return true; case kSBGRA_8888_GrPixelConfig: return true; @@ -383,6 +384,7 @@ static bool valid_premul_color_type(GrColorType ct) { case GrColorType::kRGB_565: return false; case GrColorType::kABGR_4444: return true; case GrColorType::kRGBA_8888: return true; + case GrColorType::kRGB_888x: return false; case GrColorType::kBGRA_8888: return true; case GrColorType::kRGBA_1010102: return true; case GrColorType::kGray_8: return false; @@ -402,7 +404,6 @@ static bool valid_pixel_conversion(GrColorType cpuColorType, GrPixelConfig gpuCo (!valid_premul_color_type(cpuColorType) || !valid_premul_config(gpuConfig))) { return false; } - return true; } @@ -471,10 +472,20 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceContext* dst, int left, int top, int height, GrColorType srcColorType, SkColorSpace* srcColorSpace, const void* buffer, size_t rowBytes, uint32_t pixelOpsFlags) { -#ifndef SK_LEGACY_GPU_PIXEL_OPS - return this->writeSurfacePixels2(dst, left, top, width, height, srcColorType, srcColorSpace, - buffer, rowBytes, pixelOpsFlags); + bool useLegacyPath = false; +#ifdef SK_LEGACY_GPU_PIXEL_OPS + useLegacyPath = true; #endif + // Newly added color types/configs are only supported by the new code path. + if (srcColorType == GrColorType::kRGB_888x || + dst->asSurfaceProxy()->config() == kRGB_888_GrPixelConfig) { + useLegacyPath = false; + } + + if (!useLegacyPath) { + return this->writeSurfacePixels2(dst, left, top, width, height, srcColorType, srcColorSpace, + buffer, rowBytes, pixelOpsFlags); + } // TODO: Color space conversion @@ -497,6 +508,14 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceContext* dst, int left, int top, if (!valid_pixel_conversion(srcColorType, dstProxy->config(), premul)) { return false; } + // There is no way to store alpha values in the dst. + if (GrColorTypeHasAlpha(srcColorType) && GrPixelConfigIsOpaque(dstProxy->config())) { + return false; + } + // The source has no alpha value and the dst is only alpha + if (!GrColorTypeHasAlpha(srcColorType) && GrPixelConfigIsAlphaOnly(dstProxy->config())) { + return false; + } // We need to guarantee round-trip conversion if we are reading and writing 8888 non-sRGB data, // without any color spaces attached, and the caller wants us to premul. @@ -612,10 +631,21 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top, int height, GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes, uint32_t flags) { -#ifndef SK_LEGACY_GPU_PIXEL_OPS - return this->readSurfacePixels2(src, left, top, width, height, dstColorType, dstColorSpace, - buffer, rowBytes, flags); + bool useLegacyPath = false; +#ifdef SK_LEGACY_GPU_PIXEL_OPS + useLegacyPath = true; #endif + // Newly added color types/configs are only supported by the new code path. + if (dstColorType == GrColorType::kRGB_888x || + src->asSurfaceProxy()->config() == kRGB_888_GrPixelConfig) { + useLegacyPath = false; + } + + if (!useLegacyPath) { + return this->readSurfacePixels2(src, left, top, width, height, dstColorType, dstColorSpace, + buffer, rowBytes, flags); + } + // TODO: Color space conversion ASSERT_SINGLE_OWNER_PRIV @@ -642,6 +672,18 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top, if (!valid_pixel_conversion(dstColorType, srcProxy->config(), unpremul)) { return false; } + // The source is alpha-only but the dst is not. TODO: Make non-alpha channels in the dst be 0? + if (GrPixelConfigIsAlphaOnly(srcProxy->config()) && !GrColorTypeIsAlphaOnly(dstColorType)) { + return false; + } + // The source has no alpha, the dst is alpha-only. TODO: set all values in dst to 1? + if (GrPixelConfigIsOpaque(srcProxy->config()) && GrColorTypeIsAlphaOnly(dstColorType)) { + return false; + } + // Not clear if we should unpremul in this case. + if (!GrPixelConfigIsOpaque(srcProxy->config()) && !GrColorTypeHasAlpha(dstColorType)) { + return false; + } // We need to guarantee round-trip conversion if we are reading and writing 8888 non-sRGB data, // without any color spaces attached, and the caller wants us to unpremul. @@ -913,10 +955,6 @@ bool GrContextPriv::readSurfacePixels2(GrSurfaceContext* src, int left, int top, int height, GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes, uint32_t flags) { - SkASSERT(!(flags & kDontFlush_PixelOpsFlag)); - if (flags & kDontFlush_PixelOpsFlag){ - return false; - } ASSERT_SINGLE_OWNER_PRIV RETURN_FALSE_IF_ABANDONED_PRIV SkASSERT(src); @@ -924,6 +962,11 @@ bool GrContextPriv::readSurfacePixels2(GrSurfaceContext* src, int left, int top, ASSERT_OWNED_PROXY_PRIV(src->asSurfaceProxy()); GR_CREATE_TRACE_MARKER_CONTEXT("GrContextPriv", "readSurfacePixels2", fContext); + SkASSERT(!(flags & kDontFlush_PixelOpsFlag)); + if (flags & kDontFlush_PixelOpsFlag) { + return false; + } + // MDB TODO: delay this instantiation until later in the method if (!src->asSurfaceProxy()->instantiate(this->resourceProvider())) { return false; diff --git a/src/gpu/GrSurfaceContext.cpp b/src/gpu/GrSurfaceContext.cpp index ab6c05a375..37d0b71fb0 100644 --- a/src/gpu/GrSurfaceContext.cpp +++ b/src/gpu/GrSurfaceContext.cpp @@ -49,7 +49,8 @@ bool GrSurfaceContext::readPixels(const SkImageInfo& dstInfo, void* dstBuffer, GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrSurfaceContext::readPixels"); // TODO: this seems to duplicate code in SkImage_Gpu::onReadPixels - if (kUnpremul_SkAlphaType == dstInfo.alphaType()) { + if (kUnpremul_SkAlphaType == dstInfo.alphaType() && + !GrPixelConfigIsOpaque(this->asSurfaceProxy()->config())) { flags |= GrContextPriv::kUnpremul_PixelOpsFlag; } auto colorType = SkColorTypeToGrColorType(dstInfo.colorType()); diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp index 27c9b39f5e..09155a05b3 100644 --- a/src/gpu/SkGr.cpp +++ b/src/gpu/SkGr.cpp @@ -264,7 +264,7 @@ GrPixelConfig SkImageInfo2GrPixelConfig(const SkColorType type, SkColorSpace* cs // TODO: We're checking for srgbSupport, but we can then end up picking sBGRA as our pixel // config (which may not be supported). We need a better test here. case kRGB_888x_SkColorType: - return kUnknown_GrPixelConfig; + return kRGB_888_GrPixelConfig; case kBGRA_8888_SkColorType: return (caps.srgbSupport() && cs && cs->gammaCloseToSRGB()) ? kSBGRA_8888_GrPixelConfig : kBGRA_8888_GrPixelConfig; @@ -286,49 +286,14 @@ GrPixelConfig SkImageInfo2GrPixelConfig(const SkImageInfo& info, const GrCaps& c } bool GrPixelConfigToColorType(GrPixelConfig config, SkColorType* ctOut) { - SkColorType ct; - switch (config) { - case kAlpha_8_GrPixelConfig: // fall through - case kAlpha_8_as_Alpha_GrPixelConfig: // fall through - case kAlpha_8_as_Red_GrPixelConfig: - ct = kAlpha_8_SkColorType; - break; - case kGray_8_GrPixelConfig: // fall through - case kGray_8_as_Lum_GrPixelConfig: // fall through - case kGray_8_as_Red_GrPixelConfig: - ct = kGray_8_SkColorType; - break; - case kRGB_565_GrPixelConfig: - ct = kRGB_565_SkColorType; - break; - case kRGBA_4444_GrPixelConfig: - ct = kARGB_4444_SkColorType; - break; - case kRGBA_8888_GrPixelConfig: - ct = kRGBA_8888_SkColorType; - break; - case kBGRA_8888_GrPixelConfig: - ct = kBGRA_8888_SkColorType; - break; - case kSRGBA_8888_GrPixelConfig: - ct = kRGBA_8888_SkColorType; - break; - case kSBGRA_8888_GrPixelConfig: - ct = kBGRA_8888_SkColorType; - break; - case kRGBA_1010102_GrPixelConfig: - ct = kRGBA_1010102_SkColorType; - break; - case kRGBA_half_GrPixelConfig: - ct = kRGBA_F16_SkColorType; - break; - default: - return false; - } - if (ctOut) { - *ctOut = ct; + SkColorType ct = GrColorTypeToSkColorType(GrPixelConfigToColorType(config)); + if (kUnknown_SkColorType != ct) { + if (ctOut) { + *ctOut = ct; + } + return true; } - return true; + return false; } GrPixelConfig GrRenderableConfigForColorSpace(const SkColorSpace* colorSpace) { diff --git a/src/gpu/effects/GrDitherEffect.fp b/src/gpu/effects/GrDitherEffect.fp index 7cf07da311..c51865e38b 100644 --- a/src/gpu/effects/GrDitherEffect.fp +++ b/src/gpu/effects/GrDitherEffect.fp @@ -16,6 +16,7 @@ layout(key) in int rangeType; case kGray_8_as_Lum_GrPixelConfig: case kGray_8_as_Red_GrPixelConfig: case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: diff --git a/src/gpu/effects/GrDitherEffect.h b/src/gpu/effects/GrDitherEffect.h index 2e650a68f4..d1bc72bf4e 100644 --- a/src/gpu/effects/GrDitherEffect.h +++ b/src/gpu/effects/GrDitherEffect.h @@ -25,6 +25,7 @@ public: case kGray_8_as_Lum_GrPixelConfig: case kGray_8_as_Red_GrPixelConfig: case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index e05a4bb970..9af54b1dec 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -1167,8 +1167,8 @@ void GrGLCaps::onDumpJSON(SkJSONWriter* writer) const { writer->appendHexU32("flags", fConfigTable[i].fFlags); writer->appendHexU32("b_internal", fConfigTable[i].fFormats.fBaseInternalFormat); writer->appendHexU32("s_internal", fConfigTable[i].fFormats.fSizedInternalFormat); - writer->appendHexU32("e_format", - fConfigTable[i].fFormats.fExternalFormat[kOther_ExternalFormatUsage]); + writer->appendHexU32("e_format_read_pixels", + fConfigTable[i].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage]); writer->appendHexU32( "e_format_teximage", fConfigTable[i].fFormats.fExternalFormat[kTexImage_ExternalFormatUsage]); @@ -1200,7 +1200,7 @@ bool GrGLCaps::getTexImageFormats(GrPixelConfig surfaceConfig, GrPixelConfig ext bool GrGLCaps::getReadPixelsFormat(GrPixelConfig surfaceConfig, GrPixelConfig externalConfig, GrGLenum* externalFormat, GrGLenum* externalType) const { - if (!this->getExternalFormat(surfaceConfig, externalConfig, kOther_ExternalFormatUsage, + if (!this->getExternalFormat(surfaceConfig, externalConfig, kReadPixels_ExternalFormatUsage, externalFormat, externalType)) { return false; } @@ -1322,6 +1322,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, bool disableSRGBWriteControlForAdreno4xx = false; bool disableR8TexStorageForANGLEGL = false; bool disableSRGBRenderWithMSAAForMacAMD = false; + bool disableRGB8ForMali400 = false; if (!contextOptions.fDisableDriverCorrectnessWorkarounds) { // ARB_texture_rg is part of OpenGL 3.0, but osmesa doesn't support GL_RED @@ -1348,6 +1349,8 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, #if defined(SK_BUILD_FOR_MAC) disableSRGBRenderWithMSAAForMacAMD = kATI_GrGLVendor == ctxInfo.vendor(); #endif + // Mali-400 fails ReadPixels tests, mostly with non-0xFF alpha values when read as GL_RGBA8. + disableRGB8ForMali400 = kMali4xx_GrGLRenderer == ctxInfo.renderer(); } uint32_t nonMSAARenderFlags = ConfigInfo::kRenderable_Flag | @@ -1386,14 +1389,14 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[kUnknown_GrPixelConfig].fFormats.fBaseInternalFormat = 0; fConfigTable[kUnknown_GrPixelConfig].fFormats.fSizedInternalFormat = 0; - fConfigTable[kUnknown_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = 0; + fConfigTable[kUnknown_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = 0; fConfigTable[kUnknown_GrPixelConfig].fFormats.fExternalType = 0; fConfigTable[kUnknown_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; fConfigTable[kUnknown_GrPixelConfig].fSwizzle = GrSwizzle::RGBA(); fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA; fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGBA8; - fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA; fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE; fConfigTable[kRGBA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; @@ -1415,7 +1418,41 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, } fConfigTable[kRGBA_8888_GrPixelConfig].fSwizzle = GrSwizzle::RGBA(); - fConfigTable[kBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kRGB_888_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGB; + fConfigTable[kRGB_888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGB8; + // Our external RGB data always has a byte where alpha would be. When calling read pixels we + // want to read to kRGB_888x color type and ensure that gets 0xFF written. Using GL_RGB would + // read back unaligned 24bit RGB color values. Note that this all a bit moot as we don't + // currently expect to ever read back GrColorType::kRGB_888x because our implementation of + // supportedReadPixelsColorType never returns it. + fConfigTable[kRGB_888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA; + fConfigTable[kRGB_888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE; + fConfigTable[kRGB_888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; + fConfigTable[kRGB_888_GrPixelConfig].fFlags = ConfigInfo::kTextureable_Flag; + if (kGL_GrGLStandard == standard) { + // Even in OpenGL 4.6 GL_RGB8 is required to be color renderable but not required to be a + // supported render buffer format. Since we usually use render buffers for MSAA on non-ES GL + // we don't support MSAA for GL_RGB8. On 4.2+ we could check using + // glGetInternalFormativ(GL_RENDERBUFFER, GL_RGB8, GL_INTERNALFORMAT_SUPPORTED, ...) if this + // becomes an issue. + // This also would probably work in mixed-samples mode where there is no MSAA color buffer + // but we don't support that just for simplicity's sake. + fConfigTable[kRGB_888_GrPixelConfig].fFlags |= nonMSAARenderFlags; + } else { + // 3.0 and the extension support this as a render buffer format. + if (version >= GR_GL_VER(3, 0) || ctxInfo.hasExtension("GL_OES_rgb8_rgba8")) { + fConfigTable[kRGB_888_GrPixelConfig].fFlags |= allRenderFlags; + } + } + if (texStorageSupported) { + fConfigTable[kRGB_888_GrPixelConfig].fFlags |= ConfigInfo::kCanUseTexStorage_Flag; + } + fConfigTable[kRGB_888_GrPixelConfig].fSwizzle = GrSwizzle::RGBA(); + if (disableRGB8ForMali400) { + fConfigTable[kRGB_888_GrPixelConfig].fFlags = 0; + } + + fConfigTable[kBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_BGRA; fConfigTable[kBGRA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE; fConfigTable[kBGRA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; @@ -1527,7 +1564,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_SRGB8_ALPHA8; // GL does not do srgb<->rgb conversions when transferring between cpu and gpu. Thus, the // external format is GL_RGBA. See below for note about ES2.0 and glTex[Sub]Image. - fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA; fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE; fConfigTable[kSRGBA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; @@ -1547,7 +1584,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_SRGB8_ALPHA8; // GL does not do srgb<->rgb conversions when transferring between cpu and gpu. Thus, the // external format is GL_BGRA. - fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_BGRA; fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE; fConfigTable[kSBGRA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; @@ -1567,7 +1604,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, } else { fConfigTable[kRGB_565_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGB5; } - fConfigTable[kRGB_565_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kRGB_565_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGB; fConfigTable[kRGB_565_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_SHORT_5_6_5; fConfigTable[kRGB_565_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; @@ -1593,7 +1630,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA; fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGBA4; - fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA; fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_SHORT_4_4_4_4; fConfigTable[kRGBA_4444_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType; @@ -1612,7 +1649,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA; fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGB10_A2; - fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA; fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_INT_2_10_10_10_REV; @@ -1640,7 +1677,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, } alphaInfo.fFormats.fBaseInternalFormat = GR_GL_ALPHA; alphaInfo.fFormats.fSizedInternalFormat = GR_GL_ALPHA8; - alphaInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_ALPHA; + alphaInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_ALPHA; alphaInfo.fSwizzle = GrSwizzle::AAAA(); if (fAlpha8IsRenderable && alpha8IsValidForGL) { alphaInfo.fFlags |= allRenderFlags; @@ -1651,7 +1688,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, redInfo.fFormatType = kNormalizedFixedPoint_FormatType; redInfo.fFormats.fBaseInternalFormat = GR_GL_RED; redInfo.fFormats.fSizedInternalFormat = GR_GL_R8; - redInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_RED; + redInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RED; redInfo.fSwizzle = GrSwizzle::RRRR(); // ES2 Command Buffer does not allow TexStorage with R8_EXT (so Alpha_8 and Gray_8) @@ -1680,7 +1717,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, grayLumInfo.fFormatType = kNormalizedFixedPoint_FormatType; grayLumInfo.fFormats.fBaseInternalFormat = GR_GL_LUMINANCE; grayLumInfo.fFormats.fSizedInternalFormat = GR_GL_LUMINANCE8; - grayLumInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_LUMINANCE; + grayLumInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_LUMINANCE; grayLumInfo.fSwizzle = GrSwizzle::RGBA(); if ((standard == kGL_GrGLStandard && version <= GR_GL_VER(3, 0)) || (standard == kGLES_GrGLStandard && version < GR_GL_VER(3, 0))) { @@ -1692,7 +1729,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, grayRedInfo.fFormatType = kNormalizedFixedPoint_FormatType; grayRedInfo.fFormats.fBaseInternalFormat = GR_GL_RED; grayRedInfo.fFormats.fSizedInternalFormat = GR_GL_R8; - grayRedInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_RED; + grayRedInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RED; grayRedInfo.fSwizzle = GrSwizzle::RRRA(); grayRedInfo.fFlags = ConfigInfo::kTextureable_Flag; @@ -1762,7 +1799,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[fpconfig].fFormats.fBaseInternalFormat = format; fConfigTable[fpconfig].fFormats.fSizedInternalFormat = kRGBA_float_GrPixelConfig == fpconfig ? GR_GL_RGBA32F : GR_GL_RG32F; - fConfigTable[fpconfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = format; + fConfigTable[fpconfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = format; fConfigTable[fpconfig].fFormats.fExternalType = GR_GL_FLOAT; fConfigTable[fpconfig].fFormatType = kFloat_FormatType; if (hasFPTextures) { @@ -1794,7 +1831,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, redHalf.fFormatType = kFloat_FormatType; redHalf.fFormats.fBaseInternalFormat = GR_GL_RED; redHalf.fFormats.fSizedInternalFormat = GR_GL_R16F; - redHalf.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_RED; + redHalf.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RED; redHalf.fSwizzle = GrSwizzle::RRRR(); if (textureRedSupport && hasHalfFPTextures) { redHalf.fFlags = ConfigInfo::kTextureable_Flag; @@ -1816,7 +1853,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA; fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGBA16F; - fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = + fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA; if (kGL_GrGLStandard == ctxInfo.standard() || ctxInfo.version() >= GR_GL_VER(3, 0)) { fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fExternalType = GR_GL_HALF_FLOAT; @@ -1850,10 +1887,10 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, bool useSizedRbFormats = kGLES_GrGLStandard == ctxInfo.standard(); for (int i = 0; i < kGrPixelConfigCnt; ++i) { - // Almost always we want to pass fExternalFormat[kOther_ExternalFormatUsage] as the <format> - // param to glTex[Sub]Image. + // Almost always we want to pass fExternalFormat[kReadPixels_ExternalFormatUsage] as the + // <format> param to glTex[Sub]Image. fConfigTable[i].fFormats.fExternalFormat[kTexImage_ExternalFormatUsage] = - fConfigTable[i].fFormats.fExternalFormat[kOther_ExternalFormatUsage]; + fConfigTable[i].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage]; fConfigTable[i].fFormats.fInternalFormatTexImage = useSizedTexFormats ? fConfigTable[i].fFormats.fSizedInternalFormat : fConfigTable[i].fFormats.fBaseInternalFormat; @@ -1888,6 +1925,12 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions, // unsupported. (If we have no sRGB support at all, this will get overwritten below). fConfigTable[kSBGRA_8888_GrPixelConfig].fFlags = 0; } + // On ES 2.0 we have to use GL_RGB with glTexImage as the internal/external formats must + // be the same. Moreover, if we write kRGB_888x data to a texture format on non-ES2 we want to + // be sure that we write 1 for alpha not whatever happens to be in the client provided the 'x' + // slot. + fConfigTable[kRGB_888_GrPixelConfig].fFormats.fExternalFormat[kTexImage_ExternalFormatUsage] = + GR_GL_RGB; // If BGRA is supported as an internal format it must always be specified to glTex[Sub]Image // as a base format. @@ -2561,7 +2604,10 @@ bool validate_sized_format(GrGLenum format, SkColorType ct, GrPixelConfig* confi } break; case kRGB_888x_SkColorType: - return false; + if (GR_GL_RGB8 == format) { + *config = kRGB_888_GrPixelConfig; + } + break; case kBGRA_8888_SkColorType: if (GR_GL_RGBA8 == format) { if (kGL_GrGLStandard == standard) { diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h index eed3ab3a17..19d443cc1c 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/src/gpu/gl/GrGLCaps.h @@ -419,9 +419,9 @@ public: private: enum ExternalFormatUsage { kTexImage_ExternalFormatUsage, - kOther_ExternalFormatUsage, + kReadPixels_ExternalFormatUsage, - kLast_ExternalFormatUsage = kOther_ExternalFormatUsage + kLast_ExternalFormatUsage = kReadPixels_ExternalFormatUsage }; static const int kExternalFormatUsageCnt = kLast_ExternalFormatUsage + 1; bool getExternalFormat(GrPixelConfig surfaceConfig, GrPixelConfig memoryConfig, diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp index ce81adf95f..c6e2d2537f 100644 --- a/src/gpu/gl/GrGLGpu.cpp +++ b/src/gpu/gl/GrGLGpu.cpp @@ -776,7 +776,7 @@ bool GrGLGpu::onWritePixels(GrSurface* surface, GrSurfaceOrigin origin, int left srcAsConfig, texels, mipLevelCount); } -// For GL_[UN]PACK_ALIGNMENT. +// For GL_[UN]PACK_ALIGNMENT. TODO: This really wants to be GrColorType. static inline GrGLint config_alignment(GrPixelConfig config) { switch (config) { case kAlpha_8_GrPixelConfig: @@ -793,6 +793,7 @@ static inline GrGLint config_alignment(GrPixelConfig config) { case kRGBA_half_GrPixelConfig: return 2; case kRGBA_8888_GrPixelConfig: + case kRGB_888_GrPixelConfig: // We're really talking about GrColorType::kRGB_888x here. case kBGRA_8888_GrPixelConfig: case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: diff --git a/src/gpu/gl/GrGLUtil.cpp b/src/gpu/gl/GrGLUtil.cpp index 6b44402c1b..3de894db82 100644 --- a/src/gpu/gl/GrGLUtil.cpp +++ b/src/gpu/gl/GrGLUtil.cpp @@ -380,6 +380,11 @@ GrGLRenderer GrGLGetRendererFromString(const char* rendererString) { if (0 == strncmp(rendererString, kMaliTStr, SK_ARRAY_COUNT(kMaliTStr) - 1)) { return kMaliT_GrGLRenderer; } + int mali400Num; + if (1 == sscanf(rendererString, "Mali-%d", &mali400Num) && mali400Num >= 400 && + mali400Num < 500) { + return kMali4xx_GrGLRenderer; + } if (is_renderer_angle(rendererString)) { return kANGLE_GrGLRenderer; } @@ -494,6 +499,8 @@ GrPixelConfig GrGLSizedFormatToPixelConfig(GrGLenum sizedFormat) { return kAlpha_8_as_Alpha_GrPixelConfig; case GR_GL_RGBA8: return kRGBA_8888_GrPixelConfig; + case GR_GL_RGB8: + return kRGB_888_GrPixelConfig; case GR_GL_BGRA8: return kBGRA_8888_GrPixelConfig; case GR_GL_SRGB8_ALPHA8: diff --git a/src/gpu/gl/GrGLUtil.h b/src/gpu/gl/GrGLUtil.h index 9905d75ef1..416538a749 100644 --- a/src/gpu/gl/GrGLUtil.h +++ b/src/gpu/gl/GrGLUtil.h @@ -60,12 +60,13 @@ enum GrGLRenderer { /** Either HD 6xxx or Iris 6xxx */ kIntel6xxx_GrGLRenderer, kGalliumLLVM_GrGLRenderer, + kMali4xx_GrGLRenderer, /** T-6xx, T-7xx, or T-8xx */ kMaliT_GrGLRenderer, kANGLE_GrGLRenderer, - kAMDRadeonHD7xxx_GrGLRenderer, // AMD Radeon HD 7000 Series - kAMDRadeonR9M4xx_GrGLRenderer, // AMD Radeon R9 M400 Series + kAMDRadeonHD7xxx_GrGLRenderer, // AMD Radeon HD 7000 Series + kAMDRadeonR9M4xx_GrGLRenderer, // AMD Radeon R9 M400 Series kOther_GrGLRenderer }; diff --git a/src/gpu/mtl/GrMtlUtil.mm b/src/gpu/mtl/GrMtlUtil.mm index 383cc98264..e8a736c8f1 100644 --- a/src/gpu/mtl/GrMtlUtil.mm +++ b/src/gpu/mtl/GrMtlUtil.mm @@ -21,6 +21,9 @@ bool GrPixelConfigToMTLFormat(GrPixelConfig config, MTLPixelFormat* format) { case kRGBA_8888_GrPixelConfig: *format = MTLPixelFormatRGBA8Unorm; return true; + case kRGB_888_GrPixelConfig: + // TODO: MTLPixelFormatRGB8Unorm + return false; case kBGRA_8888_GrPixelConfig: *format = MTLPixelFormatBGRA8Unorm; return true; diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp index 0d75cbc56e..83546da5c4 100644 --- a/src/gpu/vk/GrVkCaps.cpp +++ b/src/gpu/vk/GrVkCaps.cpp @@ -466,6 +466,7 @@ bool validate_image_info(VkFormat format, SkColorType ct, GrPixelConfig* config) } break; case kRGB_888x_SkColorType: + // TODO: VK_FORMAT_R8G8B8_UNORM return false; case kBGRA_8888_SkColorType: if (VK_FORMAT_B8G8R8A8_UNORM == format) { diff --git a/src/gpu/vk/GrVkUtil.cpp b/src/gpu/vk/GrVkUtil.cpp index 4f0acdb8f4..e61d7bca17 100644 --- a/src/gpu/vk/GrVkUtil.cpp +++ b/src/gpu/vk/GrVkUtil.cpp @@ -22,6 +22,9 @@ bool GrPixelConfigToVkFormat(GrPixelConfig config, VkFormat* format) { case kRGBA_8888_GrPixelConfig: *format = VK_FORMAT_R8G8B8A8_UNORM; return true; + case kRGB_888_GrPixelConfig: + // TODO: VK_FORMAT_R8G8B8_UNORM + return false; case kBGRA_8888_GrPixelConfig: *format = VK_FORMAT_B8G8R8A8_UNORM; return true; diff --git a/tests/GrSurfaceTest.cpp b/tests/GrSurfaceTest.cpp index a7c688d361..c56b1eabbb 100644 --- a/tests/GrSurfaceTest.cpp +++ b/tests/GrSurfaceTest.cpp @@ -87,6 +87,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(GrSurfaceRenderability, reporter, ctxInfo) { kRGB_565_GrPixelConfig, kRGBA_4444_GrPixelConfig, kRGBA_8888_GrPixelConfig, + kRGB_888_GrPixelConfig, kBGRA_8888_GrPixelConfig, kSRGBA_8888_GrPixelConfig, kSBGRA_8888_GrPixelConfig, @@ -97,7 +98,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(GrSurfaceRenderability, reporter, ctxInfo) { kAlpha_half_as_Red_GrPixelConfig, kRGBA_half_GrPixelConfig, }; - SkASSERT(kGrPixelConfigCnt == SK_ARRAY_COUNT(configs)); + GR_STATIC_ASSERT(kGrPixelConfigCnt == SK_ARRAY_COUNT(configs)); GrSurfaceDesc desc; desc.fWidth = 64; diff --git a/tests/PackedConfigsTextureTest.cpp b/tests/PackedConfigsTextureTest.cpp index 5f18ef3d57..adc8a3de7c 100644 --- a/tests/PackedConfigsTextureTest.cpp +++ b/tests/PackedConfigsTextureTest.cpp @@ -109,8 +109,8 @@ static void run_test(skiatest::Reporter* reporter, GrContext* context, int array controlPixelData[i + 1] = 0xFA62; } - const SkImageInfo dstInfo = SkImageInfo::Make(DEV_W, DEV_H, - kRGBA_8888_SkColorType, kOpaque_SkAlphaType); + const SkImageInfo dstInfo = + SkImageInfo::Make(DEV_W, DEV_H, kRGBA_8888_SkColorType, kPremul_SkAlphaType); for (auto origin : { kTopLeft_GrSurfaceOrigin, kBottomLeft_GrSurfaceOrigin }) { auto proxy = sk_gpu_test::MakeTextureProxyFromData(context, false, DEV_W, DEV_H, diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp index ccb2ced4fd..867f766d17 100644 --- a/tests/ReadPixelsTest.cpp +++ b/tests/ReadPixelsTest.cpp @@ -67,6 +67,7 @@ static SkPMColor get_dst_bmp_init_color(int x, int y, int w) { return SkPackARGB32(0xff, r, g , b); } +// TODO: Make this consider both ATs static SkPMColor convert_to_pmcolor(SkColorType ct, SkAlphaType at, const uint32_t* addr, bool* doUnpremul) { *doUnpremul = (kUnpremul_SkAlphaType == at); @@ -80,10 +81,12 @@ static SkPMColor convert_to_pmcolor(SkColorType ct, SkAlphaType at, const uint32 r = static_cast<U8CPU>(c[2]); a = static_cast<U8CPU>(c[3]); break; + case kRGB_888x_SkColorType: // fallthrough case kRGBA_8888_SkColorType: r = static_cast<U8CPU>(c[0]); g = static_cast<U8CPU>(c[1]); b = static_cast<U8CPU>(c[2]); + // We set this even when for kRGB_888x because our caller will validate that it is 0xff. a = static_cast<U8CPU>(c[3]); break; default: @@ -165,16 +168,13 @@ static bool check_read_pixel(SkPMColor a, SkPMColor b, bool didPremulConversion) // checks the bitmap contains correct pixels after the readPixels // if the bitmap was prefilled with pixels it checks that these weren't // overwritten in the area outside the readPixels. -static bool check_read(skiatest::Reporter* reporter, - const SkBitmap& bitmap, - int x, int y, - bool checkCanvasPixels, - bool checkBitmapPixels, - SkColorType ct, - SkAlphaType at) { - SkASSERT(ct == bitmap.colorType() && at == bitmap.alphaType()); +static bool check_read(skiatest::Reporter* reporter, const SkBitmap& bitmap, int x, int y, + bool checkSurfacePixels, bool checkBitmapPixels, + SkAlphaType surfaceAlphaType) { + SkAlphaType bmpAT = bitmap.alphaType(); + SkColorType bmpCT = bitmap.colorType(); SkASSERT(!bitmap.isNull()); - SkASSERT(checkCanvasPixels || checkBitmapPixels); + SkASSERT(checkSurfacePixels || checkBitmapPixels); int bw = bitmap.width(); int bh = bitmap.height(); @@ -184,7 +184,7 @@ static bool check_read(skiatest::Reporter* reporter, if (!clippedSrcRect.intersect(srcRect)) { clippedSrcRect.setEmpty(); } - if (kAlpha_8_SkColorType == ct) { + if (kAlpha_8_SkColorType == bmpCT) { for (int by = 0; by < bh; ++by) { for (int bx = 0; bx < bw; ++bx) { int devx = bx + srcRect.fLeft; @@ -192,11 +192,14 @@ static bool check_read(skiatest::Reporter* reporter, const uint8_t* alpha = bitmap.getAddr8(bx, by); if (clippedSrcRect.contains(devx, devy)) { - if (checkCanvasPixels) { - uint8_t canvasAlpha = SkGetPackedA32(get_src_color(devx, devy)); - if (canvasAlpha != *alpha) { - ERRORF(reporter, "Expected readback alpha (%d, %d) value 0x%02x, got 0x%02x. ", - bx, by, canvasAlpha, *alpha); + if (checkSurfacePixels) { + uint8_t surfaceAlpha = (surfaceAlphaType == kOpaque_SkAlphaType) + ? 0xFF + : SkGetPackedA32(get_src_color(devx, devy)); + if (surfaceAlpha != *alpha) { + ERRORF(reporter, + "Expected readback alpha (%d, %d) value 0x%02x, got 0x%02x. ", + bx, by, surfaceAlpha, *alpha); return false; } } @@ -220,13 +223,18 @@ static bool check_read(skiatest::Reporter* reporter, const uint32_t* pixel = bitmap.getAddr32(bx, by); if (clippedSrcRect.contains(devx, devy)) { - if (checkCanvasPixels) { - SkPMColor canvasPixel = get_src_color(devx, devy); + if (checkSurfacePixels) { + SkPMColor surfacePMColor = get_src_color(devx, devy); + if (kOpaque_SkAlphaType == surfaceAlphaType || kOpaque_SkAlphaType == bmpAT) { + surfacePMColor |= 0xFF000000; + } bool didPremul; - SkPMColor pmPixel = convert_to_pmcolor(ct, at, pixel, &didPremul); - if (!check_read_pixel(pmPixel, canvasPixel, didPremul)) { - ERRORF(reporter, "Expected readback pixel (%d, %d) value 0x%08x, got 0x%08x. " - "Readback was unpremul: %d", bx, by, canvasPixel, pmPixel, didPremul); + SkPMColor pmPixel = convert_to_pmcolor(bmpCT, bmpAT, pixel, &didPremul); + if (!check_read_pixel(pmPixel, surfacePMColor, didPremul)) { + ERRORF(reporter, + "Expected readback pixel (%d, %d) value 0x%08x, got 0x%08x. " + "Readback was unpremul: %d", + bx, by, surfacePMColor, pmPixel, didPremul); return false; } } @@ -288,11 +296,12 @@ static const struct { SkColorType fColorType; SkAlphaType fAlphaType; } gReadPixelsConfigs[] = { - { kRGBA_8888_SkColorType, kPremul_SkAlphaType }, - { kRGBA_8888_SkColorType, kUnpremul_SkAlphaType }, - { kBGRA_8888_SkColorType, kPremul_SkAlphaType }, - { kBGRA_8888_SkColorType, kUnpremul_SkAlphaType }, - { kAlpha_8_SkColorType, kPremul_SkAlphaType }, + {kRGBA_8888_SkColorType, kPremul_SkAlphaType}, + {kRGBA_8888_SkColorType, kUnpremul_SkAlphaType}, + {kRGB_888x_SkColorType, kOpaque_SkAlphaType}, + {kBGRA_8888_SkColorType, kPremul_SkAlphaType}, + {kBGRA_8888_SkColorType, kUnpremul_SkAlphaType}, + {kAlpha_8_SkColorType, kPremul_SkAlphaType}, }; const SkIRect gReadPixelsTestRects[] = { // entire thing @@ -341,8 +350,58 @@ const SkIRect gReadPixelsTestRects[] = { SkIRect::MakeLTRB(3 * DEV_W / 4, -10, DEV_W + 10, DEV_H + 10), }; +enum class ReadSuccessExpectation { + kNo, + kMaybe, + kYes, +}; + +bool check_success_expectation(ReadSuccessExpectation expectation, bool actualSuccess) { + switch (expectation) { + case ReadSuccessExpectation::kMaybe: + return true; + case ReadSuccessExpectation::kNo: + return !actualSuccess; + case ReadSuccessExpectation::kYes: + return actualSuccess; + } + return false; +} + +ReadSuccessExpectation read_should_succeed(const SkIRect& srcRect, const SkImageInfo& dstInfo, + const SkImageInfo& srcInfo, bool isGPU) { + if (!SkIRect::Intersects(srcRect, DEV_RECT)) { + return ReadSuccessExpectation::kNo; + } + if (!SkImageInfoValidConversion(dstInfo, srcInfo)) { + return ReadSuccessExpectation::kNo; + } + if (!isGPU) { + return ReadSuccessExpectation::kYes; + } + // This serves more as documentation of what currently works on the GPU rather than desired + // expectations. Once we make GrSurfaceContext color/alpha type aware and clean up some read + // pixels code we will make more scenarios work. + + // The GPU code current only does the premul->unpremul conversion, not the reverse. + if (srcInfo.alphaType() == kUnpremul_SkAlphaType && + dstInfo.alphaType() == kPremul_SkAlphaType) { + return ReadSuccessExpectation::kNo; + } + // We don't currently require reading alpha-only surfaces to succeed because of some pessimistic + // caps decisions and alpha/red complexity in GL. + if (SkColorTypeIsAlphaOnly(srcInfo.colorType())) { + return ReadSuccessExpectation::kMaybe; + } + if (!SkColorTypeIsAlwaysOpaque(srcInfo.colorType()) && + SkColorTypeIsAlwaysOpaque(dstInfo.colorType())) { + return ReadSuccessExpectation::kNo; + } + return ReadSuccessExpectation::kYes; +} + static void test_readpixels(skiatest::Reporter* reporter, const sk_sp<SkSurface>& surface, - BitmapInit lastBitmapInit) { + const SkImageInfo& surfaceInfo, BitmapInit lastBitmapInit) { SkCanvas* canvas = surface->getCanvas(); fill_src_canvas(canvas); for (size_t rect = 0; rect < SK_ARRAY_COUNT(gReadPixelsTestRects); ++rect) { @@ -363,18 +422,21 @@ static void test_readpixels(skiatest::Reporter* reporter, const sk_sp<SkSurface> bool success = surface->readPixels(bmp, srcRect.fLeft, srcRect.fTop); uint32_t idAfter = surface->generationID(); - // we expect to succeed when the read isn't fully clipped - // out. - bool expectSuccess = SkIRect::Intersects(srcRect, DEV_RECT); + // we expect to succeed when the read isn't fully clipped out and the infos are + // compatible. + bool isGPU = SkToBool(surface->getCanvas()->getGrContext()); + auto expectSuccess = read_should_succeed(srcRect, bmp.info(), surfaceInfo, isGPU); // determine whether we expected the read to succeed. - REPORTER_ASSERT(reporter, success == expectSuccess); + REPORTER_ASSERT(reporter, check_success_expectation(expectSuccess, success), + "Read succeed=%d unexpectedly, src ct/at: %d/%d, dst ct/at: %d/%d", + success, surfaceInfo.colorType(), surfaceInfo.alphaType(), + bmp.info().colorType(), bmp.info().alphaType()); // read pixels should never change the gen id REPORTER_ASSERT(reporter, idBefore == idAfter); if (success || startsWithPixels) { - check_read(reporter, bmp, srcRect.fLeft, srcRect.fTop, - success, startsWithPixels, - gReadPixelsConfigs[c].fColorType, gReadPixelsConfigs[c].fAlphaType); + check_read(reporter, bmp, srcRect.fLeft, srcRect.fTop, success, + startsWithPixels, surfaceInfo.alphaType()); } else { // if we had no pixels beforehand and the readPixels // failed then our bitmap should still not have pixels @@ -384,11 +446,12 @@ static void test_readpixels(skiatest::Reporter* reporter, const sk_sp<SkSurface> } } } + DEF_TEST(ReadPixels, reporter) { const SkImageInfo info = SkImageInfo::MakeN32Premul(DEV_W, DEV_H); auto surface(SkSurface::MakeRaster(info)); // SW readback fails a premul check when reading back to an unaligned rowbytes. - test_readpixels(reporter, surface, kLastAligned_BitmapInit); + test_readpixels(reporter, surface, info, kLastAligned_BitmapInit); } #if SK_SUPPORT_GPU DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Gpu, reporter, ctxInfo) { @@ -399,18 +462,29 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Gpu, reporter, ctxInfo) { return; } - const SkImageInfo ii = SkImageInfo::MakeN32Premul(DEV_W, DEV_H); - for (auto& origin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) { - sk_sp<SkSurface> surface(SkSurface::MakeRenderTarget(ctxInfo.grContext(), SkBudgeted::kNo, - ii, 0, origin, nullptr)); - test_readpixels(reporter, surface, kLast_BitmapInit); + static const SkImageInfo kImageInfos[] = { + SkImageInfo::Make(DEV_W, DEV_H, kRGBA_8888_SkColorType, kPremul_SkAlphaType), + SkImageInfo::Make(DEV_W, DEV_H, kBGRA_8888_SkColorType, kPremul_SkAlphaType), + SkImageInfo::Make(DEV_W, DEV_H, kRGB_888x_SkColorType, kOpaque_SkAlphaType), + SkImageInfo::Make(DEV_W, DEV_H, kAlpha_8_SkColorType, kPremul_SkAlphaType), + }; + for (const auto& ii : kImageInfos) { + for (auto& origin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) { + sk_sp<SkSurface> surface(SkSurface::MakeRenderTarget( + ctxInfo.grContext(), SkBudgeted::kNo, ii, 0, origin, nullptr)); + if (!surface) { + continue; + } + test_readpixels(reporter, surface, ii, kLast_BitmapInit); + } } } #endif #if SK_SUPPORT_GPU static void test_readpixels_texture(skiatest::Reporter* reporter, - sk_sp<GrSurfaceContext> sContext) { + sk_sp<GrSurfaceContext> sContext, + const SkImageInfo& surfaceInfo) { for (size_t rect = 0; rect < SK_ARRAY_COUNT(gReadPixelsTestRects); ++rect) { const SkIRect& srcRect = gReadPixelsTestRects[rect]; for (BitmapInit bmi = kFirstBitmapInit; bmi <= kLast_BitmapInit; bmi = nextBMI(bmi)) { @@ -426,15 +500,32 @@ static void test_readpixels_texture(skiatest::Reporter* reporter, if (startsWithPixels) { fill_dst_bmp_with_init_data(&bmp); uint32_t flags = 0; + // TODO: These two hacks can go away when the surface context knows the alpha + // type. + // Tell the read to perform an unpremul step since it doesn't know alpha type. if (gReadPixelsConfigs[c].fAlphaType == kUnpremul_SkAlphaType) { flags = GrContextPriv::kUnpremul_PixelOpsFlag; } + // The surface context doesn't know that the src is opaque. We don't support + // converting non-opaque data to opaque during a read. + if (bmp.alphaType() == kOpaque_SkAlphaType && + surfaceInfo.alphaType() != kOpaque_SkAlphaType) { + continue; + } bool success = sContext->readPixels(bmp.info(), bmp.getPixels(), bmp.rowBytes(), srcRect.fLeft, srcRect.fTop, flags); - check_read(reporter, bmp, srcRect.fLeft, srcRect.fTop, - success, true, - gReadPixelsConfigs[c].fColorType, gReadPixelsConfigs[c].fAlphaType); + auto expectSuccess = + read_should_succeed(srcRect, bmp.info(), surfaceInfo, true); + REPORTER_ASSERT( + reporter, check_success_expectation(expectSuccess, success), + "Read succeed=%d unexpectedly, src ct/at: %d/%d, dst ct/at: %d/%d", + success, surfaceInfo.colorType(), surfaceInfo.alphaType(), + bmp.info().colorType(), bmp.info().alphaType()); + if (success) { + check_read(reporter, bmp, srcRect.fLeft, srcRect.fTop, success, true, + surfaceInfo.alphaType()); + } } } } @@ -460,7 +551,8 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) { bmp.rowBytes()); sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext( std::move(proxy)); - test_readpixels_texture(reporter, std::move(sContext)); + auto info = SkImageInfo::Make(DEV_W, DEV_H, kN32_SkColorType, kPremul_SkAlphaType); + test_readpixels_texture(reporter, std::move(sContext), info); } } } diff --git a/tests/VkUploadPixelsTests.cpp b/tests/VkUploadPixelsTests.cpp index acadf7dc1d..27d30bda65 100644 --- a/tests/VkUploadPixelsTests.cpp +++ b/tests/VkUploadPixelsTests.cpp @@ -67,7 +67,7 @@ void basic_texture_test(skiatest::Reporter* reporter, GrContext* context, SkColo if (proxy) { sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext(proxy); - SkImageInfo dstInfo = SkImageInfo::Make(kWidth, kHeight, ct, kOpaque_SkAlphaType); + SkImageInfo dstInfo = SkImageInfo::Make(kWidth, kHeight, ct, kPremul_SkAlphaType); bool result = sContext->readPixels(dstInfo, dstBuffer, 0, 0, 0); REPORTER_ASSERT(reporter, result); @@ -76,7 +76,7 @@ void basic_texture_test(skiatest::Reporter* reporter, GrContext* context, SkColo kWidth, kHeight)); - dstInfo = SkImageInfo::Make(10, 2, ct, kOpaque_SkAlphaType); + dstInfo = SkImageInfo::Make(10, 2, ct, kPremul_SkAlphaType); result = sContext->writePixels(dstInfo, srcBuffer, 0, 2, 10); REPORTER_ASSERT(reporter, result); @@ -97,7 +97,7 @@ void basic_texture_test(skiatest::Reporter* reporter, GrContext* context, SkColo if (proxy) { sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext(proxy); - SkImageInfo dstInfo = SkImageInfo::Make(kWidth, kHeight, ct, kOpaque_SkAlphaType); + SkImageInfo dstInfo = SkImageInfo::Make(kWidth, kHeight, ct, kPremul_SkAlphaType); bool result = sContext->readPixels(dstInfo, dstBuffer, 0, 0, 0); REPORTER_ASSERT(reporter, result); @@ -106,7 +106,7 @@ void basic_texture_test(skiatest::Reporter* reporter, GrContext* context, SkColo kWidth, kHeight)); - dstInfo = SkImageInfo::Make(4, 5, ct, kOpaque_SkAlphaType); + dstInfo = SkImageInfo::Make(4, 5, ct, kPremul_SkAlphaType); result = sContext->writePixels(dstInfo, srcBuffer, 0, 5, 4); REPORTER_ASSERT(reporter, result); diff --git a/tests/WritePixelsTest.cpp b/tests/WritePixelsTest.cpp index f00d092d03..1f5758184f 100644 --- a/tests/WritePixelsTest.cpp +++ b/tests/WritePixelsTest.cpp @@ -7,6 +7,7 @@ #include "SkCanvas.h" #include "SkColorData.h" +#include "SkImageInfoPriv.h" #include "SkMathPriv.h" #include "SkSurface.h" #include "Test.h" @@ -67,7 +68,8 @@ static uint32_t pack_color_type(SkColorType ct, U8CPU a, U8CPU r, U8CPU g, U8CPU result[2] = r; result[3] = a; break; - case kRGBA_8888_SkColorType: + case kRGBA_8888_SkColorType: // fallthrough + case kRGB_888x_SkColorType: result[0] = r; result[1] = g; result[2] = b; @@ -144,6 +146,7 @@ static SkPMColor convert_to_PMColor(SkColorType ct, SkAlphaType at, uint32_t col } switch (ct) { case kRGBA_8888_SkColorType: + case kRGB_888x_SkColorType: // fallthrough color = SkSwizzle_RGBA_to_PMColor(color); break; case kBGRA_8888_SkColorType: @@ -176,8 +179,32 @@ static bool check_pixel(SkPMColor a, SkPMColor b, bool didPremulConversion) { SkAbs32(aB - bB) <= 1; } -static bool check_write(skiatest::Reporter* reporter, SkSurface* surf, const SkBitmap& bitmap, - int writeX, int writeY) { +bool write_should_succeed(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo, bool isGPU) { + if (!SkImageInfoValidConversion(dstInfo, srcInfo)) { + return false; + } + if (!isGPU) { + return true; + } + // The GPU backend supports writing unpremul data to a premul dst but not vice versa. + if (srcInfo.alphaType() == kPremul_SkAlphaType && + dstInfo.alphaType() == kUnpremul_SkAlphaType) { + return false; + } + if (!SkColorTypeIsAlwaysOpaque(srcInfo.colorType()) && + SkColorTypeIsAlwaysOpaque(dstInfo.colorType())) { + return false; + } + // The source has no alpha value and the dst is only alpha + if (SkColorTypeIsAlwaysOpaque(srcInfo.colorType()) && + SkColorTypeIsAlphaOnly(dstInfo.colorType())) { + return false; + } + return true; +} + +static bool check_write(skiatest::Reporter* reporter, SkSurface* surf, SkAlphaType surfaceAlphaType, + const SkBitmap& bitmap, int writeX, int writeY) { size_t canvasRowBytes; const uint32_t* canvasPixels; @@ -215,6 +242,10 @@ static bool check_write(skiatest::Reporter* reporter, SkSurface* surf, const SkB bool mul = (kUnpremul_SkAlphaType == bmInfo.alphaType()); SkPMColor bmpPMColor = convert_to_PMColor(bmInfo.colorType(), bmInfo.alphaType(), bmpColor8888); + if (bmInfo.alphaType() == kOpaque_SkAlphaType || + surfaceAlphaType == kOpaque_SkAlphaType) { + bmpPMColor |= 0xFF000000; + } if (!check_pixel(bmpPMColor, canvasPixel, mul)) { ERRORF(reporter, "Expected canvas pixel at %d, %d to be 0x%08x, got 0x%08x. " "Write performed premul: %d", cx, cy, bmpPMColor, canvasPixel, mul); @@ -293,7 +324,8 @@ DEF_TEST(WritePixelsSurfaceGenID, reporter) { REPORTER_ASSERT(reporter, genID1 != genID2); } -static void test_write_pixels(skiatest::Reporter* reporter, SkSurface* surface) { +static void test_write_pixels(skiatest::Reporter* reporter, SkSurface* surface, + const SkImageInfo& surfaceInfo) { const SkIRect testRects[] = { // entire thing DEV_RECT, @@ -347,10 +379,11 @@ static void test_write_pixels(skiatest::Reporter* reporter, SkSurface* surface) SkColorType fColorType; SkAlphaType fAlphaType; } gSrcConfigs[] = { - { kRGBA_8888_SkColorType, kPremul_SkAlphaType }, - { kRGBA_8888_SkColorType, kUnpremul_SkAlphaType }, - { kBGRA_8888_SkColorType, kPremul_SkAlphaType }, - { kBGRA_8888_SkColorType, kUnpremul_SkAlphaType }, + {kRGBA_8888_SkColorType, kPremul_SkAlphaType}, + {kRGBA_8888_SkColorType, kUnpremul_SkAlphaType}, + {kRGB_888x_SkColorType, kOpaque_SkAlphaType}, + {kBGRA_8888_SkColorType, kPremul_SkAlphaType}, + {kBGRA_8888_SkColorType, kUnpremul_SkAlphaType}, }; for (size_t r = 0; r < SK_ARRAY_COUNT(testRects); ++r) { const SkIRect& rect = testRects[r]; @@ -359,6 +392,7 @@ static void test_write_pixels(skiatest::Reporter* reporter, SkSurface* surface) const SkColorType ct = gSrcConfigs[c].fColorType; const SkAlphaType at = gSrcConfigs[c].fAlphaType; + bool isGPU = SkToBool(surface->getCanvas()->getGrContext()); fill_surface(surface); SkBitmap bmp; REPORTER_ASSERT(reporter, setup_bitmap(&bmp, ct, at, rect.width(), @@ -369,19 +403,21 @@ static void test_write_pixels(skiatest::Reporter* reporter, SkSurface* surface) surface->writePixels(bmp, rect.fLeft, rect.fTop); uint32_t idAfter = surface->generationID(); - REPORTER_ASSERT(reporter, check_write(reporter, surface, bmp, - rect.fLeft, rect.fTop)); + REPORTER_ASSERT(reporter, check_write(reporter, surface, surfaceInfo.alphaType(), + bmp, rect.fLeft, rect.fTop)); // we should change the genID iff pixels were actually written. SkIRect canvasRect = SkIRect::MakeSize(canvas->getBaseLayerSize()); SkIRect writeRect = SkIRect::MakeXYWH(rect.fLeft, rect.fTop, bmp.width(), bmp.height()); - bool intersects = SkIRect::Intersects(canvasRect, writeRect) ; - REPORTER_ASSERT(reporter, intersects == (idBefore != idAfter)); + bool expectSuccess = SkIRect::Intersects(canvasRect, writeRect) && + write_should_succeed(surfaceInfo, bmp.info(), isGPU); + REPORTER_ASSERT(reporter, expectSuccess == (idBefore != idAfter)); } } } } + DEF_TEST(WritePixels, reporter) { const SkImageInfo info = SkImageInfo::MakeN32Premul(DEV_W, DEV_H); for (auto& tightRowBytes : { true, false }) { @@ -394,9 +430,10 @@ DEF_TEST(WritePixels, reporter) { } auto surface(SkSurface::MakeRasterDirectReleaseProc(info, pixels, rowBytes, free_pixels, nullptr)); - test_write_pixels(reporter, surface.get()); + test_write_pixels(reporter, surface.get(), info); } } + #if SK_SUPPORT_GPU static void test_write_pixels(skiatest::Reporter* reporter, GrContext* context, int sampleCnt) { const SkImageInfo ii = SkImageInfo::MakeN32Premul(DEV_W, DEV_H); @@ -405,7 +442,7 @@ static void test_write_pixels(skiatest::Reporter* reporter, GrContext* context, SkBudgeted::kNo, ii, sampleCnt, origin, nullptr)); if (surface) { - test_write_pixels(reporter, surface.get()); + test_write_pixels(reporter, surface.get(), ii); } } } @@ -425,11 +462,15 @@ static void test_write_pixels_non_texture(skiatest::Reporter* reporter, GrContex for (auto& origin : { kTopLeft_GrSurfaceOrigin, kBottomLeft_GrSurfaceOrigin }) { GrBackendTexture backendTex = gpu->createTestingOnlyBackendTexture( nullptr, DEV_W, DEV_H, kSkia8888_GrPixelConfig, true, GrMipMapped::kNo); + if (!backendTex.isValid()) { + continue; + } SkColorType colorType = kN32_SkColorType; sk_sp<SkSurface> surface(SkSurface::MakeFromBackendTextureAsRenderTarget( context, backendTex, origin, sampleCnt, colorType, nullptr, nullptr)); if (surface) { - test_write_pixels(reporter, surface.get()); + auto ii = SkImageInfo::MakeN32Premul(DEV_W, DEV_H); + test_write_pixels(reporter, surface.get(), ii); } gpu->deleteTestingOnlyBackendTexture(backendTex); } diff --git a/tools/flags/SkCommonFlagsConfig.cpp b/tools/flags/SkCommonFlagsConfig.cpp index b8fd4190a6..934fe146e9 100644 --- a/tools/flags/SkCommonFlagsConfig.cpp +++ b/tools/flags/SkCommonFlagsConfig.cpp @@ -51,10 +51,12 @@ static const struct { { "gl4444", "gpu", "api=gl,color=4444" }, { "gl565", "gpu", "api=gl,color=565" }, { "glf16", "gpu", "api=gl,color=f16" }, + { "gl888x", "gpu", "api=gl,color=888x" }, { "gl1010102", "gpu", "api=gl,color=1010102" }, { "glsrgb", "gpu", "api=gl,color=srgb" }, { "glsrgbnl", "gpu", "api=gl,color=srgbnl" }, { "glesf16", "gpu", "api=gles,color=f16" }, + { "gles888x", "gpu", "api=gles,color=888x" }, { "gles1010102", "gpu", "api=gles,color=1010102" }, { "glessrgb", "gpu", "api=gles,color=srgb" }, { "glessrgbnl", "gpu", "api=gles,color=srgbnl" }, @@ -148,6 +150,7 @@ static const char configExtendedHelp[] = "\t Select framebuffer color format.\n" "\t Options:\n" "\t\t8888\t\t\tLinear 8888.\n" + "\t\t888x\t\t\tLinear 888x.\n" "\t\t4444\t\t\tLinear 4444.\n" "\t\t565\t\t\tLinear 565.\n" "\t\tf16{_gamut}\t\tLinear 16-bit floating point.\n" @@ -296,6 +299,10 @@ static bool parse_option_gpu_color(const SkString& value, *outColorType = kRGBA_8888_SkColorType; *outColorSpace = nullptr; return true; + } else if (value.equals("888x")) { + *outColorType = kRGB_888x_SkColorType; + *outColorSpace = nullptr; + return true; } else if (value.equals("4444")) { *outColorType = kARGB_4444_SkColorType; *outColorSpace = nullptr; |