From 2eae8e53ba9c68b020ff10893170b1e6a252d7a1 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Wed, 30 Oct 2019 14:04:33 -0500 Subject: [PATCH] only scale shader by paint alpha < 1 This adds `shader && paint alpha < 1.0f` to the blitter Key, so we'll generate and cache two different programs based on that bit. I put a little bit of effort into a different approach, wrapping the shader with a new shader that modulates by paint alpha when paint alpha is less than one. This should work, but I realized that it would break caching, since we're caching based on shader identity. (Not really break, but make it ineffective.) I've added some notes about cache keys in general, and that this wrapper approach would make caching less effective is I think enough to make me want to work out a "identify yourself except for uniforms" protocol for shaders (really effects, eventually color filters will need it too). Change-Id: I1a2a490657bf7442e79f87d80968302720265f7d Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251775 Reviewed-by: Mike Klein Commit-Queue: Mike Klein --- src/core/SkVMBlitter.cpp | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/core/SkVMBlitter.cpp b/src/core/SkVMBlitter.cpp index 5ba415dd59..8c2891dd6e 100644 --- a/src/core/SkVMBlitter.cpp +++ b/src/core/SkVMBlitter.cpp @@ -26,6 +26,8 @@ namespace { SkBlendMode blendMode; sk_sp colorSpace; sk_sp shader; + bool applyPaintAlphaToShader; + uint8_t unusedBytes[7]; Key withCoverage(Coverage c) const { Key k = *this; @@ -36,14 +38,26 @@ namespace { SK_END_REQUIRE_DENSE; static bool operator==(const Key& x, const Key& y) { - return x.colorType == y.colorType - && x.alphaType == y.alphaType - && x.coverage == y.coverage - && x.blendMode == y.blendMode - && x.colorSpace == y.colorSpace // SkColorSpace::Equals() would make hashing unsound. - && x.shader == y.shader; + // N.B. using SkColorSpace::Equals() would make hashing unsound: + // Keys that compare as == could have non-equal hashes. + return x.colorType == y.colorType + && x.alphaType == y.alphaType + && x.coverage == y.coverage + && x.blendMode == y.blendMode + && x.colorSpace == y.colorSpace + && x.shader == y.shader + && x.applyPaintAlphaToShader == y.applyPaintAlphaToShader; } + // TODO: we could do with better hashing and equality here in general, + // though I'm not sure how strictly important any of these ideas are: + // - use SkColorSpace::hash() and SkColorSpace::Equals() to coalesce equivalent + // color spaces by value + // - come up with a mechanism to have SkShader identify itself except for its + // uniforms, so that we can reuse programs with the same kind of shader but + // different uniforms + // - pack bits of Key more carefully + static SkString debug_name(const Key& key) { return SkStringPrintf("CT%d-AT%d-Cov%d-Blend%d-CS%d-Shader%d", key.colorType, @@ -191,11 +205,12 @@ namespace { key.colorSpace.get(), uniforms, sizeof(Uniforms), &src.r, &src.g, &src.b, &src.a)); - // TODO: skip when paint is opaque. - src.r = scale_unorm8(src.r, paint_alpha); - src.g = scale_unorm8(src.g, paint_alpha); - src.b = scale_unorm8(src.b, paint_alpha); - src.a = scale_unorm8(src.a, paint_alpha); + if (key.applyPaintAlphaToShader) { + src.r = scale_unorm8(src.r, paint_alpha); + src.g = scale_unorm8(src.g, paint_alpha); + src.b = scale_unorm8(src.b, paint_alpha); + src.a = scale_unorm8(src.a, paint_alpha); + } } if (key.coverage == Coverage::Mask3D) { @@ -326,6 +341,8 @@ namespace { paint.getBlendMode(), device.refColorSpace(), paint.refShader(), + paint.getShader() && paint.getAlphaf() < 1.0f, + {0,0,0,0,0,0,0}, } , fUniforms(sizeof(Uniforms)) {