diff --git a/src/images/SkJPEGImageEncoder.cpp b/src/images/SkJPEGImageEncoder.cpp index 845673421c..764e9c43b0 100644 --- a/src/images/SkJPEGImageEncoder.cpp +++ b/src/images/SkJPEGImageEncoder.cpp @@ -82,22 +82,17 @@ static bool set_encode_config(J_COLOR_SPACE* jpegColorType, int* numComponents, } bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, const SkEncodeOptions& opts) { - SkASSERT(!pixmap.colorSpace() || pixmap.colorSpace()->gammaCloseToSRGB() || - pixmap.colorSpace()->gammaIsLinear()); - - SkPixmap src = pixmap; - if (SkTransferFunctionBehavior::kIgnore == opts.fUnpremulBehavior) { - src.setColorSpace(nullptr); - } else { - // kCorrect behavior requires a color space. It's not actually critical in the - // jpeg case (since jpegs are opaque), but Skia color correct behavior generally + if (SkTransferFunctionBehavior::kRespect == opts.fUnpremulBehavior) { + // Respecting the transfer function requries a color space. It's not actually critical + // in the jpeg case (since jpegs are opaque), but Skia color correct behavior generally // requires pixels to be tagged with color spaces. - if (!src.colorSpace()) { + if (!pixmap.colorSpace() || (!pixmap.colorSpace()->gammaCloseToSRGB() && + !pixmap.colorSpace()->gammaIsLinear())) { return false; } } - return SkEncodeImageAsJPEG(stream, src, 100); + return SkEncodeImageAsJPEG(stream, pixmap, 100); } bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, int quality) { diff --git a/src/images/SkPNGImageEncoder.cpp b/src/images/SkPNGImageEncoder.cpp index 9f5ebd27ab..b57c32a0fc 100644 --- a/src/images/SkPNGImageEncoder.cpp +++ b/src/images/SkPNGImageEncoder.cpp @@ -51,8 +51,10 @@ static void set_icc(png_structp png_ptr, png_infop info_ptr, sk_sp icc) png_set_iCCP(png_ptr, info_ptr, name, 0, iccPtr, icc->size()); } -static transform_scanline_proc choose_proc(const SkImageInfo& info) { - const bool isGammaEncoded = info.gammaCloseToSRGB(); +static transform_scanline_proc choose_proc(const SkImageInfo& info, + SkTransferFunctionBehavior unpremulBehavior) { + const bool isSRGBTransferFn = + (SkTransferFunctionBehavior::kRespect == unpremulBehavior) && info.gammaCloseToSRGB(); switch (info.colorType()) { case kRGBA_8888_SkColorType: switch (info.alphaType()) { @@ -61,8 +63,8 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info) { case kUnpremul_SkAlphaType: return transform_scanline_memcpy; case kPremul_SkAlphaType: - return isGammaEncoded ? transform_scanline_srgbA : - transform_scanline_rgbA; + return isSRGBTransferFn ? transform_scanline_srgbA : + transform_scanline_rgbA; default: SkASSERT(false); return nullptr; @@ -74,8 +76,8 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info) { case kUnpremul_SkAlphaType: return transform_scanline_BGRA; case kPremul_SkAlphaType: - return isGammaEncoded ? transform_scanline_sbgrA : - transform_scanline_bgrA; + return isSRGBTransferFn ? transform_scanline_sbgrA : + transform_scanline_bgrA; default: SkASSERT(false); return nullptr; @@ -118,14 +120,15 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info) { opaque, the return value will always be 0. */ static inline int pack_palette(SkColorTable* ctable, png_color* SK_RESTRICT palette, - png_byte* SK_RESTRICT alphas, const SkImageInfo& info) { + png_byte* SK_RESTRICT alphas, const SkImageInfo& info, + SkTransferFunctionBehavior unpremulBehavior) { const SkPMColor* colors = ctable->readColors(); const int count = ctable->count(); SkPMColor storage[256]; if (kPremul_SkAlphaType == info.alphaType()) { // Unpremultiply the colors. const SkImageInfo rgbaInfo = info.makeColorType(kRGBA_8888_SkColorType); - transform_scanline_proc proc = choose_proc(rgbaInfo); + transform_scanline_proc proc = choose_proc(rgbaInfo, unpremulBehavior); proc((char*) storage, (const char*) colors, ctable->count(), 4, nullptr); colors = storage; } @@ -174,17 +177,13 @@ static inline int pack_palette(SkColorTable* ctable, png_color* SK_RESTRICT pale return numWithAlpha; } -static bool do_encode(SkWStream*, const SkPixmap&, int, int, png_color_8&); +static bool do_encode(SkWStream*, const SkPixmap&, int, int, png_color_8&, + SkTransferFunctionBehavior unpremulBehavior); -bool SkEncodeImageAsPNG(SkWStream* stream, const SkPixmap& src, const SkEncodeOptions& opts) { - SkASSERT(!src.colorSpace() || src.colorSpace()->gammaCloseToSRGB() || - src.colorSpace()->gammaIsLinear()); - - SkPixmap pixmap = src; - if (SkTransferFunctionBehavior::kIgnore == opts.fUnpremulBehavior) { - pixmap.setColorSpace(nullptr); - } else { - if (!pixmap.colorSpace()) { +bool SkEncodeImageAsPNG(SkWStream* stream, const SkPixmap& pixmap, const SkEncodeOptions& opts) { + if (SkTransferFunctionBehavior::kRespect == opts.fUnpremulBehavior) { + if (!pixmap.colorSpace() || (!pixmap.colorSpace()->gammaCloseToSRGB() && + !pixmap.colorSpace()->gammaIsLinear())) { return false; } } @@ -276,7 +275,7 @@ bool SkEncodeImageAsPNG(SkWStream* stream, const SkPixmap& src, const SkEncodeOp // or 4 bit indices. } - return do_encode(stream, pixmap, pngColorType, bitDepth, sig_bit); + return do_encode(stream, pixmap, pngColorType, bitDepth, sig_bit, opts.fUnpremulBehavior); } static int num_components(int pngColorType) { @@ -294,8 +293,8 @@ static int num_components(int pngColorType) { } } -static bool do_encode(SkWStream* stream, const SkPixmap& pixmap, - int pngColorType, int bitDepth, png_color_8& sig_bit) { +static bool do_encode(SkWStream* stream, const SkPixmap& pixmap, int pngColorType, int bitDepth, + png_color_8& sig_bit, SkTransferFunctionBehavior unpremulBehavior) { png_structp png_ptr; png_infop info_ptr; @@ -340,7 +339,8 @@ static bool do_encode(SkWStream* stream, const SkPixmap& pixmap, if (kIndex_8_SkColorType == pixmap.colorType()) { SkColorTable* colorTable = pixmap.ctable(); SkASSERT(colorTable); - int numTrans = pack_palette(colorTable, paletteColors, trans, pixmap.info()); + int numTrans = pack_palette(colorTable, paletteColors, trans, pixmap.info(), + unpremulBehavior); png_set_PLTE(png_ptr, info_ptr, paletteColors, colorTable->count()); if (numTrans > 0) { png_set_tRNS(png_ptr, info_ptr, trans, numTrans, nullptr); @@ -371,7 +371,7 @@ static bool do_encode(SkWStream* stream, const SkPixmap& pixmap, SkAutoSTMalloc<1024, char> rowStorage(pixmap.width() * pngBytesPerPixel); char* storage = rowStorage.get(); const char* srcImage = (const char*)pixmap.addr(); - transform_scanline_proc proc = choose_proc(pixmap.info()); + transform_scanline_proc proc = choose_proc(pixmap.info(), unpremulBehavior); for (int y = 0; y < pixmap.height(); y++) { png_bytep row_ptr = (png_bytep)storage; proc(storage, srcImage, pixmap.width(), SkColorTypeBytesPerPixel(pixmap.colorType()), diff --git a/src/images/SkWEBPImageEncoder.cpp b/src/images/SkWEBPImageEncoder.cpp index 728dbc712a..c0cd4a6cae 100644 --- a/src/images/SkWEBPImageEncoder.cpp +++ b/src/images/SkWEBPImageEncoder.cpp @@ -40,8 +40,10 @@ extern "C" { #include "webp/mux.h" } -static transform_scanline_proc choose_proc(const SkImageInfo& info) { - const bool isGammaEncoded = info.gammaCloseToSRGB(); +static transform_scanline_proc choose_proc(const SkImageInfo& info, + SkTransferFunctionBehavior unpremulBehavior) { + const bool isSRGBTransferFn = + (SkTransferFunctionBehavior::kRespect == unpremulBehavior) && info.gammaCloseToSRGB(); switch (info.colorType()) { case kRGBA_8888_SkColorType: switch (info.alphaType()) { @@ -50,8 +52,8 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info) { case kUnpremul_SkAlphaType: return transform_scanline_memcpy; case kPremul_SkAlphaType: - return isGammaEncoded ? transform_scanline_srgbA : - transform_scanline_rgbA; + return isSRGBTransferFn ? transform_scanline_srgbA : + transform_scanline_rgbA; default: return nullptr; } @@ -62,8 +64,8 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info) { case kUnpremul_SkAlphaType: return transform_scanline_BGRA; case kPremul_SkAlphaType: - return isGammaEncoded ? transform_scanline_sbgrA : - transform_scanline_bgrA; + return isSRGBTransferFn ? transform_scanline_sbgrA : + transform_scanline_bgrA; default: return nullptr; } @@ -121,21 +123,16 @@ static int stream_writer(const uint8_t* data, size_t data_size, return stream->write(data, data_size) ? 1 : 0; } -static bool do_encode(SkWStream* stream, const SkPixmap& srcPixmap, const SkEncodeOptions& opts, +static bool do_encode(SkWStream* stream, const SkPixmap& pixmap, const SkEncodeOptions& opts, int quality) { - SkASSERT(!srcPixmap.colorSpace() || srcPixmap.colorSpace()->gammaCloseToSRGB() || - srcPixmap.colorSpace()->gammaIsLinear()); - - SkPixmap pixmap = srcPixmap; - if (SkTransferFunctionBehavior::kIgnore == opts.fUnpremulBehavior) { - pixmap.setColorSpace(nullptr); - } else { - if (!pixmap.colorSpace()) { + if (SkTransferFunctionBehavior::kRespect == opts.fUnpremulBehavior) { + if (!pixmap.colorSpace() || (!pixmap.colorSpace()->gammaCloseToSRGB() && + !pixmap.colorSpace()->gammaIsLinear())) { return false; } } - const transform_scanline_proc proc = choose_proc(pixmap.info()); + const transform_scanline_proc proc = choose_proc(pixmap.info(), opts.fUnpremulBehavior); if (!proc) { return false; } @@ -162,7 +159,7 @@ static bool do_encode(SkWStream* stream, const SkPixmap& srcPixmap, const SkEnco if (kPremul_SkAlphaType == pixmap.alphaType()) { // Unpremultiply the colors. const SkImageInfo rgbaInfo = pixmap.info().makeColorType(kRGBA_8888_SkColorType); - transform_scanline_proc proc = choose_proc(rgbaInfo); + transform_scanline_proc proc = choose_proc(rgbaInfo, opts.fUnpremulBehavior); proc((char*) storage, (const char*) colors, pixmap.ctable()->count(), 4, nullptr); colors = storage; } diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp index 3897878879..2c601fadb0 100644 --- a/tests/CodecTest.cpp +++ b/tests/CodecTest.cpp @@ -1547,7 +1547,8 @@ static void encode_format(SkDynamicMemoryWStream* stream, const SkPixmap& pixmap } } -static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format) { +static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format, + SkTransferFunctionBehavior unpremulBehavior) { // Test with sRGB color space. SkBitmap srgbBitmap; SkImageInfo srgbInfo = SkImageInfo::MakeS32(1, 1, kOpaque_SkAlphaType); @@ -1557,7 +1558,7 @@ static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format) srgbBitmap.peekPixels(&pixmap); SkDynamicMemoryWStream srgbBuf; SkEncodeOptions opts; - opts.fUnpremulBehavior = SkTransferFunctionBehavior::kRespect; + opts.fUnpremulBehavior = unpremulBehavior; encode_format(&srgbBuf, pixmap, opts, format); sk_sp srgbData = srgbBuf.detachAsData(); std::unique_ptr srgbCodec(SkCodec::NewFromData(srgbData)); @@ -1587,7 +1588,10 @@ static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format) } DEF_TEST(Codec_EncodeICC, r) { - test_encode_icc(r, SkEncodedImageFormat::kPNG); - test_encode_icc(r, SkEncodedImageFormat::kJPEG); - test_encode_icc(r, SkEncodedImageFormat::kWEBP); + test_encode_icc(r, SkEncodedImageFormat::kPNG, SkTransferFunctionBehavior::kRespect); + test_encode_icc(r, SkEncodedImageFormat::kJPEG, SkTransferFunctionBehavior::kRespect); + test_encode_icc(r, SkEncodedImageFormat::kWEBP, SkTransferFunctionBehavior::kRespect); + test_encode_icc(r, SkEncodedImageFormat::kPNG, SkTransferFunctionBehavior::kIgnore); + test_encode_icc(r, SkEncodedImageFormat::kJPEG, SkTransferFunctionBehavior::kIgnore); + test_encode_icc(r, SkEncodedImageFormat::kWEBP, SkTransferFunctionBehavior::kIgnore); }