Fixed a couple of spots where sksl didn't have proper array support.
vec2 x[3] worked, but vec2[3] x didn't. Interface blocks also did
not work with array sizes.
BUG=skia:
Change-Id: I45b424891db46804f1e3c1f4793470b7b501a6de
Reviewed-on: https://skia-review.googlesource.com/8523
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 4b1eea3..18cfba7 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -584,19 +584,35 @@
}
void GLSLCodeGenerator::writeInterfaceBlock(const InterfaceBlock& intf) {
- if (intf.fVariable.fName == "gl_PerVertex") {
+ if (intf.fTypeName == "gl_PerVertex") {
return;
}
this->writeModifiers(intf.fVariable.fModifiers, true);
- this->writeLine(intf.fVariable.fType.name() + " {");
+ this->writeLine(intf.fTypeName + " {");
fIndentation++;
- for (const auto& f : intf.fVariable.fType.fields()) {
+ const Type* structType = &intf.fVariable.fType;
+ while (structType->kind() == Type::kArray_Kind) {
+ structType = &structType->componentType();
+ }
+ for (const auto& f : structType->fields()) {
this->writeModifiers(f.fModifiers, false);
this->writeType(*f.fType);
this->writeLine(" " + f.fName + ";");
}
fIndentation--;
- this->writeLine("};");
+ this->write("}");
+ if (intf.fInstanceName.size()) {
+ this->write(" ");
+ this->write(intf.fInstanceName);
+ for (const auto& size : intf.fSizes) {
+ this->write("[");
+ if (size) {
+ this->writeExpression(*size, kTopLevel_Precedence);
+ }
+ this->write("]");
+ }
+ }
+ this->writeLine(";");
}
void GLSLCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, bool global) {
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 70df81b..247766f 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -197,7 +197,6 @@
}
for (const auto& varDecl : decl.fVars) {
const Type* type = baseType;
- ASSERT(type->kind() != Type::kArray_Kind);
std::vector<std::unique_ptr<Expression>> sizes;
for (const auto& rawSize : varDecl.fSizes) {
if (rawSize) {
@@ -206,7 +205,7 @@
return nullptr;
}
SkString name = type->fName;
- uint64_t count;
+ int64_t count;
if (size->fKind == Expression::kIntLiteral_Kind) {
count = ((IntLiteral&) *size).fValue;
if (count <= 0) {
@@ -550,26 +549,67 @@
}
}
}
- Type* type = new Type(intf.fPosition, intf.fInterfaceName, fields);
+ Type* type = new Type(intf.fPosition, intf.fTypeName, fields);
old->takeOwnership(type);
- SkString name = intf.fValueName.size() > 0 ? intf.fValueName : intf.fInterfaceName;
- Variable* var = new Variable(intf.fPosition, intf.fModifiers, name, *type,
- Variable::kGlobal_Storage);
+ std::vector<std::unique_ptr<Expression>> sizes;
+ for (const auto& size : intf.fSizes) {
+ if (size) {
+ std::unique_ptr<Expression> converted = this->convertExpression(*size);
+ if (!converted) {
+ return nullptr;
+ }
+ SkString name = type->fName;
+ int64_t count;
+ if (converted->fKind == Expression::kIntLiteral_Kind) {
+ count = ((IntLiteral&) *converted).fValue;
+ if (count <= 0) {
+ fErrors.error(converted->fPosition, "array size must be positive");
+ }
+ name += "[" + to_string(count) + "]";
+ } else {
+ count = -1;
+ name += "[]";
+ }
+ type = new Type(name, Type::kArray_Kind, *type, (int) count);
+ fSymbolTable->takeOwnership((Type*) type);
+ sizes.push_back(std::move(converted));
+ } else {
+ type = new Type(type->fName + "[]", Type::kArray_Kind, *type, -1);
+ fSymbolTable->takeOwnership((Type*) type);
+ sizes.push_back(nullptr);
+ }
+ }
+ Variable* var = new Variable(intf.fPosition, intf.fModifiers,
+ intf.fInstanceName.size() ? intf.fInstanceName : intf.fTypeName,
+ *type, Variable::kGlobal_Storage);
old->takeOwnership(var);
- if (intf.fValueName.size()) {
- old->addWithoutOwnership(intf.fValueName, var);
+ if (intf.fInstanceName.size()) {
+ old->addWithoutOwnership(intf.fInstanceName, var);
} else {
for (size_t i = 0; i < fields.size(); i++) {
old->add(fields[i].fName, std::unique_ptr<Field>(new Field(intf.fPosition, *var,
(int) i)));
}
}
- return std::unique_ptr<InterfaceBlock>(new InterfaceBlock(intf.fPosition, *var, fSymbolTable));
+ return std::unique_ptr<InterfaceBlock>(new InterfaceBlock(intf.fPosition, *var,
+ intf.fTypeName,
+ intf.fInstanceName,
+ std::move(sizes),
+ fSymbolTable));
}
const Type* IRGenerator::convertType(const ASTType& type) {
const Symbol* result = (*fSymbolTable)[type.fName];
if (result && result->fKind == Symbol::kType_Kind) {
+ for (int size : type.fSizes) {
+ SkString name = result->fName + "[";
+ if (size != -1) {
+ name += to_string(size);
+ }
+ name += "]";
+ result = new Type(name, Type::kArray_Kind, (const Type&) *result, size);
+ fSymbolTable->takeOwnership((Type*) result);
+ }
return (const Type*) result;
}
fErrors.error(type.fPosition, "unknown type '" + type.fName + "'");
@@ -1315,6 +1355,21 @@
std::unique_ptr<Expression> IRGenerator::convertIndex(std::unique_ptr<Expression> base,
const ASTExpression& index) {
+ if (base->fKind == Expression::kTypeReference_Kind) {
+ if (index.fKind == ASTExpression::kInt_Kind) {
+ const Type& oldType = ((TypeReference&) *base).fValue;
+ int64_t size = ((const ASTIntLiteral&) index).fValue;
+ Type* newType = new Type(oldType.name() + "[" + to_string(size) + "]",
+ Type::kArray_Kind, oldType, size);
+ fSymbolTable->takeOwnership(newType);
+ return std::unique_ptr<Expression>(new TypeReference(fContext, base->fPosition,
+ *newType));
+
+ } else {
+ fErrors.error(base->fPosition, "array size must be a constant");
+ return nullptr;
+ }
+ }
if (base->fType.kind() != Type::kArray_Kind && base->fType.kind() != Type::kMatrix_Kind &&
base->fType.kind() != Type::kVector_Kind) {
fErrors.error(base->fPosition, "expected array, but found '" + base->fType.description() +
diff --git a/src/sksl/SkSLParser.cpp b/src/sksl/SkSLParser.cpp
index e7733cd..5bffe1e 100644
--- a/src/sksl/SkSLParser.cpp
+++ b/src/sksl/SkSLParser.cpp
@@ -415,7 +415,7 @@
}
fTypes.add(name.fText, std::unique_ptr<Type>(new Type(name.fPosition, name.fText, fields)));
return std::unique_ptr<ASTType>(new ASTType(name.fPosition, name.fText,
- ASTType::kStruct_Kind));
+ ASTType::kStruct_Kind, std::vector<int>()));
}
/* structDeclaration ((IDENTIFIER varDeclarationEnd) | SEMICOLON) */
@@ -770,7 +770,7 @@
}
}
-/* IDENTIFIER(type) */
+/* IDENTIFIER(type) (LBRACKET intLiteral? RBRACKET)* */
std::unique_ptr<ASTType> Parser::type() {
Token type;
if (!this->expect(Token::IDENTIFIER, "a type", &type)) {
@@ -780,11 +780,26 @@
this->error(type.fPosition, ("no type named '" + type.fText + "'").c_str());
return nullptr;
}
+ std::vector<int> sizes;
+ while (this->peek().fKind == Token::LBRACKET) {
+ this->expect(Token::LBRACKET, "'['");
+ if (this->peek().fKind != Token::RBRACKET) {
+ int64_t i;
+ if (this->intLiteral(&i)) {
+ sizes.push_back(i);
+ } else {
+ return nullptr;
+ }
+ } else {
+ sizes.push_back(-1);
+ }
+ this->expect(Token::RBRACKET, "']'");
+ }
return std::unique_ptr<ASTType>(new ASTType(type.fPosition, std::move(type.fText),
- ASTType::kIdentifier_Kind));
+ ASTType::kIdentifier_Kind, sizes));
}
-/* IDENTIFIER LBRACE varDeclaration* RBRACE */
+/* IDENTIFIER LBRACE varDeclaration* RBRACE (IDENTIFIER (LBRACKET expression? RBRACKET)*)? */
std::unique_ptr<ASTDeclaration> Parser::interfaceBlock(Modifiers mods) {
Token name;
if (!this->expect(Token::IDENTIFIER, "an identifier", &name)) {
@@ -807,14 +822,29 @@
decls.push_back(std::move(decl));
}
this->nextToken();
- SkString valueName;
+ SkString instanceName;
+ std::vector<std::unique_ptr<ASTExpression>> sizes;
if (this->peek().fKind == Token::IDENTIFIER) {
- valueName = this->nextToken().fText;
+ instanceName = this->nextToken().fText;
+ while (this->peek().fKind == Token::LBRACKET) {
+ this->expect(Token::LBRACKET, "'['");
+ if (this->peek().fKind != Token::RBRACKET) {
+ std::unique_ptr<ASTExpression> size = this->expression();
+ if (!size) {
+ return nullptr;
+ }
+ sizes.push_back(std::move(size));
+ } else {
+ sizes.push_back(nullptr);
+ }
+ this->expect(Token::RBRACKET, "']'");
+ }
}
this->expect(Token::SEMICOLON, "';'");
return std::unique_ptr<ASTDeclaration>(new ASTInterfaceBlock(name.fPosition, mods,
- name.fText, std::move(valueName),
- std::move(decls)));
+ name.fText, std::move(decls),
+ std::move(instanceName),
+ std::move(sizes)));
}
/* IF LPAREN expression RPAREN statement (ELSE statement)? */
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index 1158f94..0b7da57 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -1876,7 +1876,8 @@
false);
Variable intfVar(Position(), Modifiers(layout, Modifiers::kUniform_Flag), name,
intfStruct, Variable::kGlobal_Storage);
- InterfaceBlock intf(Position(), intfVar, st);
+ InterfaceBlock intf(Position(), intfVar, name, SkString(""),
+ std::vector<std::unique_ptr<Expression>>(), st);
fRTHeightStructId = this->writeInterfaceBlock(intf);
fRTHeightFieldIndex = 0;
}
diff --git a/src/sksl/ast/SkSLASTInterfaceBlock.h b/src/sksl/ast/SkSLASTInterfaceBlock.h
index 9cd99d5..7647fb5 100644
--- a/src/sksl/ast/SkSLASTInterfaceBlock.h
+++ b/src/sksl/ast/SkSLASTInterfaceBlock.h
@@ -25,31 +25,41 @@
// valueName is empty when it was not present in the source
ASTInterfaceBlock(Position position,
Modifiers modifiers,
- SkString interfaceName,
- SkString valueName,
- std::vector<std::unique_ptr<ASTVarDeclarations>> declarations)
+ SkString typeName,
+ std::vector<std::unique_ptr<ASTVarDeclarations>> declarations,
+ SkString instanceName,
+ std::vector<std::unique_ptr<ASTExpression>> sizes)
: INHERITED(position, kInterfaceBlock_Kind)
, fModifiers(modifiers)
- , fInterfaceName(std::move(interfaceName))
- , fValueName(std::move(valueName))
- , fDeclarations(std::move(declarations)) {}
+ , fTypeName(std::move(typeName))
+ , fDeclarations(std::move(declarations))
+ , fInstanceName(std::move(instanceName))
+ , fSizes(std::move(sizes)) {}
SkString description() const override {
- SkString result = fModifiers.description() + fInterfaceName + " {\n";
+ SkString result = fModifiers.description() + fTypeName + " {\n";
for (size_t i = 0; i < fDeclarations.size(); i++) {
result += fDeclarations[i]->description() + "\n";
}
result += "}";
- if (fValueName.size()) {
- result += " " + fValueName;
+ if (fInstanceName.size()) {
+ result += " " + fInstanceName;
+ for (const auto& size : fSizes) {
+ result += "[";
+ if (size) {
+ result += size->description();
+ }
+ result += "]";
+ }
}
return result + ";";
}
const Modifiers fModifiers;
- const SkString fInterfaceName;
- const SkString fValueName;
+ const SkString fTypeName;
const std::vector<std::unique_ptr<ASTVarDeclarations>> fDeclarations;
+ const SkString fInstanceName;
+ const std::vector<std::unique_ptr<ASTExpression>> fSizes;
typedef ASTDeclaration INHERITED;
};
diff --git a/src/sksl/ast/SkSLASTType.h b/src/sksl/ast/SkSLASTType.h
index 9cbded1..b95c3d7 100644
--- a/src/sksl/ast/SkSLASTType.h
+++ b/src/sksl/ast/SkSLASTType.h
@@ -21,10 +21,11 @@
kStruct_Kind
};
- ASTType(Position position, SkString name, Kind kind)
+ ASTType(Position position, SkString name, Kind kind, std::vector<int> sizes)
: INHERITED(position)
, fName(std::move(name))
- , fKind(kind) {}
+ , fKind(kind)
+ , fSizes(std::move(sizes)) {}
SkString description() const override {
return fName;
@@ -34,6 +35,9 @@
const Kind fKind;
+ // array sizes, -1 meaning unspecified
+ const std::vector<int> fSizes;
+
typedef ASTPositionNode INHERITED;
};
diff --git a/src/sksl/ir/SkSLInterfaceBlock.h b/src/sksl/ir/SkSLInterfaceBlock.h
index 0141953..0de37c5 100644
--- a/src/sksl/ir/SkSLInterfaceBlock.h
+++ b/src/sksl/ir/SkSLInterfaceBlock.h
@@ -25,23 +25,43 @@
* At the IR level, this is represented by a single variable of struct type.
*/
struct InterfaceBlock : public ProgramElement {
- InterfaceBlock(Position position, const Variable& var, std::shared_ptr<SymbolTable> typeOwner)
- : INHERITED(position, kInterfaceBlock_Kind)
+ InterfaceBlock(Position position, const Variable& var, SkString typeName, SkString instanceName,
+ std::vector<std::unique_ptr<Expression>> sizes,
+ std::shared_ptr<SymbolTable> typeOwner)
+ : INHERITED(position, kInterfaceBlock_Kind)
, fVariable(std::move(var))
- , fTypeOwner(typeOwner) {
- ASSERT(fVariable.fType.kind() == Type::kStruct_Kind);
- }
+ , fTypeName(std::move(typeName))
+ , fInstanceName(std::move(instanceName))
+ , fSizes(std::move(sizes))
+ , fTypeOwner(typeOwner) {}
SkString description() const override {
- SkString result = fVariable.fModifiers.description() + fVariable.fName + " {\n";
- for (size_t i = 0; i < fVariable.fType.fields().size(); i++) {
- result += fVariable.fType.fields()[i].description() + "\n";
+ SkString result = fVariable.fModifiers.description() + fTypeName + " {\n";
+ const Type* structType = &fVariable.fType;
+ while (structType->kind() == Type::kArray_Kind) {
+ structType = &structType->componentType();
}
- result += "};";
- return result;
+ for (const auto& f : structType->fields()) {
+ result += f.description() + "\n";
+ }
+ result += "}";
+ if (fInstanceName.size()) {
+ result += " " + fInstanceName;
+ for (const auto& size : fSizes) {
+ result += "[";
+ if (size) {
+ result += size->description();
+ }
+ result += "]";
+ }
+ }
+ return result + ";";
}
const Variable& fVariable;
+ const SkString fTypeName;
+ const SkString fInstanceName;
+ const std::vector<std::unique_ptr<Expression>> fSizes;
const std::shared_ptr<SymbolTable> fTypeOwner;
typedef ProgramElement INHERITED;
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index 533c203..a57fd9d 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -187,6 +187,7 @@
"bool w;"
"};"
"void main() {"
+ " sk_FragColor = vec4(x, y[0], y[1], 0);"
"}",
*SkSL::ShaderCapsFactory::Default(),
"#version 400\n"
@@ -198,6 +199,39 @@
" bool w;\n"
"};\n"
"void main() {\n"
+ " sk_FragColor = vec4(x, y[0], y[1], 0.0);\n"
+ "}\n");
+ test(r,
+ "uniform testBlock {"
+ "float x;"
+ "} test;"
+ "void main() {"
+ " sk_FragColor = vec4(test.x);"
+ "}",
+ *SkSL::ShaderCapsFactory::Default(),
+ "#version 400\n"
+ "out vec4 sk_FragColor;\n"
+ "uniform testBlock {\n"
+ " float x;\n"
+ "} test;\n"
+ "void main() {\n"
+ " sk_FragColor = vec4(test.x);\n"
+ "}\n");
+ test(r,
+ "uniform testBlock {"
+ "float x;"
+ "} test[2];"
+ "void main() {"
+ " sk_FragColor = vec4(test[1].x);"
+ "}",
+ *SkSL::ShaderCapsFactory::Default(),
+ "#version 400\n"
+ "out vec4 sk_FragColor;\n"
+ "uniform testBlock {\n"
+ " float x;\n"
+ "} test[2];\n"
+ "void main() {\n"
+ " sk_FragColor = vec4(test[1].x);\n"
"}\n");
}
@@ -702,4 +736,18 @@
" sk_FragColor = vec4(gl_ClipDistance[0]);\n"
"}\n");
}
+
+DEF_TEST(SkSLArrayTypes, r) {
+ test(r,
+ "void main() { vec2 x[2] = vec2[2](vec2(1), vec2(2));"
+ "vec2[2] y = vec2[2](vec2(3), vec2(4)); }",
+ *SkSL::ShaderCapsFactory::Default(),
+ "#version 400\n"
+ "out vec4 sk_FragColor;\n"
+ "void main() {\n"
+ " vec2 x[2] = vec2[2](vec2(1.0), vec2(2.0));\n"
+ " vec2[2] y = vec2[2](vec2(3.0), vec2(4.0));\n"
+ "}\n");
+}
+
#endif