From 1950e0a868774330330555a9a368992218f42240 Mon Sep 17 00:00:00 2001 From: Matt Sarett Date: Mon, 12 Jun 2017 16:17:30 -0400 Subject: Always encode images with sRGB encoded pixels Bug: skia: Change-Id: Icb25bc21a30e88f21df5b0e267d5a3a05535e44a Reviewed-on: https://skia-review.googlesource.com/19544 Commit-Queue: Matt Sarett Commit-Queue: Mike Klein Reviewed-by: Mike Klein --- src/images/SkImageEncoderFns.h | 18 ++++++++++++++++-- src/images/SkJpegEncoder.cpp | 36 +++++++++++++----------------------- src/images/SkPngEncoder.cpp | 20 +++++++++----------- src/images/SkWebpEncoder.cpp | 11 +---------- 4 files changed, 39 insertions(+), 46 deletions(-) (limited to 'src/images') diff --git a/src/images/SkImageEncoderFns.h b/src/images/SkImageEncoderFns.h index 6a917d0db6..6012a45697 100644 --- a/src/images/SkImageEncoderFns.h +++ b/src/images/SkImageEncoderFns.h @@ -15,6 +15,7 @@ #include "SkBitmap.h" #include "SkColor.h" #include "SkColorPriv.h" +#include "SkColorSpace_Base.h" #include "SkICC.h" #include "SkOpts.h" #include "SkPreConfig.h" @@ -256,6 +257,7 @@ static inline void transform_scanline_F16(char* SK_RESTRICT dst, const char* SK_ int width, int, const SkPMColor*) { SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, (const void**) &src); + p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, (void**) &dst); p.run(0,0, width); } @@ -268,6 +270,7 @@ static inline void transform_scanline_F16_premul(char* SK_RESTRICT dst, const ch SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, (const void**) &src); p.append(SkRasterPipeline::unpremul); + p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, (void**) &dst); p.run(0,0, width); } @@ -312,10 +315,21 @@ static inline void transform_scanline_F16_to_premul_8888(char* SK_RESTRICT dst, p.run(0,0, width); } -static inline sk_sp icc_from_color_space(const SkColorSpace& cs) { +static inline sk_sp icc_from_color_space(const SkImageInfo& info) { + SkColorSpace* cs = info.colorSpace(); + if (!cs) { + return nullptr; + } + + sk_sp owned; + if (kRGBA_F16_SkColorType == info.colorType()) { + owned = as_CSB(cs)->makeSRGBGamma(); + cs = owned.get(); + } + SkColorSpaceTransferFn fn; SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor); - if (cs.isNumericalTransferFn(&fn) && cs.toXYZD50(&toXYZD50)) { + if (cs->isNumericalTransferFn(&fn) && cs->toXYZD50(&toXYZD50)) { return SkICC::WriteToICC(fn, toXYZD50); } diff --git a/src/images/SkJpegEncoder.cpp b/src/images/SkJpegEncoder.cpp index eaea77e0e0..a0ee398615 100644 --- a/src/images/SkJpegEncoder.cpp +++ b/src/images/SkJpegEncoder.cpp @@ -207,29 +207,19 @@ std::unique_ptr SkJpegEncoder::Make(SkWStream* dst, const SkPixmap& s jpeg_set_quality(encoderMgr->cinfo(), options.fQuality, TRUE); jpeg_start_compress(encoderMgr->cinfo(), TRUE); - if (SkColorSpace* cs = src.colorSpace()) { - sk_sp owned; - if (src.colorType() == kRGBA_F16_SkColorType) { - // We'll be converting to 8-bit sRGB, so we'd better tag it that way. - owned = as_CSB(src.colorSpace())->makeSRGBGamma(); - cs = owned.get(); - } - - sk_sp icc = icc_from_color_space(*cs); - if (icc) { - // Create a contiguous block of memory with the icc signature followed by the profile. - sk_sp markerData = - SkData::MakeUninitialized(kICCMarkerHeaderSize + icc->size()); - uint8_t* ptr = (uint8_t*) markerData->writable_data(); - memcpy(ptr, kICCSig, sizeof(kICCSig)); - ptr += sizeof(kICCSig); - *ptr++ = 1; // This is the first marker. - *ptr++ = 1; // Out of one total markers. - memcpy(ptr, icc->data(), icc->size()); - - jpeg_write_marker(encoderMgr->cinfo(), kICCMarker, markerData->bytes(), - markerData->size()); - } + sk_sp icc = icc_from_color_space(src.info()); + if (icc) { + // Create a contiguous block of memory with the icc signature followed by the profile. + sk_sp markerData = + SkData::MakeUninitialized(kICCMarkerHeaderSize + icc->size()); + uint8_t* ptr = (uint8_t*) markerData->writable_data(); + memcpy(ptr, kICCSig, sizeof(kICCSig)); + ptr += sizeof(kICCSig); + *ptr++ = 1; // This is the first marker. + *ptr++ = 1; // Out of one total markers. + memcpy(ptr, icc->data(), icc->size()); + + jpeg_write_marker(encoderMgr->cinfo(), kICCMarker, markerData->bytes(), markerData->size()); } return std::unique_ptr(new SkJpegEncoder(std::move(encoderMgr), src)); diff --git a/src/images/SkPngEncoder.cpp b/src/images/SkPngEncoder.cpp index cc925207b1..b0c2b23dfb 100644 --- a/src/images/SkPngEncoder.cpp +++ b/src/images/SkPngEncoder.cpp @@ -53,7 +53,7 @@ public: bool setHeader(const SkImageInfo& srcInfo, const SkPngEncoder::Options& options); bool setPalette(const SkImageInfo& srcInfo, SkColorTable* colorTable, SkTransferFunctionBehavior); - bool setColorSpace(SkColorSpace* colorSpace); + bool setColorSpace(const SkImageInfo& info); bool writeInfo(const SkImageInfo& srcInfo); void chooseProc(const SkImageInfo& srcInfo, SkTransferFunctionBehavior unpremulBehavior); @@ -355,8 +355,8 @@ bool SkPngEncoderMgr::setPalette(const SkImageInfo& srcInfo, SkColorTable* color return true; } -static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkColorSpace& colorSpace) { - sk_sp icc = icc_from_color_space(colorSpace); +static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkImageInfo& info) { + sk_sp icc = icc_from_color_space(info); if (!icc) { return; } @@ -372,17 +372,15 @@ static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkColorSpace& png_set_iCCP(png_ptr, info_ptr, name, 0, iccPtr, icc->size()); } -bool SkPngEncoderMgr::setColorSpace(SkColorSpace* colorSpace) { +bool SkPngEncoderMgr::setColorSpace(const SkImageInfo& info) { if (setjmp(png_jmpbuf(fPngPtr))) { return false; } - if (colorSpace) { - if (colorSpace->isSRGB()) { - png_set_sRGB(fPngPtr, fInfoPtr, PNG_sRGB_INTENT_PERCEPTUAL); - } else { - set_icc(fPngPtr, fInfoPtr, *colorSpace); - } + if (info.colorSpace() && info.colorSpace()->isSRGB()) { + png_set_sRGB(fPngPtr, fInfoPtr, PNG_sRGB_INTENT_PERCEPTUAL); + } else { + set_icc(fPngPtr, fInfoPtr, info); } return true; @@ -429,7 +427,7 @@ std::unique_ptr SkPngEncoder::Make(SkWStream* dst, const SkPixmap& sr return nullptr; } - if (!encoderMgr->setColorSpace(src.colorSpace())) { + if (!encoderMgr->setColorSpace(src.info())) { return nullptr; } diff --git a/src/images/SkWebpEncoder.cpp b/src/images/SkWebpEncoder.cpp index 81ed7e1845..296d4f4cf0 100644 --- a/src/images/SkWebpEncoder.cpp +++ b/src/images/SkWebpEncoder.cpp @@ -194,16 +194,7 @@ bool SkWebpEncoder::Encode(SkWStream* stream, const SkPixmap& pixmap, const Opti // If there is no need to embed an ICC profile, we write directly to the input stream. // Otherwise, we will first encode to |tmp| and use a mux to add the ICC chunk. libwebp // forces us to have an encoded image before we can add a profile. - sk_sp icc; - if (SkColorSpace* cs = pixmap.colorSpace()) { - sk_sp owned; - if (pixmap.colorType() == kRGBA_F16_SkColorType) { - // We'll be converting to 8-bit sRGB, so we'd better tag it that way. - owned = as_CSB(pixmap.colorSpace())->makeSRGBGamma(); - cs = owned.get(); - } - icc = icc_from_color_space(*cs); - } + sk_sp icc = icc_from_color_space(pixmap.info()); SkDynamicMemoryWStream tmp; pic.custom_ptr = icc ? (void*)&tmp : (void*)stream; -- cgit v1.2.3