From 4eebd9eed06039d265f06edb759765731f963271 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Wed, 11 Jul 2018 14:49:51 -0400 Subject: collapse parametric_{r,g,b} into parametric, remove _a and gamma_dst parametric_a and gamma_dst were unused outside of unit tests. In all other cases, we always use parametric_{r,g,b} together and always pass them the same argument. So we can collapse them into a single stage like gamma and to/from_srgb. Change-Id: I08cea896c7744f97b4f4bf9e029f5d643e45e177 Reviewed-on: https://skia-review.googlesource.com/140576 Commit-Queue: Mike Klein Reviewed-by: Brian Osman --- bench/SkRasterPipelineBench.cpp | 8 ++------ src/core/SkColorSpaceXformSteps.cpp | 8 ++------ src/core/SkConvertPixels.cpp | 8 ++------ src/core/SkRasterPipeline.h | 3 +-- src/effects/SkHighContrastFilter.cpp | 8 ++------ src/effects/SkToSRGBColorFilter.cpp | 4 +--- src/opts/SkRasterPipeline_opts.h | 27 ++++++++++++--------------- tests/ParametricStageTest.cpp | 8 ++++---- 8 files changed, 26 insertions(+), 48 deletions(-) diff --git a/bench/SkRasterPipelineBench.cpp b/bench/SkRasterPipelineBench.cpp index 5ddf43dd21..6dd7415148 100644 --- a/bench/SkRasterPipelineBench.cpp +++ b/bench/SkRasterPipelineBench.cpp @@ -130,12 +130,8 @@ public: SkRasterPipeline p(&alloc); p.append_constant_color(&alloc, c); if (fParametric) { - p.append(SkRasterPipeline::parametric_r, &from_2dot2); - p.append(SkRasterPipeline::parametric_g, &from_2dot2); - p.append(SkRasterPipeline::parametric_b, &from_2dot2); - p.append(SkRasterPipeline::parametric_r, & to_2dot2); - p.append(SkRasterPipeline::parametric_g, & to_2dot2); - p.append(SkRasterPipeline::parametric_b, & to_2dot2); + p.append(SkRasterPipeline::parametric, &from_2dot2); + p.append(SkRasterPipeline::parametric, & to_2dot2); } else { p.append(SkRasterPipeline::gamma, &from_2dot2.fG); p.append(SkRasterPipeline::gamma, & to_2dot2.fG); diff --git a/src/core/SkColorSpaceXformSteps.cpp b/src/core/SkColorSpaceXformSteps.cpp index 77a9c6b05d..3abdb2f05a 100644 --- a/src/core/SkColorSpaceXformSteps.cpp +++ b/src/core/SkColorSpaceXformSteps.cpp @@ -115,9 +115,7 @@ void SkColorSpaceXformSteps::apply(SkRasterPipeline* p) const { srcTF.fF == 0) { p->append(SkRasterPipeline::gamma, &srcTF.fG); } else { - p->append(SkRasterPipeline::parametric_r, &srcTF); - p->append(SkRasterPipeline::parametric_g, &srcTF); - p->append(SkRasterPipeline::parametric_b, &srcTF); + p->append(SkRasterPipeline::parametric, &srcTF); } } if (flags.gamut_transform) { @@ -134,9 +132,7 @@ void SkColorSpaceXformSteps::apply(SkRasterPipeline* p) const { dstTFInv.fF == 0) { p->append(SkRasterPipeline::gamma, &dstTFInv.fG); } else { - p->append(SkRasterPipeline::parametric_r, &dstTFInv); - p->append(SkRasterPipeline::parametric_g, &dstTFInv); - p->append(SkRasterPipeline::parametric_b, &dstTFInv); + p->append(SkRasterPipeline::parametric, &dstTFInv); } } if (flags.premul) { p->append(SkRasterPipeline::premul); } diff --git a/src/core/SkConvertPixels.cpp b/src/core/SkConvertPixels.cpp index 25645d485c..0434b92c2e 100644 --- a/src/core/SkConvertPixels.cpp +++ b/src/core/SkConvertPixels.cpp @@ -308,9 +308,7 @@ static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, size if (is_just_gamma(srcFn)) { pipeline.append(SkRasterPipeline::gamma, &srcFn.fG); } else { - pipeline.append(SkRasterPipeline::parametric_r, &srcFn); - pipeline.append(SkRasterPipeline::parametric_g, &srcFn); - pipeline.append(SkRasterPipeline::parametric_b, &srcFn); + pipeline.append(SkRasterPipeline::parametric, &srcFn); } } @@ -340,9 +338,7 @@ static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, size if (is_just_gamma(dstFn)) { pipeline.append(SkRasterPipeline::gamma, &dstFn.fG); } else { - pipeline.append(SkRasterPipeline::parametric_r, &dstFn); - pipeline.append(SkRasterPipeline::parametric_g, &dstFn); - pipeline.append(SkRasterPipeline::parametric_b, &dstFn); + pipeline.append(SkRasterPipeline::parametric, &dstFn); } } diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h index 0efdae518a..be197e4c6e 100644 --- a/src/core/SkRasterPipeline.h +++ b/src/core/SkRasterPipeline.h @@ -69,8 +69,7 @@ M(matrix_translate) M(matrix_scale_translate) \ M(matrix_2x3) M(matrix_3x3) M(matrix_3x4) M(matrix_4x5) M(matrix_4x3) \ M(matrix_perspective) \ - M(parametric_r) M(parametric_g) M(parametric_b) \ - M(parametric_a) M(gamma) M(gamma_dst) \ + M(parametric) M(gamma) \ M(mirror_x) M(repeat_x) \ M(mirror_y) M(repeat_y) \ M(decal_x) M(decal_y) M(decal_x_and_y) \ diff --git a/src/effects/SkHighContrastFilter.cpp b/src/effects/SkHighContrastFilter.cpp index be6180a531..2d0b0f97fe 100644 --- a/src/effects/SkHighContrastFilter.cpp +++ b/src/effects/SkHighContrastFilter.cpp @@ -73,9 +73,7 @@ void SkHighContrast_Filter::onAppendStages(SkRasterPipeline* p, square->G = 2.0f; square->A = 1.0f; square->B = square->C = square->D = square->E = square->F = 0; - p->append(SkRasterPipeline::parametric_r, square); - p->append(SkRasterPipeline::parametric_g, square); - p->append(SkRasterPipeline::parametric_b, square); + p->append(SkRasterPipeline::parametric, square); } if (fConfig.fGrayscale) { @@ -122,9 +120,7 @@ void SkHighContrast_Filter::onAppendStages(SkRasterPipeline* p, sqrt->G = 0.5f; sqrt->A = 1.0f; sqrt->B = sqrt->C = sqrt->D = sqrt->E = sqrt->F = 0; - p->append(SkRasterPipeline::parametric_r, sqrt); - p->append(SkRasterPipeline::parametric_g, sqrt); - p->append(SkRasterPipeline::parametric_b, sqrt); + p->append(SkRasterPipeline::parametric, sqrt); } if (!shaderIsOpaque) { diff --git a/src/effects/SkToSRGBColorFilter.cpp b/src/effects/SkToSRGBColorFilter.cpp index a9bd65c625..ba01c6b2c4 100644 --- a/src/effects/SkToSRGBColorFilter.cpp +++ b/src/effects/SkToSRGBColorFilter.cpp @@ -29,9 +29,7 @@ void SkToSRGBColorFilter::onAppendStages(SkRasterPipeline* p, p->append(SkRasterPipeline::from_srgb); } else if (fSrcColorSpace->isNumericalTransferFn(&srcFn)) { auto copy = alloc->make(srcFn); - p->append(SkRasterPipeline::parametric_r, copy); - p->append(SkRasterPipeline::parametric_g, copy); - p->append(SkRasterPipeline::parametric_b, copy); + p->append(SkRasterPipeline::parametric, copy); } else { SkDEBUGFAIL("Looks like we got a table transfer function here, quite unexpectedly."); // TODO: If we really need to handle this, we can, but I don't think Ganesh does. diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h index 1266970285..0dce9cfdb6 100644 --- a/src/opts/SkRasterPipeline_opts.h +++ b/src/opts/SkRasterPipeline_opts.h @@ -1483,26 +1483,24 @@ STAGE(byte_tables, const void* ctx) { // TODO: rename Tables SkJumper_ByteTable a = from_byte(gather(tables->a, to_unorm(a, 255))); } -SI F parametric(F v, const SkJumper_ParametricTransferFunction* ctx) { - F r = if_then_else(v <= ctx->D, mad(ctx->C, v, ctx->F) - , approx_powf(mad(ctx->A, v, ctx->B), ctx->G) + ctx->E); - return min(max(r, 0), 1.0f); // Clamp to [0,1], with argument order mattering to handle NaN. +STAGE(parametric, const SkJumper_ParametricTransferFunction* ctx) { + auto fn = [&](F v) { + F r = if_then_else(v <= ctx->D, mad(ctx->C, v, ctx->F) + , approx_powf(mad(ctx->A, v, ctx->B), ctx->G) + ctx->E); + // Clamp to [0,1], with argument order mattering to handle NaN. + // TODO: should we really be clamping here? + return min(max(r, 0), 1.0f); + }; + r = fn(r); + g = fn(g); + b = fn(b); } -STAGE(parametric_r, const SkJumper_ParametricTransferFunction* ctx) { r = parametric(r, ctx); } -STAGE(parametric_g, const SkJumper_ParametricTransferFunction* ctx) { g = parametric(g, ctx); } -STAGE(parametric_b, const SkJumper_ParametricTransferFunction* ctx) { b = parametric(b, ctx); } -STAGE(parametric_a, const SkJumper_ParametricTransferFunction* ctx) { a = parametric(a, ctx); } STAGE(gamma, const float* G) { r = approx_powf(r, *G); g = approx_powf(g, *G); b = approx_powf(b, *G); } -STAGE(gamma_dst, const float* G) { - dr = approx_powf(dr, *G); - dg = approx_powf(dg, *G); - db = approx_powf(db, *G); -} STAGE(load_a8, const SkJumper_MemoryCtx* ctx) { auto ptr = ptr_at_xy(ctx, dx,dy); @@ -3220,8 +3218,7 @@ static NotImplemented byte_tables, colorburn, colordodge, softlight, hue, saturation, color, luminosity, matrix_3x3, matrix_3x4, matrix_4x5, matrix_4x3, - parametric_r, parametric_g, parametric_b, parametric_a, - gamma, gamma_dst, + parametric, gamma, rgb_to_hsl, hsl_to_rgb, gauss_a_to_rgba, mirror_x, repeat_x, diff --git a/tests/ParametricStageTest.cpp b/tests/ParametricStageTest.cpp index 6d98ac451b..1b943e9fbf 100644 --- a/tests/ParametricStageTest.cpp +++ b/tests/ParametricStageTest.cpp @@ -22,10 +22,7 @@ static void check_error(skiatest::Reporter* r, float limit, SkColorSpaceTransfer SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f32, &ip); - p.append(SkRasterPipeline::parametric_r, &fn); - p.append(SkRasterPipeline::parametric_g, &fn); - p.append(SkRasterPipeline::parametric_b, &fn); - p.append(SkRasterPipeline::parametric_a, &fn); + p.append(SkRasterPipeline::parametric, &fn); p.append(SkRasterPipeline::store_f32, &op); p.run(0,0, 256/4,1); @@ -34,6 +31,9 @@ static void check_error(skiatest::Reporter* r, float limit, SkColorSpaceTransfer for (int i = 0; i < 256; i++) { float want = (in[i] <= fn.fD) ? fn.fC * in[i] + fn.fF : powf(in[i] * fn.fA + fn.fB, fn.fG) + fn.fE; + if (i % 4 == 3) { // alpha should stay unchanged. + want = in[i]; + } float err = fabsf(out[i] - want); if (err > limit) { ERRORF(r, "At %d, error was %g (got %g, want %g)", i, err, out[i], want); -- cgit v1.2.3