clean up legacy sRGB stages
Nothing terribly interesting.
Change-Id: I8956c5bc3fa9098337bad8fa143de1d95f987f9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291655
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/src/core/SkBlitter_Sprite.cpp b/src/core/SkBlitter_Sprite.cpp
index df7d9a7..af6222b 100644
--- a/src/core/SkBlitter_Sprite.cpp
+++ b/src/core/SkBlitter_Sprite.cpp
@@ -134,7 +134,7 @@
: kPremul_SkAlphaType;
fAlloc->make<SkColorSpaceXformSteps>(srcCS, srcAT,
dstCS, kPremul_SkAlphaType)
- ->apply(&p, fSource.colorType());
+ ->apply(&p);
}
if (fPaintColor.fA != 1.0f) {
p.append(SkRasterPipeline::scale_1_float, &fPaintColor.fA);
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index e3af3f4..3731cc5 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -203,9 +203,7 @@
rec.fPipeline->append(SkRasterPipeline::unpremul);
}
- // TODO: is it valuable to thread this through appendStages()?
- bool shaderIsNormalized = false;
- fSteps.apply(rec.fPipeline, shaderIsNormalized);
+ fSteps.apply(rec.fPipeline);
if (!shaderIsOpaque) {
rec.fPipeline->append(SkRasterPipeline::premul);
diff --git a/src/core/SkColorSpaceXformSteps.cpp b/src/core/SkColorSpaceXformSteps.cpp
index d4eed8d..1d90dc2 100644
--- a/src/core/SkColorSpaceXformSteps.cpp
+++ b/src/core/SkColorSpaceXformSteps.cpp
@@ -65,9 +65,6 @@
src-> transferFn(&this->srcTF );
dst->invTransferFn(&this->dstTFInv);
- this->srcTF_is_sRGB = src->gammaCloseToSRGB();
- this->dstTF_is_sRGB = dst->gammaCloseToSRGB();
-
// If we linearize then immediately reencode with the same transfer function, skip both.
if ( this->flags.linearize &&
!this->flags.gamut_transform &&
@@ -131,29 +128,12 @@
}
}
-void SkColorSpaceXformSteps::apply(SkRasterPipeline* p, bool src_is_normalized) const {
-#ifndef SK_LEGACY_SRGB_STAGES
- src_is_normalized = false;
-#endif
- if (flags.unpremul) { p->append(SkRasterPipeline::unpremul); }
- if (flags.linearize) {
- if (src_is_normalized && srcTF_is_sRGB) {
- p->append(SkRasterPipeline::from_srgb);
- } else {
- p->append_transfer_function(srcTF);
- }
- }
- if (flags.gamut_transform) {
- p->append(SkRasterPipeline::matrix_3x3, &src_to_dst_matrix);
- }
- if (flags.encode) {
- if (src_is_normalized && dstTF_is_sRGB) {
- p->append(SkRasterPipeline::to_srgb);
- } else {
- p->append_transfer_function(dstTFInv);
- }
- }
- if (flags.premul) { p->append(SkRasterPipeline::premul); }
+void SkColorSpaceXformSteps::apply(SkRasterPipeline* p) const {
+ if (flags.unpremul) { p->append(SkRasterPipeline::unpremul); }
+ if (flags.linearize) { p->append_transfer_function(srcTF); }
+ if (flags.gamut_transform) { p->append(SkRasterPipeline::matrix_3x3, &src_to_dst_matrix); }
+ if (flags.encode) { p->append_transfer_function(dstTFInv); }
+ if (flags.premul) { p->append(SkRasterPipeline::premul); }
}
skvm::Color sk_program_transfer_fn(skvm::Builder* p, skvm::Uniforms* uniforms,
diff --git a/src/core/SkColorSpaceXformSteps.h b/src/core/SkColorSpaceXformSteps.h
index 7d15259..03c9aab 100644
--- a/src/core/SkColorSpaceXformSteps.h
+++ b/src/core/SkColorSpaceXformSteps.h
@@ -42,17 +42,11 @@
dst.colorSpace(), dst.alphaType()) {}
void apply(float rgba[4]) const;
- void apply(SkRasterPipeline*, bool src_is_normalized) const;
+ void apply(SkRasterPipeline*) const;
skvm::Color program(skvm::Builder*, skvm::Uniforms*, skvm::Color) const;
- void apply(SkRasterPipeline* p, SkColorType srcCT) const {
- return this->apply(p, SkColorTypeIsNormalized(srcCT));
- }
-
Flags flags;
- bool srcTF_is_sRGB,
- dstTF_is_sRGB;
skcms_TransferFunction srcTF, // Apply for linearize.
dstTFInv; // Apply for encode.
float src_to_dst_matrix[9]; // Apply this 3x3 column-major matrix for gamut_transform.
diff --git a/src/core/SkConvertPixels.cpp b/src/core/SkConvertPixels.cpp
index 2eb840c..c3dd1d2 100644
--- a/src/core/SkConvertPixels.cpp
+++ b/src/core/SkConvertPixels.cpp
@@ -211,7 +211,7 @@
SkRasterPipeline_<256> pipeline;
pipeline.append_load(srcInfo.colorType(), &src);
- steps.apply(&pipeline, srcInfo.colorType());
+ steps.apply(&pipeline);
pipeline.append_gamut_clamp_if_normalized(dstInfo);
diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h
index 316e884..04d677b 100644
--- a/src/core/SkRasterPipeline.h
+++ b/src/core/SkRasterPipeline.h
@@ -44,7 +44,6 @@
M(unpremul) M(premul) M(premul_dst) \
M(force_opaque) M(force_opaque_dst) \
M(set_rgb) M(unbounded_set_rgb) M(swap_rb) M(swap_rb_dst) \
- M(from_srgb) M(to_srgb) \
M(black_color) M(white_color) \
M(uniform_color) M(unbounded_uniform_color) M(uniform_color_dst) \
M(seed_shader) M(dither) \
diff --git a/src/gpu/GrDataUtils.cpp b/src/gpu/GrDataUtils.cpp
index 8ba9bff..7e6a6c7 100644
--- a/src/gpu/GrDataUtils.cpp
+++ b/src/gpu/GrDataUtils.cpp
@@ -599,7 +599,7 @@
pipeline.append_transfer_function(*skcms_sRGB_TransferFunction());
}
if (alphaOrCSConversion) {
- steps->apply(&pipeline, srcIsNormalized);
+ steps->apply(&pipeline);
}
if (clampGamut) {
append_clamp_gamut(&pipeline);
diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h
index 339d7a8..8c7969b 100644
--- a/src/opts/SkRasterPipeline_opts.h
+++ b/src/opts/SkRasterPipeline_opts.h
@@ -1912,45 +1912,6 @@
b = fn(b);
}
-STAGE(from_srgb, Ctx::None) {
- auto fn = [](F s) {
- U32 sign;
- s = strip_sign(s, &sign);
- auto lo = s * (1/12.92f);
- auto hi = mad(s*s, mad(s, 0.3000f, 0.6975f), 0.0025f);
- return apply_sign(if_then_else(s < 0.055f, lo, hi), sign);
- };
- r = fn(r);
- g = fn(g);
- b = fn(b);
-}
-STAGE(to_srgb, Ctx::None) {
- auto fn = [](F l) {
- U32 sign;
- l = strip_sign(l, &sign);
- // We tweak c and d for each instruction set to make sure fn(1) is exactly 1.
- #if defined(JUMPER_IS_SSE2) || defined(JUMPER_IS_SSE41) || \
- defined(JUMPER_IS_AVX ) || defined(JUMPER_IS_HSW ) || defined(JUMPER_IS_SKX)
- const float c = 1.130048394203f,
- d = 0.141357362270f;
- #elif defined(JUMPER_IS_NEON)
- const float c = 1.129999995232f,
- d = 0.141381442547f;
- #else
- const float c = 1.129999995232f,
- d = 0.141377761960f;
- #endif
- F t = rsqrt(l);
- auto lo = l * 12.92f;
- auto hi = mad(t, mad(t, -0.0024542345f, 0.013832027f), c)
- * rcp(d + t);
- return apply_sign(if_then_else(l < 0.00465985f, lo, hi), sign);
- };
- r = fn(r);
- g = fn(g);
- b = fn(b);
-}
-
STAGE(load_a8, const SkRasterPipeline_MemoryCtx* ctx) {
auto ptr = ptr_at_xy<const uint8_t>(ctx, dx,dy);
@@ -4253,8 +4214,6 @@
NOT_IMPLEMENTED(unbounded_uniform_color)
NOT_IMPLEMENTED(unpremul)
NOT_IMPLEMENTED(dither) // TODO
- NOT_IMPLEMENTED(from_srgb)
- NOT_IMPLEMENTED(to_srgb)
NOT_IMPLEMENTED(load_16161616)
NOT_IMPLEMENTED(load_16161616_dst)
NOT_IMPLEMENTED(store_16161616)
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index 93bada2..a1a94cc 100755
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -464,14 +464,6 @@
};
auto append_misc = [&] {
- // This is an inessential optimization... it's logically safe to set this to false.
- // But if...
- // - we know the image is definitely normalized, and
- // - we're doing some color space conversion, and
- // - sRGB curves are involved,
- // then we can use slightly faster math that doesn't work well outside [0,1].
- bool src_is_normalized = SkColorTypeIsNormalized(info.colorType());
-
SkColorSpace* cs = info.colorSpace();
SkAlphaType at = info.alphaType();
@@ -480,7 +472,6 @@
SkColor4f rgb = rec.fPaint.getColor4f();
p->append_set_rgb(alloc, rgb);
- src_is_normalized = rgb.fitsInBytes();
cs = sk_srgb_singleton();
at = kUnpremul_SkAlphaType;
}
@@ -491,13 +482,12 @@
p->append(at == kUnpremul_SkAlphaType || fClampAsIfUnpremul
? SkRasterPipeline::clamp_1
: SkRasterPipeline::clamp_a);
- src_is_normalized = true;
}
// Transform color space and alpha type to match shader convention (dst CS, premul alpha).
alloc->make<SkColorSpaceXformSteps>(cs, at,
rec.fDstCS, kPremul_SkAlphaType)
- ->apply(p, src_is_normalized);
+ ->apply(p);
return true;
};
diff --git a/src/shaders/SkShader.cpp b/src/shaders/SkShader.cpp
index c1c09e7..766b961 100644
--- a/src/shaders/SkShader.cpp
+++ b/src/shaders/SkShader.cpp
@@ -189,7 +189,7 @@
rec.fPipeline->append(SkRasterPipeline::callback, cb);
rec.fAlloc->make<SkColorSpaceXformSteps>(sk_srgb_singleton(), kPremul_SkAlphaType,
rec.fDstCS, kPremul_SkAlphaType)
- ->apply(rec.fPipeline, true);
+ ->apply(rec.fPipeline);
return true;
}
return false;
diff --git a/tests/SRGBTest.cpp b/tests/SRGBTest.cpp
index a2c09d9..2eea2ce 100644
--- a/tests/SRGBTest.cpp
+++ b/tests/SRGBTest.cpp
@@ -13,33 +13,31 @@
#include <math.h>
DEF_TEST(srgb_roundtrip, r) {
- for (int normalized = 0; normalized < 2; normalized++) {
- uint32_t reds[256];
- for (int i = 0; i < 256; i++) {
- reds[i] = i;
- }
+ uint32_t reds[256];
+ for (int i = 0; i < 256; i++) {
+ reds[i] = i;
+ }
- SkRasterPipeline_MemoryCtx ptr = { reds, 0 };
+ SkRasterPipeline_MemoryCtx ptr = { reds, 0 };
- sk_sp<SkColorSpace> sRGB = SkColorSpace::MakeSRGB(),
- linear = sRGB->makeLinearGamma();
- const SkAlphaType upm = kUnpremul_SkAlphaType;
+ sk_sp<SkColorSpace> sRGB = SkColorSpace::MakeSRGB(),
+ linear = sRGB->makeLinearGamma();
+ const SkAlphaType upm = kUnpremul_SkAlphaType;
- SkColorSpaceXformSteps linearize{ sRGB.get(),upm, linear.get(),upm},
- reencode {linear.get(),upm, sRGB.get(),upm};
+ SkColorSpaceXformSteps linearize{ sRGB.get(),upm, linear.get(),upm},
+ reencode {linear.get(),upm, sRGB.get(),upm};
- SkRasterPipeline_<256> p;
- p.append(SkRasterPipeline::load_8888, &ptr);
- linearize.apply(&p, !!normalized);
- reencode .apply(&p, !!normalized);
- p.append(SkRasterPipeline::store_8888, &ptr);
+ SkRasterPipeline_<256> p;
+ p.append(SkRasterPipeline::load_8888, &ptr);
+ linearize.apply(&p);
+ reencode .apply(&p);
+ p.append(SkRasterPipeline::store_8888, &ptr);
- p.run(0,0,256,1);
+ p.run(0,0,256,1);
- for (int i = 0; i < 256; i++) {
- if (reds[i] != (uint32_t)i) {
- ERRORF(r, "%d doesn't round trip, %d", i, reds[i]);
- }
+ for (int i = 0; i < 256; i++) {
+ if (reds[i] != (uint32_t)i) {
+ ERRORF(r, "%d doesn't round trip, %d", i, reds[i]);
}
}
}
@@ -60,7 +58,7 @@
SkSTArenaAlloc<256> alloc;
SkRasterPipeline p(&alloc);
p.append_constant_color(&alloc, color);
- steps.apply(&p, true/*inputs are normalized*/);
+ steps.apply(&p);
p.append(SkRasterPipeline::store_f32, &dst);
p.run(0,0,4,1);
@@ -76,16 +74,12 @@
}
// Linearize and then re-encode pixel values, testing that the output is close to the input.
-static void test_roundtripping(skiatest::Reporter* r,
- sk_sp<SkColorSpace> cs,
- float range,
- float tolerance,
- bool normalized) {
+DEF_TEST(srgb_roundtrip_extended, r) {
static const int kSteps = 128;
SkColor4f rgba[kSteps];
auto expected = [=](int i) {
- float scale = range / (3*kSteps);
+ float scale = 10000.0f / (3*kSteps);
return SkColor4f{
(3*i+0) * scale,
(3*i+1) * scale,
@@ -100,6 +94,7 @@
SkRasterPipeline_MemoryCtx ptr = { rgba, 0 };
+ sk_sp<SkColorSpace> cs = SkColorSpace::MakeSRGB();
sk_sp<SkColorSpace> linear = cs->makeLinearGamma();
const SkAlphaType upm = kUnpremul_SkAlphaType;
@@ -108,14 +103,14 @@
SkRasterPipeline_<256> p;
p.append(SkRasterPipeline::load_f32, &ptr);
- linearize.apply(&p, normalized);
- reencode .apply(&p, normalized);
+ linearize.apply(&p);
+ reencode .apply(&p);
p.append(SkRasterPipeline::store_f32, &ptr);
p.run(0,0,kSteps,1);
auto close = [=](float x, float y) {
return x == y
- || (x/y < tolerance && y/x < tolerance);
+ || (x/y < 1.001f && y/x < 1.001f);
};
for (int i = 0; i < kSteps; i++) {
@@ -130,11 +125,3 @@
REPORTER_ASSERT(r, close(rgba[i].fB, want.fB));
}
}
-
-DEF_TEST(srgb_roundtrip_extended, r) {
- // We're lying when we set normalized=true, but it allows us to test the to_srgb/from_srgb path.
- test_roundtripping(r, SkColorSpace::MakeSRGB(), 2.0f, 1.025f, true);
-
- // This normalized=false path should have much better round-tripping properties.
- test_roundtripping(r, SkColorSpace::MakeSRGB(), 10000.0f, 1.001f, false);
-}
