Follow up of material parser:

- Fix CMakeLists to correctly include src files in matp test
- Replace std::string to std::string_view
FIXES=440579712

Author: trie94

libs/filament-matp/CMakeLists.txt

@@ -13,7 +13,6 @@ set(PUBLIC_HDRS
 )
 
 set(SRCS
-        src/DirIncluder.h
         src/JsonishLexeme.h
         src/JsonishLexer.h
         src/JsonishParser.h
@@ -22,7 +21,6 @@ set(SRCS
         src/MaterialLexeme.h
         src/MaterialLexer.h
         src/ParametersProcessor.h
-        src/DirIncluder.cpp
         src/JsonishLexer.cpp
         src/JsonishParser.cpp
         src/MaterialLexer.cpp
@@ -56,7 +54,6 @@ project(test_matp)
 set(TARGET test_matp)
 set(SRCS
     tests/test_matp.cpp
-    tests/test_includer.cpp
     tests/TestMaterialParser.h
     tests/test_compute_material.cpp
 )

libs/filament-matp/include/filament-matp/MaterialParser.h

@@ -39,10 +39,10 @@ class MaterialParser {
     // 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.
+    // When the input shader has #includes, it has to be resolved before calling into parse.
     bool parse(
             filamat::MaterialBuilder& builder,
-            const Config& config,
-            Config::Input* input, ssize_t& size, std::unique_ptr<const char[]>& buffer);
+            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(
             const Config& config, ssize_t& size, std::unique_ptr<const char[]>& buffer);

libs/filament-matp/src/MaterialParser.cpp

@@ -27,7 +27,6 @@
 #include <utils/Log.h>
 #include <utils/JobSystem.h>
 
-#include "DirIncluder.h"
 #include "JsonishLexer.h"
 #include "JsonishParser.h"
 #include "MaterialLexeme.h"
@@ -458,11 +457,7 @@ bool MaterialParser::processTemplateSubstitutions(
 }
 
 bool MaterialParser::parse(filamat::MaterialBuilder& builder,
-        const Config& config,
-        Config::Input* input, ssize_t& size, std::unique_ptr<const char[]>& buffer) {
-
-    utils::Path const materialFilePath = utils::Path(input->getName()).getAbsolutePath();
-    assert(materialFilePath.isFile());
+        const Config& config, ssize_t& size, std::unique_ptr<const char[]>& buffer) {
 
     if (builder.getFeatureLevel() > config.getFeatureLevel()) {
         std::cerr << "Material feature level (" << +builder.getFeatureLevel()
@@ -489,15 +484,9 @@ bool MaterialParser::parse(filamat::MaterialBuilder& builder,
             return reflectParameters(builder);
     }
 
-    // Set the root include directory to the directory containing the material file.
-    DirIncluder includer;
-    includer.setIncludeDirectory(materialFilePath.getParent());
-
     builder
             .noSamplerValidation(config.noSamplerValidation())
             .includeEssl1(config.includeEssl1())
-            .includeCallback(includer)
-            .fileName(materialFilePath.getName().c_str())
             .platform(config.getPlatform())
             .targetApi(config.getTargetApi())
             .optimization(config.getOptimizationLevel())

tools/matc/CMakeLists.txt

@@ -12,13 +12,15 @@ set(TARGET matlang)
 set(HDRS
         src/matc/CommandlineConfig.h
         src/matc/Compiler.h
+        src/matc/DirIncluder.h
         src/matc/MaterialCompiler.h
         )
 
 set(SRCS
         src/matc/Compiler.cpp
         src/matc/MaterialCompiler.cpp
         src/matc/CommandlineConfig.cpp
+        src/matc/DirIncluder.cpp
         )
 
 # ==================================================================================================
@@ -62,3 +64,16 @@ set_target_properties(${TARGET} PROPERTIES FOLDER Tools)
 # Installation
 # ==================================================================================================
 install(TARGETS ${TARGET} RUNTIME DESTINATION bin)
+
+# ==================================================================================================
+# Tests
+# ==================================================================================================
+project(test_matc)
+set(TARGET test_matc)
+set(SRCS tests/test_includer.cpp)
+
+add_executable(${TARGET} ${SRCS})
+
+target_link_libraries(${TARGET} matlang gtest)
+
+set_target_properties(test_matc PROPERTIES FOLDER Tests)

libs/filament-matp/src/DirIncluder.cpp renamed to tools/matc/src/matc/DirIncluder.cpp

@@ -20,7 +20,7 @@
 
 #include <fstream>
 
-namespace matp {
+namespace matc {
 
 bool DirIncluder::operator()(const utils::CString& includedBy, filamat::IncludeResult& result) {
     auto getHeaderPath = [&result, &includedBy, this]() {
@@ -66,5 +66,5 @@ bool DirIncluder::operator()(const utils::CString& includedBy, filamat::IncludeR
     return true;
 }
 
-} // namespace matp
+} // namespace matc
 

libs/filament-matp/src/DirIncluder.h renamed to tools/matc/src/matc/DirIncluder.h

@@ -21,7 +21,7 @@
 
 #include <utils/Path.h>
 
-namespace matp {
+namespace matc {
 
 // Functor callback handler used to resolve includes relative to a root include directory.
 class DirIncluder {
@@ -38,6 +38,6 @@ class DirIncluder {
 
 };
 
-} // namespace matp
+} // namespace matc
 
 #endif

tools/matc/src/matc/MaterialCompiler.cpp

@@ -16,6 +16,8 @@
 
 #include "MaterialCompiler.h"
 
+#include "DirIncluder.h"
+
 #include <memory>
 #include <iostream>
 #include <utility>
@@ -52,21 +54,33 @@ bool MaterialCompiler::run(const matp::Config& config) {
 
     mParser.processTemplateSubstitutions(config, size, buffer);
 
+    utils::Path const materialFilePath = utils::Path(input->getName()).getAbsolutePath();
+    assert(materialFilePath.isFile());
+
     if (config.rawShaderMode()) {
-        utils::Path const materialFilePath = utils::Path(config.getInput()->getName()).getAbsolutePath();
-        assert(materialFilePath.isFile());
         const std::string extension = materialFilePath.getExtension();
         glslang::InitializeProcess();
-        bool const success = compileRawShader(buffer.get(), size, config.isDebug(), config.getOutput(),
-                extension.c_str());
+        bool const success =
+                compileRawShader(
+                        buffer.get(), size,
+                        config.isDebug(),
+                        config.getOutput(),
+                        extension.c_str());
         glslang::FinalizeProcess();
         return success;
     }
 
     MaterialBuilder::init();
     MaterialBuilder builder;
 
-    if (!mParser.parse(builder, config, input, size, buffer)) {
+    // Set the root include directory to the directory containing the material file.
+    DirIncluder includer;
+    includer.setIncludeDirectory(materialFilePath.getParent());
+
+    builder.includeCallback(includer)
+            .fileName(materialFilePath.getName().c_str());
+
+    if (!mParser.parse(builder, config, size, buffer)) {
         return false;
     }