Include non-ES2 numeric types in the root symbol table.
Previously, we hid non-ES2 numeric types from Runtime Effect code by
only including them in the private symbol table. Now, they are present
in the root symbol table, but marked with a new flag that identifies
them as disallowed in ES2.
The IR generator now enforces that strict-ES2 code doesn't contain types
that aren't allowed. This has two benefits:
- Intrinsic functions in sksl_public can now reference these types
- Error reporting is nicer
Change-Id: I32375de4efdcb57b74a8a1692fb2ee315a003336
Bug: skia:12348, skia:11115
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439997
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 89e5dad..2c8c8ca 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -149,15 +149,30 @@
TYPE( Float), TYPE( Float2), TYPE( Float3), TYPE( Float4),
TYPE( Half), TYPE( Half2), TYPE( Half3), TYPE( Half4),
TYPE( Int), TYPE( Int2), TYPE( Int3), TYPE( Int4),
+ TYPE( UInt), TYPE( UInt2), TYPE( UInt3), TYPE( UInt4),
+ TYPE( Short), TYPE( Short2), TYPE( Short3), TYPE( Short4),
+ TYPE(UShort), TYPE(UShort2), TYPE(UShort3), TYPE(UShort4),
TYPE( Bool), TYPE( Bool2), TYPE( Bool3), TYPE( Bool4),
- TYPE(Float2x2), TYPE(Float3x3), TYPE(Float4x4),
- TYPE( Half2x2), TYPE( Half3x3), TYPE(Half4x4),
+ TYPE(Float2x2), TYPE(Float2x3), TYPE(Float2x4),
+ TYPE(Float3x2), TYPE(Float3x3), TYPE(Float3x4),
+ TYPE(Float4x2), TYPE(Float4x3), TYPE(Float4x4),
+
+ TYPE(Half2x2), TYPE(Half2x3), TYPE(Half2x4),
+ TYPE(Half3x2), TYPE(Half3x3), TYPE(Half3x4),
+ TYPE(Half4x2), TYPE(Half4x3), TYPE(Half4x4),
TYPE(SquareMat), TYPE(SquareHMat),
+ TYPE(Mat), TYPE(HMat),
- TYPE(GenType), TYPE(GenHType), TYPE(GenIType), TYPE(GenBType),
- TYPE(Vec), TYPE(HVec), TYPE(IVec), TYPE(BVec),
+ // TODO(skia:12349): generic short/ushort
+ TYPE(GenType), TYPE(GenIType), TYPE(GenUType),
+ TYPE(GenHType), /* (GenSType) (GenUSType) */
+ TYPE(GenBType),
+
+ TYPE(Vec), TYPE(IVec), TYPE(UVec),
+ TYPE(HVec), TYPE(SVec), TYPE(USVec),
+ TYPE(BVec),
TYPE(ColorFilter),
TYPE(Shader),
@@ -175,23 +190,6 @@
auto privateSymbolTable = std::make_shared<SymbolTable>(parent, /*builtin=*/true);
const SkSL::Symbol* privateTypes[] = {
- TYPE( UInt), TYPE( UInt2), TYPE( UInt3), TYPE( UInt4),
- TYPE( Short), TYPE( Short2), TYPE( Short3), TYPE( Short4),
- TYPE(UShort), TYPE(UShort2), TYPE(UShort3), TYPE(UShort4),
-
- TYPE(GenUType), TYPE(UVec),
- TYPE(SVec), TYPE(USVec),
-
- TYPE(Float2x3), TYPE(Float2x4),
- TYPE(Float3x2), TYPE(Float3x4),
- TYPE(Float4x2), TYPE(Float4x3),
-
- TYPE(Half2x3), TYPE(Half2x4),
- TYPE(Half3x2), TYPE(Half3x4),
- TYPE(Half4x2), TYPE(Half4x3),
-
- TYPE(Mat), TYPE(HMat),
-
TYPE(Sampler1D), TYPE(Sampler2D), TYPE(Sampler3D),
TYPE(SamplerExternalOES),
TYPE(Sampler2DRect),
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 20d6307..e82e9c0 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -1113,14 +1113,14 @@
}
}
-bool IRGenerator::typeContainsPrivateFields(const Type& type) {
+static bool type_contains_private_fields(const Type& type) {
// Checks for usage of private types, including fields inside a struct.
if (type.isPrivate()) {
return true;
}
if (type.isStruct()) {
for (const auto& f : type.fields()) {
- if (this->typeContainsPrivateFields(*f.fType)) {
+ if (type_contains_private_fields(*f.fType)) {
return true;
}
}
@@ -1142,9 +1142,15 @@
"type '" + name + "' not allowed in this context");
return nullptr;
}
- if (!fIsBuiltinCode && this->typeContainsPrivateFields(*result)) {
- this->errorReporter().error(type.fOffset, "type '" + name + "' is private");
- return nullptr;
+ if (!fIsBuiltinCode) {
+ if (type_contains_private_fields(*result)) {
+ this->errorReporter().error(type.fOffset, "type '" + name + "' is private");
+ return nullptr;
+ }
+ if (this->strictES2Mode() && !result->allowedInES2()) {
+ this->errorReporter().error(type.fOffset, "type '" + name + "' is not supported");
+ return nullptr;
+ }
}
if (isArray) {
auto iter = type.begin();
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index e0d7a3e..2668ceb 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -227,7 +227,6 @@
std::unique_ptr<Statement> getNormalizeSkPositionCode();
void checkValid(const Expression& expr);
- bool typeContainsPrivateFields(const Type& type);
bool setRefKind(Expression& expr, VariableReference::RefKind kind);
void copyIntrinsicIfNeeded(const FunctionDeclaration& function);
void findAndDeclareBuiltinVariables();
diff --git a/src/sksl/ir/SkSLType.cpp b/src/sksl/ir/SkSLType.cpp
index be79e03..562231a 100644
--- a/src/sksl/ir/SkSLType.cpp
+++ b/src/sksl/ir/SkSLType.cpp
@@ -50,6 +50,10 @@
return this->componentType().bitWidth();
}
+ bool allowedInES2() const override {
+ return fComponentType.allowedInES2();
+ }
+
private:
using INHERITED = Type;
@@ -128,7 +132,7 @@
public:
static constexpr TypeKind kTypeKind = TypeKind::kScalar;
- ScalarType(const char* name, const char* abbrev, NumberKind numberKind, int8_t priority,
+ ScalarType(skstd::string_view name, const char* abbrev, NumberKind numberKind, int8_t priority,
int8_t bitWidth)
: INHERITED(name, abbrev, kTypeKind)
, fNumberKind(numberKind)
@@ -159,6 +163,10 @@
return true;
}
+ bool allowedInES2() const override {
+ return fNumberKind != NumberKind::kUnsigned;
+ }
+
private:
using INHERITED = Type;
@@ -201,6 +209,10 @@
return true;
}
+ bool allowedInES2() const override {
+ return fColumns == fRows;
+ }
+
private:
using INHERITED = Type;
@@ -306,6 +318,12 @@
return true;
}
+ bool allowedInES2() const override {
+ return std::all_of(fFields.begin(), fFields.end(), [](const Field& f) {
+ return f.fType->allowedInES2();
+ });
+ }
+
private:
using INHERITED = Type;
@@ -344,6 +362,10 @@
return true;
}
+ bool allowedInES2() const override {
+ return fComponentType.allowedInES2();
+ }
+
private:
using INHERITED = Type;
@@ -373,7 +395,7 @@
return std::make_unique<LiteralType>(name, scalarType, priority);
}
-std::unique_ptr<Type> Type::MakeMatrixType(const char* name, const char* abbrev,
+std::unique_ptr<Type> Type::MakeMatrixType(skstd::string_view name, const char* abbrev,
const Type& componentType, int columns, int8_t rows) {
return std::make_unique<MatrixType>(name, abbrev, componentType, columns, rows);
}
@@ -387,7 +409,7 @@
return std::unique_ptr<Type>(new Type(name, abbrev, typeKind));
}
-std::unique_ptr<Type> Type::MakeScalarType(const char* name, const char* abbrev,
+std::unique_ptr<Type> Type::MakeScalarType(skstd::string_view name, const char* abbrev,
Type::NumberKind numberKind, int8_t priority,
int8_t bitWidth) {
return std::make_unique<ScalarType>(name, abbrev, numberKind, priority, bitWidth);
@@ -406,7 +428,7 @@
isMultisampled, isSampled);
}
-std::unique_ptr<Type> Type::MakeVectorType(const char* name, const char* abbrev,
+std::unique_ptr<Type> Type::MakeVectorType(skstd::string_view name, const char* abbrev,
const Type& componentType, int columns) {
return std::make_unique<VectorType>(name, abbrev, componentType, columns);
}
diff --git a/src/sksl/ir/SkSLType.h b/src/sksl/ir/SkSLType.h
index 9836205..41850ea 100644
--- a/src/sksl/ir/SkSLType.h
+++ b/src/sksl/ir/SkSLType.h
@@ -125,7 +125,7 @@
int8_t priority);
/** Create a matrix type. */
- static std::unique_ptr<Type> MakeMatrixType(const char* name, const char* abbrev,
+ static std::unique_ptr<Type> MakeMatrixType(skstd::string_view name, const char* abbrev,
const Type& componentType, int columns,
int8_t rows);
@@ -133,7 +133,7 @@
static std::unique_ptr<Type> MakeSamplerType(const char* name, const Type& textureType);
/** Create a scalar type. */
- static std::unique_ptr<Type> MakeScalarType(const char* name, const char* abbrev,
+ static std::unique_ptr<Type> MakeScalarType(skstd::string_view name, const char* abbrev,
Type::NumberKind numberKind, int8_t priority,
int8_t bitWidth);
@@ -153,7 +153,7 @@
bool isMultisampled, bool isSampled);
/** Create a vector type. */
- static std::unique_ptr<Type> MakeVectorType(const char* name, const char* abbrev,
+ static std::unique_ptr<Type> MakeVectorType(skstd::string_view name, const char* abbrev,
const Type& componentType, int columns);
template <typename T>
@@ -198,6 +198,10 @@
return this->name().starts_with("$");
}
+ virtual bool allowedInES2() const {
+ return true;
+ }
+
bool operator==(const Type& other) const {
return this->name() == other.name();
}
diff --git a/tests/sksl/runtime_errors/ES3Types.skvm b/tests/sksl/runtime_errors/ES3Types.skvm
index 69e8f45..6c9e5d4 100644
--- a/tests/sksl/runtime_errors/ES3Types.skvm
+++ b/tests/sksl/runtime_errors/ES3Types.skvm
@@ -1,4 +1,29 @@
### Compilation failed:
-error: 1: expected ';', but found 'x'
-1 error
+error: 1: type 'float2x3' is not supported
+error: 2: type 'float2x4' is not supported
+error: 3: type 'float3x2' is not supported
+error: 4: type 'float3x4' is not supported
+error: 5: type 'float4x2' is not supported
+error: 6: type 'float4x3' is not supported
+error: 8: type 'half2x3' is not supported
+error: 9: type 'half2x4' is not supported
+error: 10: type 'half3x2' is not supported
+error: 11: type 'half3x4' is not supported
+error: 12: type 'half4x2' is not supported
+error: 13: type 'half4x3' is not supported
+error: 15: type 'uint' is not supported
+error: 16: type 'uint2' is not supported
+error: 17: type 'uint3' is not supported
+error: 18: type 'uint4' is not supported
+error: 20: type 'ushort' is not supported
+error: 21: type 'ushort2' is not supported
+error: 22: type 'ushort3' is not supported
+error: 23: type 'ushort4' is not supported
+error: 25: type 'uint4' is not supported
+error: 26: type 'ushort' is not supported
+error: 27: type 'float2x3' is not supported
+error: 29: type 'Sh42' is not supported
+error: 30: type 'Sus' is not supported
+error: 31: type 'Sau' is not supported
+26 errors