diff --git a/libs/filament-matp/include/filament-matp/MaterialParser.h b/libs/filament-matp/include/filament-matp/MaterialParser.h index a8461fdb5d..e186815f05 100644 --- a/libs/filament-matp/include/filament-matp/MaterialParser.h +++ b/libs/filament-matp/include/filament-matp/MaterialParser.h @@ -20,6 +20,7 @@ #include +#include #include "Config.h" namespace filamat { @@ -40,45 +41,48 @@ public: // Call MaterialBuilder::init before passing in the builder; call MaterialBuilder::build to // create filamat::Package after. // When the input shader has #includes, it has to be resolved before calling into parse. - bool parse( + utils::Status parse( filamat::MaterialBuilder& builder, const Config& config, ssize_t& size, std::unique_ptr& buffer); // Replaces macro keywords with user specified ones. Must be called before parse. - bool processTemplateSubstitutions( + utils::Status processTemplateSubstitutions( const Config& config, ssize_t& size, std::unique_ptr& buffer); private: friend class ::TestMaterialParser; - bool parseMaterial(const char* buffer, size_t size, + utils::Status parseMaterial(const char* buffer, size_t size, filamat::MaterialBuilder& builder) const noexcept; - bool processMaterial(const MaterialLexeme&, + utils::Status processMaterial(const MaterialLexeme&, filamat::MaterialBuilder& builder) const noexcept; - bool processVertexShader(const MaterialLexeme&, + utils::Status processVertexShader(const MaterialLexeme&, filamat::MaterialBuilder& builder) const noexcept; - bool processFragmentShader(const MaterialLexeme&, + utils::Status processFragmentShader(const MaterialLexeme&, filamat::MaterialBuilder& builder) const noexcept; - bool processComputeShader(const MaterialLexeme&, + utils::Status processComputeShader(const MaterialLexeme&, filamat::MaterialBuilder& builder) const noexcept; - bool ignoreLexeme(const MaterialLexeme&, filamat::MaterialBuilder& builder) const noexcept; + utils::Status ignoreLexeme( + const MaterialLexeme&, filamat::MaterialBuilder& builder) const noexcept; - bool parseMaterialAsJSON(const char* buffer, size_t size, + utils::Status parseMaterialAsJSON(const char* buffer, size_t size, filamat::MaterialBuilder& builder) const noexcept; - bool processMaterialJSON(const JsonishValue*, + utils::Status processMaterialJSON(const JsonishValue*, filamat::MaterialBuilder& builder) const noexcept; - bool processVertexShaderJSON(const JsonishValue*, + utils::Status processVertexShaderJSON(const JsonishValue*, filamat::MaterialBuilder& builder) const noexcept; - bool processFragmentShaderJSON(const JsonishValue*, + utils::Status processFragmentShaderJSON(const JsonishValue*, filamat::MaterialBuilder& builder) const noexcept; - bool processComputeShaderJSON(const JsonishValue*, + utils::Status processComputeShaderJSON(const JsonishValue*, filamat::MaterialBuilder& builder) const noexcept; - bool ignoreLexemeJSON(const JsonishValue*, filamat::MaterialBuilder& builder) const noexcept; - bool isValidJsonStart(const char* buffer, size_t size) const noexcept; + utils::Status ignoreLexemeJSON( + const JsonishValue*, filamat::MaterialBuilder& builder) const noexcept; + utils::Status isValidJsonStart(const char* buffer, size_t size) const noexcept; - bool processMaterialParameters(filamat::MaterialBuilder& builder, const Config& config) const; + utils::Status processMaterialParameters( + filamat::MaterialBuilder& builder, const Config& config) const; // Member function pointer type, this is used to implement a Command design // pattern. - using MaterialConfigProcessor = bool (MaterialParser::*) + using MaterialConfigProcessor = utils::Status (MaterialParser::*) (const MaterialLexeme&, filamat::MaterialBuilder& builder) const; // Map used to store Command pattern function pointers. // Using string_view is generally not recommended in a map, but the string keys are program constants, @@ -86,7 +90,7 @@ private: std::unordered_map mConfigProcessor; // The same, but for pure JSON syntax - using MaterialConfigProcessorJSON = bool (MaterialParser::*) + using MaterialConfigProcessorJSON = utils::Status (MaterialParser::*) (const JsonishValue*, filamat::MaterialBuilder& builder) const; std::unordered_map mConfigProcessorJSON; }; diff --git a/libs/filament-matp/src/MaterialParser.cpp b/libs/filament-matp/src/MaterialParser.cpp index 6880fd2f11..a1a322f903 100644 --- a/libs/filament-matp/src/MaterialParser.cpp +++ b/libs/filament-matp/src/MaterialParser.cpp @@ -16,24 +16,23 @@ #include -#include -#include -#include - -#include - -#include - -#include -#include - #include "JsonishLexer.h" #include "JsonishParser.h" #include "MaterialLexeme.h" #include "MaterialLexer.h" #include "ParametersProcessor.h" + +#include +#include #include +#include +#include +#include + +#include +#include + using namespace utils; using namespace filamat; using namespace std::placeholders; @@ -60,7 +59,7 @@ MaterialParser::MaterialParser() { mConfigProcessorJSON[CONFIG_KEY_TOOL] = &MaterialParser::ignoreLexemeJSON; } -bool MaterialParser::processMaterial(const MaterialLexeme& jsonLexeme, +utils::Status MaterialParser::processMaterial(const MaterialLexeme& jsonLexeme, MaterialBuilder& builder) const noexcept { JsonishLexer jlexer; @@ -70,21 +69,14 @@ bool MaterialParser::processMaterial(const MaterialLexeme& jsonLexeme, std::unique_ptr const json = parser.parse(); if (json == nullptr) { - std::cerr << "JsonishParser error (see above)." << std::endl; - return false; + return utils::Status::internal("JsonishParser error (see above)."); } ParametersProcessor parametersProcessor; - bool const ok = parametersProcessor.process(builder, *json); - if (!ok) { - std::cerr << "Error while processing material." << std::endl; - return false; - } - - return true; + return parametersProcessor.process(builder, *json); } -bool MaterialParser::processVertexShader(const MaterialLexeme& lexeme, +utils::Status MaterialParser::processVertexShader(const MaterialLexeme& lexeme, MaterialBuilder& builder) const noexcept { MaterialLexeme const trimmedLexeme = lexeme.trimBlockMarkers(); @@ -93,10 +85,10 @@ bool MaterialParser::processVertexShader(const MaterialLexeme& lexeme, // getLine() returns a line number, with 1 being the first line, but .material wants a 0-based // line number offset, where 0 is the first line. builder.materialVertex(shaderStr.c_str(), trimmedLexeme.getLine() - 1); - return true; + return utils::Status::ok(); } -bool MaterialParser::processFragmentShader(const MaterialLexeme& lexeme, +utils::Status MaterialParser::processFragmentShader(const MaterialLexeme& lexeme, MaterialBuilder& builder) const noexcept { MaterialLexeme const trimmedLexeme = lexeme.trimBlockMarkers(); @@ -105,150 +97,147 @@ bool MaterialParser::processFragmentShader(const MaterialLexeme& lexeme, // getLine() returns a line number, with 1 being the first line, but .material wants a 0-based // line number offset, where 0 is the first line. builder.material(shaderStr.c_str(), trimmedLexeme.getLine() - 1); - return true; + return utils::Status::ok(); } -bool MaterialParser::processComputeShader(const MaterialLexeme& lexeme, +utils::Status MaterialParser::processComputeShader(const MaterialLexeme& lexeme, MaterialBuilder& builder) const noexcept { return MaterialParser::processFragmentShader(lexeme, builder); } -bool MaterialParser::ignoreLexeme(const MaterialLexeme&, MaterialBuilder&) const noexcept { - return true; +utils::Status MaterialParser::ignoreLexeme(const MaterialLexeme&, MaterialBuilder&) const noexcept { + return utils::Status::ok(); } -static bool reflectParameters(const MaterialBuilder& builder) { +static utils::Status reflectParameters(const MaterialBuilder& builder) { size_t const count = builder.getParameterCount(); const MaterialBuilder::ParameterList& parameters = builder.getParameters(); + utils::io::sstream ss; - std::cout << "{" << std::endl; - std::cout << " \"parameters\": [" << std::endl; + ss << "{" << utils::io::endl; + ss << " \"parameters\": [" << utils::io::endl; for (size_t i = 0; i < count; i++) { const MaterialBuilder::Parameter& parameter = parameters[i]; - std::cout << " {" << std::endl; - std::cout << R"( "name": ")" << parameter.name.c_str() << "\"," << std::endl; + ss << " {" << utils::io::endl; + ss << R"( "name": ")" << parameter.name.c_str() << "\"," << utils::io::endl; if (parameter.isSampler()) { - std::cout << R"( "type": ")" << - Enums::toString(parameter.samplerType) << "\"," << std::endl; - std::cout << R"( "format": ")" << - Enums::toString(parameter.format) << "\"," << std::endl; - std::cout << R"( "precision": ")" << - Enums::toString(parameter.precision) << "\"," << std::endl; - std::cout << R"( "multisample": ")" << - (parameter.multisample ? "true" : "false")<< "\"" << std::endl; + ss << R"( "type": ")" << + Enums::toString(parameter.samplerType) << "\"," << utils::io::endl; + ss << R"( "format": ")" << + Enums::toString(parameter.format) << "\"," << utils::io::endl; + ss << R"( "precision": ")" << + Enums::toString(parameter.precision) << "\"," << utils::io::endl; + ss << R"( "multisample": ")" << + (parameter.multisample ? "true" : "false")<< "\"" << utils::io::endl; } else if (parameter.isUniform()) { - std::cout << R"( "type": ")" << - Enums::toString(parameter.uniformType) << "\"," << std::endl; - std::cout << R"( "size": ")" << parameter.size << "\"" << std::endl; + ss << R"( "type": ")" << + Enums::toString(parameter.uniformType) << "\"," << utils::io::endl; + ss << R"( "size": ")" << parameter.size << "\"" << utils::io::endl; } else if (parameter.isSubpass()) { - std::cout << R"( "type": ")" << - Enums::toString(parameter.subpassType) << "\"," << std::endl; - std::cout << R"( "format": ")" << - Enums::toString(parameter.format) << "\"," << std::endl; - std::cout << R"( "precision": ")" << - Enums::toString(parameter.precision) << "\"" << std::endl; + ss << R"( "type": ")" << + Enums::toString(parameter.subpassType) << "\"," << utils::io::endl; + ss << R"( "format": ")" << + Enums::toString(parameter.format) << "\"," << utils::io::endl; + ss << R"( "precision": ")" << + Enums::toString(parameter.precision) << "\"" << utils::io::endl; } - std::cout << " }"; - if (i < count - 1) std::cout << ","; - std::cout << std::endl; + ss << " }"; + if (i < count - 1) ss << ","; + ss << utils::io::endl; } - std::cout << " ]" << std::endl; - std::cout << "}" << std::endl; + ss << " ]" << utils::io::endl; + ss << "}" << utils::io::endl; - return true; + return {utils::StatusCode::OK, ss.c_str()}; } -bool MaterialParser::processMaterialJSON(const JsonishValue* value, +utils::Status MaterialParser::processMaterialJSON(const JsonishValue* value, filamat::MaterialBuilder& builder) const noexcept { if (!value) { - std::cerr << "'material' block does not have a value, one is required." << std::endl; - return false; + return utils::Status::invalidArgument( + "'material' block does not have a value, one is required."); } if (value->getType() != JsonishValue::OBJECT) { - std::cerr << "'material' block has an invalid type: " + utils::io::sstream errorMessage; + errorMessage << "'material' block has an invalid type: " << JsonishValue::typeToString(value->getType()) - << ", should be OBJECT." - << std::endl; - return false; + << ", should be OBJECT."; + + return utils::Status::invalidArgument(errorMessage.c_str()); } ParametersProcessor parametersProcessor; - bool const ok = parametersProcessor.process(builder, *value->toJsonObject()); - if (!ok) { - std::cerr << "Error while processing material." << std::endl; - return false; - } - - return true; + return parametersProcessor.process(builder, *value->toJsonObject()); } -bool MaterialParser::processVertexShaderJSON(const JsonishValue* value, +utils::Status MaterialParser::processVertexShaderJSON(const JsonishValue* value, filamat::MaterialBuilder& builder) const noexcept { if (!value) { - std::cerr << "'vertex' block does not have a value, one is required." << std::endl; - return false; + return utils::Status::invalidArgument( + "'vertex' block does not have a value, one is required."); } if (value->getType() != JsonishValue::STRING) { - std::cerr << "'vertex' block has an invalid type: " + utils::io::sstream errorMessage; + errorMessage << "'vertex' block has an invalid type: " << JsonishValue::typeToString(value->getType()) - << ", should be STRING." - << std::endl; - return false; + << ", should be STRING."; + return utils::Status::invalidArgument(errorMessage.c_str()); } builder.materialVertex(value->toJsonString()->getString().c_str()); - return true; + return utils::Status::ok(); } -bool MaterialParser::processFragmentShaderJSON(const JsonishValue* value, +utils::Status MaterialParser::processFragmentShaderJSON(const JsonishValue* value, filamat::MaterialBuilder& builder) const noexcept { if (!value) { - std::cerr << "'fragment' block does not have a value, one is required." << std::endl; - return false; + return utils::Status::invalidArgument( + "'fragment' block does not have a value, one is required."); } if (value->getType() != JsonishValue::STRING) { - std::cerr << "'fragment' block has an invalid type: " + utils::io::sstream errorMessage; + errorMessage << "'fragment' block has an invalid type: " << JsonishValue::typeToString(value->getType()) - << ", should be STRING." - << std::endl; - return false; + << ", should be STRING."; + return utils::Status::invalidArgument(errorMessage.c_str()); } builder.material(value->toJsonString()->getString().c_str()); - return true; + return utils::Status::ok(); } -bool MaterialParser::processComputeShaderJSON(const JsonishValue* value, + +utils::Status MaterialParser::processComputeShaderJSON(const JsonishValue* value, filamat::MaterialBuilder& builder) const noexcept { if (!value) { - std::cerr << "'compute' block does not have a value, one is required." << std::endl; - return false; + return utils::Status::invalidArgument( + "'compute' block does not have a value, one is required."); } if (value->getType() != JsonishValue::STRING) { - std::cerr << "'compute' block has an invalid type: " + utils::io::sstream errorMessage; + errorMessage << "'compute' block has an invalid type: " << JsonishValue::typeToString(value->getType()) - << ", should be STRING." - << std::endl; - return false; + << ", should be STRING."; + return utils::Status::invalidArgument(errorMessage.c_str()); } builder.material(value->toJsonString()->getString().c_str()); - return true; + return utils::Status::ok(); } -bool MaterialParser::ignoreLexemeJSON(const JsonishValue*, +utils::Status MaterialParser::ignoreLexemeJSON(const JsonishValue*, filamat::MaterialBuilder&) const noexcept { - return true; + return utils::Status::ok(); } -bool MaterialParser::isValidJsonStart(const char* buffer, size_t size) const noexcept { +utils::Status MaterialParser::isValidJsonStart(const char* buffer, size_t size) const noexcept { // Skip all whitespace characters. const char* end = buffer + size; while (buffer != end && isspace(buffer[0])) { @@ -257,31 +246,36 @@ bool MaterialParser::isValidJsonStart(const char* buffer, size_t size) const noe // A buffer made only of whitespace is not a valid JSON start. if (buffer == end) { - return false; + return utils::Status::invalidArgument( + "A buffer made only of whitespace is not a valid JSON start."); } const char c = buffer[0]; // Take care of block, array, and string if (c == '{' || c == '[' || c == '"') { - return true; + return utils::Status::ok(); } // boolean true if (c == 't' && (end - buffer) > 3 && strncmp(buffer, "true", 4) != 0) { - return true; + return utils::Status::ok(); } // boolean false if (c == 'f' && (end - buffer) > 4 && strncmp(buffer, "false", 5) != 0) { - return true; + return utils::Status::ok(); } // null literal - return c == 'n' && (end - buffer) > 3 && strncmp(buffer, "null", 5) != 0; + if (c == 'n' && (end - buffer) > 3 && strncmp(buffer, "null", 5) != 0) { + return utils::Status::ok(); + } + + return utils::Status::invalidArgument("Unknown character while parsing JSON."); } -bool MaterialParser::parseMaterialAsJSON(const char* buffer, size_t size, +utils::Status MaterialParser::parseMaterialAsJSON(const char* buffer, size_t size, filamat::MaterialBuilder& builder) const noexcept { JsonishLexer jlexer; @@ -290,31 +284,29 @@ bool MaterialParser::parseMaterialAsJSON(const char* buffer, size_t size, JsonishParser parser(jlexer.getLexemes()); std::unique_ptr json = parser.parse(); if (json == nullptr) { - std::cerr << "Could not parse JSON material file" << std::endl; - return false; + return utils::Status::internal("Could not parse JSON material file"); } for (auto& entry : json->getEntries()) { const std::string& key = entry.first; if (mConfigProcessorJSON.find(key) == mConfigProcessorJSON.end()) { - std::cerr << "Unknown identifier '" << key << "'" << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Unknown identifier '" << key << "'"; + return utils::Status::invalidArgument(errorMessage.c_str()); } // Retrieve function member pointer MaterialConfigProcessorJSON const p = mConfigProcessorJSON.at(key); // Call it. - bool const ok = (*this.*p)(entry.second, builder); - if (!ok) { - std::cerr << "Error while processing block with key:'" << key << "'" << std::endl; - return false; + if (utils::Status status = (*this.*p)(entry.second, builder); !status.isOk()) { + return status; } } - return true; + return utils::Status::ok(); } -bool MaterialParser::parseMaterial(const char* buffer, size_t size, +utils::Status MaterialParser::parseMaterial(const char* buffer, size_t size, filamat::MaterialBuilder& builder) const noexcept { MaterialLexer materialLexer; @@ -325,38 +317,42 @@ bool MaterialParser::parseMaterial(const char* buffer, size_t size, // a binary file. for (auto lexeme : lexemes) { if (lexeme.getType() == MaterialType::UNKNOWN) { - std::cerr << "Unexpected character at line:" << lexeme.getLine() - << " position:" << lexeme.getLinePosition() << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Unexpected character at line:" << lexeme.getLine() + << " position:" << lexeme.getLinePosition(); + return utils::Status::invalidArgument(errorMessage.c_str()); } } // Make a first quick pass just to make sure the format was respected (the material format is // a series of IDENTIFIER, BLOCK pairs). if (lexemes.size() < 2) { - std::cerr << "Input MUST be an alternation of [identifier, block] pairs." << std::endl; - return false; + return utils::Status::invalidArgument( + "Input MUST be an alternation of [identifier, block] pairs."); } for (size_t i = 0; i < lexemes.size(); i += 2) { auto lexeme = lexemes.at(i); if (lexeme.getType() != MaterialType::IDENTIFIER) { - std::cerr << "An identifier was expected at line:" << lexeme.getLine() - << " position:" << lexeme.getLinePosition() << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "An identifier was expected at line:" << lexeme.getLine() + << " position:" << lexeme.getLinePosition(); + return utils::Status::invalidArgument(errorMessage.c_str()); } if (i == lexemes.size() - 1) { - std::cerr << "Identifier at line:" << lexeme.getLine() + utils::io::sstream errorMessage; + errorMessage << "Identifier at line:" << lexeme.getLine() << " position:" << lexeme.getLinePosition() - << " must be followed by a block." << std::endl; - return false; + << " must be followed by a block."; + return utils::Status::invalidArgument(errorMessage.c_str()); } auto nextLexeme = lexemes.at(i + 1); if (nextLexeme.getType() != MaterialType::BLOCK) { - std::cerr << "A block was expected at line:" << lexeme.getLine() - << " position:" << lexeme.getLinePosition() << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "A block was expected at line:" << lexeme.getLine() + << " position:" << lexeme.getLinePosition(); + return utils::Status::invalidArgument(errorMessage.c_str()); } } @@ -366,35 +362,37 @@ bool MaterialParser::parseMaterial(const char* buffer, size_t size, if (lexeme.getType() == MaterialType::IDENTIFIER) { identifier = lexeme.getStringValue(); if (mConfigProcessor.find(identifier) == mConfigProcessor.end()) { - std::cerr << "Unknown identifier '" + utils::io::sstream errorMessage; + errorMessage << "Unknown identifier '" << identifier << "' at line:" << lexeme.getLine() - << " position:" << lexeme.getLinePosition() << std::endl; - return false; + << " position:" << lexeme.getLinePosition(); + return utils::Status::invalidArgument(errorMessage.c_str()); } } else if (lexeme.getType() == MaterialType::BLOCK) { MaterialConfigProcessor const processor = mConfigProcessor.at(identifier); - if (!(*this.*processor)(lexeme, builder)) { - std::cerr << "Error while processing block with key:'" << identifier << "'" - << std::endl; - return false; + if (utils::Status status = (*this.*processor)(lexeme, builder); !status.isOk()) { + return status; } } } - return true; + return utils::Status::ok(); } -bool MaterialParser::processMaterialParameters(filamat::MaterialBuilder& builder, +utils::Status MaterialParser::processMaterialParameters(filamat::MaterialBuilder& builder, const Config& config) const { ParametersProcessor parametersProcessor; - bool ok = true; + utils::Status status; for (const auto& param : config.getMaterialParameters()) { - ok &= parametersProcessor.process(builder, param.first, param.second); + utils::Status s = parametersProcessor.process(builder, param.first, param.second); + if (!s.isOk()) { + status = s; + } } - return ok; + return status; } -bool MaterialParser::processTemplateSubstitutions( +utils::Status MaterialParser::processTemplateSubstitutions( const Config& config, ssize_t& size, std::unique_ptr& buffer) { const auto& templateMap = config.getTemplateMap(); ssize_t modifiedSize = size; @@ -408,8 +406,7 @@ bool MaterialParser::processTemplateSubstitutions( ssize_t endCursor = cursor; while (true) { if (endCursor == size) { - std::cerr << "Unexpected end of file" << std::endl; - return false; + return utils::Status::internal("Unexpected end of file"); } if (buffer[endCursor] == '}') { break; @@ -422,8 +419,9 @@ bool MaterialParser::processTemplateSubstitutions( modifiedSize -= macro.size() + 3; modifiedSize += iter->second.size(); } else { - std::cerr << "Undefined template macro:" << macro << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Undefined template macro:" << macro; + return utils::Status::invalidArgument(errorMessage.c_str()); } modified = true; } @@ -453,28 +451,30 @@ bool MaterialParser::processTemplateSubstitutions( buffer = std::move(modifiedBuffer); size = modifiedSize; } - return true; + return utils::Status::ok(); } -bool MaterialParser::parse(filamat::MaterialBuilder& builder, +utils::Status MaterialParser::parse(filamat::MaterialBuilder& builder, const Config& config, ssize_t& size, std::unique_ptr& buffer) { if (builder.getFeatureLevel() > config.getFeatureLevel()) { - std::cerr << "Material feature level (" << +builder.getFeatureLevel() - << ") is higher than maximum allowed (" << +config.getFeatureLevel() << ")" << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Material feature level (" << +builder.getFeatureLevel() + << ") is higher than maximum allowed (" << +config.getFeatureLevel() << ")"; + return utils::Status::invalidArgument(errorMessage.c_str()); } // Before attempting an expensive lex, let's find out if we were sent pure JSON. - bool parsed; - if (isValidJsonStart(buffer.get(), size_t(size))) { - parsed = parseMaterialAsJSON(buffer.get(), size_t(size), builder); + utils::Status parsedStatus; + if (utils::Status validJson = isValidJsonStart(buffer.get(), size_t(size)); + validJson.isOk()) { + parsedStatus = parseMaterialAsJSON(buffer.get(), size_t(size), builder); } else { - parsed = parseMaterial(buffer.get(), size_t(size), builder); + parsedStatus = parseMaterial(buffer.get(), size_t(size), builder); } - if (!parsed) { - return false; + if (!parsedStatus.isOk()) { + return parsedStatus; } switch (config.getReflectionTarget()) { @@ -500,12 +500,12 @@ bool MaterialParser::parse(filamat::MaterialBuilder& builder, builder.shaderDefine(define.first.c_str(), define.second.c_str()); } - if (!processMaterialParameters(builder, config)) { - std::cerr << "Error while processing material parameters." << std::endl; - return false; + if (utils::Status processedStatus = processMaterialParameters(builder, config); + !processedStatus.isOk()) { + return processedStatus; } - return true; + return utils::Status::ok(); } } // namespace matp \ No newline at end of file diff --git a/libs/filament-matp/src/ParametersProcessor.cpp b/libs/filament-matp/src/ParametersProcessor.cpp index 489d4ad2c8..0fbaa49a64 100644 --- a/libs/filament-matp/src/ParametersProcessor.cpp +++ b/libs/filament-matp/src/ParametersProcessor.cpp @@ -17,6 +17,9 @@ #include "ParametersProcessor.h" #include +#include +#include +#include #include #include @@ -28,7 +31,6 @@ #include #include #include - #include using namespace filamat; @@ -37,15 +39,16 @@ using namespace utils; namespace matp { template -static bool logEnumIssue(const std::string& key, const JsonishString& value, +static utils::Status logEnumIssue(const std::string& key, const JsonishString& value, const std::unordered_map& map) noexcept { - std::cerr << "Error while processing key '" << key << "' value." << std::endl; - std::cerr << "Value '" << value.getString() << "' is invalid. Valid values are:" - << std::endl; + utils::io::sstream errorMessage; + errorMessage << "Error while processing key '" << key << "' value." << utils::io::endl; + errorMessage << "Value '" << value.getString() << "' is invalid. Valid values are:" + << utils::io::endl; for (const auto& entries : map) { - std::cerr << " " << entries.first << std::endl; + errorMessage << " " << entries.first << utils::io::endl; } - return false; + return utils::Status::invalidArgument(errorMessage.c_str()); } template @@ -71,12 +74,12 @@ static MaterialBuilder::Variable intToVariable(size_t i) noexcept { } } -static bool processName(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processName(MaterialBuilder& builder, const JsonishValue& value) { builder.name(value.toJsonString()->getString().c_str()); - return true; + return utils::Status::ok(); } -static bool processInterpolation(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processInterpolation(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "smooth", MaterialBuilder::Interpolation::SMOOTH }, { "flat", MaterialBuilder::Interpolation::FLAT }, @@ -86,7 +89,7 @@ static bool processInterpolation(MaterialBuilder& builder, const JsonishValue& v return logEnumIssue("interpolation", *interpolationString, strToEnum); } builder.interpolation(stringToEnum(strToEnum, interpolationString->getString())); - return true; + return utils::Status::ok(); } /** @@ -133,39 +136,34 @@ static ssize_t extractArraySize(std::string& type) { return (ssize_t)std::stoul(type.c_str() + start + 1, nullptr); } -static bool processParameter(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { +static utils::Status processParameter(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { const JsonishValue* typeValue = jsonObject.getValue("type"); if (!typeValue) { - std::cerr << "parameters: entry without key 'type'." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: entry without key 'type'."); } if (typeValue->getType() != JsonishValue::STRING) { - std::cerr << "parameters: type value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: type value must be STRING."); } const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "parameters: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "parameters: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: name value must be STRING."); } const JsonishValue* transformNameValue = jsonObject.getValue("transformName"); if (transformNameValue && transformNameValue->getType() != JsonishValue::STRING) { - std::cerr << "parameters: transformName value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument( + "parameters: transformName value must be STRING."); } const JsonishValue* precisionValue = jsonObject.getValue("precision"); if (precisionValue) { if (precisionValue->getType() != JsonishValue::STRING) { - std::cerr << "parameters: precision must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: precision must be a STRING."); } auto precisionString = precisionValue->toJsonString(); if (!Enums::isValid(precisionString->getString())){ @@ -176,8 +174,7 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json const JsonishValue* formatValue = jsonObject.getValue("format"); if (formatValue) { if (formatValue->getType() != JsonishValue::STRING) { - std::cerr << "parameters: format must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: format must be a STRING."); } auto formatString = formatValue->toJsonString(); if (!Enums::isValid(formatString->getString())){ @@ -188,15 +185,13 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json const JsonishValue* filterableValue = jsonObject.getValue("filterable"); if (filterableValue) { if (filterableValue->getType() != JsonishValue::BOOL) { - std::cerr << "parameters: filterable must be a BOOL." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: filterable must be a BOOL."); } } const JsonishValue* multiSampleValue = jsonObject.getValue("multisample"); if (multiSampleValue) { if (multiSampleValue->getType() != JsonishValue::BOOL) { - std::cerr << "parameters: multisample must be a BOOL." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: multisample must be a BOOL."); } } @@ -209,8 +204,7 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json if (stagesValue) { ShaderStageFlags parsedStages = ShaderStageFlags::NONE; if (stagesValue->getType() != JsonishValue::ARRAY) { - std::cerr << "parameters: stages must be an ARRAY." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: stages must be an ARRAY."); } for (auto value: stagesValue->toJsonArray()->getElements()) { if (value->getType() == JsonishValue::Type::STRING) { @@ -220,15 +214,16 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json if (Enums::isValid(stageString)) { parsedStages |= Enums::toEnum(stageString); } else { - std::cerr << "stages: the stage '" << stageString - << "' for parameter with name '" << nameString - << "' is not a valid shader stage." << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "stages: the stage '" << stageString << + "' for parameter with name '" << nameString << + "' is not a valid shader stage."; + return utils::Status::invalidArgument(errorMessage.c_str()); } continue; } - std::cerr << "parameters: stages must be an array of STRINGs." << std::endl; - return false; + return utils::Status::invalidArgument( + "parameters: stages must be an array of STRINGs."); } stages = parsedStages; } @@ -237,10 +232,11 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json if (Enums::isValid(typeString)) { if (stages.has_value()) { - std::cerr << "parameters: the uniform parameter with name '" << nameString << "'" + utils::io::sstream errorMessage; + errorMessage << "parameters: the uniform parameter with name '" << nameString << "'" << " has shader stages specified. Shader stages are only supported for" - << " samplers." << std::endl; - return false; + << " samplers."; + return utils::Status::invalidArgument(errorMessage.c_str()); } MaterialBuilder::UniformType const type = Enums::toEnum(typeString); ParameterPrecision precision = ParameterPrecision::DEFAULT; @@ -255,10 +251,11 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json } } else if (Enums::isValid(typeString)) { if (arraySize > 0) { - std::cerr << "parameters: the parameter with name '" << nameString << "'" + utils::io::sstream errorMessage; + errorMessage << "parameters: the parameter with name '" << nameString << "'" << " is an array of samplers of size " << arraySize << ". Arrays of samplers" - << " are currently not supported." << std::endl; - return false; + << " are currently not supported."; + return utils::Status::invalidArgument(errorMessage.c_str()); } MaterialBuilder::SamplerType const type = Enums::toEnum(typeString); @@ -270,15 +267,15 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json precisionValue->toJsonString()->getString()) : ParameterPrecision::DEFAULT; if (format == SamplerFormat::SHADOW) { - std::cerr << "Materials should not be able to define a shadow sampler"; - return false; + return utils::Status::invalidArgument( + "Materials should not be able to define a shadow sampler"); } if (format == SamplerFormat::INT && filterableValue) { - std::cerr << "parameters: the parameter with name '" << nameString << "'" - << " is an integer sampler. The `filterable` attribute must not be defined." - << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "parameters: the parameter with name '" << nameString << "'" + << " is an integer sampler. The `filterable` attribute must not be defined."; + return utils::Status::invalidArgument(errorMessage.c_str()); } // For samplers without `filterable` defined, we use the following logic @@ -292,11 +289,11 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json if (transformNameValue) { if (type != MaterialBuilder::SamplerType::SAMPLER_EXTERNAL) { - std::cerr << "parameters: the parameter with name '" << nameString << "'" + utils::io::sstream errorMessage; + errorMessage << "parameters: the parameter with name '" << nameString << "'" << " is a sampler of type " << typeString << " and has a transformName." - << " Transform names are only supported for external samplers." - << std::endl; - return false; + << " Transform names are only supported for external samplers."; + return utils::Status::invalidArgument(errorMessage.c_str()); } auto transformName = transformNameValue->toJsonString()->getString(); builder.parameter(nameString.c_str(), type, format, precision, filterable, @@ -307,49 +304,49 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json } } else { - std::cerr << "parameters: the type '" << typeString + utils::io::sstream errorMessage; + errorMessage << "parameters: the type '" << typeString << "' for parameter with name '" << nameString << "' is neither a valid uniform " - << "type nor a valid sampler type." << std::endl; - return false; + << "type nor a valid sampler type."; + return utils::Status::invalidArgument(errorMessage.c_str()); + } - return true; + return utils::Status::ok(); } -static bool processParameters(MaterialBuilder& builder, const JsonishValue& v) { +static utils::Status processParameters(MaterialBuilder& builder, const JsonishValue& v) { auto jsonArray = v.toJsonArray(); - bool ok = true; + utils::Status status; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok &= processParameter(builder, *value->toJsonObject()); + utils::Status s = processParameter(builder, *value->toJsonObject()); + if (!s.isOk()) { + status = s; + } continue; } - std::cerr << "parameters must be an array of OBJECTs." << std::endl; - return false; + return utils::Status::invalidArgument("parameters must be an array of OBJECTs."); } - return ok; + return status; } -static bool processConstant(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { +static utils::Status processConstant(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { const JsonishValue* typeValue = jsonObject.getValue("type"); if (!typeValue) { - std::cerr << "constants: entry without key 'type'." << std::endl; - return false; + return utils::Status::invalidArgument("constants: entry without key 'type'."); } if (typeValue->getType() != JsonishValue::STRING) { - std::cerr << "constants: type value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("constants: type value must be STRING."); } const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "constants: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("constants: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "constants: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("constants: name value must be STRING."); } auto typeString = typeValue->toJsonString()->getString(); @@ -363,9 +360,8 @@ static bool processConstant(MaterialBuilder& builder, const JsonishObject& jsonO int32_t intDefault = 0; if (defaultValue) { if (defaultValue->getType() != JsonishValue::NUMBER) { - std::cerr << "constants: INT constants must have NUMBER default value" - << std::endl; - return false; + return utils::Status::invalidArgument( + "constants: INT constants must have NUMBER default value"); } // FIXME: Jsonish doesn't distinguish between integers and floats. intDefault = (int32_t)defaultValue->toJsonNumber()->getFloat(); @@ -377,9 +373,8 @@ static bool processConstant(MaterialBuilder& builder, const JsonishObject& jsonO float floatDefault = 0.0f; if (defaultValue) { if (defaultValue->getType() != JsonishValue::NUMBER) { - std::cerr << "constants: FLOAT constants must have NUMBER default value" - << std::endl; - return false; + return utils::Status::invalidArgument( + "constants: FLOAT constants must have NUMBER default value"); } floatDefault = defaultValue->toJsonNumber()->getFloat(); } @@ -390,9 +385,8 @@ static bool processConstant(MaterialBuilder& builder, const JsonishObject& jsonO bool boolDefault = false; if (defaultValue) { if (defaultValue->getType() != JsonishValue::BOOL) { - std::cerr << "constants: BOOL constants must have BOOL default value" - << std::endl; - return false; + return utils::Status::invalidArgument( + "constants: BOOL constants must have BOOL default value"); } boolDefault = defaultValue->toJsonBool()->getBool(); } @@ -400,58 +394,56 @@ static bool processConstant(MaterialBuilder& builder, const JsonishObject& jsonO break; } } else { - std::cerr << "constants: the type '" << typeString + utils::io::sstream errorMessage; + errorMessage << "constants: the type '" << typeString << "' for constant with name '" << nameString << "' is not a valid constant " - << "parameter type." << std::endl; - return false; + << "parameter type."; + return utils::Status::invalidArgument(errorMessage.c_str()); } - return true; + return utils::Status::ok(); } -static bool processConstants(MaterialBuilder& builder, const JsonishValue& v) { +static utils::Status processConstants(MaterialBuilder& builder, const JsonishValue& v) { auto jsonArray = v.toJsonArray(); - bool ok = true; + utils::Status status; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok &= processConstant(builder, *value->toJsonObject()); + utils::Status s = processConstant(builder, *value->toJsonObject()); + if (!s.isOk()) { + status = s; + } continue; } - std::cerr << "constants must be an array of OBJECTs." << std::endl; - return false; + return utils::Status::invalidArgument("constants must be an array of OBJECTs."); } - return ok; + return utils::Status::ok(); } -static bool processBufferField(filament::BufferInterfaceBlock::Builder& builder, +static utils::Status processBufferField(filament::BufferInterfaceBlock::Builder& builder, const JsonishObject& jsonObject) noexcept { const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "buffers: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "buffers: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: name value must be STRING."); } const JsonishValue* typeValue = jsonObject.getValue("type"); if (!typeValue) { - std::cerr << "buffers: entry without key 'type'." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: entry without key 'type'."); } if (typeValue->getType() != JsonishValue::STRING) { - std::cerr << "buffers: type value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: type value must be STRING."); } const JsonishValue* precisionValue = jsonObject.getValue("precision"); if (precisionValue) { if (precisionValue->getType() != JsonishValue::STRING) { - std::cerr << "buffers: precision must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: precision must be a STRING."); } auto precisionString = precisionValue->toJsonString(); @@ -480,15 +472,16 @@ static bool processBufferField(filament::BufferInterfaceBlock::Builder& builder, { nameString.data(), nameString.size() }, uint32_t(arraySize), type, precision } }); } } else { - std::cerr << "buffers: the type '" << typeString << "' for parameter with name '" - << nameString << "' is not a valid buffer field type." << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "buffers: the type '" << typeString << "' for parameter with name '" + << nameString << "' is not a valid buffer field type."; + return utils::Status::invalidArgument(errorMessage.c_str()); } - return true; + return utils::Status::ok(); } -static bool processBuffer(MaterialBuilder& builder, +static utils::Status processBuffer(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { filament::BufferInterfaceBlock::Builder bibb; @@ -498,32 +491,26 @@ static bool processBuffer(MaterialBuilder& builder, const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "buffers: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "buffers: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: name value must be STRING."); } const JsonishValue* qualifiersValue = jsonObject.getValue("qualifiers"); if (!qualifiersValue) { - std::cerr << "buffers: entry without key 'qualifiers'." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: entry without key 'qualifiers'."); } if (qualifiersValue->getType() != JsonishValue::ARRAY) { - std::cerr << "buffers: qualifiers value must be an ARRAY." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: qualifiers value must be an ARRAY."); } const JsonishValue* fieldsValue = jsonObject.getValue("fields"); if (!fieldsValue) { - std::cerr << "buffers: entry without key 'fields'." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: entry without key 'fields'."); } if (fieldsValue->getType() != JsonishValue::ARRAY) { - std::cerr << "buffers: fields value must be an ARRAY." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: fields value must be an ARRAY."); } auto nameString = nameValue->toJsonString()->getString(); @@ -547,72 +534,71 @@ static bool processBuffer(MaterialBuilder& builder, } continue; } - std::cerr << "buffers: qualifiers must be an array of STRINGs." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: qualifiers must be an array of STRINGs."); } - bool ok = true; + utils::Status status; for (auto value : fieldsValue->toJsonArray()->getElements()) { if (bibb.hasVariableSizeArray()) { - std::cerr << "buffers: a variable size array must be the only and last field." << std::endl; - return false; + return utils::Status::invalidArgument( + "buffers: a variable size array must be the only and last field."); } if (value->getType() == JsonishValue::Type::OBJECT) { - ok &= processBufferField(bibb, *value->toJsonObject()); + utils::Status s = processBufferField(bibb, *value->toJsonObject()); + if (!s.isOk()) { + status = s; + } continue; } - std::cerr << "buffers: fields must be an array of OBJECTs." << std::endl; - return false; + return utils::Status::invalidArgument("buffers: fields must be an array of OBJECTs."); } builder.buffer(bibb.build()); - return ok; + return status; } -static bool processBuffers(MaterialBuilder& builder, const JsonishValue& v) { +static utils::Status processBuffers(MaterialBuilder& builder, const JsonishValue& v) { auto jsonArray = v.toJsonArray(); - bool ok = true; + utils::Status status; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok &= processBuffer(builder, *value->toJsonObject()); + utils::Status s = processBuffer(builder, *value->toJsonObject()); + if (!s.isOk()) { + status = s; + } continue; } - std::cerr << "buffers must be an array of OBJECTs." << std::endl; - return false; + return utils::Status::invalidArgument("buffers must be an array of OBJECTs."); } - return ok; + return status; } -static bool processSubpass(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { +static utils::Status processSubpass( + MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { const JsonishValue* typeValue = jsonObject.getValue("type"); if (!typeValue) { - std::cerr << "subpasses: entry without key 'type'." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses: entry without key 'type'."); } if (typeValue->getType() != JsonishValue::STRING) { - std::cerr << "subpasses: type value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses: type value must be STRING."); } const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "subpasses: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "subpasses: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses: name value must be STRING."); } const JsonishValue* precisionValue = jsonObject.getValue("precision"); if (precisionValue) { if (precisionValue->getType() != JsonishValue::STRING) { - std::cerr << "subpasses: precision must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses: precision must be a STRING."); } auto precisionString = precisionValue->toJsonString(); @@ -624,8 +610,7 @@ static bool processSubpass(MaterialBuilder& builder, const JsonishObject& jsonOb const JsonishValue* formatValue = jsonObject.getValue("format"); if (formatValue) { if (formatValue->getType() != JsonishValue::STRING) { - std::cerr << "subpasses: format must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses: format must be a STRING."); } auto formatString = formatValue->toJsonString(); @@ -641,10 +626,11 @@ static bool processSubpass(MaterialBuilder& builder, const JsonishObject& jsonOb if (Enums::isValid(typeString)) { if (arraySize > 0) { - std::cerr << "subpasses: the parameter with name '" << nameString << "'" + utils::io::sstream errorMessage; + errorMessage << "subpasses: the parameter with name '" << nameString << "'" << " is an array of subpasses of size " << arraySize << ". Arrays of subpasses" - << " are currently not supported." << std::endl; - return false; + << " are currently not supported."; + return utils::Status::invalidArgument(errorMessage.c_str()); } MaterialBuilder::SubpassType type = Enums::toEnum(typeString); @@ -664,37 +650,42 @@ static bool processSubpass(MaterialBuilder& builder, const JsonishObject& jsonOb builder.subpass(type, nameString.c_str()); } } else { - std::cerr << "subpasses: the type '" << typeString + utils::io::sstream errorMessage; + errorMessage << "subpasses: the type '" << typeString << "' for parameter with name '" << nameString << "' is neither a valid uniform " - << "type nor a valid sampler type." << std::endl; - return false; + << "type nor a valid sampler type."; + return utils::Status::invalidArgument(errorMessage.c_str()); } - return true; + return utils::Status::ok(); } -static bool processSubpasses(MaterialBuilder& builder, const JsonishValue& v) { +static utils::Status processSubpasses(MaterialBuilder& builder, const JsonishValue& v) { auto jsonArray = v.toJsonArray(); - bool ok = true; + utils::Status status; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok &= processSubpass(builder, *value->toJsonObject()); + utils::Status s = processSubpass(builder, *value->toJsonObject()); + if (!s.isOk()) { + status = s; + } continue; } - std::cerr << "subpasses must be an array of OBJECTs." << std::endl; - return false; + return utils::Status::invalidArgument("subpasses must be an array of OBJECTs."); } - return ok; + return status; } -static bool processVariables(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processVariables(MaterialBuilder& builder, const JsonishValue& value) { const JsonishArray* jsonArray = value.toJsonArray(); const auto& elements = jsonArray->getElements(); if (elements.size() > MaterialBuilder::MATERIAL_VARIABLES_COUNT) { - std::cerr << "variables: Max array size is " << MaterialBuilder::MATERIAL_VARIABLES_COUNT << "." << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "variables: Max array size is " + << MaterialBuilder::MATERIAL_VARIABLES_COUNT << "."; + return utils::Status::invalidArgument(errorMessage.c_str()); } for (size_t i = 0; i < elements.size(); i++) { @@ -709,19 +700,16 @@ static bool processVariables(MaterialBuilder& builder, const JsonishValue& value const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "variables: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("variables: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "variables: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("variables: name value must be STRING."); } const JsonishValue* precisionValue = jsonObject.getValue("precision"); if (precisionValue) { if (precisionValue->getType() != JsonishValue::STRING) { - std::cerr << "variables: precision must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("variables: precision must be a STRING."); } auto precisionString = precisionValue->toJsonString(); if (!Enums::isValid(precisionString->getString())){ @@ -739,16 +727,17 @@ static bool processVariables(MaterialBuilder& builder, const JsonishValue& value nameString = elementValue->toJsonString()->getString(); builder.variable(v, nameString.c_str()); } else { - std::cerr << "variables: array index " << i << " is not a STRING. found:" << - JsonishValue::typeToString(elementValue->getType()) << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "variables: array index " << i << " is not a STRING. found:" << + JsonishValue::typeToString(elementValue->getType()); + return utils::Status::invalidArgument(errorMessage.c_str()); } } - return true; + return utils::Status::ok(); } -static bool processRequires(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processRequires(MaterialBuilder& builder, const JsonishValue& value) { using Attribute = filament::VertexAttribute; static const std::unordered_map strToEnum { { "color", Attribute::COLOR }, @@ -767,8 +756,7 @@ static bool processRequires(MaterialBuilder& builder, const JsonishValue& value) }; for (auto v : value.toJsonArray()->getElements()) { if (v->getType() != JsonishValue::Type::STRING) { - std::cerr << "requires: entries must be STRINGs." << std::endl; - return false; + return utils::Status::invalidArgument("requires: entries must be STRINGs."); } auto jsonString = v->toJsonString(); @@ -779,10 +767,10 @@ static bool processRequires(MaterialBuilder& builder, const JsonishValue& value) builder.require(stringToEnum(strToEnum, jsonString->getString())); } - return true; + return utils::Status::ok(); } -static bool processBlending(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processBlending(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "add", MaterialBuilder::BlendingMode::ADD }, { "masked", MaterialBuilder::BlendingMode::MASKED }, @@ -799,10 +787,10 @@ static bool processBlending(MaterialBuilder& builder, const JsonishValue& value) } builder.blending(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok(); } -static bool processBlendFunction(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processBlendFunction(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum{ { "zero", MaterialBuilder::BlendFunction::ZERO }, { "one", MaterialBuilder::BlendFunction::ONE }, @@ -818,7 +806,7 @@ static bool processBlendFunction(MaterialBuilder& builder, const JsonishValue& v }; if (value.getType() != JsonishValue::Type::OBJECT) { - std::cerr << "blendFunction must be an OBJECT." << std::endl; + return utils::Status::invalidArgument("blendFunction must be an OBJECT."); } JsonishObject const* const jsonObject = value.toJsonObject(); @@ -835,20 +823,23 @@ static bool processBlendFunction(MaterialBuilder& builder, const JsonishValue& v const char* key = entry.first; const JsonishValue* v = jsonObject->getValue(key); if (!v) { - std::cerr << "blendFunction: entry without '" << key << "' key." << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "blendFunction: entry without '" << key << "' key."; + return utils::Status::invalidArgument(errorMessage.c_str()); } if (v->getType() != JsonishValue::STRING) { - std::cerr << "blendFunction: '" << key << "' value must be STRING." << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "blendFunction: '" << key << "' value must be STRING."; + return utils::Status::invalidArgument(errorMessage.c_str()); } *entry.second = stringToEnum(strToEnum, v->toJsonString()->getString()); } builder.customBlendFunctions(srcRGB, srcA, dstRGB, dstA); - return true; + return utils::Status::ok(); } -static bool processPostLightingBlending(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processPostLightingBlending( + MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "add", MaterialBuilder::BlendingMode::ADD }, { "opaque", MaterialBuilder::BlendingMode::OPAQUE }, @@ -862,10 +853,10 @@ static bool processPostLightingBlending(MaterialBuilder& builder, const JsonishV } builder.postLightingBlending(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok(); } -static bool processVertexDomain(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processVertexDomain(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "device", MaterialBuilder::VertexDomain::DEVICE}, { "object", MaterialBuilder::VertexDomain::OBJECT}, @@ -878,10 +869,10 @@ static bool processVertexDomain(MaterialBuilder& builder, const JsonishValue& va } builder.vertexDomain(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok(); } -static bool processCulling(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processCulling(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "back", MaterialBuilder::CullingMode::BACK }, { "front", MaterialBuilder::CullingMode::FRONT }, @@ -894,10 +885,10 @@ static bool processCulling(MaterialBuilder& builder, const JsonishValue& value) } builder.culling(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok(); } -static bool processQuality(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processQuality(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "low", MaterialBuilder::ShaderQuality::LOW }, { "normal", MaterialBuilder::ShaderQuality::NORMAL }, @@ -910,10 +901,10 @@ static bool processQuality(MaterialBuilder& builder, const JsonishValue& value) } builder.quality(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok(); } -static bool processFeatureLevel(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processFeatureLevel(MaterialBuilder& builder, const JsonishValue& value) { using filament::backend::FeatureLevel; JsonishNumber const* const number = value.toJsonNumber(); FeatureLevel featureLevel; @@ -926,14 +917,15 @@ static bool processFeatureLevel(MaterialBuilder& builder, const JsonishValue& va } else if (number->getFloat() == 3.0f) { featureLevel = FeatureLevel::FEATURE_LEVEL_3; } else { - std::cerr << "featureLevel: invalid value " << number->getFloat() << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "featureLevel: invalid value " << number->getFloat(); + return utils::Status::invalidArgument(errorMessage.c_str()); } builder.featureLevel(featureLevel); - return true; + return utils::Status::ok(); } -static bool processGroupSizes(MaterialBuilder& builder, const JsonishValue& v) { +static utils::Status processGroupSizes(MaterialBuilder& builder, const JsonishValue& v) { auto jsonArray = v.toJsonArray(); filament::math::uint3 groupSize{ 1, 1, 1 }; @@ -941,8 +933,7 @@ static bool processGroupSizes(MaterialBuilder& builder, const JsonishValue& v) { for (auto value : jsonArray->getElements()) { if (index >= 3) { - std::cerr << "groupSize: must be an array no larger than 3" << std::endl; - return false; + return utils::Status::invalidArgument("groupSize: must be an array no larger than 3"); } if (value->getType() == JsonishValue::Type::NUMBER) { JsonishNumber const* const number = value->toJsonNumber(); @@ -950,20 +941,20 @@ static bool processGroupSizes(MaterialBuilder& builder, const JsonishValue& v) { if (aFloat > 0 && floor(aFloat) == aFloat) { groupSize[index] = uint32_t(floor(aFloat)); } else { - std::cerr << "groupSize: invalid value " << aFloat << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage<< "groupSize: invalid value " << aFloat; + return utils::Status::invalidArgument(errorMessage.c_str()); } index++; continue; } - std::cerr << "groupSize must be an array of NUMBERs." << std::endl; - return false; + return utils::Status::invalidArgument("groupSize must be an array of NUMBERs."); } builder.groupSize(groupSize); - return true; + return utils::Status::ok(); } -static bool processStereoscopicType(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processStereoscopicType(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum{ { "instanced", MaterialBuilder::StereoscopicType::INSTANCED }, { "multiview", MaterialBuilder::StereoscopicType::MULTIVIEW }, @@ -974,26 +965,24 @@ static bool processStereoscopicType(MaterialBuilder& builder, const JsonishValue } builder.stereoscopicType(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok(); } -static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { +static utils::Status processOutput( + MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { const JsonishValue* nameValue = jsonObject.getValue("name"); if (!nameValue) { - std::cerr << "outputs: entry without 'name' key." << std::endl; - return false; + return utils::Status::invalidArgument("outputs: entry without 'name' key."); } if (nameValue->getType() != JsonishValue::STRING) { - std::cerr << "outputs: name value must be STRING." << std::endl; - return false; + return utils::Status::invalidArgument("outputs: name value must be STRING."); } const JsonishValue* targetValue = jsonObject.getValue("target"); if (targetValue) { if (targetValue->getType() != JsonishValue::STRING) { - std::cerr << "outputs: target must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("outputs: target must be a STRING."); } auto targetString = targetValue->toJsonString(); @@ -1005,8 +994,7 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj const JsonishValue* precisionValue = jsonObject.getValue("precision"); if (precisionValue) { if (precisionValue->getType() != JsonishValue::STRING) { - std::cerr << "parameters: precision must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("parameters: precision must be a STRING."); } auto precisionString = precisionValue->toJsonString(); @@ -1018,8 +1006,7 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj const JsonishValue* typeValue = jsonObject.getValue("type"); if (typeValue) { if (typeValue->getType() != JsonishValue::STRING) { - std::cerr << "outputs: type must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("outputs: type must be a STRING."); } auto typeString = typeValue->toJsonString(); @@ -1031,8 +1018,7 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj const JsonishValue* qualifierValue = jsonObject.getValue("qualifier"); if (qualifierValue) { if (qualifierValue->getType() != JsonishValue::STRING) { - std::cerr << "outputs: qualifier must be a STRING." << std::endl; - return false; + return utils::Status::invalidArgument("outputs: qualifier must be a STRING."); } auto qualifierString = qualifierValue->toJsonString(); @@ -1044,8 +1030,7 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj const JsonishValue* locationValue = jsonObject.getValue("location"); if (locationValue) { if (locationValue->getType() != JsonishValue::NUMBER) { - std::cerr << "outputs: location must be a NUMBER." << std::endl; - return false; + return utils::Status::invalidArgument("outputs: location must be a NUMBER."); } } @@ -1082,50 +1067,52 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj builder.output(qualifier, target, precision, type, name, location); - return true; + return utils::Status::ok(); } -static bool processOutputs(MaterialBuilder& builder, const JsonishValue& v) { +static utils::Status processOutputs(MaterialBuilder& builder, const JsonishValue& v) { auto jsonArray = v.toJsonArray(); - bool ok = true; + utils::Status status; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok &= processOutput(builder, *value->toJsonObject()); + utils::Status s = processOutput(builder, *value->toJsonObject()); + if (!s.isOk()) { + status = s; + } continue; } - std::cerr << "outputs must be an array of OBJECTs." << std::endl; - return false; + return utils::Status::invalidArgument("outputs must be an array of OBJECTs."); } - return ok; + return status; } -static bool processColorWrite(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processColorWrite(MaterialBuilder& builder, const JsonishValue& value) { builder.colorWrite(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok(); } -static bool processDepthWrite(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processDepthWrite(MaterialBuilder& builder, const JsonishValue& value) { builder.depthWrite(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok(); } -static bool processInstanced(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processInstanced(MaterialBuilder& builder, const JsonishValue& value) { builder.instanced(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok(); } -static bool processDepthCull(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processDepthCull(MaterialBuilder& builder, const JsonishValue& value) { builder.depthCulling(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok(); } -static bool processDoubleSided(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processDoubleSided(MaterialBuilder& builder, const JsonishValue& value) { builder.doubleSided(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processTransparencyMode(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processTransparencyMode(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "default", MaterialBuilder::TransparencyMode::DEFAULT }, { "twoPassesOneSide", MaterialBuilder::TransparencyMode::TWO_PASSES_ONE_SIDE }, @@ -1137,94 +1124,94 @@ static bool processTransparencyMode(MaterialBuilder& builder, const JsonishValue } builder.transparencyMode(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processMaskThreshold(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processMaskThreshold(MaterialBuilder& builder, const JsonishValue& value) { builder.maskThreshold(value.toJsonNumber()->getFloat()); - return true; + return utils::Status::ok();; } -static bool processAlphaToCoverage(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processAlphaToCoverage(MaterialBuilder& builder, const JsonishValue& value) { builder.alphaToCoverage(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processShadowMultiplier(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processShadowMultiplier(MaterialBuilder& builder, const JsonishValue& value) { builder.shadowMultiplier(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processTransparentShadow(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processTransparentShadow(MaterialBuilder& builder, const JsonishValue& value) { builder.transparentShadow(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processSpecularAntiAliasing(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processSpecularAntiAliasing(MaterialBuilder& builder, const JsonishValue& value) { builder.specularAntiAliasing(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processSpecularAntiAliasingVariance(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processSpecularAntiAliasingVariance(MaterialBuilder& builder, const JsonishValue& value) { builder.specularAntiAliasingVariance(value.toJsonNumber()->getFloat()); - return true; + return utils::Status::ok();; } -static bool processSpecularAntiAliasingThreshold(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processSpecularAntiAliasingThreshold(MaterialBuilder& builder, const JsonishValue& value) { builder.specularAntiAliasingThreshold(value.toJsonNumber()->getFloat()); - return true; + return utils::Status::ok();; } -static bool processClearCoatIorChange(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processClearCoatIorChange(MaterialBuilder& builder, const JsonishValue& value) { builder.clearCoatIorChange(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processFlipUV(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processFlipUV(MaterialBuilder& builder, const JsonishValue& value) { builder.flipUV(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processLinearFog(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processLinearFog(MaterialBuilder& builder, const JsonishValue& value) { builder.linearFog(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processShadowFarAttenuation(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processShadowFarAttenuation(MaterialBuilder& builder, const JsonishValue& value) { builder.shadowFarAttenuation(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processMultiBounceAO(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processMultiBounceAO(MaterialBuilder& builder, const JsonishValue& value) { builder.multiBounceAmbientOcclusion(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processFramebufferFetch(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processFramebufferFetch(MaterialBuilder& builder, const JsonishValue& value) { if (value.toJsonBool()->getBool()) { builder.enableFramebufferFetch(); } - return true; + return utils::Status::ok();; } -static bool processVertexDomainDeviceJittered(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processVertexDomainDeviceJittered(MaterialBuilder& builder, const JsonishValue& value) { builder.vertexDomainDeviceJittered(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processLegacyMorphing(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processLegacyMorphing(MaterialBuilder& builder, const JsonishValue& value) { if (value.toJsonBool()->getBool()) { builder.useLegacyMorphing(); } - return true; + return utils::Status::ok();; } -static bool processCustomSurfaceShading(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processCustomSurfaceShading(MaterialBuilder& builder, const JsonishValue& value) { builder.customSurfaceShading(value.toJsonBool()->getBool()); - return true; + return utils::Status::ok();; } -static bool processSpecularAmbientOcclusion(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processSpecularAmbientOcclusion(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "none", MaterialBuilder::SpecularAmbientOcclusion::NONE }, { "simple", MaterialBuilder::SpecularAmbientOcclusion::SIMPLE }, @@ -1239,10 +1226,10 @@ static bool processSpecularAmbientOcclusion(MaterialBuilder& builder, const Json } builder.specularAmbientOcclusion(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processShading(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processShading(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "cloth", MaterialBuilder::Shading::CLOTH }, { "lit", MaterialBuilder::Shading::LIT }, @@ -1256,10 +1243,10 @@ static bool processShading(MaterialBuilder& builder, const JsonishValue& value) } builder.shading(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processDomain(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processDomain(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "surface", MaterialBuilder::MaterialDomain::SURFACE }, { "postprocess", MaterialBuilder::MaterialDomain::POST_PROCESS }, @@ -1271,10 +1258,10 @@ static bool processDomain(MaterialBuilder& builder, const JsonishValue& value) { } builder.materialDomain(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processRefractionMode(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processRefractionMode(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum{ { "none", MaterialBuilder::RefractionMode::NONE }, { "cubemap", MaterialBuilder::RefractionMode::CUBEMAP }, @@ -1286,10 +1273,10 @@ static bool processRefractionMode(MaterialBuilder& builder, const JsonishValue& } builder.refractionMode(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processReflectionMode(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processReflectionMode(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "default", MaterialBuilder::ReflectionMode ::DEFAULT }, { "screenspace", MaterialBuilder::ReflectionMode::SCREEN_SPACE }, @@ -1300,10 +1287,10 @@ static bool processReflectionMode(MaterialBuilder& builder, const JsonishValue& } builder.reflectionMode(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processRefractionType(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processRefractionType(MaterialBuilder& builder, const JsonishValue& value) { static const std::unordered_map strToEnum { { "solid", MaterialBuilder::RefractionType::SOLID }, { "thin", MaterialBuilder::RefractionType::THIN }, @@ -1314,10 +1301,10 @@ static bool processRefractionType(MaterialBuilder& builder, const JsonishValue& } builder.refractionType(stringToEnum(strToEnum, jsonString->getString())); - return true; + return utils::Status::ok();; } -static bool processVariantFilter(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processVariantFilter(MaterialBuilder& builder, const JsonishValue& value) { // We avoid using an initializer list for this particular map to avoid build errors that are // due to static initialization ordering. using filament::Variant; @@ -1341,30 +1328,32 @@ static bool processVariantFilter(MaterialBuilder& builder, const JsonishValue& v for (size_t i = 0; i < elements.size(); i++) { auto elementValue = elements[i]; if (elementValue->getType() != JsonishValue::Type::STRING) { - std::cerr << "variant_filter: array index " << i << + utils::io::sstream errorMessage; + errorMessage << "variant_filter: array index " << i << " is not a STRING. found:" << - JsonishValue::typeToString(elementValue->getType()) << std::endl; - return false; + JsonishValue::typeToString(elementValue->getType()); + return utils::Status::invalidArgument(errorMessage.c_str()); } const std::string& s = elementValue->toJsonString()->getString(); if (!isStringValidEnum(strToEnum, s)) { - std::cerr << "variant_filter: variant " << s << - " is not a valid variant" << std::endl; + utils::io::sstream errorMessage; + errorMessage << "variant_filter: variant " << s << " is not a valid variant"; + return utils::Status::invalidArgument(errorMessage.c_str()); } variantFilter |= (uint32_t)strToEnum.at(s); } builder.variantFilter(variantFilter); - return true; + return utils::Status::ok(); } -static bool processUseDefaultDepthVariant(MaterialBuilder& builder, const JsonishValue& value) { +static utils::Status processUseDefaultDepthVariant(MaterialBuilder& builder, const JsonishValue& value) { if (value.toJsonBool()->getBool()) { builder.useDefaultDepthVariant(); } - return true; + return utils::Status::ok();; } ParametersProcessor::ParametersProcessor() { @@ -1419,7 +1408,7 @@ ParametersProcessor::ParametersProcessor() { mParameters["shadowFarAttenuation"] = { &processShadowFarAttenuation, Type::BOOL }; } -bool ParametersProcessor::process(MaterialBuilder& builder, const JsonishObject& jsonObject) { +utils::Status ParametersProcessor::process(MaterialBuilder& builder, const JsonishObject& jsonObject) { for(const auto& entry : jsonObject.getEntries()) { const std::string& key = entry.first; const JsonishValue* field = entry.second; @@ -1430,26 +1419,29 @@ bool ParametersProcessor::process(MaterialBuilder& builder, const JsonishObject& // Verify type is what was expected. if (mParameters.at(key).rootAssert != field->getType()) { - std::cerr << "Value for key:\"" << key << "\" is not what was expected" << std::endl; - std::cerr << "Got :\"" << JsonishValue::typeToString(field->getType())<< "\" but expected '" - << JsonishValue::typeToString(mParameters.at(key).rootAssert) << "'" << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Value for key:\"" << key << "\" is not what was expected." + << utils::io::endl; + errorMessage << "Got :\"" << JsonishValue::typeToString(field->getType()) + << "\" but expected '" + << JsonishValue::typeToString(mParameters.at(key).rootAssert) << "'"; + return utils::Status::invalidArgument(errorMessage.c_str()); } auto fPointer = mParameters[key].callback; - bool ok = fPointer(builder, *field); - if (!ok) { + if (utils::Status status = fPointer(builder, *field); !status.isOk()) { std::cerr << "Error while processing material json, key:\"" << key << "\"" << std::endl; - return false; + return status; } } - return true; + return utils::Status::ok();; } -bool ParametersProcessor::process(filamat::MaterialBuilder& builder, const std::string& key, const std::string& value) { +utils::Status ParametersProcessor::process(filamat::MaterialBuilder& builder, const std::string& key, const std::string& value) { if (mParameters.find(key) == mParameters.end()) { - std::cerr << "Ignoring config entry (unknown key): \"" << key << "\"" << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Ignoring config entry (unknown key): \"" << key << "\""; + return utils::Status::invalidArgument(errorMessage.c_str()); } std::unique_ptr var; @@ -1472,19 +1464,18 @@ bool ParametersProcessor::process(filamat::MaterialBuilder& builder, const std:: var = std::make_unique(value); break; default: - std::cerr << "Unsupported type: \"" - << JsonishValue::typeToString(mParameters.at(key).rootAssert) - << "\"" << std::endl; - return false; + utils::io::sstream errorMessage; + errorMessage << "Unsupported type: \"" + << JsonishValue::typeToString(mParameters.at(key).rootAssert) << "\""; + return utils::Status::invalidArgument(errorMessage.c_str()); } auto fPointer = mParameters[key].callback; - bool ok = fPointer(builder, *var); - if (!ok) { + if (utils::Status status = fPointer(builder, *var); !status.isOk()) { std::cerr << "Error while processing material param, key:\"" << key << "\"" << std::endl; - return false; + return status; } - return true; + return utils::Status::ok(); } } // namespace matp diff --git a/libs/filament-matp/src/ParametersProcessor.h b/libs/filament-matp/src/ParametersProcessor.h index 41e54c8507..f0e7751e9b 100644 --- a/libs/filament-matp/src/ParametersProcessor.h +++ b/libs/filament-matp/src/ParametersProcessor.h @@ -25,6 +25,7 @@ #include "JsonishParser.h" #include +#include namespace matp { @@ -33,12 +34,12 @@ class ParametersProcessor { public: ParametersProcessor(); ~ParametersProcessor() = default; - bool process(filamat::MaterialBuilder& builder, const JsonishObject& jsonObject); - bool process(filamat::MaterialBuilder& builder, const std::string& key, const std::string& value); + utils::Status process(filamat::MaterialBuilder& builder, const JsonishObject& jsonObject); + utils::Status process(filamat::MaterialBuilder& builder, const std::string& key, const std::string& value); private: - using Callback = bool (*)(filamat::MaterialBuilder& builder, const JsonishValue& value); + using Callback = utils::Status (*)(filamat::MaterialBuilder& builder, const JsonishValue& value); struct ParameterInfo { Callback callback; diff --git a/libs/filament-matp/tests/TestMaterialParser.h b/libs/filament-matp/tests/TestMaterialParser.h index 0c35927412..b64a27aae7 100644 --- a/libs/filament-matp/tests/TestMaterialParser.h +++ b/libs/filament-matp/tests/TestMaterialParser.h @@ -25,12 +25,12 @@ public: explicit TestMaterialParser(const matp::MaterialParser& materialParser) : mMaterialParser(materialParser) {} - bool parseMaterial(const char* buffer, size_t size, filamat::MaterialBuilder& builder) + utils::Status parseMaterial(const char* buffer, size_t size, filamat::MaterialBuilder& builder) noexcept{ return mMaterialParser.parseMaterial(buffer, size, builder); } - bool parseMaterialAsJSON(const char* buffer, size_t size, filamat::MaterialBuilder& builder) + utils::Status parseMaterialAsJSON(const char* buffer, size_t size, filamat::MaterialBuilder& builder) noexcept{ return mMaterialParser.parseMaterialAsJSON(buffer, size, builder); } diff --git a/libs/filament-matp/tests/test_compute_material.cpp b/libs/filament-matp/tests/test_compute_material.cpp index 97ed00eeec..208ec27cbd 100644 --- a/libs/filament-matp/tests/test_compute_material.cpp +++ b/libs/filament-matp/tests/test_compute_material.cpp @@ -41,9 +41,10 @@ TEST(TestParseAndComputeMaterial, JsonMaterialCompilerSimple) { filamat::MaterialBuilder::init(); filamat::MaterialBuilder builder; - bool result = testParser.parseMaterial(jsonMaterialSourceSimple.data(), jsonMaterialSourceSimple.size(), builder); + utils::Status result = testParser.parseMaterial( + jsonMaterialSourceSimple.data(), jsonMaterialSourceSimple.size(), builder); - EXPECT_TRUE(result); + EXPECT_EQ(result.getCode(), utils::StatusCode::OK); utils::JobSystem js; js.adopt(); diff --git a/libs/filament-matp/tests/test_matp.cpp b/libs/filament-matp/tests/test_matp.cpp index ee465b1e51..a55c8cdb14 100644 --- a/libs/filament-matp/tests/test_matp.cpp +++ b/libs/filament-matp/tests/test_matp.cpp @@ -93,8 +93,9 @@ TEST_F(MaterialLexer, MaterialParser) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial(materialSource.c_str(), materialSource.size(), unused); - EXPECT_EQ(result, true); + utils::Status result = + testParser.parseMaterial(materialSource.c_str(), materialSource.size(), unused); + EXPECT_EQ(result.getCode(), utils::StatusCode::OK); } TEST_F(MaterialLexer, NoSpaceBetweenBlockAndIdentifier) { @@ -111,16 +112,18 @@ TEST_F(MaterialLexer, MaterialParserWithToolSection) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial(materialSourceWithTool.c_str(), materialSourceWithTool.size(), unused); - EXPECT_EQ(result, true); + utils::Status result =testParser.parseMaterial( + materialSourceWithTool.c_str(), materialSourceWithTool.size(), unused); + EXPECT_EQ(result.getCode(), utils::StatusCode::OK); } TEST_F(MaterialLexer, MaterialParserWithCommentedBraces) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial(materialSourceWithCommentedBraces.c_str(), materialSourceWithCommentedBraces.size(), unused); - EXPECT_EQ(result, true); + utils::Status result = testParser.parseMaterial( + materialSourceWithCommentedBraces.c_str(), materialSourceWithCommentedBraces.size(), unused); + EXPECT_EQ(result.getCode(), utils::StatusCode::OK); } TEST_F(MaterialLexer, MaterialParserErrorOnlyIdentifier) { @@ -130,9 +133,9 @@ TEST_F(MaterialLexer, MaterialParserErrorOnlyIdentifier) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial( + utils::Status result = testParser.parseMaterial( sourceMissingIdentifier.c_str(), sourceMissingIdentifier.size(), unused); - EXPECT_EQ(result, false); + EXPECT_EQ(result.getCode(), utils::StatusCode::INVALID_ARGUMENT); } TEST_F(MaterialLexer, MaterialParserErrorMissingBlock) { @@ -143,9 +146,9 @@ TEST_F(MaterialLexer, MaterialParserErrorMissingBlock) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial( + utils::Status result = testParser.parseMaterial( sourceMissingBlock.c_str(), sourceMissingBlock.size(), unused); - EXPECT_EQ(result, false); + EXPECT_EQ(result.getCode(), utils::StatusCode::INVALID_ARGUMENT); } TEST_F(MaterialLexer, MaterialParserErrorTwoBlock) { @@ -155,8 +158,9 @@ TEST_F(MaterialLexer, MaterialParserErrorTwoBlock) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial(sourceTwoBlock.c_str(), sourceTwoBlock.size(), unused); - EXPECT_EQ(result, false); + utils::Status result = + testParser.parseMaterial(sourceTwoBlock.c_str(), sourceTwoBlock.size(), unused); + EXPECT_EQ(result.getCode(), utils::StatusCode::INVALID_ARGUMENT); } TEST_F(MaterialLexer, MaterialParserSyntaxError) { @@ -166,8 +170,8 @@ TEST_F(MaterialLexer, MaterialParserSyntaxError) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterial(sourceSyntaxError.c_str(), sourceSyntaxError.size(), unused); - EXPECT_EQ(result, false); + utils::Status result = testParser.parseMaterial(sourceSyntaxError.c_str(), sourceSyntaxError.size(), unused); + EXPECT_EQ(result.getCode(), utils::StatusCode::INVALID_ARGUMENT); } static std::string jsonMaterialSource(R"( @@ -290,8 +294,9 @@ TEST_F(MaterialLexer, JsonMaterialParser) { matp::MaterialParser parser; TestMaterialParser testParser(parser); filamat::MaterialBuilder unused; - bool result = testParser.parseMaterialAsJSON(jsonMaterialSource.c_str(), jsonMaterialSource.size(), unused); - EXPECT_EQ(result, true); + utils::Status result = testParser.parseMaterialAsJSON( + jsonMaterialSource.c_str(), jsonMaterialSource.size(), unused); + EXPECT_EQ(result.getCode(), utils::StatusCode::OK); } int main(int argc, char** argv) { diff --git a/libs/utils/include/utils/Status.h b/libs/utils/include/utils/Status.h index c2c494c60c..fe06fb18d3 100644 --- a/libs/utils/include/utils/Status.h +++ b/libs/utils/include/utils/Status.h @@ -38,7 +38,7 @@ enum class StatusCode { /** * Returns the StatusCode to indicate whether the request was successful. - * If successful, it returns OK with no error message, if not it returns + * If successful, it returns OK with an optional message, if not it returns * other codes with an optional error message. */ class UTILS_PUBLIC Status { @@ -49,14 +49,14 @@ public: Status() : mStatusCode(StatusCode::OK) {} /** - * Creates a new Status with the given status code and error message. + * Creates a new Status with the given status code and supplementary message. * * @param statusCode The status code to use. - * @param errorMessage An optional error message. + * @param message An optional message, usually contains the reason for the failure. */ - Status(StatusCode statusCode, std::string_view errorMessage) : + Status(StatusCode statusCode, std::string_view message) : mStatusCode(statusCode), - mErrorMessage(errorMessage.data(), errorMessage.length()) {} + mMessage(message.data(), message.length()) {} Status(const Status& other) = default; @@ -68,7 +68,7 @@ public: Status& operator=(Status&& other) noexcept = default; bool operator==(const Status& other) const { - return mStatusCode == other.mStatusCode && mErrorMessage == other.mErrorMessage; + return mStatusCode == other.mStatusCode && mMessage == other.mMessage; } bool operator!=(const Status& other) const { @@ -92,11 +92,11 @@ public: } /** - * Returns the error message for this Status. - * @return The error message string. Will be empty if the status is OK. + * Returns the message for this Status. + * @return The message string. Can be empty if it's not set. */ - std::string_view getErrorMessage() const { - return std::string_view(mErrorMessage.begin(), mErrorMessage.end()); + std::string_view getMessage() const { + return std::string_view(mMessage.begin(), mMessage.end()); } /** @@ -112,6 +112,14 @@ public: return {}; } + /** + * Creates a success Status with a StatusCode of OK with a supplementary message. + * @return a success Status with a StatusCode of OK with a supplementary message. + */ + static Status ok(std::string_view message) { + return {StatusCode::OK, message}; + } + /** * Creates an error Status with an INTERNAL status code. * @param message The error message to include. @@ -132,8 +140,8 @@ public: private: StatusCode mStatusCode; - // Reason for the error if exists. - utils::CString mErrorMessage; + // Additional message for the Status. Usually contains the reason for the error. + utils::CString mMessage; }; utils::io::ostream& operator<<(utils::io::ostream& os, const Status& status); diff --git a/libs/utils/src/Status.cpp b/libs/utils/src/Status.cpp index a6a9f21016..f495f8ebfc 100644 --- a/libs/utils/src/Status.cpp +++ b/libs/utils/src/Status.cpp @@ -29,7 +29,7 @@ utils::io::ostream& operator<<(utils::io::ostream& os, const Status& status) { case StatusCode::INTERNAL: os << "Internal error"; break; } - os << ", error message: " << status.getErrorMessage(); + os << ", with a message: " << status.getMessage(); return os; } } // namespace utils diff --git a/libs/utils/test/test_Status.cpp b/libs/utils/test/test_Status.cpp index 68628c9018..84c73ad9b6 100644 --- a/libs/utils/test/test_Status.cpp +++ b/libs/utils/test/test_Status.cpp @@ -29,13 +29,13 @@ TEST(StatusTest, Constructor) { std::string_view errorMessage = "invalid"; Status status(StatusCode::INVALID_ARGUMENT, errorMessage); EXPECT_EQ(status.getCode(), StatusCode::INVALID_ARGUMENT); - EXPECT_EQ(status.getErrorMessage(), errorMessage); + EXPECT_EQ(status.getMessage(), errorMessage); } TEST(StatusTest, CopyOperator) { Status status1 = Status::ok(); EXPECT_EQ(status1.getCode(), StatusCode::OK); - EXPECT_EQ(status1.getErrorMessage(), ""); + EXPECT_EQ(status1.getMessage(), ""); Status status2(StatusCode::INTERNAL, "internal error"); status1 = status2; @@ -52,14 +52,14 @@ TEST(StatusTest, CopyConstructor) { TEST(StatusTest, MoveOperator) { Status status; EXPECT_EQ(status.getCode(), StatusCode::OK); - EXPECT_EQ(status.getErrorMessage(), ""); + EXPECT_EQ(status.getMessage(), ""); std::string_view errorMessage = "internal error"; Status another(StatusCode::INTERNAL, errorMessage); status = std::move(another); EXPECT_EQ(status.getCode(), StatusCode::INTERNAL); - EXPECT_EQ(status.getErrorMessage(), errorMessage); + EXPECT_EQ(status.getMessage(), errorMessage); } TEST(StatusTest, MoveConstructor) { @@ -68,7 +68,7 @@ TEST(StatusTest, MoveConstructor) { Status moved(std::move(original)); EXPECT_EQ(moved.getCode(), StatusCode::INTERNAL); - EXPECT_EQ(moved.getErrorMessage(), errorMessage); + EXPECT_EQ(moved.getMessage(), errorMessage); } TEST(StatusTest, Equality) { @@ -97,6 +97,12 @@ TEST(StatusTest, StaticOk) { EXPECT_EQ(Status::ok(), expected); } +TEST(StatusTest, StaticOkWithMessage) { + std::string_view supplementaryMessage = "some debug string"; + Status expected(StatusCode::OK, supplementaryMessage); + EXPECT_EQ(Status::ok(supplementaryMessage), expected); +} + TEST(StatusTest, StaticInvalidArgumentError) { std::string_view errorMessage = "invalid argument"; Status expected(StatusCode::INVALID_ARGUMENT, errorMessage); diff --git a/tools/matc/src/matc/MaterialCompiler.cpp b/tools/matc/src/matc/MaterialCompiler.cpp index 4ce91aca00..95ce7409d4 100644 --- a/tools/matc/src/matc/MaterialCompiler.cpp +++ b/tools/matc/src/matc/MaterialCompiler.cpp @@ -80,13 +80,16 @@ bool MaterialCompiler::run(const matp::Config& config) { builder.includeCallback(includer) .fileName(materialFilePath.getName().c_str()); - if (!mParser.parse(builder, config, size, buffer)) { + utils::Status status = mParser.parse(builder, config, size, buffer); + if (!status.isOk()) { + std::cerr << status.getMessage() << std::endl; return false; } // If we're reflecting parameters, the MaterialParser will have handled it inside of parse(). // We should return here to avoid actually building a material. if (config.getReflectionTarget() != matp::Config::Metadata::NONE) { + std::cout << status.getMessage() << std::endl; return true; }