change the API of Extra Defines, nuke the GLSL JIT macros

Author: devshgraphicsprogramming

include/nbl/asset/utils/IShaderCompiler.h

@@ -115,8 +115,12 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
 			std::string_view sourceIdentifier = "";
 			system::logger_opt_ptr logger = nullptr;
 			const CIncludeFinder* includeFinder = nullptr;
-			uint32_t maxSelfInclusionCount = 4u;
-			core::SRange<const char* const> extraDefines = {nullptr, nullptr};
+			struct SMacroDefinition
+			{
+				std::string_view identifier;
+				std::string_view definition;
+			};
+			std::span<const SMacroDefinition> extraDefines = {};
 		};
 
 		// https://github.com/microsoft/DirectXShaderCompiler/blob/main/docs/SPIR-V.rst#debugging
@@ -290,8 +294,6 @@ class NBL_API2 IShaderCompiler : public core::IReferenceCounted
 
 		virtual void insertIntoStart(std::string& code, std::ostringstream&& ins) const = 0;
 
-		void insertExtraDefines(std::string& code, const core::SRange<const char* const>& defines) const;
-
 		core::smart_refctd_ptr<system::ISystem> m_system;
 	private:
 		core::smart_refctd_ptr<CIncludeFinder> m_defaultIncludeFinder;

include/nbl/video/ILogicalDevice.h

@@ -477,13 +477,6 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
         // OpenGL: const egl::CEGL::Context*
         // Vulkan: const VkDevice*
         virtual const void* getNativeHandle() const = 0;
-        
-        // these are the defines which shall be added to any IGPUShader which has its source as GLSL
-        inline core::SRange<const char* const> getExtraShaderDefines() const
-        {
-            const char* const* begin = m_extraShaderDefines.data();
-            return {begin,begin+m_extraShaderDefines.size()};
-        }
 
     protected:
         ILogicalDevice(core::smart_refctd_ptr<IAPIConnection>&& api, IPhysicalDevice* physicalDevice, const SCreationParams& params)
@@ -563,37 +556,6 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
         ) = 0;
         virtual core::smart_refctd_ptr<IGPUGraphicsPipeline> createGraphicsPipeline_impl(IGPUPipelineCache* pipelineCache, IGPUGraphicsPipeline::SCreationParams&& params) = 0;
         virtual bool createGraphicsPipelines_impl(IGPUPipelineCache* pipelineCache, core::SRange<const IGPUGraphicsPipeline::SCreationParams> params, core::smart_refctd_ptr<IGPUGraphicsPipeline>* output) = 0;
-        
-        void addCommonShaderDefines(std::ostringstream& pool, const bool runningInRenderDoc);
-
-        template<typename... Args>
-        inline void addShaderDefineToPool(std::ostringstream& pool, const char* define, Args&&... args)
-        {
-            const ptrdiff_t pos = pool.tellp();
-            m_extraShaderDefines.push_back(reinterpret_cast<const char*>(pos));
-            pool << define << " ";
-            ((pool << (std::is_same<uint8_t, Args>::value ? static_cast<uint32_t>(std::forward<Args>(args)) : std::forward<Args>(args))), ...);
-        }
-        inline void finalizeShaderDefinePool(std::ostringstream&& pool)
-        {
-            m_ShaderDefineStringPool.resize(static_cast<size_t>(pool.tellp())+m_extraShaderDefines.size());
-            const auto data = ptrdiff_t(m_ShaderDefineStringPool.data());
-
-            const auto str = pool.str();
-            size_t nullCharsWritten = 0u;
-            for (auto i=0u; i<m_extraShaderDefines.size(); i++)
-            {
-                auto& dst = m_extraShaderDefines[i];
-                const auto len = (i!=(m_extraShaderDefines.size()-1u) ? ptrdiff_t(m_extraShaderDefines[i+1]):str.length())-ptrdiff_t(dst);
-                const char* src = str.data()+ptrdiff_t(dst);
-                dst += data+(nullCharsWritten++);
-                memcpy(const_cast<char*>(dst),src,len);
-                const_cast<char*>(dst)[len] = 0;
-            }
-        }
-
-        core::vector<char> m_ShaderDefineStringPool;
-        core::vector<const char*> m_extraShaderDefines;
 
         core::smart_refctd_ptr<asset::CCompilerSet> m_compilerSet;
         core::smart_refctd_ptr<IAPIConnection> m_api;

src/nbl/asset/utils/CGLSLCompiler.cpp

