From cd3e13a130e3d99790e80b329ef6d13591615fe0 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Tue, 10 Jul 2018 15:52:06 +0000 Subject: [PATCH] Reland "transform paint color to dst colorspace" This is a reland of 6eb36214461794626291a01c3150f7239e4a91a3 I've added an unbounded_uniform_color stage to allow lowp to keep working when colors are in range and to force floats when colors go out of range, most commonly in "narrow" config. Original change's description: > transform paint color to dst colorspace > > Hopefully I can start to knock off effects that need > to be converted to dst colorspace one at a time now. > > My rough list is, > - paint color <---- > - image shader > - sprite blitter > - gradients? > - mode color filter > - lighting color filter > - high contrast filter > - toSRGB color filter > > This change cuts the diffs between 8888 and srgb from > ~540 to ~500. > > I left myself a note about not being able to interpret null > to sRGB at a high level yet. That'd cause a ton of 8888 diffs, > and I think SkColorSpaceXformCanvas diffs too. > > Change-Id: Id66a63e0e92130927f267719aeccb8bbcd92973a > Reviewed-on: https://skia-review.googlesource.com/140244 > Reviewed-by: Brian Osman > Commit-Queue: Mike Klein Change-Id: I56dbf3ad3547d668e4b41f3126ef61d70b295f7c Reviewed-on: https://skia-review.googlesource.com/140460 Reviewed-by: Mike Klein Commit-Queue: Mike Klein --- src/core/SkRasterPipeline.cpp | 29 +++++++++++++++++----------- src/core/SkRasterPipeline.h | 2 +- src/core/SkRasterPipelineBlitter.cpp | 27 +++++++++++++++++++++++++- src/opts/SkRasterPipeline_opts.h | 7 +++++++ 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/core/SkRasterPipeline.cpp b/src/core/SkRasterPipeline.cpp index 46658969cd..4ee1be0372 100644 --- a/src/core/SkRasterPipeline.cpp +++ b/src/core/SkRasterPipeline.cpp @@ -21,7 +21,8 @@ void SkRasterPipeline::reset() { } void SkRasterPipeline::append(StockStage stage, void* ctx) { - SkASSERT(stage != uniform_color); // Please use append_constant_color(). + SkASSERT(stage != uniform_color); // Please use append_constant_color(). + SkASSERT(stage != unbounded_uniform_color); // Please use append_constant_color(). this->unchecked_append(stage, ctx); } void SkRasterPipeline::unchecked_append(StockStage stage, void* ctx) { @@ -90,9 +91,7 @@ void SkRasterPipeline::dump() const { #endif void SkRasterPipeline::append_constant_color(SkArenaAlloc* alloc, const float rgba[4]) { - SkASSERT(0 <= rgba[0] && rgba[0] <= 1); - SkASSERT(0 <= rgba[1] && rgba[1] <= 1); - SkASSERT(0 <= rgba[2] && rgba[2] <= 1); + // r,g,b might be outside [0,1], but alpha should probably always be in [0,1]. SkASSERT(0 <= rgba[3] && rgba[3] <= 1); if (rgba[0] == 0 && rgba[1] == 0 && rgba[2] == 0 && rgba[3] == 1) { @@ -106,14 +105,22 @@ void SkRasterPipeline::append_constant_color(SkArenaAlloc* alloc, const float rg Sk4f color = Sk4f::Load(rgba); color.store(&ctx->r); - // To make loads more direct, we store 8-bit values in 16-bit slots. - color = color * 255.0f + 0.5f; - ctx->rgba[0] = (uint16_t)color[0]; - ctx->rgba[1] = (uint16_t)color[1]; - ctx->rgba[2] = (uint16_t)color[2]; - ctx->rgba[3] = (uint16_t)color[3]; + // uniform_color requires colors in range and can go lowp, + // while unbounded_uniform_color supports out-of-range colors too but not lowp. + if (0 <= rgba[0] && rgba[0] <= 1 && + 0 <= rgba[1] && rgba[1] <= 1 && + 0 <= rgba[2] && rgba[2] <= 1) { + // To make loads more direct, we store 8-bit values in 16-bit slots. + color = color * 255.0f + 0.5f; + ctx->rgba[0] = (uint16_t)color[0]; + ctx->rgba[1] = (uint16_t)color[1]; + ctx->rgba[2] = (uint16_t)color[2]; + ctx->rgba[3] = (uint16_t)color[3]; + this->unchecked_append(uniform_color, ctx); + } else { + this->unchecked_append(unbounded_uniform_color, ctx); + } - this->unchecked_append(uniform_color, ctx); INC_COLOR; } diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h index 3c14480132..0efdae518a 100644 --- a/src/core/SkRasterPipeline.h +++ b/src/core/SkRasterPipeline.h @@ -42,7 +42,7 @@ M(force_opaque) M(force_opaque_dst) \ M(set_rgb) M(swap_rb) \ M(from_srgb) M(to_srgb) \ - M(black_color) M(white_color) M(uniform_color) \ + M(black_color) M(white_color) M(uniform_color) M(unbounded_uniform_color) \ M(seed_shader) M(dither) \ M(load_a8) M(load_a8_dst) M(store_a8) M(gather_a8) \ M(load_g8) M(load_g8_dst) M(gather_g8) \ diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp index 66a4a62734..3643637f0b 100644 --- a/src/core/SkRasterPipelineBlitter.cpp +++ b/src/core/SkRasterPipelineBlitter.cpp @@ -12,6 +12,7 @@ #include "SkColor.h" #include "SkColorFilter.h" #include "SkColorSpaceXformer.h" +#include "SkColorSpaceXformSteps.h" #include "SkOpts.h" #include "SkPM4f.h" #include "SkPM4fPriv.h" @@ -85,12 +86,36 @@ private: typedef SkBlitter INHERITED; }; +static SkPM4f premul_in_dst_colorspace(SkColor color, SkColorSpace* dstCS) { + float rgba[4]; + swizzle_rb(SkNx_cast(Sk4b::Load(&color)) * (1/255.0f)).store(rgba); + + // SkColors are always sRGB. + auto srcCS = SkColorSpace::MakeSRGB().get(); + + // If dstCS is null, no color space transformation is needed (and apply() will just premul). + if (!dstCS) { dstCS = srcCS; } + + SkColorSpaceXformSteps(srcCS, kUnpremul_SkAlphaType, dstCS) + .apply(rgba); + + return {{rgba[0], rgba[1], rgba[2], rgba[3]}}; +} + SkBlitter* SkCreateRasterPipelineBlitter(const SkPixmap& dst, const SkPaint& paint, const SkMatrix& ctm, SkArenaAlloc* alloc) { + // For legacy/SkColorSpaceXformCanvas to keep working, + // we need to sometimes still need to distinguish null dstCS from sRGB. +#if 0 + SkColorSpace* dstCS = dst.colorSpace() ? dst.colorSpace() + : SkColorSpace::MakeSRGB().get(); +#else SkColorSpace* dstCS = dst.colorSpace(); - SkPM4f paintColor = SkPM4f_from_SkColor(paint.getColor(), dstCS); +#endif + SkPM4f paintColor = premul_in_dst_colorspace(paint.getColor(), dstCS); + auto shader = as_SB(paint.getShader()); SkRasterPipeline_<256> shaderPipeline; diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h index 23b5e22829..1266970285 100644 --- a/src/opts/SkRasterPipeline_opts.h +++ b/src/opts/SkRasterPipeline_opts.h @@ -976,6 +976,12 @@ STAGE(uniform_color, const SkJumper_UniformColorCtx* c) { b = c->b; a = c->a; } +STAGE(unbounded_uniform_color, const SkJumper_UniformColorCtx* c) { + r = c->r; + g = c->g; + b = c->b; + a = c->a; +} // splats opaque-black into r,g,b,a STAGE(black_color, Ctx::None) { @@ -3204,6 +3210,7 @@ using NotImplemented = void(*)(void); static NotImplemented callback, load_rgba, store_rgba, clamp_0, clamp_1, + unbounded_uniform_color, unpremul, dither, from_srgb, from_srgb_dst, to_srgb, load_f16 , load_f16_dst , store_f16 , gather_f16,