Bug fixes 1

Hazardu · Hazardu · commit 07862397ee56 · 2024-01-18T16:55:17.000+01:00
diff --git a/include/nbl/asset/utils/CHLSLCompiler.h b/include/nbl/asset/utils/CHLSLCompiler.h
@@ -7,10 +7,6 @@
 
 #include "nbl/asset/utils/ISPIRVOptimizer.h"
 #include "nbl/asset/utils/IShaderCompiler.h"
-#include <wrl.h>
-#include <combaseapi.h>
-#include <sstream>
-#include <dxc/dxcapi.h>
 
 
 
@@ -27,18 +23,7 @@ namespace nbl::asset
 class NBL_API2 CHLSLCompiler final : public IShaderCompiler
 {
 	public:
-
-		struct DxcCompilationResult
-		{
-			Microsoft::WRL::ComPtr<IDxcBlobEncoding> errorMessages;
-			Microsoft::WRL::ComPtr<IDxcBlob> objectBlob;
-			Microsoft::WRL::ComPtr<IDxcResult> compileResult;
-
-			std::string GetErrorMessagesString()
-			{
-				return std::string(reinterpret_cast<char*>(errorMessages->GetBufferPointer()), errorMessages->GetBufferSize());
-			}
-		};
+		
 
 		IShader::E_CONTENT_TYPE getCodeContentType() const override { return IShader::E_CONTENT_TYPE::ECT_HLSL; };
 
@@ -70,7 +55,11 @@ class NBL_API2 CHLSLCompiler final : public IShaderCompiler
 							
 		void insertIntoStart(std::string& code, std::ostringstream&& ins) const override;
 
-		DxcCompilationResult dxcCompile(std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options) const;
+		struct SdxcCompileResult {
+			uint8_t *begin;
+			size_t size;
+		};
+		SdxcCompileResult dxcCompile(std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options) const;
 	protected:
 		
 
diff --git a/include/nbl/system/ISystem.h b/include/nbl/system/ISystem.h
@@ -149,6 +149,8 @@ class NBL_API2 ISystem : public core::IReferenceCounted
                 m_cachedArchiveFiles.removeObject(dummy,pathAlias);
         }
 
+        void unmountBuiltins();
+
         //
         struct SystemInfo
         {
diff --git a/src/nbl/asset/utils/CHLSLCompiler.cpp b/src/nbl/asset/utils/CHLSLCompiler.cpp
@@ -13,6 +13,10 @@
 #include <regex>
 #include <iterator>
 #include <codecvt>
+#include <wrl.h>
+#include <combaseapi.h>
+#include <sstream>
+#include <dxc/dxcapi.h>
 
 
 using namespace nbl;
@@ -31,6 +35,19 @@ struct DXC
 };
 }
 
+struct DxcCompilationResult
+{
+    Microsoft::WRL::ComPtr<IDxcBlobEncoding> errorMessages;
+    Microsoft::WRL::ComPtr<IDxcBlob> objectBlob;
+    Microsoft::WRL::ComPtr<IDxcResult> compileResult;
+
+    std::string GetErrorMessagesString()
+    {
+        return std::string(reinterpret_cast<char*>(errorMessages->GetBufferPointer()), errorMessages->GetBufferSize());
+    }
+};
+
+
 CHLSLCompiler::CHLSLCompiler(core::smart_refctd_ptr<system::ISystem>&& system)
     : IShaderCompiler(std::move(system))
 {
@@ -53,7 +70,7 @@ CHLSLCompiler::~CHLSLCompiler()
     delete m_dxcCompilerTypes;
 }
 
