Detect alpha-preservation in ternary expressions.
It occurred to me that this was fairly easy to check. I'm not sure if
any of our shaders would actually exercise this path.
Change-Id: Iab22688b00a2d55333717f4ea9a0d92cc8e5e756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/665857
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/analysis/SkSLReturnsInputAlpha.cpp b/src/sksl/analysis/SkSLReturnsInputAlpha.cpp
index 17a5dea..2a6d977 100644
--- a/src/sksl/analysis/SkSLReturnsInputAlpha.cpp
+++ b/src/sksl/analysis/SkSLReturnsInputAlpha.cpp
@@ -21,6 +21,7 @@
#include "src/sksl/ir/SkSLFunctionDefinition.h"
#include "src/sksl/ir/SkSLReturnStatement.h"
#include "src/sksl/ir/SkSLSwizzle.h"
+#include "src/sksl/ir/SkSLTernaryExpression.h"
#include "src/sksl/ir/SkSLType.h"
#include "src/sksl/ir/SkSLVariable.h"
#include "src/sksl/ir/SkSLVariableReference.h"
@@ -69,11 +70,11 @@
return true;
}
if (expr.is<Swizzle>()) {
- // It's a swizzle: `input.___a`.
+ // It's a swizzle: check for `input.___a`.
return this->isInputSwizzleEndingWithAlpha(expr);
}
if (expr.is<ConstructorSplat>() || expr.is<ConstructorCompound>()) {
- // This is a splat or compound constructor with `input.a` as its final component.
+ // This is a splat or compound constructor; check for `input.a` as its final component.
const AnyConstructor& ctor = expr.asAnyConstructor();
return this->returnsInputAlpha(*ctor.argumentSpan().back());
}
@@ -82,6 +83,12 @@
const Expression& arg = *expr.as<ConstructorCompoundCast>().argument();
return arg.type().componentType().isFloat() && this->returnsInputAlpha(arg);
}
+ if (expr.is<TernaryExpression>()) {
+ // Both sides of the ternary must preserve input alpha.
+ const TernaryExpression& ternary = expr.as<TernaryExpression>();
+ return this->returnsInputAlpha(*ternary.ifTrue()) &&
+ this->returnsInputAlpha(*ternary.ifFalse());
+ }
// We weren't able to pattern-match here.
return false;
}
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index f6754bd..4da4066 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -1401,7 +1401,11 @@
expectAlphaUnchanged("half4 main(half4 color) { return half4(half2(1), half2(color.a)); }");
expectAlphaUnchanged("half4 main(half4 color) { return half4(color.a); }");
expectAlphaUnchanged("half4 main(half4 color) { return half4(float4(color.baba)); }");
-
+ expectAlphaUnchanged("half4 main(half4 color) { return color.r != color.g ? color :"
+ " color.000a; }");
+ expectAlphaUnchanged("half4 main(half4 color) { return color.a == color.r ? color.rrra : "
+ "color.g == color.b ? color.ggga : "
+ " color.bbba; }");
// These swizzles don't end in alpha.
expectAlphaChanged("half4 main(half4 color) { return color.argb; }");
expectAlphaChanged("half4 main(half4 color) { return color.rrrr; }");
@@ -1412,6 +1416,11 @@
// This splat constructor doesn't use alpha.
expectAlphaChanged("half4 main(half4 color) { return half4(color.r); }");
+ // These ternaries don't return alpha on both sides
+ expectAlphaChanged("half4 main(half4 color) { return color.a > 0 ? half4(0) : color; }");
+ expectAlphaChanged("half4 main(half4 color) { return color.g < 1 ? color.bgra : color.abgr; }");
+ expectAlphaChanged("half4 main(half4 color) { return color.b > 0.5 ? half4(0) : half4(1); }");
+
// Performing arithmetic on the input causes it to report as "alpha changed" even if the
// arithmetic is a no-op; we aren't smart enough to see through it.
expectAlphaChanged("half4 main(half4 color) { return color + half4(1,1,1,0); }");
