Require that runtime effect child variables are 'uniform'
Bug: skia:11374
Change-Id: I63d605eabbe514a0469d00d8a671969874f3edd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393081
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt
index 2699db1..07e7348 100644
--- a/RELEASE_NOTES.txt
+++ b/RELEASE_NOTES.txt
@@ -19,6 +19,10 @@
are not supported.
https://review.skia.org/391856
+ * SkRuntimeEffect requires that 'shader' variables be declared as 'uniform'. The deprecated
+ syntax of 'in shader' is no longer supported.
+ https://review.skia.org/393081
+
* * *
Milestone 90
diff --git a/resources/sksl/runtime_errors/IllegalShaderUse.rte b/resources/sksl/runtime_errors/IllegalShaderUse.rte
index 67f06b8..2216d39 100644
--- a/resources/sksl/runtime_errors/IllegalShaderUse.rte
+++ b/resources/sksl/runtime_errors/IllegalShaderUse.rte
@@ -1,11 +1,10 @@
-// Expect 5 errors
+// Expect >= 7 errors (currently 9, due to double-reporting)
// Correct declaration (used in some test functions)
uniform shader s1;
uniform shader s2;
// Incorrect shader declarations (they must be uniform)
-// TODO(skbug.com/11374): These are not detected as errors yet
shader s3;
in shader s4;
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 4757f36..6fea273 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -294,11 +294,14 @@
}
}
if (this->programKind() == ProgramKind::kRuntimeEffect) {
- // TODO(skbug.com/11374): Remove this special case, 'in' should be prohibited in all cases
- if ((modifiers.fFlags & Modifiers::kIn_Flag) && !baseType->isEffectChild()) {
+ if (modifiers.fFlags & Modifiers::kIn_Flag) {
this->errorReporter().error(offset, "'in' variables not permitted in runtime effects");
}
}
+ if (baseType->isEffectChild() && !(modifiers.fFlags & Modifiers::kUniform_Flag)) {
+ this->errorReporter().error(
+ offset, "variables of type '" + baseType->displayName() + "' must be uniform");
+ }
if ((modifiers.fLayout.fFlags & Layout::kKey_Flag) &&
(modifiers.fFlags & Modifiers::kUniform_Flag)) {
this->errorReporter().error(offset, "'key' is not permitted on 'uniform' variables");
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index fca5ce7..5305ba7 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -274,12 +274,6 @@
effect.child("child") = rgbwShader;
effect.test(0xFF0000FF, 0xFFFF0000, 0xFF00FF00, 0xFFFFFFFF);
- // Legacy behavior - shaders can be declared 'in' rather than 'uniform'
- effect.build("in shader child;"
- "half4 main() { return sample(child); }");
- effect.child("child") = rgbwShader;
- effect.test(0xFF0000FF, 0xFF00FF00, 0xFFFF0000, 0xFFFFFFFF);
-
//
// Helper functions
//
diff --git a/tests/sksl/runtime_errors/IllegalShaderUse.skvm b/tests/sksl/runtime_errors/IllegalShaderUse.skvm
index 840154b..5690524 100644
--- a/tests/sksl/runtime_errors/IllegalShaderUse.skvm
+++ b/tests/sksl/runtime_errors/IllegalShaderUse.skvm
@@ -1,8 +1,12 @@
### Compilation failed:
-error: 13: variables of type 'shader' must be global
-error: 14: parameters of type 'shader' not allowed
-error: 15: functions may not return opaque type 'shader'
-error: 16: cannot construct 'shader'
-error: 17: ternary expression of opaque type 'shader' not allowed
-5 errors
+error: 8: variables of type 'shader' must be uniform
+error: 9: 'in' variables not permitted in runtime effects
+error: 9: variables of type 'shader' must be uniform
+error: 12: variables of type 'shader' must be global
+error: 12: variables of type 'shader' must be uniform
+error: 13: parameters of type 'shader' not allowed
+error: 14: functions may not return opaque type 'shader'
+error: 15: cannot construct 'shader'
+error: 16: ternary expression of opaque type 'shader' not allowed
+9 errors
