Fix const globals in Metal
We were emitting this at global scope (not in Globals). That would lead
to errors about the variable needing to be in the constant address
space. (You can see the result in ConstArray.metal - the old code was
invalid). Also, we were already making references use _globals, so the
code was double-wrong (or half-right, depending on your perspective).
After the core change, writeVarDeclaration was only used for local
scope, and writeModifiers never used the 'globalContext' parameter.
The removal of finishLine() changed every test output, unfortunately.
Change-Id: Icc1356ba2cc3c339b2f5759b3d18523fd39395bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408356
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/codegen/SkSLMetalCodeGenerator.cpp b/src/sksl/codegen/SkSLMetalCodeGenerator.cpp
index c265756..864acfc 100644
--- a/src/sksl/codegen/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLMetalCodeGenerator.cpp
@@ -233,7 +233,7 @@
separator = ", ";
const Variable* param = function.parameters()[index];
- this->writeModifiers(param->modifiers(), /*globalContext=*/false);
+ this->writeModifiers(param->modifiers());
const Type* type = outVars[index] ? &outVars[index]->type() : &arguments[index]->type();
this->writeType(*type);
@@ -1690,7 +1690,7 @@
}
this->write(separator);
separator = ", ";
- this->writeModifiers(param->modifiers(), /*globalContext=*/false);
+ this->writeModifiers(param->modifiers());
const Type* type = ¶m->type();
this->writeType(*type);
if (param->modifiers().fFlags & Modifiers::kOut_Flag) {
@@ -1772,8 +1772,7 @@
this->write(buffer.str());
}
-void MetalCodeGenerator::writeModifiers(const Modifiers& modifiers,
- bool globalContext) {
+void MetalCodeGenerator::writeModifiers(const Modifiers& modifiers) {
if (modifiers.fFlags & Modifiers::kOut_Flag) {
this->write("thread ");
}
@@ -1786,7 +1785,7 @@
if ("sk_PerVertex" == intf.typeName()) {
return;
}
- this->writeModifiers(intf.variable().modifiers(), /*globalContext=*/true);
+ this->writeModifiers(intf.variable().modifiers());
this->write("struct ");
this->writeLine(intf.typeName() + " {");
const Type* structType = &intf.variable().type();
@@ -1856,7 +1855,7 @@
return;
}
currentOffset += fieldSize;
- this->writeModifiers(field.fModifiers, /*globalContext=*/false);
+ this->writeModifiers(field.fModifiers);
this->writeType(*fieldType);
this->write(" ");
this->writeName(field.fName);
@@ -1878,11 +1877,8 @@
this->write(name);
}
-void MetalCodeGenerator::writeVarDeclaration(const VarDeclaration& varDecl, bool global) {
- if (global && !(varDecl.var().modifiers().fFlags & Modifiers::kConst_Flag)) {
- return;
- }
- this->writeModifiers(varDecl.var().modifiers(), global);
+void MetalCodeGenerator::writeVarDeclaration(const VarDeclaration& varDecl) {
+ this->writeModifiers(varDecl.var().modifiers());
this->writeType(varDecl.var().type());
this->write(" ");
this->writeName(varDecl.var().name());
@@ -1906,7 +1902,7 @@
this->writeReturnStatement(s.as<ReturnStatement>());
break;
case Statement::Kind::kVarDeclaration:
- this->writeVarDeclaration(s.as<VarDeclaration>(), false);
+ this->writeVarDeclaration(s.as<VarDeclaration>());
break;
case Statement::Kind::kIf:
this->writeIfStatement(s.as<IfStatement>());
@@ -2208,16 +2204,17 @@
const GlobalVarDeclaration& global = element->as<GlobalVarDeclaration>();
const VarDeclaration& decl = global.declaration()->as<VarDeclaration>();
const Variable& var = decl.var();
- if ((!var.modifiers().fFlags && -1 == var.modifiers().fLayout.fBuiltin) ||
- var.type().typeKind() == Type::TypeKind::kSampler) {
- if (var.type().typeKind() == Type::TypeKind::kSampler) {
- // Samplers are represented as a "texture/sampler" duo in the global struct.
- visitor->visitTexture(var.type(), var.name());
- visitor->visitSampler(var.type(), String(var.name()) + SAMPLER_SUFFIX);
- } else {
- // Visit a regular variable.
- visitor->visitVariable(var, decl.value().get());
- }
+ if (var.type().typeKind() == Type::TypeKind::kSampler) {
+ // Samplers are represented as a "texture/sampler" duo in the global struct.
+ visitor->visitTexture(var.type(), var.name());
+ visitor->visitSampler(var.type(), String(var.name()) + SAMPLER_SUFFIX);
+ continue;
+ }
+
+ if (!(var.modifiers().fFlags & ~Modifiers::kConst_Flag) &&
+ -1 == var.modifiers().fLayout.fBuiltin) {
+ // Visit a regular variable.
+ visitor->visitVariable(var, decl.value().get());
}
}
}
@@ -2250,6 +2247,7 @@
void visitVariable(const Variable& var, const Expression* value) override {
this->addElement();
fCodeGen->write(" ");
+ fCodeGen->writeModifiers(var.modifiers());
fCodeGen->writeType(var.type());
fCodeGen->write(" ");
fCodeGen->writeName(var.name());
@@ -2329,19 +2327,8 @@
switch (e.kind()) {
case ProgramElement::Kind::kExtension:
break;
- case ProgramElement::Kind::kGlobalVar: {
- const GlobalVarDeclaration& global = e.as<GlobalVarDeclaration>();
- const VarDeclaration& decl = global.declaration()->as<VarDeclaration>();
- int builtin = decl.var().modifiers().fLayout.fBuiltin;
- if (-1 == builtin) {
- // normal var
- this->writeVarDeclaration(decl, true);
- this->finishLine();
- } else if (SK_FRAGCOLOR_BUILTIN == builtin) {
- // ignore
- }
+ case ProgramElement::Kind::kGlobalVar:
break;
- }
case ProgramElement::Kind::kInterfaceBlock:
// handled in writeInterfaceBlocks, do nothing
break;
@@ -2355,8 +2342,7 @@
this->writeFunctionPrototype(e.as<FunctionPrototype>());
break;
case ProgramElement::Kind::kModifiers:
- this->writeModifiers(e.as<ModifiersDeclaration>().modifiers(),
- /*globalContext=*/true);
+ this->writeModifiers(e.as<ModifiersDeclaration>().modifiers());
this->writeLine(";");
break;
case ProgramElement::Kind::kEnum: