Refactored SkSL function creation and error handling
This breaks up the giant IRGenerator::convertFunction method into more-
manageable chunks, moves the functionality into FunctionDeclaration,
and funnels the DSL through it so it receives the same error checking.
Change-Id: Icf2ac650ab3d5276d8c0134062a4e7e220f9bf32
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402778
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl.gni b/gn/sksl.gni
index ad9a00d..266e8ff 100644
--- a/gn/sksl.gni
+++ b/gn/sksl.gni
@@ -132,6 +132,7 @@
"$_src/sksl/ir/SkSLForStatement.h",
"$_src/sksl/ir/SkSLFunctionCall.cpp",
"$_src/sksl/ir/SkSLFunctionCall.h",
+ "$_src/sksl/ir/SkSLFunctionDeclaration.cpp",
"$_src/sksl/ir/SkSLFunctionDeclaration.h",
"$_src/sksl/ir/SkSLFunctionDefinition.h",
"$_src/sksl/ir/SkSLFunctionPrototype.h",
diff --git a/include/sksl/DSLFunction.h b/include/sksl/DSLFunction.h
index 60f14e4..5aab49f 100644
--- a/include/sksl/DSLFunction.h
+++ b/include/sksl/DSLFunction.h
@@ -76,8 +76,8 @@
DSLExpression call(SkTArray<DSLExpression> args);
- const SkSL::Type* fReturnType;
- const SkSL::FunctionDeclaration* fDecl;
+ const SkSL::Type* fReturnType = nullptr;
+ const SkSL::FunctionDeclaration* fDecl = nullptr;
};
} // namespace dsl
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 67c9321..ffe4a5c 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -334,7 +334,7 @@
Modifiers::kFlat_Flag | Modifiers::kNoPerspective_Flag;
}
// TODO(skbug.com/11301): Migrate above checks into building a mask of permitted layout flags
- this->checkModifiers(offset, modifiers, permitted, /*permittedLayoutFlags=*/~0);
+ CheckModifiers(fContext, offset, modifiers, permitted, /*permittedLayoutFlags=*/~0);
}
std::unique_ptr<Variable> IRGenerator::convertVar(int offset, const Modifiers& modifiers,
@@ -795,15 +795,17 @@
return ExpressionStatement::Make(fContext, std::move(result));
}
-void IRGenerator::checkModifiers(int offset,
+void IRGenerator::CheckModifiers(const Context& context,
+ int offset,
const Modifiers& modifiers,
int permittedModifierFlags,
int permittedLayoutFlags) {
+ ErrorReporter& errorReporter = context.fErrors;
int flags = modifiers.fFlags;
auto checkModifier = [&](Modifiers::Flag flag, const char* name) {
if (flags & flag) {
if (!(permittedModifierFlags & flag)) {
- this->errorReporter().error(offset, "'" + String(name) + "' is not permitted here");
+ errorReporter.error(offset, "'" + String(name) + "' is not permitted here");
}
flags &= ~flag;
}
@@ -824,8 +826,8 @@
auto checkLayout = [&](Layout::Flag flag, const char* name) {
if (layoutFlags & flag) {
if (!(permittedLayoutFlags & flag)) {
- this->errorReporter().error(
- offset, "layout qualifier '" + String(name) + "' is not permitted here");
+ errorReporter.error(offset, "layout qualifier '" + String(name) +
+ "' is not permitted here");
}
layoutFlags &= ~flag;
}
@@ -969,49 +971,15 @@
if (returnType == nullptr) {
return;
}
- if (returnType->isArray()) {
- this->errorReporter().error(
- f.fOffset, "functions may not return type '" + returnType->displayName() + "'");
- return;
- }
- if (this->strictES2Mode() && returnType->isOrContainsArray()) {
- this->errorReporter().error(f.fOffset,
- "functions may not return structs containing arrays");
- return;
- }
- if (!fIsBuiltinCode && !returnType->isVoid() && returnType->componentType().isOpaque()) {
- this->errorReporter().error(
- f.fOffset,
- "functions may not return opaque type '" + returnType->displayName() + "'");
- return;
- }
const ASTNode::FunctionData& funcData = f.getFunctionData();
bool isMain = (funcData.fName == "main");
- // Check function modifiers.
- this->checkModifiers(
- f.fOffset,
- funcData.fModifiers,
- Modifiers::kHasSideEffects_Flag | Modifiers::kInline_Flag | Modifiers::kNoInline_Flag,
- /*permittedLayoutFlags=*/0);
- if ((funcData.fModifiers.fFlags & Modifiers::kInline_Flag) &&
- (funcData.fModifiers.fFlags & Modifiers::kNoInline_Flag)) {
- this->errorReporter().error(f.fOffset, "functions cannot be both 'inline' and 'noinline'");
- }
-
- auto typeIsValidForColor = [&](const Type& type) {
- return type == *fContext.fTypes.fHalf4 || type == *fContext.fTypes.fFloat4;
- };
-
- // Check modifiers on each function parameter.
- std::vector<const Variable*> parameters;
+ std::vector<std::unique_ptr<Variable>> parameters;
+ parameters.reserve(funcData.fParameterCount);
for (size_t i = 0; i < funcData.fParameterCount; ++i) {
const ASTNode& param = *(iter++);
SkASSERT(param.fKind == ASTNode::Kind::kParameter);
const ASTNode::ParameterData& pd = param.getParameterData();
- this->checkModifiers(param.fOffset, pd.fModifiers,
- Modifiers::kConst_Flag | Modifiers::kIn_Flag | Modifiers::kOut_Flag,
- /*permittedLayoutFlags=*/0);
auto paramIter = param.begin();
const Type* type = this->convertType(*(paramIter++));
if (!type) {
@@ -1024,211 +992,40 @@
}
type = fSymbolTable->addArrayDimension(type, arraySize);
}
- // Only the (builtin) declarations of 'sample' are allowed to have shader/colorFilter or FP
- // parameters. You can pass other opaque types to functions safely; this restriction is
- // specific to "child" objects.
- if ((type->isEffectChild() || type->isFragmentProcessor()) && !fIsBuiltinCode) {
- this->errorReporter().error(
- param.fOffset, "parameters of type '" + type->displayName() + "' not allowed");
- return;
- }
- Modifiers m = pd.fModifiers;
- if (isMain && (this->programKind() == ProgramKind::kRuntimeColorFilter ||
- this->programKind() == ProgramKind::kRuntimeShader ||
- this->programKind() == ProgramKind::kFragmentProcessor)) {
- // We verify that the signature is fully correct later. For now, if this is an .fp or
- // runtime effect of any flavor, a float2 param is supposed to be the coords, and
- // a half4/float parameter is supposed to be the input color:
- if (*type == *fContext.fTypes.fFloat2) {
- m.fLayout.fBuiltin = SK_MAIN_COORDS_BUILTIN;
- } else if(typeIsValidForColor(*type)) {
- m.fLayout.fBuiltin = SK_INPUT_COLOR_BUILTIN;
- }
- }
- if (isMain && (this->programKind() == ProgramKind::kFragment)) {
- // For testing purposes, we have .sksl inputs that are treated as both runtime effects
- // and fragment shaders. To make that work, fragment shaders are allowed to have a
- // coords parameter. We turn it into sk_FragCoord.
- if (*type == *fContext.fTypes.fFloat2) {
- m.fLayout.fBuiltin = SK_FRAGCOORD_BUILTIN;
- }
- }
-
- const Variable* var = fSymbolTable->takeOwnershipOfSymbol(
- std::make_unique<Variable>(param.fOffset, this->modifiersPool().add(m), pd.fName,
- type, fIsBuiltinCode, Variable::Storage::kParameter));
- parameters.push_back(var);
+ parameters.push_back(std::make_unique<Variable>(param.fOffset,
+ this->modifiersPool().add(pd.fModifiers),
+ pd.fName,
+ type,
+ fIsBuiltinCode,
+ Variable::Storage::kParameter));
}
- auto paramIsCoords = [&](int idx) {
- const Variable* p = parameters[idx];
- return p->type() == *fContext.fTypes.fFloat2 &&
- p->modifiers().fFlags == 0 &&
- p->modifiers().fLayout.fBuiltin == (this->programKind() == ProgramKind::kFragment
- ? SK_FRAGCOORD_BUILTIN
- : SK_MAIN_COORDS_BUILTIN);
- };
-
- auto paramIsInputColor = [&](int idx) {
- return typeIsValidForColor(parameters[idx]->type()) &&
- parameters[idx]->modifiers().fFlags == 0 &&
- parameters[idx]->modifiers().fLayout.fBuiltin == SK_INPUT_COLOR_BUILTIN;
- };
-
- // Check the function signature of `main`.
- if (isMain) {
- switch (this->programKind()) {
- case ProgramKind::kRuntimeColorFilter: {
- // (half4|float4) main(half4|float4)
- if (!typeIsValidForColor(*returnType)) {
- this->errorReporter().error(f.fOffset,
- "'main' must return: 'vec4', 'float4', or 'half4'");
- return;
- }
- bool validParams = (parameters.size() == 1 && paramIsInputColor(0));
- if (!validParams) {
- this->errorReporter().error(
- f.fOffset, "'main' parameter must be 'vec4', 'float4', or 'half4'");
- return;
- }
- break;
- }
- case ProgramKind::kRuntimeShader: {
- // (half4|float4) main(float2) -or- (half4|float4) main(float2, half4|float4)
- if (!typeIsValidForColor(*returnType)) {
- this->errorReporter().error(f.fOffset,
- "'main' must return: 'vec4', 'float4', or 'half4'");
- return;
- }
- bool validParams =
- (parameters.size() == 1 && paramIsCoords(0)) ||
- (parameters.size() == 2 && paramIsCoords(0) && paramIsInputColor(1));
- if (!validParams) {
- this->errorReporter().error(
- f.fOffset, "'main' parameters must be (float2, (vec4|float4|half4)?)");
- }
- break;
- }
- case ProgramKind::kFragmentProcessor: {
- if (*returnType != *fContext.fTypes.fHalf4) {
- this->errorReporter().error(f.fOffset, ".fp 'main' must return 'half4'");
- return;
- }
- bool validParams = (parameters.size() == 0) ||
- (parameters.size() == 1 && paramIsCoords(0));
- if (!validParams) {
- this->errorReporter().error(
- f.fOffset, ".fp 'main' must be declared main() or main(float2)");
- return;
- }
- break;
- }
- case ProgramKind::kGeneric:
- // No rules apply here
- break;
- case ProgramKind::kFragment: {
- bool validParams = (parameters.size() == 0) ||
- (parameters.size() == 1 && paramIsCoords(0));
- if (!validParams) {
- this->errorReporter().error(f.fOffset,
- "shader 'main' must be main() or main(float2)");
- return;
- }
- break;
- }
- case ProgramKind::kVertex:
- case ProgramKind::kGeometry:
- if (parameters.size()) {
- this->errorReporter().error(f.fOffset,
- "shader 'main' must have zero parameters");
- }
- break;
- }
+ // Conservatively assume all user-defined functions have side effects.
+ Modifiers declModifiers = funcData.fModifiers;
+ if (!fIsBuiltinCode) {
+ declModifiers.fFlags |= Modifiers::kHasSideEffects_Flag;
}
- // Find existing declarations and report conflicts.
- const FunctionDeclaration* decl = nullptr;
- const Symbol* entry = (*fSymbolTable)[funcData.fName];
- if (entry) {
- std::vector<const FunctionDeclaration*> functions;
- switch (entry->kind()) {
- case Symbol::Kind::kUnresolvedFunction:
- functions = entry->as<UnresolvedFunction>().functions();
- break;
- case Symbol::Kind::kFunctionDeclaration:
- functions.push_back(&entry->as<FunctionDeclaration>());
- break;
- default:
- this->errorReporter().error(f.fOffset,
- "symbol '" + funcData.fName + "' was already defined");
- return;
- }
- for (const FunctionDeclaration* other : functions) {
- SkASSERT(other->name() == funcData.fName);
- if (parameters.size() == other->parameters().size()) {
- bool match = true;
- for (size_t i = 0; i < parameters.size(); i++) {
- if (parameters[i]->type() != other->parameters()[i]->type()) {
- match = false;
- break;
- }
- }
- if (match) {
- if (*returnType != other->returnType()) {
- FunctionDeclaration newDecl(f.fOffset,
- this->modifiersPool().add(funcData.fModifiers),
- funcData.fName,
- parameters,
- returnType,
- fIsBuiltinCode);
- this->errorReporter().error(
- f.fOffset, "functions '" + newDecl.description() + "' and '" +
- other->description() + "' differ only in return type");
- return;
- }
- decl = other;
- for (size_t i = 0; i < parameters.size(); i++) {
- if (parameters[i]->modifiers() != other->parameters()[i]->modifiers()) {
- this->errorReporter().error(
- f.fOffset,
- "modifiers on parameter " + to_string((uint64_t)i + 1) +
- " differ between declaration and definition");
- return;
- }
- }
- if (other->definition() && !other->isBuiltin()) {
- this->errorReporter().error(
- f.fOffset, "duplicate definition of " + other->description());
- return;
- }
- break;
- }
- }
- }
+ if (fContext.fConfig->fSettings.fForceNoInline) {
+ // Apply the `noinline` modifier to every function. This allows us to test Runtime
+ // Effects without any inlining, even when the code is later added to a paint.
+ declModifiers.fFlags &= ~Modifiers::kInline_Flag;
+ declModifiers.fFlags |= Modifiers::kNoInline_Flag;
}
+
+ const FunctionDeclaration* decl = FunctionDeclaration::Convert(
+ fContext,
+ *fSymbolTable,
+ *fModifiers,
+ f.fOffset,
+ this->modifiersPool().add(declModifiers),
+ funcData.fName,
+ std::move(parameters),
+ returnType,
+ fIsBuiltinCode);
if (!decl) {
- // Conservatively assume all user-defined functions have side effects.
- Modifiers declModifiers = funcData.fModifiers;
- if (!fIsBuiltinCode) {
- declModifiers.fFlags |= Modifiers::kHasSideEffects_Flag;
- }
-
- if (fContext.fConfig->fSettings.fForceNoInline) {
- // Apply the `noinline` modifier to every function. This allows us to test Runtime
- // Effects without any inlining, even when the code is later added to a paint.
- declModifiers.fFlags &= ~Modifiers::kInline_Flag;
- declModifiers.fFlags |= Modifiers::kNoInline_Flag;
- }
-
- // Create a new declaration.
- decl = fSymbolTable->add(
- std::make_unique<FunctionDeclaration>(f.fOffset,
- this->modifiersPool().add(declModifiers),
- funcData.fName,
- parameters,
- returnType,
- fIsBuiltinCode));
+ return;
}
if (iter == f.end()) {
// If there's no body, we've found a prototype.
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index 0691d00..b4ec32b 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -142,6 +142,12 @@
void pushSymbolTable();
void popSymbolTable();
+ static void CheckModifiers(const Context& context,
+ int offset,
+ const Modifiers& modifiers,
+ int permittedModifierFlags,
+ int permittedLayoutFlags);
+
const Context& fContext;
private:
@@ -158,10 +164,6 @@
*/
std::unique_ptr<ModifiersPool> releaseModifiers();
- void checkModifiers(int offset,
- const Modifiers& modifiers,
- int permittedModifierFlags,
- int permittedLayoutFlags);
void checkVarDeclaration(int offset,
const Modifiers& modifiers,
const Type* baseType,
diff --git a/src/sksl/dsl/DSLFunction.cpp b/src/sksl/dsl/DSLFunction.cpp
index 7e2971e..df74480 100644
--- a/src/sksl/dsl/DSLFunction.cpp
+++ b/src/sksl/dsl/DSLFunction.cpp
@@ -20,7 +20,7 @@
void DSLFunction::init(const DSLType& returnType, const char* name,
std::vector<DSLVar*> params) {
- std::vector<const Variable*> paramVars;
+ std::vector<std::unique_ptr<Variable>> paramVars;
paramVars.reserve(params.size());
bool isMain = !strcmp(name, "main");
auto typeIsValidForColor = [&](const SkSL::Type& type) {
@@ -59,20 +59,24 @@
}
}
}
- paramVars.push_back(&DSLWriter::Var(*param));
+ std::unique_ptr<SkSL::Variable> paramVar = DSLWriter::ParameterVar(*param);
+ SkASSERT(paramVar);
+ paramVars.push_back(std::move(paramVar));
param->fDeclaration = nullptr;
}
- SkSL::SymbolTable& symbols = *DSLWriter::SymbolTable();
- fDecl = symbols.add(std::make_unique<SkSL::FunctionDeclaration>(
- /*offset=*/-1,
- DSLWriter::Modifiers(SkSL::Modifiers()),
- isMain ? name : DSLWriter::Name(name),
- std::move(paramVars), fReturnType,
- /*builtin=*/false));
+ fDecl = SkSL::FunctionDeclaration::Convert(DSLWriter::Context(),
+ *DSLWriter::SymbolTable(),
+ *DSLWriter::IRGenerator().fModifiers, /*offset=*/-1,
+ DSLWriter::Modifiers(SkSL::Modifiers()),
+ isMain ? name : DSLWriter::Name(name),
+ std::move(paramVars), fReturnType,
+ /*isBuiltin=*/false);
}
void DSLFunction::define(DSLBlock block) {
- SkASSERT(fDecl);
+ if (!fDecl) {
+ return;
+ }
SkASSERTF(!fDecl->definition(), "function '%s' already defined", fDecl->description().c_str());
std::unique_ptr<Statement> body = block.release();
DSLWriter::IRGenerator().finalizeFunction(*fDecl, body.get());
diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp
index d0de74c..16c0306 100644
--- a/src/sksl/dsl/priv/DSLWriter.cpp
+++ b/src/sksl/dsl/priv/DSLWriter.cpp
@@ -226,6 +226,22 @@
return *var.fVar;
}
+std::unique_ptr<SkSL::Variable> DSLWriter::ParameterVar(DSLVar& var) {
+ // This should only be called on undeclared parameter variables, but we allow the creation to go
+ // ahead regardless so we don't have to worry about null pointers potentially sneaking in and
+ // breaking things. DSLFunction is responsible for reporting errors for invalid parameters.
+ std::unique_ptr<SkSL::Variable> skslVar = DSLWriter::IRGenerator().convertVar(
+ /*offset=*/-1,
+ var.fModifiers.fModifiers,
+ &var.fType.skslType(),
+ var.fName,
+ /*isArray=*/false,
+ /*arraySize=*/nullptr,
+ var.fStorage);
+ var.fVar = skslVar.get();
+ return skslVar;
+}
+
std::unique_ptr<SkSL::Statement> DSLWriter::Declaration(DSLVar& var) {
Var(var);
return std::move(var.fDeclaration);
diff --git a/src/sksl/dsl/priv/DSLWriter.h b/src/sksl/dsl/priv/DSLWriter.h
index 3c379d0..cc5d7a8 100644
--- a/src/sksl/dsl/priv/DSLWriter.h
+++ b/src/sksl/dsl/priv/DSLWriter.h
@@ -86,11 +86,16 @@
static const SkSL::Modifiers* Modifiers(const SkSL::Modifiers& modifiers);
/**
- * Returns the SkSL variable corresponding to a DSLVar.
+ * Returns the SkSL variable corresponding to a (non-parameter) DSLVar.
*/
static const SkSL::Variable& Var(DSLVar& var);
/**
+ * Creates an SkSL variable corresponding to a parameter DSLVar.
+ */
+ static std::unique_ptr<SkSL::Variable> ParameterVar(DSLVar& var);
+
+ /**
* Returns the SkSL declaration corresponding to a DSLVar.
*/
static std::unique_ptr<SkSL::Statement> Declaration(DSLVar& var);
diff --git a/src/sksl/ir/SkSLFunctionDeclaration.cpp b/src/sksl/ir/SkSLFunctionDeclaration.cpp
new file mode 100644
index 0000000..56c73cf
--- /dev/null
+++ b/src/sksl/ir/SkSLFunctionDeclaration.cpp
@@ -0,0 +1,298 @@
+/*
+ * Copyright 2021 Google LLC.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/sksl/ir/SkSLFunctionDeclaration.h"
+
+#include "src/sksl/SkSLCompiler.h"
+#include "src/sksl/SkSLIRGenerator.h"
+#include "src/sksl/ir/SkSLUnresolvedFunction.h"
+
+namespace SkSL {
+
+static bool check_modifiers(const Context& context, int offset, const Modifiers& modifiers) {
+ IRGenerator::CheckModifiers(
+ context,
+ offset,
+ modifiers,
+ Modifiers::kHasSideEffects_Flag | Modifiers::kInline_Flag | Modifiers::kNoInline_Flag,
+ /*permittedLayoutFlags=*/0);
+ if ((modifiers.fFlags & Modifiers::kInline_Flag) &&
+ (modifiers.fFlags & Modifiers::kNoInline_Flag)) {
+ context.fErrors.error(offset, "functions cannot be both 'inline' and 'noinline'");
+ return false;
+ }
+ return true;
+}
+
+static bool check_return_type(const Context& context, int offset, const Type& returnType,
+ bool isBuiltin) {
+ ErrorReporter& errors = context.fErrors;
+ if (returnType.isArray()) {
+ errors.error(offset, "functions may not return type '" + returnType.displayName() + "'");
+ return false;
+ }
+ if (context.fConfig->strictES2Mode() && returnType.isOrContainsArray()) {
+ errors.error(offset, "functions may not return structs containing arrays");
+ return false;
+ }
+ if (!isBuiltin && !returnType.isVoid() && returnType.componentType().isOpaque()) {
+ errors.error(offset, "functions may not return opaque type '" + returnType.displayName() +
+ "'");
+ return false;
+ }
+ return true;
+}
+
+static bool check_parameters(const Context& context, ModifiersPool& modifiersPool,
+ std::vector<std::unique_ptr<Variable>>& parameters, bool isMain,
+ bool isBuiltin) {
+ auto typeIsValidForColor = [&](const Type& type) {
+ return type == *context.fTypes.fHalf4 || type == *context.fTypes.fFloat4;
+ };
+
+ // Check modifiers on each function parameter.
+ for (auto& param : parameters) {
+ IRGenerator::CheckModifiers(context, param->fOffset, param->modifiers(),
+ Modifiers::kConst_Flag | Modifiers::kIn_Flag |
+ Modifiers::kOut_Flag, /*permittedLayoutFlags=*/0);
+ const Type& type = param->type();
+ // Only the (builtin) declarations of 'sample' are allowed to have shader/colorFilter or FP
+ // parameters. You can pass other opaque types to functions safely; this restriction is
+ // specific to "child" objects.
+ if ((type.isEffectChild() || type.isFragmentProcessor()) && !isBuiltin) {
+ context.fErrors.error(param->fOffset, "parameters of type '" + type.displayName() +
+ "' not allowed");
+ return false;
+ }
+
+ Modifiers m = param->modifiers();
+ ProgramKind kind = context.fConfig->fKind;
+ if (isMain && (kind == ProgramKind::kRuntimeColorFilter ||
+ kind == ProgramKind::kRuntimeShader ||
+ kind == ProgramKind::kFragmentProcessor)) {
+ // We verify that the signature is fully correct later. For now, if this is an .fp or
+ // runtime effect of any flavor, a float2 param is supposed to be the coords, and
+ // a half4/float parameter is supposed to be the input color:
+ if (type == *context.fTypes.fFloat2) {
+ m.fLayout.fBuiltin = SK_MAIN_COORDS_BUILTIN;
+ } else if(typeIsValidForColor(type)) {
+ m.fLayout.fBuiltin = SK_INPUT_COLOR_BUILTIN;
+ }
+ if (m.fLayout.fBuiltin) {
+ param->setModifiers(modifiersPool.add(m));
+ }
+ }
+ if (isMain && (kind == ProgramKind::kFragment)) {
+ // For testing purposes, we have .sksl inputs that are treated as both runtime effects
+ // and fragment shaders. To make that work, fragment shaders are allowed to have a
+ // coords parameter. We turn it into sk_FragCoord.
+ if (type == *context.fTypes.fFloat2) {
+ m.fLayout.fBuiltin = SK_FRAGCOORD_BUILTIN;
+ param->setModifiers(modifiersPool.add(m));
+ }
+ }
+ }
+ return true;
+}
+
+static bool check_main_signature(const Context& context, int offset, const Type& returnType,
+ std::vector<std::unique_ptr<Variable>>& parameters,
+ bool isBuiltin) {
+ ErrorReporter& errors = context.fErrors;
+ ProgramKind kind = context.fConfig->fKind;
+
+ auto typeIsValidForColor = [&](const Type& type) {
+ return type == *context.fTypes.fHalf4 || type == *context.fTypes.fFloat4;
+ };
+
+ auto paramIsCoords = [&](int idx) {
+ const Variable& p = *parameters[idx];
+ return p.type() == *context.fTypes.fFloat2 &&
+ p.modifiers().fFlags == 0 &&
+ p.modifiers().fLayout.fBuiltin == (kind == ProgramKind::kFragment
+ ? SK_FRAGCOORD_BUILTIN
+ : SK_MAIN_COORDS_BUILTIN);
+ };
+
+ auto paramIsInputColor = [&](int idx) {
+ return typeIsValidForColor(parameters[idx]->type()) &&
+ parameters[idx]->modifiers().fFlags == 0 &&
+ parameters[idx]->modifiers().fLayout.fBuiltin == SK_INPUT_COLOR_BUILTIN;
+ };
+
+ switch (kind) {
+ case ProgramKind::kRuntimeColorFilter: {
+ // (half4|float4) main(half4|float4)
+ if (!typeIsValidForColor(returnType)) {
+ errors.error(offset, "'main' must return: 'vec4', 'float4', or 'half4'");
+ return false;
+ }
+ bool validParams = (parameters.size() == 1 && paramIsInputColor(0));
+ if (!validParams) {
+ errors.error(offset, "'main' parameter must be 'vec4', 'float4', or 'half4'");
+ return false;
+ }
+ break;
+ }
+ case ProgramKind::kRuntimeShader: {
+ // (half4|float4) main(float2) -or- (half4|float4) main(float2, half4|float4)
+ if (!typeIsValidForColor(returnType)) {
+ errors.error(offset, "'main' must return: 'vec4', 'float4', or 'half4'");
+ return false;
+ }
+ bool validParams =
+ (parameters.size() == 1 && paramIsCoords(0)) ||
+ (parameters.size() == 2 && paramIsCoords(0) && paramIsInputColor(1));
+ if (!validParams) {
+ errors.error(offset, "'main' parameters must be (float2, (vec4|float4|half4)?)");
+ return false;
+ }
+ break;
+ }
+ case ProgramKind::kFragmentProcessor: {
+ if (returnType != *context.fTypes.fHalf4) {
+ errors.error(offset, ".fp 'main' must return 'half4'");
+ return false;
+ }
+ bool validParams = (parameters.size() == 0) ||
+ (parameters.size() == 1 && paramIsCoords(0));
+ if (!validParams) {
+ errors.error(offset, ".fp 'main' must be declared main() or main(float2)");
+ return false;
+ }
+ break;
+ }
+ case ProgramKind::kGeneric:
+ // No rules apply here
+ break;
+ case ProgramKind::kFragment: {
+ bool validParams = (parameters.size() == 0) ||
+ (parameters.size() == 1 && paramIsCoords(0));
+ if (!validParams) {
+ errors.error(offset, "shader 'main' must be main() or main(float2)");
+ return false;
+ }
+ break;
+ }
+ case ProgramKind::kVertex:
+ case ProgramKind::kGeometry:
+ if (parameters.size()) {
+ errors.error(offset, "shader 'main' must have zero parameters");
+ return false;
+ }
+ break;
+ }
+ return true;
+}
+
+/**
+ * Checks for a previously existing declaration of this function, reporting errors if there is an
+ * incompatible symbol. Returns true and sets outExistingDecl to point to the existing declaration
+ * (or null if none) on success, returns false on error.
+ */
+static bool find_existing_declaration(const Context& context, SymbolTable& symbols, int offset,
+ StringFragment name,
+ std::vector<std::unique_ptr<Variable>>& parameters,
+ const Type* returnType, bool isBuiltin,
+ const FunctionDeclaration** outExistingDecl) {
+ ErrorReporter& errors = context.fErrors;
+ const Symbol* entry = symbols[name];
+ *outExistingDecl = nullptr;
+ if (entry) {
+ std::vector<const FunctionDeclaration*> functions;
+ switch (entry->kind()) {
+ case Symbol::Kind::kUnresolvedFunction:
+ functions = entry->as<UnresolvedFunction>().functions();
+ break;
+ case Symbol::Kind::kFunctionDeclaration:
+ functions.push_back(&entry->as<FunctionDeclaration>());
+ break;
+ default:
+ errors.error(offset, "symbol '" + name + "' was already defined");
+ return false;
+ }
+ for (const FunctionDeclaration* other : functions) {
+ SkASSERT(name == other->name());
+ if (parameters.size() != other->parameters().size()) {
+ continue;
+ }
+ bool match = true;
+ for (size_t i = 0; i < parameters.size(); i++) {
+ if (parameters[i]->type() != other->parameters()[i]->type()) {
+ match = false;
+ break;
+ }
+ }
+ if (!match) {
+ continue;
+ }
+ if (*returnType != other->returnType()) {
+ std::vector<const Variable*> paramPtrs;
+ paramPtrs.reserve(parameters.size());
+ for (std::unique_ptr<Variable>& param : parameters) {
+ paramPtrs.push_back(param.get());
+ }
+ FunctionDeclaration invalidDecl(offset,
+ &other->modifiers(),
+ name,
+ std::move(paramPtrs),
+ returnType,
+ isBuiltin);
+ errors.error(offset,
+ "functions '" + invalidDecl.description() + "' and '" +
+ other->description() + "' differ only in return type");
+ return false;
+ }
+ for (size_t i = 0; i < parameters.size(); i++) {
+ if (parameters[i]->modifiers() != other->parameters()[i]->modifiers()) {
+ errors.error(offset,
+ "modifiers on parameter " + to_string((uint64_t)i + 1) +
+ " differ between declaration and definition");
+ return false;
+ }
+ }
+ if (other->definition() && !other->isBuiltin()) {
+ errors.error(offset, "duplicate definition of " + other->description());
+ return false;
+ }
+ *outExistingDecl = other;
+ break;
+ }
+ }
+ return true;
+}
+
+const FunctionDeclaration* FunctionDeclaration::Convert(const Context& context,
+ SymbolTable& symbols, ModifiersPool& modifiersPool, int offset, const Modifiers* modifiers,
+ StringFragment name, std::vector<std::unique_ptr<Variable>> parameters,
+ const Type* returnType, bool isBuiltin) {
+ bool isMain = (name == "main");
+
+ const FunctionDeclaration* decl = nullptr;
+ if (!check_modifiers(context, offset, *modifiers) ||
+ !check_return_type(context, offset, *returnType, isBuiltin) ||
+ !check_parameters(context, modifiersPool, parameters, isMain, isBuiltin) ||
+ (isMain && !check_main_signature(context, offset, *returnType, parameters, isBuiltin)) ||
+ !find_existing_declaration(context, symbols, offset, name, parameters, returnType,
+ isBuiltin, &decl)) {
+ return nullptr;
+ }
+ std::vector<const Variable*> finalParameters;
+ finalParameters.reserve(parameters.size());
+ for (auto& param : parameters) {
+ finalParameters.push_back(symbols.takeOwnershipOfSymbol(std::move(param)));
+ }
+ if (decl) {
+ return decl;
+ }
+ auto result = std::make_unique<FunctionDeclaration>(offset, modifiers, name,
+ std::move(finalParameters), returnType,
+ isBuiltin);
+ return symbols.add(std::move(result));
+}
+
+} // namespace SkSL
diff --git a/src/sksl/ir/SkSLFunctionDeclaration.h b/src/sksl/ir/SkSLFunctionDeclaration.h
index 8dda492..da3b4e6 100644
--- a/src/sksl/ir/SkSLFunctionDeclaration.h
+++ b/src/sksl/ir/SkSLFunctionDeclaration.h
@@ -9,6 +9,7 @@
#define SKSL_FUNCTIONDECLARATION
#include "include/private/SkSLModifiers.h"
+#include "include/private/SkSLProgramKind.h"
#include "include/private/SkSLSymbol.h"
#include "include/private/SkTArray.h"
#include "src/sksl/ir/SkSLExpression.h"
@@ -38,6 +39,16 @@
, fBuiltin(builtin)
, fIsMain(name == "main") {}
+ static const FunctionDeclaration* Convert(const Context& context,
+ SymbolTable& symbols,
+ ModifiersPool& modifiersPool,
+ int offset,
+ const Modifiers* modifiers,
+ StringFragment name,
+ std::vector<std::unique_ptr<Variable>> parameters,
+ const Type* returnType,
+ bool isBuiltin);
+
const Modifiers& modifiers() const {
return *fModifiers;
}
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
index 92796fb..717e386 100644
--- a/src/sksl/ir/SkSLVariable.h
+++ b/src/sksl/ir/SkSLVariable.h
@@ -56,6 +56,10 @@
return *fModifiers;
}
+ void setModifiers(const Modifiers* modifiers) {
+ fModifiers = modifiers;
+ }
+
bool isBuiltin() const {
return fBuiltin;
}
diff --git a/tests/SkDSLRuntimeEffectTest.cpp b/tests/SkDSLRuntimeEffectTest.cpp
index 0f65030..997ac8b 100644
--- a/tests/SkDSLRuntimeEffectTest.cpp
+++ b/tests/SkDSLRuntimeEffectTest.cpp
@@ -128,7 +128,8 @@
effect.start();
Var gColor(kUniform_Modifier, kFloat4_Type);
DeclareGlobal(gColor);
- Function(kHalf4_Type, "main").define(
+ Var p(kFloat2_Type, "p");
+ Function(kHalf4_Type, "main", p).define(
Return(Half4(gColor))
);
effect.end();
@@ -143,7 +144,8 @@
effect.start();
Var gColor(kUniform_Modifier, kInt4_Type);
DeclareGlobal(gColor);
- Function(kHalf4_Type, "main").define(
+ Var p(kFloat2_Type, "p");
+ Function(kHalf4_Type, "main", p).define(
Return(Half4(gColor) / 255)
);
effect.end();
@@ -158,7 +160,8 @@
// make sure we're not saturating unexpectedly.
{
effect.start();
- Function(kHalf4_Type, "main").define(
+ Var p(kFloat2_Type, "p");
+ Function(kHalf4_Type, "main", p).define(
Return(Half4(0.498 * (Half2(Swizzle(sk_FragCoord(), X, Y)) - 0.5), 0, 1))
);
effect.end();
diff --git a/tests/SkSLDSLTest.cpp b/tests/SkSLDSLTest.cpp
index dc806f6..fb80da3 100644
--- a/tests/SkSLDSLTest.cpp
+++ b/tests/SkSLDSLTest.cpp
@@ -1322,13 +1322,13 @@
DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) {
AutoDSLContext context(ctxInfo.directContext()->priv().getGpu(), /*markVarsDeclared=*/false);
- Var coords(kHalf2_Type, "coords");
+ Var coords(kFloat2_Type, "coords");
DSLFunction(kVoid_Type, "main", coords).define(
sk_FragColor() = Half4(coords, 0, 1)
);
REPORTER_ASSERT(r, DSLWriter::ProgramElements().size() == 1);
EXPECT_EQUAL(*DSLWriter::ProgramElements()[0],
- "void main(half2 coords) { (sk_FragColor = half4(coords, 0.0, 1.0)); }");
+ "void main(float2 coords) { (sk_FragColor = half4(half2(coords), 0.0, 1.0)); }");
{
DSLWriter::Reset();
