diff options
author | Brian Osman <brianosman@google.com> | 2018-07-12 14:44:27 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2018-07-12 20:54:14 +0000 |
commit | b62f50cf763279fa0d0aa2f80624de02c7a1c2fb (patch) | |
tree | a4aba5466ff7e3136bd97c35718a678354b430ea /src/images | |
parent | 5bc4fc8f3f26e9e14cccee4d200309e4b500b8d3 (diff) |
Replace nearly all kRespect with kIgnore
- Encoders and decoders always assume kIgnore.
- They are less opinionated about F16 and color space,
we just trust the color space that's passed in, and
put that directly in the image (no sRGB encoding).
- SkBitmap and SkPixmap read/write pixels functions were
defaulting to kResepct, those are now always kIgnore.
- Many other bits of plumbing are simplified, and I
added a default of kIgnore to SkImage::makeColorSpace,
so we can phase out that argument entirely.
- Still need to add defaults to other public APIs that
take SkTransferFunctionBehavior.
- This makes gold think that we've dramatically changed
the contents of all F16 images, but that's because
it doesn't understand the (now linear) color space
that's embedded. Once we triage them all once, they
will work fine (and they'll look perfect in the browser).
Bug: skia:
Change-Id: I62fa090f96cae1b67d181ce14bd91f34ff2ed747
Reviewed-on: https://skia-review.googlesource.com/140570
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Diffstat (limited to 'src/images')
-rw-r--r-- | src/images/SkImageEncoder.cpp | 3 | ||||
-rw-r--r-- | src/images/SkImageEncoderFns.h | 65 | ||||
-rw-r--r-- | src/images/SkJpegEncoder.cpp | 13 | ||||
-rw-r--r-- | src/images/SkPngEncoder.cpp | 20 | ||||
-rw-r--r-- | src/images/SkWebpEncoder.cpp | 13 |
5 files changed, 13 insertions, 101 deletions
diff --git a/src/images/SkImageEncoder.cpp b/src/images/SkImageEncoder.cpp index 7472a02f51..946a67f231 100644 --- a/src/images/SkImageEncoder.cpp +++ b/src/images/SkImageEncoder.cpp @@ -40,19 +40,16 @@ bool SkEncodeImage(SkWStream* dst, const SkPixmap& src, case SkEncodedImageFormat::kJPEG: { SkJpegEncoder::Options opts; opts.fQuality = quality; - opts.fBlendBehavior = SkTransferFunctionBehavior::kIgnore; return SkJpegEncoder::Encode(dst, src, opts); } case SkEncodedImageFormat::kPNG: { SkPngEncoder::Options opts; - opts.fUnpremulBehavior = SkTransferFunctionBehavior::kIgnore; return SkPngEncoder::Encode(dst, src, opts); } case SkEncodedImageFormat::kWEBP: { SkWebpEncoder::Options opts; opts.fCompression = SkWebpEncoder::Compression::kLossy; opts.fQuality = quality; - opts.fUnpremulBehavior = SkTransferFunctionBehavior::kIgnore; return SkWebpEncoder::Encode(dst, src, opts); } default: diff --git a/src/images/SkImageEncoderFns.h b/src/images/SkImageEncoderFns.h index 6bf0081b26..cd9972952d 100644 --- a/src/images/SkImageEncoderFns.h +++ b/src/images/SkImageEncoderFns.h @@ -163,24 +163,6 @@ static inline void transform_scanline_bgrA(char* SK_RESTRICT dst, const char* SK SkUnpremultiplyRow<true>((uint32_t*) dst, (const uint32_t*) src, width); } -template <bool kIsRGBA> -static inline void transform_scanline_unpremultiply_sRGB(void* dst, const void* src, int width) { - SkJumper_MemoryCtx src_ctx = { (void*)src, 0 }, - dst_ctx = { (void*)dst, 0 }; - SkRasterPipeline_<256> p; - if (kIsRGBA) { - p.append(SkRasterPipeline::load_8888, &src_ctx); - } else { - p.append(SkRasterPipeline::load_bgra, &src_ctx); - } - - p.append(SkRasterPipeline::from_srgb); - p.append(SkRasterPipeline::unpremul); - p.append(SkRasterPipeline::to_srgb); - p.append(SkRasterPipeline::store_8888, &dst_ctx); - p.run(0,0, width,1); -} - /** * Premultiply RGBA to rgbA. */ @@ -191,39 +173,6 @@ static inline void transform_scanline_to_premul_legacy(char* SK_RESTRICT dst, } /** - * Premultiply RGBA to rgbA linearly. - */ -static inline void transform_scanline_to_premul_linear(char* SK_RESTRICT dst, - const char* SK_RESTRICT src, - int width, int, const SkPMColor*) { - SkJumper_MemoryCtx src_ctx = { (void*)src, 0 }, - dst_ctx = { (void*)dst, 0 }; - SkRasterPipeline_<256> p; - p.append(SkRasterPipeline::load_8888, &src_ctx); - p.append(SkRasterPipeline::from_srgb); - p.append(SkRasterPipeline::premul); - p.append(SkRasterPipeline::to_srgb); - p.append(SkRasterPipeline::store_8888, &dst_ctx); - p.run(0,0, width,1); -} - -/** - * Transform from kPremul, kRGBA_8888_SkColorType to 4-bytes-per-pixel unpremultiplied RGBA. - */ -static inline void transform_scanline_srgbA(char* SK_RESTRICT dst, const char* SK_RESTRICT src, - int width, int, const SkPMColor*) { - transform_scanline_unpremultiply_sRGB<true>(dst, src, width); -} - -/** - * Transform from kPremul, kBGRA_8888_SkColorType to 4-bytes-per-pixel unpremultiplied RGBA. - */ -static inline void transform_scanline_sbgrA(char* SK_RESTRICT dst, const char* SK_RESTRICT src, - int width, int, const SkPMColor*) { - transform_scanline_unpremultiply_sRGB<false>(dst, src, width); -} - -/** * Transform from kUnpremul, kBGRA_8888_SkColorType to 4-bytes-per-pixel unpremultiplied RGBA. */ static inline void transform_scanline_BGRA(char* SK_RESTRICT dst, const char* SK_RESTRICT src, @@ -336,7 +285,6 @@ static inline void transform_scanline_F16(char* SK_RESTRICT dst, const char* SK_ p.append(SkRasterPipeline::load_f16, &src_ctx); p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, &dst_ctx); p.run(0,0, width,1); } @@ -353,7 +301,6 @@ static inline void transform_scanline_F16_premul(char* SK_RESTRICT dst, const ch p.append(SkRasterPipeline::unpremul); p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, &dst_ctx); p.run(0,0, width,1); } @@ -370,7 +317,6 @@ static inline void transform_scanline_F16_to_8888(char* SK_RESTRICT dst, p.append(SkRasterPipeline::load_f16, &src_ctx); p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_8888, &dst_ctx); p.run(0,0, width,1); } @@ -388,7 +334,6 @@ static inline void transform_scanline_F16_premul_to_8888(char* SK_RESTRICT dst, p.append(SkRasterPipeline::unpremul); p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_8888, &dst_ctx); p.run(0,0, width,1); } @@ -405,7 +350,6 @@ static inline void transform_scanline_F16_to_premul_8888(char* SK_RESTRICT dst, p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); p.append(SkRasterPipeline::premul); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_8888, &dst_ctx); p.run(0,0, width,1); } @@ -421,7 +365,6 @@ static inline void transform_scanline_F32(char* SK_RESTRICT dst, const char* SK_ p.append(SkRasterPipeline::load_f32, &src_ctx); p.append(SkRasterPipeline::clamp_0); // F32 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, &dst_ctx); p.run(0,0, width,1); } @@ -438,7 +381,6 @@ static inline void transform_scanline_F32_premul(char* SK_RESTRICT dst, const ch p.append(SkRasterPipeline::unpremul); p.append(SkRasterPipeline::clamp_0); // F32 values may be out of [0,1] range, so clamp. p.append(SkRasterPipeline::clamp_1); - p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, &dst_ctx); p.run(0,0, width,1); } @@ -449,13 +391,6 @@ static inline sk_sp<SkData> icc_from_color_space(const SkImageInfo& info) { return nullptr; } - sk_sp<SkColorSpace> owned; - if (kRGBA_F16_SkColorType == info.colorType() || - kRGBA_F32_SkColorType == info.colorType()) { - owned = cs->makeSRGBGamma(); - cs = owned.get(); - } - SkColorSpaceTransferFn fn; SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor); if (cs->isNumericalTransferFn(&fn) && cs->toXYZD50(&toXYZD50)) { diff --git a/src/images/SkJpegEncoder.cpp b/src/images/SkJpegEncoder.cpp index 9c139f707b..1c1fb15e2e 100644 --- a/src/images/SkJpegEncoder.cpp +++ b/src/images/SkJpegEncoder.cpp @@ -74,15 +74,7 @@ bool SkJpegEncoderMgr::setParams(const SkImageInfo& srcInfo, const SkJpegEncoder return (transform_scanline_proc) nullptr; } - // Note that kRespect mode is only supported with sRGB or linear transfer functions. - // The legacy code path is incidentally correct when the transfer function is linear. - const bool isSRGBTransferFn = srcInfo.gammaCloseToSRGB() && - (SkTransferFunctionBehavior::kRespect == options.fBlendBehavior); - if (isSRGBTransferFn) { - return transform_scanline_to_premul_linear; - } else { - return transform_scanline_to_premul_legacy; - } + return transform_scanline_to_premul_legacy; }; J_COLOR_SPACE jpegColorType = JCS_EXT_RGBA; @@ -118,8 +110,7 @@ bool SkJpegEncoderMgr::setParams(const SkImageInfo& srcInfo, const SkJpegEncoder numComponents = 1; break; case kRGBA_F16_SkColorType: - if (!srcInfo.colorSpace() || - SkTransferFunctionBehavior::kRespect != options.fBlendBehavior) { + if (!srcInfo.colorSpace()) { return false; } diff --git a/src/images/SkPngEncoder.cpp b/src/images/SkPngEncoder.cpp index 7520b9044d..0ca2551735 100644 --- a/src/images/SkPngEncoder.cpp +++ b/src/images/SkPngEncoder.cpp @@ -55,7 +55,7 @@ public: bool setHeader(const SkImageInfo& srcInfo, const SkPngEncoder::Options& options); bool setColorSpace(const SkImageInfo& info); bool writeInfo(const SkImageInfo& srcInfo); - void chooseProc(const SkImageInfo& srcInfo, SkTransferFunctionBehavior unpremulBehavior); + void chooseProc(const SkImageInfo& srcInfo); png_structp pngPtr() { return fPngPtr; } png_infop infoPtr() { return fInfoPtr; } @@ -230,10 +230,7 @@ bool SkPngEncoderMgr::setHeader(const SkImageInfo& srcInfo, const SkPngEncoder:: return true; } -static transform_scanline_proc choose_proc(const SkImageInfo& info, - SkTransferFunctionBehavior unpremulBehavior) { - const bool isSRGBTransferFn = - (SkTransferFunctionBehavior::kRespect == unpremulBehavior) && info.gammaCloseToSRGB(); +static transform_scanline_proc choose_proc(const SkImageInfo& info) { switch (info.colorType()) { case kRGBA_8888_SkColorType: switch (info.alphaType()) { @@ -242,8 +239,7 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info, case kUnpremul_SkAlphaType: return transform_scanline_memcpy; case kPremul_SkAlphaType: - return isSRGBTransferFn ? transform_scanline_srgbA : - transform_scanline_rgbA; + return transform_scanline_rgbA; default: SkASSERT(false); return nullptr; @@ -255,8 +251,7 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info, case kUnpremul_SkAlphaType: return transform_scanline_BGRA; case kPremul_SkAlphaType: - return isSRGBTransferFn ? transform_scanline_sbgrA : - transform_scanline_bgrA; + return transform_scanline_bgrA; default: SkASSERT(false); return nullptr; @@ -369,9 +364,8 @@ bool SkPngEncoderMgr::writeInfo(const SkImageInfo& srcInfo) { return true; } -void SkPngEncoderMgr::chooseProc(const SkImageInfo& srcInfo, - SkTransferFunctionBehavior unpremulBehavior) { - fProc = choose_proc(srcInfo, unpremulBehavior); +void SkPngEncoderMgr::chooseProc(const SkImageInfo& srcInfo) { + fProc = choose_proc(srcInfo); } std::unique_ptr<SkEncoder> SkPngEncoder::Make(SkWStream* dst, const SkPixmap& src, @@ -397,7 +391,7 @@ std::unique_ptr<SkEncoder> SkPngEncoder::Make(SkWStream* dst, const SkPixmap& sr return nullptr; } - encoderMgr->chooseProc(src.info(), options.fUnpremulBehavior); + encoderMgr->chooseProc(src.info()); return std::unique_ptr<SkPngEncoder>(new SkPngEncoder(std::move(encoderMgr), src)); } diff --git a/src/images/SkWebpEncoder.cpp b/src/images/SkWebpEncoder.cpp index 2cb55d2f04..bda34dcb01 100644 --- a/src/images/SkWebpEncoder.cpp +++ b/src/images/SkWebpEncoder.cpp @@ -41,10 +41,7 @@ extern "C" { #include "webp/mux.h" } -static transform_scanline_proc choose_proc(const SkImageInfo& info, - SkTransferFunctionBehavior unpremulBehavior) { - const bool isSRGBTransferFn = - (SkTransferFunctionBehavior::kRespect == unpremulBehavior) && info.gammaCloseToSRGB(); +static transform_scanline_proc choose_proc(const SkImageInfo& info) { switch (info.colorType()) { case kRGBA_8888_SkColorType: switch (info.alphaType()) { @@ -53,8 +50,7 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info, case kUnpremul_SkAlphaType: return transform_scanline_memcpy; case kPremul_SkAlphaType: - return isSRGBTransferFn ? transform_scanline_srgbA : - transform_scanline_rgbA; + return transform_scanline_rgbA; default: return nullptr; } @@ -65,8 +61,7 @@ static transform_scanline_proc choose_proc(const SkImageInfo& info, case kUnpremul_SkAlphaType: return transform_scanline_BGRA; case kPremul_SkAlphaType: - return isSRGBTransferFn ? transform_scanline_sbgrA : - transform_scanline_bgrA; + return transform_scanline_bgrA; default: return nullptr; } @@ -113,7 +108,7 @@ bool SkWebpEncoder::Encode(SkWStream* stream, const SkPixmap& pixmap, const Opti return false; } - const transform_scanline_proc proc = choose_proc(pixmap.info(), opts.fUnpremulBehavior); + const transform_scanline_proc proc = choose_proc(pixmap.info()); if (!proc) { return false; } |