use from/to_srgb only when known to be in [0,1]

Our srgb_roundtrip_extended unit test has shown that our from_srgb and
to_srgb stages start to break down outside a range of about [-2,2], and
they appear to asymptote to roundtripping to y=8, not an identity y=x.

We'll continue to use these stages where we know ahead of time that
the inputs are all in [0,1], and otherwise fall back on ::parametric.

Guarded by SK_LEGACY_SRGB_STAGE_CHOICE.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I046b2d37a629df9b0ea687f8df6cad864110430f
Reviewed-on: https://skia-review.googlesource.com/c/165766
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
Mike Klein 2018-10-29 09:39:52 -04:00 committed by Skia Commit-Bot
parent be83793ce2
commit c23d1ab9b0
9 changed files with 53 additions and 34 deletions

View File

@ -130,7 +130,7 @@ public:
: kPremul_SkAlphaType;
fAlloc->make<SkColorSpaceXformSteps>(srcCS, srcAT,
dstCS, kPremul_SkAlphaType)
->apply(&p);
->apply(&p, fSource.colorType());
}
if (fPaintColor.fA != 1.0f) {
p.append(SkRasterPipeline::scale_1_float, &fPaintColor.fA);

View File

@ -231,7 +231,11 @@ public:
if (!shaderIsOpaque) {
p->append(SkRasterPipeline::unpremul);
}
fSteps.apply(p);
// TODO: is it valuable to thread this through appendStages()?
bool shaderIsNormalized = false;
fSteps.apply(p, shaderIsNormalized);
if (!shaderIsOpaque) {
p->append(SkRasterPipeline::premul);
}

View File

@ -136,10 +136,13 @@ void SkColorSpaceXformSteps::apply(float* rgba) const {
}
}
void SkColorSpaceXformSteps::apply(SkRasterPipeline* p) const {
void SkColorSpaceXformSteps::apply(SkRasterPipeline* p, bool src_is_normalized) const {
#if defined(SK_LEGACY_SRGB_STAGE_CHOICE)
src_is_normalized = true;
#endif
if (flags.unpremul) { p->append(SkRasterPipeline::unpremul); }
if (flags.linearize) {
if (srcTF_is_sRGB) {
if (src_is_normalized && srcTF_is_sRGB) {
p->append(SkRasterPipeline::from_srgb);
} else if (srcTF.fA == 1 &&
srcTF.fB == 0 &&
@ -156,7 +159,7 @@ void SkColorSpaceXformSteps::apply(SkRasterPipeline* p) const {
p->append(SkRasterPipeline::matrix_3x3, &src_to_dst_matrix);
}
if (flags.encode) {
if (dstTF_is_sRGB) {
if (src_is_normalized && dstTF_is_sRGB) {
p->append(SkRasterPipeline::to_srgb);
} else if (dstTFInv.fA == 1 &&
dstTFInv.fB == 0 &&

View File

@ -34,7 +34,11 @@ struct SkColorSpaceXformSteps {
SkColorSpace* dst, SkAlphaType dstAT);
void apply(float rgba[4]) const;
void apply(SkRasterPipeline*) const;
void apply(SkRasterPipeline*, bool src_is_normalized) const;
void apply(SkRasterPipeline* p, SkColorType srcCT) const {
this->apply(p, srcCT < kRGBA_F16_SkColorType);
}
Flags flags;

View File

@ -25,7 +25,7 @@ SkColorSpaceXformer::SkColorSpaceXformer(sk_sp<SkColorSpace> dst)
SkRasterPipeline p(&fAlloc);
p.append(SkRasterPipeline::load_8888, &fFromSRGBSrc);
p.append(SkRasterPipeline::swap_rb);
fFromSRGBSteps.apply(&p);
fFromSRGBSteps.apply(&p, kBGRA_8888_SkColorType);
p.append(SkRasterPipeline::swap_rb);
p.append(SkRasterPipeline::store_8888, &fFromSRGBDst);
fFromSRGB = p.compile();

View File

@ -174,7 +174,7 @@ static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, size
SkRasterPipeline_<256> pipeline;
pipeline.append_load(srcInfo.colorType(), &src);
steps.apply(&pipeline);
steps.apply(&pipeline, srcInfo.colorType());
pipeline.append_gamut_clamp_if_normalized(dstInfo);

View File

@ -21,9 +21,10 @@ void SkToSRGBColorFilter::onAppendStages(SkRasterPipeline* p,
SkColorSpace* /*dst color space*/,
SkArenaAlloc* alloc,
bool shaderIsOpaque) const {
bool shaderIsNormalized = false;
alloc->make<SkColorSpaceXformSteps>(fSrcColorSpace.get(), kPremul_SkAlphaType,
sk_srgb_singleton() , kPremul_SkAlphaType)
->apply(p);
->apply(p, shaderIsNormalized);
}
sk_sp<SkColorFilter> SkToSRGBColorFilter::Make(sk_sp<SkColorSpace> srcColorSpace) {

View File

@ -382,7 +382,7 @@ bool SkImageShader::onAppendStages(const StageRec& rec) const {
}
alloc->make<SkColorSpaceXformSteps>(srcCS , kPremul_SkAlphaType,
rec.fDstCS, kPremul_SkAlphaType)
->apply(p);
->apply(p, info.colorType());
}
return true;

View File

@ -13,31 +13,33 @@
#include <math.h>
DEF_TEST(srgb_roundtrip, r) {
uint32_t reds[256];
for (int i = 0; i < 256; i++) {
reds[i] = i;
}
for (int normalized = 0; normalized < 2; normalized++) {
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);
reencode .apply(&p);
p.append(SkRasterPipeline::store_8888, &ptr);
SkRasterPipeline_<256> p;
p.append(SkRasterPipeline::load_8888, &ptr);
linearize.apply(&p, !!normalized);
reencode .apply(&p, !!normalized);
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]);
}
}
}
}
@ -58,7 +60,7 @@ DEF_TEST(srgb_edge_cases, r) {
SkSTArenaAlloc<256> alloc;
SkRasterPipeline p(&alloc);
p.append_constant_color(&alloc, color);
steps.apply(&p);
steps.apply(&p, true/*inputs are normalized*/);
p.append(SkRasterPipeline::store_f32, &dst);
p.run(0,0,4,1);
@ -77,7 +79,8 @@ DEF_TEST(srgb_edge_cases, r) {
static void test_roundtripping(skiatest::Reporter* r,
sk_sp<SkColorSpace> cs,
float range,
float tolerance) {
float tolerance,
bool normalized) {
static const int kSteps = 128;
SkColor4f rgba[kSteps];
@ -105,8 +108,8 @@ static void test_roundtripping(skiatest::Reporter* r,
SkRasterPipeline_<256> p;
p.append(SkRasterPipeline::load_f32, &ptr);
linearize.apply(&p);
reencode .apply(&p);
linearize.apply(&p, normalized);
reencode .apply(&p, normalized);
p.append(SkRasterPipeline::store_f32, &ptr);
p.run(0,0,kSteps,1);
@ -129,5 +132,9 @@ static void test_roundtripping(skiatest::Reporter* r,
}
DEF_TEST(srgb_roundtrip_extended, r) {
test_roundtripping(r, SkColorSpace::MakeSRGB(), 2.0f, 1.025f);
// 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);
}