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>
diff --git a/src/images/SkImageEncoder.cpp b/src/images/SkImageEncoder.cpp
index 7472a02..946a67f 100644
--- a/src/images/SkImageEncoder.cpp
+++ b/src/images/SkImageEncoder.cpp
@@ -40,19 +40,16 @@
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 6bf0081..cd99729 100644
--- a/src/images/SkImageEncoderFns.h
+++ b/src/images/SkImageEncoderFns.h
@@ -163,24 +163,6 @@
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 @@
}
/**
- * 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 9c139f7..1c1fb15 100644
--- a/src/images/SkJpegEncoder.cpp
+++ b/src/images/SkJpegEncoder.cpp
@@ -74,15 +74,7 @@
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 @@
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 7520b90..0ca2551 100644
--- a/src/images/SkPngEncoder.cpp
+++ b/src/images/SkPngEncoder.cpp
@@ -55,7 +55,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 2cb55d2..bda34dc 100644
--- a/src/images/SkWebpEncoder.cpp
+++ b/src/images/SkWebpEncoder.cpp
@@ -41,10 +41,7 @@
#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 @@
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 @@
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 @@
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;
}