From 4773fc4647d3732a2fe3ceaf13dc03491f215e69 Mon Sep 17 00:00:00 2001 From: yein Date: Fri, 7 Nov 2025 15:23:54 -0800 Subject: [PATCH] Move the include resolution functionality to matp (#9414) * Move the include resolution functionality to matp - matp::MaterialParser now has a function resolveIncludes that returns a pair of status and resolve string. - all the include resolution classes are moved to matp private src. - matp::resolveIncludes is renamed to matp::resolveIncludesRecursively and only used internally. - added insertLineDirectives and insertLineDirectiveChecks in Config; add those in the CommandlineConfig. - moved output format to public use. Note: MaterialParser::resolveIncludes could take a includer instead of the materialFilePath, but i decided to go with the materialFilePath because the most common use case is resolving from the file's directory. This allows the parser to just create the default DirIncluder internally, and we don't need to expose it publicly. * add const to the buffer param --- libs/filament-matp/CMakeLists.txt | 7 +++ .../include/filament-matp/Config.h | 5 ++ .../include/filament-matp/MaterialParser.h | 12 +++- .../filament-matp/src}/DirIncluder.cpp | 4 +- .../filament-matp/src}/DirIncluder.h | 4 +- .../filament-matp/src}/IncludeCallback.h | 9 +-- .../filament-matp/src}/Includes.cpp | 34 ++++++----- .../filament-matp/src}/Includes.h | 12 ++-- libs/filament-matp/src/MaterialParser.cpp | 31 ++++++++++ .../filament-matp}/tests/Dir/Bar.h | 0 .../filament-matp}/tests/Dir/Baz.h | 0 .../matc => libs/filament-matp}/tests/Foo.h | 0 .../filament-matp}/tests/MockIncluder.h | 4 +- .../filament-matp}/tests/test_includer.cpp | 15 ++--- .../filament-matp}/tests/test_includes.cpp | 61 ++++++++++--------- libs/filament-matp/tests/test_matp.cpp | 8 +-- tools/matc/CMakeLists.txt | 19 ------ tools/matc/src/matc/CommandlineConfig.cpp | 28 +++++++-- tools/matc/src/matc/MaterialCompiler.cpp | 31 +++------- 19 files changed, 162 insertions(+), 122 deletions(-) rename {tools/matc/src/matc => libs/filament-matp/src}/DirIncluder.cpp (98%) rename {tools/matc/src/matc => libs/filament-matp/src}/DirIncluder.h (96%) rename {tools/matc/src/matc => libs/filament-matp/src}/IncludeCallback.h (92%) rename {tools/matc/src/matc => libs/filament-matp/src}/Includes.cpp (91%) rename {tools/matc/src/matc => libs/filament-matp/src}/Includes.h (82%) rename {tools/matc => libs/filament-matp}/tests/Dir/Bar.h (100%) rename {tools/matc => libs/filament-matp}/tests/Dir/Baz.h (100%) rename {tools/matc => libs/filament-matp}/tests/Foo.h (100%) rename {tools/matc => libs/filament-matp}/tests/MockIncluder.h (95%) rename {tools/matc => libs/filament-matp}/tests/test_includer.cpp (89%) rename {tools/matc => libs/filament-matp}/tests/test_includes.cpp (89%) diff --git a/libs/filament-matp/CMakeLists.txt b/libs/filament-matp/CMakeLists.txt index 15d6953110..4fdc2d3910 100644 --- a/libs/filament-matp/CMakeLists.txt +++ b/libs/filament-matp/CMakeLists.txt @@ -13,6 +13,11 @@ set(PUBLIC_HDRS ) set(SRCS + src/DirIncluder.h + src/DirIncluder.cpp + src/IncludeCallback.h + src/Includes.h + src/Includes.cpp src/JsonishLexeme.h src/JsonishLexer.h src/JsonishParser.h @@ -53,6 +58,8 @@ install(DIRECTORY ${PUBLIC_HDR_DIR}/ DESTINATION include) project(test_matp) set(TARGET test_matp) set(SRCS + tests/test_includes.cpp + tests/test_includer.cpp tests/test_matp.cpp tests/TestMaterialParser.h tests/test_compute_material.cpp diff --git a/libs/filament-matp/include/filament-matp/Config.h b/libs/filament-matp/include/filament-matp/Config.h index a01f9b3a59..9f61bb7158 100644 --- a/libs/filament-matp/include/filament-matp/Config.h +++ b/libs/filament-matp/include/filament-matp/Config.h @@ -155,6 +155,9 @@ public: return mWorkarounds; } + bool getInsertLineDirectives() const noexcept { return mInsertLineDirectives; } + bool getInsertLineDirectiveChecks() const noexcept { return mInsertLineDirectiveChecks; } + protected: bool mDebug = false; bool mIsValid = true; @@ -174,6 +177,8 @@ protected: filament::UserVariantFilterMask mVariantFilter = 0; Workarounds mWorkarounds = Workarounds::ALL; bool mIncludeEssl1 = true; + bool mInsertLineDirectives = true; + bool mInsertLineDirectiveChecks = true; }; } // namespace matp diff --git a/libs/filament-matp/include/filament-matp/MaterialParser.h b/libs/filament-matp/include/filament-matp/MaterialParser.h index e186815f05..f354b935cf 100644 --- a/libs/filament-matp/include/filament-matp/MaterialParser.h +++ b/libs/filament-matp/include/filament-matp/MaterialParser.h @@ -20,9 +20,11 @@ #include -#include #include "Config.h" +#include +#include + namespace filamat { class MaterialBuilder; } @@ -36,7 +38,13 @@ class MaterialLexeme; class MaterialParser { public: MaterialParser(); - + // Resolves #include directives in the material by looking at the given file structure. + // Returns the string of resolved material with the state that indicates whether the include + // resolution was successful. + std::pair resolveIncludes( + const std::unique_ptr& buffer, ssize_t size, + const utils::Path& materialFilePath, + bool insertLineDirectives, bool insertLineDirectiveCheck); // Parses a string material so that it can be used in MaterialBuilder. // Call MaterialBuilder::init before passing in the builder; call MaterialBuilder::build to // create filamat::Package after. diff --git a/tools/matc/src/matc/DirIncluder.cpp b/libs/filament-matp/src/DirIncluder.cpp similarity index 98% rename from tools/matc/src/matc/DirIncluder.cpp rename to libs/filament-matp/src/DirIncluder.cpp index 937495f35d..5387fb1891 100644 --- a/tools/matc/src/matc/DirIncluder.cpp +++ b/libs/filament-matp/src/DirIncluder.cpp @@ -20,7 +20,7 @@ #include -namespace matc { +namespace matp { bool DirIncluder::operator()(const utils::CString& includedBy, IncludeResult& result) { auto getHeaderPath = [&result, &includedBy, this]() { @@ -66,5 +66,5 @@ bool DirIncluder::operator()(const utils::CString& includedBy, IncludeResult& re return true; } -} // namespace matc +} // namespace matp diff --git a/tools/matc/src/matc/DirIncluder.h b/libs/filament-matp/src/DirIncluder.h similarity index 96% rename from tools/matc/src/matc/DirIncluder.h rename to libs/filament-matp/src/DirIncluder.h index 4be7cf048a..38a978602c 100644 --- a/tools/matc/src/matc/DirIncluder.h +++ b/libs/filament-matp/src/DirIncluder.h @@ -21,7 +21,7 @@ #include -namespace matc { +namespace matp { // Functor callback handler used to resolve includes relative to a root include directory. class DirIncluder { @@ -38,6 +38,6 @@ private: }; -} // namespace matc +} // namespace matp #endif diff --git a/tools/matc/src/matc/IncludeCallback.h b/libs/filament-matp/src/IncludeCallback.h similarity index 92% rename from tools/matc/src/matc/IncludeCallback.h rename to libs/filament-matp/src/IncludeCallback.h index 64e534c378..64daf27366 100644 --- a/tools/matc/src/matc/IncludeCallback.h +++ b/libs/filament-matp/src/IncludeCallback.h @@ -18,10 +18,11 @@ #define TNT_INCLUDER_H #include +#include #include -namespace matc { +namespace matp { struct IncludeResult { // The include name of the root file, as if it were being included. @@ -30,7 +31,7 @@ struct IncludeResult { const utils::CString includeName; // The following fields should be filled out by the IncludeCallback when processing an include, - // or when calling resolveIncludes for the root file. + // or when calling resolveIncludesRecursively for the root file. // The full contents of the include file. This may contain additional, recursive include // directives. @@ -58,12 +59,12 @@ struct IncludeResult { * @param result is the IncludeResult that the callback should fill out. * @return true, if the include was resolved successfully, false otherwise. * - * For an example of implementing this callback, see tools/matc/src/matc/DirIncluder.h. + * For an example of implementing this callback, see DirIncluder.h. */ using IncludeCallback = std::function; -} // namespace matc +} // namespace matp #endif diff --git a/tools/matc/src/matc/Includes.cpp b/libs/filament-matp/src/Includes.cpp similarity index 91% rename from tools/matc/src/matc/Includes.cpp rename to libs/filament-matp/src/Includes.cpp index 0ba2f8ad35..33ae0c44bd 100644 --- a/tools/matc/src/matc/Includes.cpp +++ b/libs/filament-matp/src/Includes.cpp @@ -19,10 +19,11 @@ #include #include #include +#include #include -namespace matc { +namespace matp { static bool isWhitespace(char c) { return (c == ' ' || c == '\f' || c == '\n' || c == '\r' || c == '\t' || c == '\v'); @@ -75,12 +76,11 @@ static void findCommentRanges(const char (&beginToken)[BTSize], const char (&end } -bool resolveIncludes(IncludeResult& root, IncludeCallback callback, +utils::Status resolveIncludesRecursively(IncludeResult& root, IncludeCallback callback, const ResolveOptions& options, size_t depth) { if (depth > 30) { // This is probably an include cycle. Stop here and report an error so we don't overflow. - utils::slog.e << "Include depth > 30. Include cycle?" << utils::io::endl; - return false; + return utils::Status::invalidArgument("Include depth > 30. Include cycle?"); } utils::CString& text = root.text; @@ -88,7 +88,7 @@ bool resolveIncludes(IncludeResult& root, IncludeCallback callback, std::vector includes = parseForIncludes(text); if (includes.empty()) { // No more to resolve. - return true; + return utils::Status::ok(); } const size_t lineNumberOffset = includes[0].line; @@ -196,28 +196,30 @@ bool resolveIncludes(IncludeResult& root, IncludeCallback callback, const auto include = includes[0]; // Ask the includer to resolve this include. if (!callback) { - return false; + return utils::Status::invalidArgument("No callback is provided"); } - IncludeResult resolved { + IncludeResult includeResult{ .includeName = include.name }; - if (!callback(root.name, resolved)) { - utils::slog.e << "The included file \"" << include.name.c_str() + if (!callback(root.name, includeResult)) { + utils::io::sstream errorMessage; + errorMessage << "The included file \"" << include.name.c_str() << "\" could not be found." << utils::io::endl; - return false; + return utils::Status::invalidArgument(errorMessage.c_str()); } // Recursively resolve all of its includes. - if (!resolveIncludes(resolved, callback, options, depth + 1)) { - return false; + if (utils::Status resolveStatus = + resolveIncludesRecursively(includeResult, callback, options, depth + 1); + !resolveStatus.isOk()) { + return resolveStatus; } - text.replace(include.startPosition, include.length, resolved.text); + text.replace(include.startPosition, include.length, includeResult.text); includes = parseForIncludes(text); } - - return true; + return utils::Status::ok(); } std::vector parseForIncludes(const utils::CString& source) { @@ -308,4 +310,4 @@ std::vector parseForIncludes(const utils::CString& source) { return results; } -} // namespace matc +} // namespace matp diff --git a/tools/matc/src/matc/Includes.h b/libs/filament-matp/src/Includes.h similarity index 82% rename from tools/matc/src/matc/Includes.h rename to libs/filament-matp/src/Includes.h index 683796be4a..58edcd7d5f 100644 --- a/tools/matc/src/matc/Includes.h +++ b/libs/filament-matp/src/Includes.h @@ -20,10 +20,11 @@ #include "IncludeCallback.h" #include +#include #include -namespace matc { +namespace matp { struct ResolveOptions { // If true, insert #line directives before / after each include. @@ -39,9 +40,10 @@ struct ResolveOptions { }; // Recursively handle all the includes inside of root. -// Returns true if all includes were handled successfully, false otherwise. -// callback may be null, in which case any #include directives found will result in a failure. -bool resolveIncludes(IncludeResult& root, IncludeCallback callback, +// Returns utils::Status::StatusCode::OK if all includes were handled successfully, otherwise an +// error status with a failure message. +// Callback may be null, in which case any #include directives found will result in a failure. +utils::Status resolveIncludesRecursively(IncludeResult& root, IncludeCallback callback, const ResolveOptions& options, size_t depth = 0); struct FoundInclude { @@ -53,6 +55,6 @@ struct FoundInclude { std::vector parseForIncludes(const utils::CString& source); -} // namespace matc +} // namespace matp #endif diff --git a/libs/filament-matp/src/MaterialParser.cpp b/libs/filament-matp/src/MaterialParser.cpp index ab3a24d3c4..06815db190 100644 --- a/libs/filament-matp/src/MaterialParser.cpp +++ b/libs/filament-matp/src/MaterialParser.cpp @@ -16,6 +16,8 @@ #include +#include "DirIncluder.h" +#include "Includes.h" #include "JsonishLexer.h" #include "JsonishParser.h" #include "MaterialLexeme.h" @@ -393,6 +395,35 @@ utils::Status MaterialParser::processMaterialParameters(filamat::MaterialBuilder return status; } +std::pair MaterialParser::resolveIncludes( + const std::unique_ptr& buffer, ssize_t size, + const utils::Path& materialFilePath, + bool insertLineDirectives, bool insertLineDirectiveCheck) { + + ResolveOptions const options { + .insertLineDirectives = insertLineDirectives, + .insertLineDirectiveCheck = insertLineDirectiveCheck + }; + + // Inline material (i.e. resolve #include directives) + DirIncluder includer; + includer.setIncludeDirectory(materialFilePath.getParent()); + utils::CString fileName = utils::CString(materialFilePath.getName().c_str()); + + // This is both the source and the result. + // This will be mutated by the below `resolveIncludesRecursively` call. + IncludeResult result { + .includeName = fileName, + .text = utils::CString(buffer.get(), size), + .name = CString("") + }; + + utils::Status resolveStatus = + matp::resolveIncludesRecursively(result, std::move(includer), options); + + return { resolveStatus, result.text }; +} + utils::Status MaterialParser::processTemplateSubstitutions( const Config& config, ssize_t& size, std::unique_ptr& buffer) { const auto& templateMap = config.getTemplateMap(); diff --git a/tools/matc/tests/Dir/Bar.h b/libs/filament-matp/tests/Dir/Bar.h similarity index 100% rename from tools/matc/tests/Dir/Bar.h rename to libs/filament-matp/tests/Dir/Bar.h diff --git a/tools/matc/tests/Dir/Baz.h b/libs/filament-matp/tests/Dir/Baz.h similarity index 100% rename from tools/matc/tests/Dir/Baz.h rename to libs/filament-matp/tests/Dir/Baz.h diff --git a/tools/matc/tests/Foo.h b/libs/filament-matp/tests/Foo.h similarity index 100% rename from tools/matc/tests/Foo.h rename to libs/filament-matp/tests/Foo.h diff --git a/tools/matc/tests/MockIncluder.h b/libs/filament-matp/tests/MockIncluder.h similarity index 95% rename from tools/matc/tests/MockIncluder.h rename to libs/filament-matp/tests/MockIncluder.h index 999cf23ce2..26f08cb5c3 100644 --- a/tools/matc/tests/MockIncluder.h +++ b/libs/filament-matp/tests/MockIncluder.h @@ -14,7 +14,7 @@ * limitations under the License. */ -#include +#include "IncludeCallback.h" #include @@ -37,7 +37,7 @@ public: return *this; } - bool operator()(const utils::CString& includedBy, matc::IncludeResult& result) { + bool operator()(const utils::CString& includedBy, matp::IncludeResult& result) { auto key = result.includeName.c_str(); auto found = mIncludeMap.find(key); diff --git a/tools/matc/tests/test_includer.cpp b/libs/filament-matp/tests/test_includer.cpp similarity index 89% rename from tools/matc/tests/test_includer.cpp rename to libs/filament-matp/tests/test_includer.cpp index a47c4bfb12..e7d0673742 100644 --- a/tools/matc/tests/test_includer.cpp +++ b/libs/filament-matp/tests/test_includer.cpp @@ -16,17 +16,17 @@ #include -#include +#include "DirIncluder.h" using namespace utils; -using namespace matc; +using namespace matp; const utils::Path root = utils::Path(__FILE__).getParent(); // TODO: these tests are disabled as they fail on CI, which needs investigation. TEST(DirIncluder, DISABLED_IncludeNonexistent) { - matc::DirIncluder includer; + DirIncluder includer; { IncludeResult i { .includeName = CString("nonexistent.h") @@ -45,7 +45,7 @@ TEST(DirIncluder, DISABLED_IncludeNonexistent) { } TEST(DirIncluder, DISABLED_IncludeFile) { - matc::DirIncluder includer; + DirIncluder includer; includer.setIncludeDirectory(root); IncludeResult result { @@ -63,7 +63,7 @@ TEST(DirIncluder, DISABLED_IncludeFile) { } TEST(DirIncluder, DISABLED_IncludeFileFromIncluder) { - matc::DirIncluder includer; + DirIncluder includer; includer.setIncludeDirectory(root); utils::CString includerFile((root + "Dir/Baz.h").c_str()); @@ -77,8 +77,3 @@ TEST(DirIncluder, DISABLED_IncludeFileFromIncluder) { EXPECT_STREQ((root + "Dir/Bar.h").c_str(), result.name.c_str()); } - -int main(int argc, char** argv) { - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} \ No newline at end of file diff --git a/tools/matc/tests/test_includes.cpp b/libs/filament-matp/tests/test_includes.cpp similarity index 89% rename from tools/matc/tests/test_includes.cpp rename to libs/filament-matp/tests/test_includes.cpp index 482fb77736..90371795b4 100644 --- a/tools/matc/tests/test_includes.cpp +++ b/libs/filament-matp/tests/test_includes.cpp @@ -16,9 +16,9 @@ #include +#include "Includes.h" #include "MockIncluder.h" -#include #include #include @@ -27,7 +27,7 @@ #include using namespace utils; -using namespace matc; +using namespace matp; // ------------------------------------------------------------------------------------------------- @@ -211,8 +211,8 @@ TEST(IncludeResolver, NoIncludes) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ("no includes", source.text.c_str()); } @@ -224,8 +224,8 @@ TEST(IncludeResolver, SingleInclude) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ("include", source.text.c_str()); } @@ -245,8 +245,8 @@ TEST(IncludeResolver, MultipleIncludes) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ(utils::CString(R"( 1 2 @@ -270,8 +270,8 @@ TEST(IncludeResolver, IncludeWithinInclude) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ(utils::CString(R"( 1 3 @@ -293,8 +293,8 @@ TEST(IncludeResolver, Includers) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ(utils::CString(R"( 3 )").c_str(), source.text.c_str()); @@ -313,8 +313,8 @@ TEST(IncludeResolver, IncludeFailure) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_FALSE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_FALSE(resolved.isOk()); } { utils::CString code(R"( @@ -326,8 +326,8 @@ TEST(IncludeResolver, IncludeFailure) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); - EXPECT_FALSE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); + EXPECT_FALSE(resolved.isOk()); } } @@ -344,9 +344,9 @@ TEST(IncludeResolver, Cycle) { IncludeResult source { .text = code }; - bool result = resolveIncludes(source, includer, {}); + utils::Status resolved = resolveIncludesRecursively(source, includer, {}); // Include cycles are disallowed. We should still terminate in finite time and report false. - EXPECT_FALSE(result); + EXPECT_FALSE(resolved.isOk()); } // Helper function that lets us write comparison cases with line breaks. @@ -395,8 +395,8 @@ TEST(IncludeResolver, SingleIncludeLineDirective) { .includeName = utils::CString("root.h"), .text = code }; - bool result = resolveIncludes(source, includer, options); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, options); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ_TRIMMARGIN(R"( | #line 1 "root.h" | #line 1 "test.h" @@ -421,8 +421,8 @@ TEST(IncludeResolver, MultipleIncludesLineDirective) { .includeName = utils::CString("root.h"), .text = code, }; - bool result = resolveIncludes(source, includer, options); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, options); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ_TRIMMARGIN(R"( | #line 1 "root.h" @@ -453,8 +453,8 @@ TEST(IncludeResolver, MultipleIncludesSameLineLineDirective) { .includeName = utils::CString("root.h"), .text = code }; - bool result = resolveIncludes(source, includer, options); - EXPECT_TRUE(result); + utils::Status resolved = resolveIncludesRecursively(source, includer, options); + EXPECT_TRUE(resolved.isOk()); EXPECT_STREQ_TRIMMARGIN(R"( | #line 1 "root.h" @@ -518,7 +518,8 @@ TEST_F(MaterialBuilder, Include) { .text = utils::CString(shaderCode.c_str(), shaderCode.size()), }; - EXPECT_TRUE(resolveIncludes(source, std::move(includer), options)); + utils::Status resolved = resolveIncludesRecursively(source, std::move(includer), options); + EXPECT_TRUE(resolved.isOk()); // Set the material with the resolved material. mBuilder.material(source.text.c_str()); @@ -545,8 +546,8 @@ TEST_F(MaterialBuilder, IncludeVertex) { IncludeResult source { .text = utils::CString(shaderCode.c_str(), shaderCode.size()), }; - - EXPECT_TRUE(resolveIncludes(source, std::move(includer), options)); + utils::Status resolved = resolveIncludesRecursively(source, std::move(includer), options); + EXPECT_TRUE(resolved.isOk()); // Set the materialVertex with the resolved material. mBuilder.materialVertex(source.text.c_str()); @@ -572,7 +573,8 @@ TEST_F(MaterialBuilder, IncludeWithinFunction) { .text = utils::CString(shaderCode.c_str(), shaderCode.size()), }; - EXPECT_TRUE(resolveIncludes(source, std::move(includer), options)); + utils::Status resolved = resolveIncludesRecursively(source, std::move(includer), options); + EXPECT_TRUE(resolved.isOk()); // Set the material with the resolved material. mBuilder.material(source.text.c_str()); @@ -592,7 +594,8 @@ TEST_F(MaterialBuilder, IncludeFailure) { .text = utils::CString(shaderCode.c_str(), shaderCode.size()), }; - EXPECT_FALSE(resolveIncludes(source, std::move(includer), options)); + utils::Status resolved = resolveIncludesRecursively(source, std::move(includer), options); + EXPECT_FALSE(resolved.isOk()); mBuilder.material(source.text.c_str()); diff --git a/libs/filament-matp/tests/test_matp.cpp b/libs/filament-matp/tests/test_matp.cpp index 9b26984e46..a8b5bb97d3 100644 --- a/libs/filament-matp/tests/test_matp.cpp +++ b/libs/filament-matp/tests/test_matp.cpp @@ -17,13 +17,13 @@ #include #include "TestMaterialParser.h" - -#include - -#include #include "JsonishLexer.h" #include "JsonishParser.h" #include "MaterialLexer.h" +#include "MockIncluder.h" + +#include +#include class MaterialLexer: public ::testing::Test { protected: diff --git a/tools/matc/CMakeLists.txt b/tools/matc/CMakeLists.txt index 837f560356..6711f10d66 100644 --- a/tools/matc/CMakeLists.txt +++ b/tools/matc/CMakeLists.txt @@ -12,17 +12,12 @@ set(TARGET matlang) set(HDRS src/matc/CommandlineConfig.h src/matc/Compiler.h - src/matc/DirIncluder.h - src/matc/Includes.h - src/matc/IncludeCallback.h src/matc/MaterialCompiler.h ) set(SRCS src/matc/Compiler.cpp src/matc/CommandlineConfig.cpp - src/matc/DirIncluder.cpp - src/matc/Includes.cpp src/matc/MaterialCompiler.cpp ) @@ -68,17 +63,3 @@ set_target_properties(${TARGET} PROPERTIES FOLDER Tools) # ================================================================================================== install(TARGETS ${TARGET} RUNTIME DESTINATION bin) -# ================================================================================================== -# Tests -# ================================================================================================== -project(test_matc) -set(TARGET test_matc) -set(SRCS tests/test_includer.cpp tests/test_includes.cpp) - -add_executable(${TARGET} ${SRCS}) - -target_link_libraries(${TARGET} matlang gtest) - -target_include_directories(${TARGET} PRIVATE ${filamat_SOURCE_DIR}/src) - -set_target_properties(test_matc PROPERTIES FOLDER Tests) diff --git a/tools/matc/src/matc/CommandlineConfig.cpp b/tools/matc/src/matc/CommandlineConfig.cpp index 887f5a4cb5..ff32c49df0 100644 --- a/tools/matc/src/matc/CommandlineConfig.cpp +++ b/tools/matc/src/matc/CommandlineConfig.cpp @@ -68,6 +68,8 @@ static const option OPTIONS[] = { { "no-sampler-validation", no_argument, nullptr, 'F' }, { "save-raw-variants", no_argument, nullptr, 'R' }, { "workarounds", required_argument, nullptr, 'W' }, + { "no-insert-line-directives", no_argument, nullptr, 'n' }, + { "no-insert-line-directive-check", no_argument, nullptr, 'N' }, { nullptr, 0, nullptr, 0 } // termination of the option list }; @@ -142,13 +144,26 @@ static void usage(char* name) { " Reflect the specified metadata as JSON: parameters\n\n" " --variant-filter=, -V \n" " Filter out specified comma-separated variants:\n" - " directionalLighting, dynamicLighting, shadowReceiver, skinning, vsm, fog," + " directionalLighting, dynamicLighting, shadowReceiver, skinning, vsm, fog,\n" " ssr (screen-space reflections), stereo\n" - " This variant filter is merged with the filter from the material, if any\n\n" + " This variant filter is merged with the filter from the material, if any.\n\n" " --workarounds, -W\n" " Workarounds to apply: all or none. (default is none).\n\n" " --version, -v\n" " Print the material version number\n\n" + " --output-format, -f\n" + " Specify output format: blob (default), header or mat.\n" + " When specifying the output as mat, it will return a mat\n" + " with #include directives resolved.\n\n" + " --no-insert-line-directives\n" + " By default insert #line directives before / after each #include.\n" + " Add this to omit line directives.\n\n" + " --no-insert-line-directive-check\n" + " By default it surrounds line directives with\n" + " #if defined(GL_GOOGLE_cpp_style_line_directive)\n" + " Some drivers may complain about the use of cpp style #line directives\n" + " if they don't support it.\n\n" + "Internal use and debugging only:\n" " --optimize-none, -g, -O0\n" " Disable all shader optimizations, for debugging\n\n" @@ -156,9 +171,6 @@ static void usage(char* name) { " Optimize shaders by running only the preprocessor\n\n" " --raw, -w\n" " Compile a raw GLSL shader into a SPIRV binary chunk\n\n" - " --output-format, -f\n" - " Specify output format: blob (default), header or mat. When specifying the " - " output as mat, it will return a mat with #include directives resolved.\n\n" " --debug, -d\n" " Generate extra data for debugging\n\n" " --no-sampler-validation, -F\n" @@ -388,6 +400,12 @@ bool CommandlineConfig::parse() { case 'R': mSaveRawVariants = true; break; + case 'n': + mInsertLineDirectives = false; + break; + case 'N': + mInsertLineDirectiveChecks = false; + break; } } diff --git a/tools/matc/src/matc/MaterialCompiler.cpp b/tools/matc/src/matc/MaterialCompiler.cpp index 6cb2445029..749f6f7698 100644 --- a/tools/matc/src/matc/MaterialCompiler.cpp +++ b/tools/matc/src/matc/MaterialCompiler.cpp @@ -16,9 +16,6 @@ #include "MaterialCompiler.h" -#include "DirIncluder.h" -#include "Includes.h" - #include #include #include @@ -27,7 +24,6 @@ #include -#include #include #include #include @@ -69,31 +65,22 @@ bool MaterialCompiler::run(const matp::Config& config) { glslang::FinalizeProcess(); return success; } - // Inline material (i.e. resolve #include directives) - DirIncluder includer; - includer.setIncludeDirectory(materialFilePath.getParent()); - utils::CString fileName = utils::CString(materialFilePath.getName().c_str()); + auto [resolvedStatus, resolvedString] = + mParser.resolveIncludes(buffer, size, materialFilePath, + config.getInsertLineDirectives(), + config.getInsertLineDirectiveChecks()); - ResolveOptions const options { - .insertLineDirectives = true, - .insertLineDirectiveCheck = true - }; - // This is both source and result, this will be mutated by `resolveIncludes` call. - IncludeResult result { - .includeName = fileName, - .text = utils::CString(buffer.get(), size), - .name = CString("") - }; - - if (!resolveIncludes(result, std::move(includer), options)) { + // It has failed to resolve the include directives. + if (!resolvedStatus.isOk()) { + std::cerr << resolvedStatus.getMessage() << std::endl; return false; } // Now that the buffer is mutated, we need to update the buffer pointer and the size // before parsing. - size = result.text.size(); + size = resolvedString.size(); auto modifiedBuffer = std::make_unique(size); - std::strncpy(modifiedBuffer.get(), result.text.c_str(), size); + std::strncpy(modifiedBuffer.get(), resolvedString.c_str(), size); buffer = std::move(modifiedBuffer); if (config.getOutputFormat() == matp::Config::OutputFormat::MAT) {