Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use utils::Status in MaterialParser (#9285)

Browse Source 
* Use utils::Status in MaterialParser

* Use utils::sstream instead of std::stringstream

* Remove remaining std::cerr and dep; update MaterialParser::reflectParameters

* make error message in utils::Status more generic

---------

Co-authored-by: Powei Feng <powei@google.com>
This commit is contained in:
yein
2025-10-07 11:27:11 -07:00
committed by GitHub
parent c839915a00
commit f52476a323
11 changed files with 554 additions and 535 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
libs/filament-matp/include/filament-matp/MaterialParser.h
View File

@@ -20,6 +20,7 @@
#include <unordered_map>
#include <utils/Status.h>
#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<const char[]>& 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<const char[]>& 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<std::string_view, MaterialConfigProcessor> 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<std::string_view, MaterialConfigProcessorJSON> mConfigProcessorJSON;
};

318
libs/filament-matp/src/MaterialParser.cpp
View File

@@ -16,24 +16,23 @@
#include <filament-matp/MaterialParser.h>
#include <memory>
#include <iostream>
#include <utility>
#include <filamat/MaterialBuilder.h>
#include <filamat/Enums.h>
#include <utils/Log.h>
#include <utils/JobSystem.h>
#include "JsonishLexer.h"
#include "JsonishParser.h"
#include "MaterialLexeme.h"
#include "MaterialLexer.h"
#include "ParametersProcessor.h"
#include <filamat/Enums.h>
#include <filamat/MaterialBuilder.h>
#include <filament-matp/Config.h>
#include <utils/sstream.h>
#include <utils/Status.h>
#include <utils/JobSystem.h>
#include <memory>
#include <utility>
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<JsonishObject> 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<JsonishObject> 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<const char[]>& 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<const char[]>& 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

623
libs/filament-matp/src/ParametersProcessor.cpp
View File

File diff suppressed because it is too large Load Diff

7
libs/filament-matp/src/ParametersProcessor.h
View File

@@ -25,6 +25,7 @@
#include "JsonishParser.h"
#include <filamat/MaterialBuilder.h>
#include <utils/Status.h>
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;

4
libs/filament-matp/tests/TestMaterialParser.h
View File

@@ -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);
}

5
libs/filament-matp/tests/test_compute_material.cpp
View File

@@ -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();

37
libs/filament-matp/tests/test_matp.cpp
View File

@@ -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) {

32
libs/utils/include/utils/Status.h
View File

@@ -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);

2
libs/utils/src/Status.cpp
View File

@@ -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

16
libs/utils/test/test_Status.cpp
View File

@@ -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);

5
tools/matc/src/matc/MaterialCompiler.cpp
View File

@@ -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;
}