@@ -133,9 +133,12 @@ CGLSLCompiler::CGLSLCompiler(core::smart_refctd_ptr<system::ISystem>&& system)
 
 std::string CGLSLCompiler::preprocessShader(std::string&& code, IShader::E_SHADER_STAGE& stage, const SPreprocessorOptions& preprocessOptions) const
 {
-    if (preprocessOptions.extraDefines.size())
+    if (!preprocessOptions.extraDefines.empty())
     {
-        insertExtraDefines(code, preprocessOptions.extraDefines);
+        std::ostringstream insertion;
+        for (const auto& define : preprocessOptions.extraDefines)
+            insertion << "#define " << define.identifier << " " << define.definition << "\n";
+        insertIntoStart(code,std::move(insertion));
     }
     IShaderCompiler::disableAllDirectivesExceptIncludes(code);
     disableGlDirectives(code);
@@ -145,7 +148,7 @@ std::string CGLSLCompiler::preprocessShader(std::string&& code, IShader::E_SHADE
 
     if (preprocessOptions.includeFinder != nullptr)
     {
-        options.SetIncluder(std::make_unique<impl::Includer>(preprocessOptions.includeFinder, m_system.get(), preprocessOptions.maxSelfInclusionCount + 1u));//custom #include handler
+        options.SetIncluder(std::make_unique<impl::Includer>(preprocessOptions.includeFinder, m_system.get(), /*maxSelfInclusionCount*/5));//custom #include handler
     }
     const shaderc_shader_kind scstage = stage == IShader::ESS_UNKNOWN ? shaderc_glsl_infer_from_source : ESStoShadercEnum(stage);
     auto res = comp.PreprocessGlsl(code, scstage, preprocessOptions.sourceIdentifier.data(), options);

src/nbl/asset/utils/CHLSLCompiler.cpp

@@ -292,16 +292,8 @@ std::string CHLSLCompiler::preprocessShader(std::string&& code, IShader::E_SHADE
     // 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
-    for (auto iter = preprocessOptions.extraDefines.begin(); iter!=preprocessOptions.extraDefines.end(); iter++)
-    {
-        std::string s = *iter;
-        size_t firstParenthesis = s.find(')');
-        if (firstParenthesis == -1) firstParenthesis = 0;
-        size_t firstWhitespace = s.find(' ', firstParenthesis);
-        if (firstWhitespace != -1)
-            s[firstWhitespace] = '=';
-        context.add_macro_definition(s);
-    }
+    for (const auto& define : preprocessOptions.extraDefines)
+        context.add_macro_definition(define.identifier.data()+core::string("=")+define.definition.data());
 
     // preprocess
     std::stringstream stream = std::stringstream();

src/nbl/asset/utils/IShaderCompiler.cpp

@@ -289,16 +289,3 @@ std::optional<std::string> IShaderCompiler::CIncludeFinder::tryIncludeGenerators
 
     return std::nullopt;
 }
-
-void IShaderCompiler::insertExtraDefines(std::string& code, const core::SRange<const char* const>& defines) const
-{
-    if (defines.empty())
-        return;
-
-    std::ostringstream insertion;
-    for (auto i = 0u; i < defines.size(); ++i)
-    {
-        insertion << "#define " << defines[i] << "\n";
-    }
-    insertIntoStart(code, std::move(insertion));
-}

src/nbl/video/CVulkanLogicalDevice.h

@@ -68,11 +68,6 @@ class CVulkanLogicalDevice final : public ILogicalDevice
                 (*m_queues)[ix] = new CThreadSafeGPUQueueAdapter(this, new CVulkanQueue(this, rdoc, vkinst, q, famIx, flags, priority));
             }
         }
-        
-        std::ostringstream pool;
-        bool runningInRenderdoc = (rdoc != nullptr);
-        addCommonShaderDefines(pool,runningInRenderdoc);
-        finalizeShaderDefinePool(std::move(pool));
 
         m_dummyDSLayout = createDescriptorSetLayout(nullptr, nullptr);
     }
@@ -703,7 +698,7 @@ class CVulkanLogicalDevice final : public ILogicalDevice
             commonCompileOptions.preprocessorOptions.logger = (m_physicalDevice->getDebugCallback()) ? m_physicalDevice->getDebugCallback()->getLogger() : nullptr;
             commonCompileOptions.preprocessorOptions.includeFinder = compiler->getDefaultIncludeFinder(); // to resolve includes before compilation
             commonCompileOptions.preprocessorOptions.sourceIdentifier = cpushader->getFilepathHint().c_str();
-            commonCompileOptions.preprocessorOptions.extraDefines = getExtraShaderDefines();
+            commonCompileOptions.preprocessorOptions.extraDefines = {};
 
             commonCompileOptions.stage = shaderStage;
             commonCompileOptions.debugInfoFlags =