-CHLSLCompiler::DxcCompilationResult CHLSLCompiler::dxcCompile(std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options) const
+CHLSLCompiler::SdxcCompileResult CHLSLCompiler::dxcCompile(std::string& source, LPCWSTR* args, uint32_t argCount, const CHLSLCompiler::SOptions& options) const
 {
     // Append Commandline options into source only if debugInfoFlags will emit source
     auto sourceEmittingFlags =
@@ -63,7 +80,7 @@ CHLSLCompiler::DxcCompilationResult CHLSLCompiler::dxcCompile(std::string& sourc
     if ((options.debugInfoFlags.value & sourceEmittingFlags) != CHLSLCompiler::E_DEBUG_INFO_FLAGS::EDIF_NONE)
     {
         std::ostringstream insertion;
-        insertion << "#pragma compile_flags ";
+        insertion << "#pragma wave dxc_compile_flags( ";
 
         std::wstring_convert<std::codecvt_utf8<wchar_t>, wchar_t> conv;
         for (uint32_t arg = 0; arg < argCount; arg ++)
@@ -72,7 +89,7 @@ CHLSLCompiler::DxcCompilationResult CHLSLCompiler::dxcCompile(std::string& sourc
             insertion << str.c_str() << " ";
         }
 
-        insertion << "\n";
+        insertion << ")\n";
         insertIntoStart(source, std::move(insertion));
     }
     nbl::asset::impl::DXC* dxc = m_dxcCompilerTypes;
@@ -114,7 +131,7 @@ CHLSLCompiler::DxcCompilationResult CHLSLCompiler::dxcCompile(std::string& sourc
     else
     {
         options.preprocessorOptions.logger.log("DXC Compilation Failed:\n%s", system::ILogger::ELL_ERROR, errorMessagesString.c_str());
-        return result;
+        return { (uint8_t*) result.objectBlob->GetBufferPointer(), result.objectBlob->GetBufferSize() };
     }
 
     ComPtr<IDxcBlob> resultingBlob;
@@ -123,7 +140,7 @@ CHLSLCompiler::DxcCompilationResult CHLSLCompiler::dxcCompile(std::string& sourc
 
     result.objectBlob = resultingBlob;
 
-    return result;
+    return { (uint8_t*)result.objectBlob->GetBufferPointer(), result.objectBlob->GetBufferSize() };
 }
 
 
@@ -135,7 +152,7 @@ std::string CHLSLCompiler::preprocessShader(std::string&& code, IShader::E_SHADE
     nbl::wave::context context(code.begin(),code.end(),preprocessOptions.sourceIdentifier.data(),{preprocessOptions});
     context.add_macro_definition("__HLSL_VERSION");
    
-     // instead of defining extraDefines as "NBL_GLSL_LIMIT_MAX_IMAGE_DIMENSION_1D 32768", 
+    // instead of defining extraDefines as "NBL_GLSL_LIMIT_MAX_IMAGE_DIMENSION_1D 32768", 
     // now define them as "NBL_GLSL_LIMIT_MAX_IMAGE_DIMENSION_1D=32768" 
     // to match boost wave syntax
     // https://www.boost.org/doc/libs/1_82_0/libs/wave/doc/class_reference_context.html#:~:text=Maintain%20defined%20macros-,add_macro_definition,-bool%20add_macro_definition
@@ -307,13 +324,13 @@ core::smart_refctd_ptr<ICPUShader> CHLSLCompiler::compileToSPIRV(const char* cod
     if (arg_storage)
         delete[] arg_storage;
 
-    if (!compileResult.objectBlob)
+    if (!compileResult.begin)
     {
         return nullptr;
     }
 
-    auto outSpirv = core::make_smart_refctd_ptr<ICPUBuffer>(compileResult.objectBlob->GetBufferSize());
-    memcpy(outSpirv->getPointer(), compileResult.objectBlob->GetBufferPointer(), compileResult.objectBlob->GetBufferSize());
+    auto outSpirv = core::make_smart_refctd_ptr<ICPUBuffer>(compileResult.size);
+    memcpy(outSpirv->getPointer(), compileResult.begin, compileResult.size);
     
     // Optimizer step
     if (hlslOptions.spirvOptimizer)
diff --git a/src/nbl/asset/utils/waveContext.h b/src/nbl/asset/utils/waveContext.h
@@ -105,17 +105,28 @@ struct preprocessing_hooks final : public boost::wave::context_policies::default
         
         if (strcmp(optionStr, "dxc_compile_flags") == 0 && hash_token_occurences == 1) {
             m_dxc_compile_flags_override.clear();
+            std::string arg = "";
             for (auto valueIter = values.begin(); valueIter != values.end(); valueIter++) {
                 std::string compiler_option_s = std::string(valueIter->get_value().c_str());
-                // if this compiler flag is encapsulated in quotation marks, strip them
-                //if (compiler_option_s[0] == '"' && compiler_option_s[compiler_option_s.length() - 1] == '"')
-                //    compiler_option_s = compiler_option_s.substr(1, compiler_option_s.length() - 2);
-
-                // if the compiler flag is a separator that is a comma or whitespace, do not add to list of flag overrides 
-                if (compiler_option_s == "," || compiler_option_s == " ")
-                    continue;
-                m_dxc_compile_flags_override.push_back(compiler_option_s);
+                //the compiler_option_s is a token thus can be only part of the actual argument, i.e. "-spirv" will be split into tokens [ "-", "spirv" ]
+                //for dxc_compile_flags just join the strings until it finds a whitespace or end of args
+
+                // if the compiler flag is a separator that is a whitespace, do not add to list of flag overrides 
+                if (compiler_option_s == " ") 
+                {
+                    //reset
+                    arg.clear();
+                    m_dxc_compile_flags_override.push_back(arg);
+                }
+                else 
+                {
+                    //append
+                    arg += compiler_option_s;
+                }
             }
+            if(arg.size() > 0)
+                m_dxc_compile_flags_override.push_back(arg);
+        
             return true;
         }
 
@@ -479,6 +490,9 @@ template<> inline bool boost::wave::impl::pp_iterator_functor<nbl::wave::context
         else
             result = includeFinder->getIncludeRelative(ctx.get_current_directory(),file_path);
     }
+    else {
+        ctx.get_hooks().m_logger.log("Include finder not assigned, preprocessor will not include file " + file_path, nbl::system::ILogger::ELL_ERROR);
+    }
 
     if (!result)
     {
diff --git a/src/nbl/system/ISystem.cpp b/src/nbl/system/ISystem.cpp
@@ -304,4 +304,8 @@ bool ISystem::ICaller::flushMapping(IFile* file, size_t offset, size_t size)
     else if (flags&IFile::ECF_COHERENT)
         return true;
     return flushMapping_impl(file,offset,size);
+}
+
+void  ISystem::unmountBuiltins() {
+
 }
diff --git a/tools/ndt/main.cpp b/tools/ndt/main.cpp
@@ -60,6 +60,8 @@ class ShaderCompiler final : public system::IApplicationFramework
 		{
 			if (argv[i] == "-no-nbl-builtins")
 			{
+				m_logger->log("Unmounting builtins.");
+				m_system->unmountBuiltins();
 				no_nbl_builtins = true;
 			}
 			else if (argv[i] == "-Fo")
@@ -78,9 +80,14 @@ class ShaderCompiler final : public system::IApplicationFramework
 		auto compilation_result = compile_shader(shader_code, file_to_compile);
 
 		// writie compiled shader to file as bytes
-		std::fstream output_file(output_filepath, std::ios::out | std::ios::binary);
-		output_file.write((const char*)compilation_result.objectBlob->GetBufferPointer(), compilation_result.objectBlob->GetBufferSize());
-		output_file.close();
+		if (compilation_result.begin) {
+			std::fstream output_file(output_filepath, std::ios::out | std::ios::binary);
+			output_file.write((const char*)compilation_result.begin, compilation_result.size);
+			output_file.close();
+		}
+		else {
+			m_logger->log("Shader compilation failed.", ILogger::ELL_ERROR);
+		}
 		/*std::string command = "dxc.exe";
 		for (std::string arg : arguments)
 		{
@@ -103,7 +110,7 @@ class ShaderCompiler final : public system::IApplicationFramework
 
 private:
 
-	CHLSLCompiler::DxcCompilationResult compile_shader(std::string& shader_code, std::string_view sourceIdentifier) {
+	CHLSLCompiler::SdxcCompileResult compile_shader(std::string& shader_code, std::string_view sourceIdentifier) {
 		constexpr uint32_t WorkgroupSize = 256;
 		constexpr uint32_t WorkgroupCount = 2048;
 		const string WorkgroupSizeAsStr = std::to_string(WorkgroupSize);
@@ -123,6 +130,9 @@ class ShaderCompiler final : public system::IApplicationFramework
 		options.preprocessorOptions.logger = m_logger.get();
 		options.preprocessorOptions.extraDefines = { &WorkgroupSizeDefine,&WorkgroupSizeDefine + 1 };
 
+		auto includeFinder = make_smart_refctd_ptr<IShaderCompiler::CIncludeFinder>(m_system);
+		options.preprocessorOptions.includeFinder = includeFinder.get();
+
 		std::vector<std::string> dxc_compile_flags_from_pragma = {};
 		auto preprocessed_shader_code = hlslcompiler->preprocessShader(std::move(shader_code), options.stage, dxc_compile_flags_from_pragma, options.preprocessorOptions);
 

Original file line number	Diff line number	Diff line change
`@@ -149,6 +149,8 @@ class NBL_API2 ISystem : public core::IReferenceCounted`
`149`	`149`	`m_cachedArchiveFiles.removeObject(dummy,pathAlias);`
`150`	`150`	`}`
`151`	`151`
	`152`	`+ void unmountBuiltins();`
	`153`	`+`
`152`	`154`	`//`
`153`	`155`	`struct SystemInfo`
`154`	`156`	`{`
Original file line number	Diff line number	Diff line change
`@@ -304,4 +304,8 @@ bool ISystem::ICaller::flushMapping(IFile* file, size_t offset, size_t size)`
`304`	`304`	`else if (flags&IFile::ECF_COHERENT)`
`305`	`305`	`return true;`
`306`	`306`	`return flushMapping_impl(file,offset,size);`
	`307`	`+}`
	`308`	`+`
	`309`	`+void ISystem::unmountBuiltins() {`
	`310`	`+`
`307`	`311`	`}`