From 38231967651683d64a5584d93b79b21f5877695b Mon Sep 17 00:00:00 2001 From: Magnus Norddahl Date: Sun, 13 Apr 2025 17:15:13 +0200 Subject: [PATCH] Revert "Remove resource system voodoo and make some lines slightly more readable" This reverts commit 9bc869e1a6a9a7162b38e51b5fefed4042041a5f. Revert "Fix race condition" This reverts commit 883e3ce9073f86213c53cd516d55fa1659e14def. Revert "Run the GLSL compiler on the worker thread" This reverts commit da0ff9ab04377a6367ae3ca4ee45699c5bb5863b. --- src/common/filesystem/source/resourcefile.cpp | 14 ++--- .../vulkan/pipelines/vk_renderpass.cpp | 33 +++++------ .../vulkan/pipelines/vk_renderpass.h | 2 - .../rendering/vulkan/shaders/vk_shader.cpp | 32 +++++------ .../rendering/vulkan/shaders/vk_shader.h | 11 +--- .../vulkan/shaders/vk_shadercache.cpp | 55 +++++++------------ .../rendering/vulkan/shaders/vk_shadercache.h | 11 ++-- src/common/rendering/vulkan/vk_levelmesh.cpp | 4 +- .../rendering/vulkan/vk_lightmapper.cpp | 4 +- 9 files changed, 62 insertions(+), 104 deletions(-) diff --git a/src/common/filesystem/source/resourcefile.cpp b/src/common/filesystem/source/resourcefile.cpp index 09414907d..3fd4676b0 100644 --- a/src/common/filesystem/source/resourcefile.cpp +++ b/src/common/filesystem/source/resourcefile.cpp @@ -742,17 +742,11 @@ FileReader FResourceFile::GetEntryReader(uint32_t entry, int readertype, int rea else { FileReader fri; - if (readertype == READER_NEW || !mainThread) - fri.OpenFile(FileName, Entries[entry].Position, Entries[entry].CompressedSize); - else - fri.OpenFilePart(Reader, Entries[entry].Position, Entries[entry].CompressedSize); - + if (readertype == READER_NEW || !mainThread) fri.OpenFile(FileName, Entries[entry].Position, Entries[entry].CompressedSize); + else fri.OpenFilePart(Reader, Entries[entry].Position, Entries[entry].CompressedSize); int flags = DCF_TRANSFEROWNER | DCF_EXCEPTIONS; - if (readertype == READER_CACHED) - flags |= DCF_CACHED; - else if (readerflags & READERFLAG_SEEKABLE) - flags |= DCF_SEEKABLE; - + if (readertype == READER_CACHED) flags |= DCF_CACHED; + else if (readerflags & READERFLAG_SEEKABLE) flags |= DCF_SEEKABLE; OpenDecompressor(fr, fri, Entries[entry].Length, Entries[entry].Method, flags); } } diff --git a/src/common/rendering/vulkan/pipelines/vk_renderpass.cpp b/src/common/rendering/vulkan/pipelines/vk_renderpass.cpp index fe1bddbcd..026b2dc78 100644 --- a/src/common/rendering/vulkan/pipelines/vk_renderpass.cpp +++ b/src/common/rendering/vulkan/pipelines/vk_renderpass.cpp @@ -276,7 +276,6 @@ VkVertexFormat *VkRenderPassManager::GetVertexFormat(int index) VulkanPipelineLayout* VkRenderPassManager::GetPipelineLayout(bool levelmesh, int UserUniformSize) { - std::unique_lock lock(PipelineLayoutsMutex); auto &layout = PipelineLayouts[levelmesh][UserUniformSize]; if (layout) return layout.get(); @@ -525,38 +524,34 @@ PipelineData* VkRenderPassSetup::GetSpecializedPipeline(const VkPipelineKey& key { // We haven't seen this before. Build it on the worker thread. - SpecializedPipelines[key] = {}; // Mark as seen - struct TaskData { - VkPipelineKey key; + std::unique_ptr builder; std::unique_ptr pipeline; - UniformStructHolder uniforms; }; auto data = std::make_shared(); - data->key = key; + + // Compile the GLSL on the main thread (mainly due to the resource system) + UniformStructHolder uniforms; + data->builder = CreatePipeline(key, false, uniforms); + SpecializedPipelines[key].Uniforms = uniforms; // Schedule the pipeline building on a worker thread VkPipelineKey k = key; auto passManager = fb->GetRenderPassManager(); passManager->RunOnWorkerThread([=]() { - auto builder = CreatePipeline(data->key, false, data->uniforms); - cycle_t ct; ct.ResetAndClock(); - data->pipeline = builder->Create(fb->GetDevice()); + data->pipeline = data->builder->Create(fb->GetDevice()); ct.Unclock(); const auto duration = ct.TimeMS(); passManager->RunOnMainThread([=]() { auto& slot = SpecializedPipelines[k]; if (!slot.pipeline) - { slot.pipeline = std::move(data->pipeline); - slot.Uniforms = std::move(data->uniforms); - } pipeline_time += duration; ++pipeline_count; @@ -615,7 +610,7 @@ std::unique_ptr VkRenderPassSetup::CreateWithStats(GraphicsPipel std::unique_ptr VkRenderPassSetup::CreatePipeline(const VkPipelineKey& key, bool isUberShader, UniformStructHolder& Uniforms) { - VkShaderProgram* program = fb->GetShaderManager()->GetGameShader(key.ShaderKey, isUberShader); + VkShaderProgram* program = fb->GetShaderManager()->Get(key.ShaderKey, isUberShader); Uniforms.Clear(); Uniforms = program->Uniforms; @@ -728,19 +723,19 @@ void VkRenderPassSetup::PrecompileFragmentShaderLibrary(const VkPipelineKey& key struct TaskData { - VkPipelineKey key; + std::unique_ptr builder; std::unique_ptr pipeline; }; auto data = std::make_shared(); - data->key = key; + data->builder = CreateFragmentShaderLibrary(key, isUberShader); auto passManager = fb->GetRenderPassManager(); passManager->RunOnWorkerThread([=]() { cycle_t ct; ct.ResetAndClock(); - data->pipeline = CreateFragmentShaderLibrary(data->key, isUberShader)->Create(fb->GetDevice()); + data->pipeline = data->builder->Create(fb->GetDevice()); ct.Unclock(); const auto duration = ct.TimeMS(); @@ -772,7 +767,7 @@ VulkanPipeline* VkRenderPassSetup::GetFragmentShaderLibrary(const VkPipelineKey& std::unique_ptr VkRenderPassSetup::LinkPipeline(const VkPipelineKey& key, bool isUberShader, UniformStructHolder& Uniforms) { - VkShaderProgram* program = fb->GetShaderManager()->GetGameShader(key.ShaderKey, isUberShader); + VkShaderProgram* program = fb->GetShaderManager()->Get(key.ShaderKey, isUberShader); Uniforms.Clear(); Uniforms = program->Uniforms; @@ -802,7 +797,7 @@ std::unique_ptr VkRenderPassSetup::CreateVertexInputLibrary(int std::unique_ptr VkRenderPassSetup::CreateVertexShaderLibrary(const VkPipelineKey& key, bool isUberShader) { - VkShaderProgram* program = fb->GetShaderManager()->GetGameShader(key.ShaderKey, isUberShader); + VkShaderProgram* program = fb->GetShaderManager()->Get(key.ShaderKey, isUberShader); GraphicsPipelineBuilder builder; builder.Cache(fb->GetRenderPassManager()->GetCache()); builder.Flags(VK_PIPELINE_CREATE_LIBRARY_BIT_KHR); @@ -817,7 +812,7 @@ std::unique_ptr VkRenderPassSetup::CreateVertexShaderLibrary(con std::unique_ptr VkRenderPassSetup::CreateFragmentShaderLibrary(const VkPipelineKey& key, bool isUberShader) { - VkShaderProgram* program = fb->GetShaderManager()->GetGameShader(key.ShaderKey, isUberShader); + VkShaderProgram* program = fb->GetShaderManager()->Get(key.ShaderKey, isUberShader); auto builder = std::make_unique(); builder->Cache(fb->GetRenderPassManager()->GetCache()); builder->Flags(VK_PIPELINE_CREATE_LIBRARY_BIT_KHR); diff --git a/src/common/rendering/vulkan/pipelines/vk_renderpass.h b/src/common/rendering/vulkan/pipelines/vk_renderpass.h index c454a89ed..1a692fc78 100644 --- a/src/common/rendering/vulkan/pipelines/vk_renderpass.h +++ b/src/common/rendering/vulkan/pipelines/vk_renderpass.h @@ -11,7 +11,6 @@ #include #include #include -#include class VulkanRenderDevice; class ColorBlendAttachmentBuilder; @@ -185,7 +184,6 @@ private: VulkanRenderDevice* fb = nullptr; std::map> RenderPassSetup; - std::mutex PipelineLayoutsMutex; std::map> PipelineLayouts[2]; std::vector VertexFormats; diff --git a/src/common/rendering/vulkan/shaders/vk_shader.cpp b/src/common/rendering/vulkan/shaders/vk_shader.cpp index cf8ccffe8..9e833bb21 100644 --- a/src/common/rendering/vulkan/shaders/vk_shader.cpp +++ b/src/common/rendering/vulkan/shaders/vk_shader.cpp @@ -91,15 +91,11 @@ void VkShaderManager::Deinit() RemoveVkPPShader(PPShaders.back()); } -VkShaderProgram* VkShaderManager::GetGameShader(const VkShaderKey& key, bool isUberShader) +VkShaderProgram* VkShaderManager::Get(const VkShaderKey& key, bool isUberShader) { - std::unique_lock lock(GameShaders.Mutex); - - std::unique_ptr& program = isUberShader ? GameShaders.Generic[key.GeneralizedShaderKey()] : GameShaders.Specialized[key]; - if (program) - return program.get(); - - program = std::make_unique(); + VkShaderProgram& program = isUberShader ? generic[key.GeneralizedShaderKey()] : specialized[key]; + if (program.frag) + return &program; const char* mainvp = "shaders/scene/vert_main.glsl"; const char* mainfp = "shaders/scene/frag_main.glsl"; @@ -131,9 +127,9 @@ VkShaderProgram* VkShaderManager::GetGameShader(const VkShaderKey& key, bool isU customKey.Layout.AlphaTest = false; const auto& desc = effectshaders[key.SpecialEffect]; - program->vert = LoadVertShader(desc.ShaderName, mainvp, nullptr, desc.defines, key, nullptr, isUberShader); + program.vert = LoadVertShader(desc.ShaderName, mainvp, nullptr, desc.defines, key, nullptr, isUberShader); if (!key.NoFragmentShader) - program->frag = LoadFragShader(desc.ShaderName, desc.fp1, desc.fp2, desc.fp3, desc.fp4, desc.fp5, desc.defines, key, nullptr, isUberShader); + program.frag = LoadFragShader(desc.ShaderName, desc.fp1, desc.fp2, desc.fp3, desc.fp4, desc.fp5, desc.defines, key, nullptr, isUberShader); } else { @@ -171,9 +167,9 @@ VkShaderProgram* VkShaderManager::GetGameShader(const VkShaderKey& key, bool isU if (key.EffectState < FIRST_USER_SHADER) { const auto& desc = defaultshaders[key.EffectState]; - program->vert = LoadVertShader(desc.ShaderName, mainvp, nullptr, desc.Defines, key, nullptr, isUberShader); + program.vert = LoadVertShader(desc.ShaderName, mainvp, nullptr, desc.Defines, key, nullptr, isUberShader); if (!key.NoFragmentShader) - program->frag = LoadFragShader(desc.ShaderName, mainfp, desc.material_lump, desc.mateffect_lump, desc.lightmodel_lump_shared, desc.lightmodel_lump, desc.Defines, key, nullptr, isUberShader); + program.frag = LoadFragShader(desc.ShaderName, mainfp, desc.material_lump, desc.mateffect_lump, desc.lightmodel_lump_shared, desc.lightmodel_lump, desc.Defines, key, nullptr, isUberShader); } else { @@ -181,14 +177,14 @@ VkShaderProgram* VkShaderManager::GetGameShader(const VkShaderKey& key, bool isU const FString& name = ExtractFileBase(desc.shader.GetChars()); FString defines = defaultshaders[desc.shaderType].Defines + desc.defines; - program->vert = LoadVertShader(name, mainvp, desc.vertshader.IsEmpty() ? nullptr : desc.vertshader.GetChars(), defines.GetChars(), key, &desc, isUberShader); + program.vert = LoadVertShader(name, mainvp, desc.vertshader.IsEmpty() ? nullptr : desc.vertshader.GetChars(), defines.GetChars(), key, &desc, isUberShader); if (!key.NoFragmentShader) - program->frag = LoadFragShader(name, mainfp, desc.shader.GetChars(), defaultshaders[desc.shaderType].mateffect_lump, defaultshaders[desc.shaderType].lightmodel_lump_shared, defaultshaders[desc.shaderType].lightmodel_lump, defines.GetChars(), key, &desc, isUberShader); + program.frag = LoadFragShader(name, mainfp, desc.shader.GetChars(), defaultshaders[desc.shaderType].mateffect_lump, defaultshaders[desc.shaderType].lightmodel_lump_shared, defaultshaders[desc.shaderType].lightmodel_lump, defines.GetChars(), key, &desc, isUberShader); - desc.Uniforms.WriteUniforms(program->Uniforms); + desc.Uniforms.WriteUniforms(program.Uniforms); } } - return program.get(); + return &program; } enum class FieldCondition @@ -634,12 +630,12 @@ FString VkShaderManager::GetVersionBlock() FString VkShaderManager::LoadPublicShaderLump(const char* lumpname) { - return fb->GetShaderCache()->GetPublicFile(lumpname)->Code; + return fb->GetShaderCache()->GetPublicFile(lumpname).Code; } FString VkShaderManager::LoadPrivateShaderLump(const char* lumpname) { - return fb->GetShaderCache()->GetPrivateFile(lumpname)->Code; + return fb->GetShaderCache()->GetPrivateFile(lumpname).Code; } VkPPShader* VkShaderManager::GetVkShader(PPShader* shader) diff --git a/src/common/rendering/vulkan/shaders/vk_shader.h b/src/common/rendering/vulkan/shaders/vk_shader.h index 526d6aaf1..b50f2af4a 100644 --- a/src/common/rendering/vulkan/shaders/vk_shader.h +++ b/src/common/rendering/vulkan/shaders/vk_shader.h @@ -4,7 +4,6 @@ #include #include #include -#include #include "vectors.h" #include "matrix.h" #include "name.h" @@ -169,7 +168,7 @@ public: void Deinit(); - VkShaderProgram* GetGameShader(const VkShaderKey& key, bool isUberShader); + VkShaderProgram* Get(const VkShaderKey& key, bool isUberShader); bool CompileNextShader() { return true; } @@ -195,12 +194,8 @@ private: VulkanRenderDevice* fb = nullptr; - struct - { - std::mutex Mutex; - std::map> Generic; - std::map> Specialized; - } GameShaders; + std::map generic; + std::map specialized; std::list PPShaders; diff --git a/src/common/rendering/vulkan/shaders/vk_shadercache.cpp b/src/common/rendering/vulkan/shaders/vk_shadercache.cpp index ccc6dd764..f28c49c87 100644 --- a/src/common/rendering/vulkan/shaders/vk_shadercache.cpp +++ b/src/common/rendering/vulkan/shaders/vk_shadercache.cpp @@ -30,8 +30,6 @@ std::vector VkShaderCache::Compile(ShaderType type, const TArrayView VkShaderCache::Compile(ShaderType type, const TArrayViewChecksum != includeinfo.Checksum) + const VkCachedShaderLump& lumpinfo = includeinfo.PrivateLump ? GetPrivateFile(includeinfo.LumpName) : GetPublicFile(includeinfo.LumpName); + if (lumpinfo.Checksum != includeinfo.Checksum) { foundChanges = true; break; @@ -65,8 +63,6 @@ std::vector VkShaderCache::Compile(ShaderType type, const TArrayView VkShaderCache::Compile(ShaderType type, const TArrayViewGetDevice()); cachedCompile.LastUsed = LaunchTime; - lock.lock(); - auto& c = CodeCache[checksum]; c = std::move(cachedCompile); return c.Code; @@ -94,12 +88,12 @@ ShaderIncludeResult VkShaderCache::OnInclude(VkCachedCompile& cachedCompile, FSt if (depth > 8) I_Error("Too much include recursion!"); - const VkCachedShaderLump* file = system ? GetPrivateFile(headerName) : GetPublicFile(headerName); + const VkCachedShaderLump& file = system ? GetPrivateFile(headerName) : GetPublicFile(headerName); VkCachedInclude cacheinfo; cacheinfo.LumpName = headerName; cacheinfo.PrivateLump = system; - cacheinfo.Checksum = file->Checksum; + cacheinfo.Checksum = file.Checksum; cachedCompile.Includes.push_back(std::move(cacheinfo)); FString includeguardname; @@ -110,7 +104,7 @@ ShaderIncludeResult VkShaderCache::OnInclude(VkCachedCompile& cachedCompile, FSt code << "#ifndef " << includeguardname.GetChars() << "\n"; code << "#define " << includeguardname.GetChars() << "\n"; code << "#line 1\n"; - code << includeFilter(file->Code.GetChars()) << "\n"; + code << includeFilter(file.Code.GetChars()) << "\n"; code << "#endif\n"; return ShaderIncludeResult(headerName.GetChars(), code.GetChars()); @@ -135,45 +129,34 @@ FString VkShaderCache::CalcSha1(const FString& str) return sha1.final(); } -const VkCachedShaderLump* VkShaderCache::GetPublicFile(const FString& lumpname) +const VkCachedShaderLump& VkShaderCache::GetPublicFile(const FString& lumpname) { - std::unique_lock lock(FileMutex); - auto it = PublicFiles.find(lumpname); if (it != PublicFiles.end()) - return it->second.get(); + return it->second; - auto cacheinfo = std::make_unique(); - cacheinfo->Code = LoadPublicShaderLump(lumpname.GetChars()); - cacheinfo->Checksum = CalcSha1(cacheinfo->Code); + VkCachedShaderLump cacheinfo; + cacheinfo.Code = LoadPublicShaderLump(lumpname.GetChars()); + cacheinfo.Checksum = CalcSha1(cacheinfo.Code); auto& result = PublicFiles[lumpname]; result = std::move(cacheinfo); - return result.get(); + return result; } -const VkCachedShaderLump* VkShaderCache::GetPrivateFile(const FString& lumpname) +const VkCachedShaderLump& VkShaderCache::GetPrivateFile(const FString& lumpname) { - std::unique_lock lock(FileMutex); - auto it = PrivateFiles.find(lumpname); if (it != PrivateFiles.end()) - return it->second.get(); + return it->second; - auto cacheinfo = std::make_unique(); - cacheinfo->Code = LoadPrivateShaderLump(lumpname.GetChars()); - cacheinfo->Checksum = CalcSha1(cacheinfo->Code); + VkCachedShaderLump cacheinfo; + cacheinfo.Code = LoadPrivateShaderLump(lumpname.GetChars()); + cacheinfo.Checksum = CalcSha1(cacheinfo.Code); auto& result = PrivateFiles[lumpname]; result = std::move(cacheinfo); - return result.get(); -} - -FString GetStringFromLumpThreadsafe(int lump) -{ - auto reader = fileSystem.OpenFileReader(lump, FileSys::READER_NEW, 0); - auto data = reader.Read(); - return FString(static_cast(data.data()), data.size()); + return result; } FString VkShaderCache::LoadPublicShaderLump(const char* lumpname) @@ -181,14 +164,14 @@ FString VkShaderCache::LoadPublicShaderLump(const char* lumpname) int lump = fileSystem.CheckNumForFullName(lumpname, 0); if (lump == -1) lump = fileSystem.CheckNumForFullName(lumpname); if (lump == -1) I_Error("Unable to load '%s'", lumpname); - return GetStringFromLumpThreadsafe(lump); + return GetStringFromLump(lump); } FString VkShaderCache::LoadPrivateShaderLump(const char* lumpname) { int lump = fileSystem.CheckNumForFullName(lumpname, 0); if (lump == -1) I_Error("Unable to load '%s'", lumpname); - return GetStringFromLumpThreadsafe(lump); + return GetStringFromLump(lump); } static uint8_t readUInt8(FileReader& fr) { uint8_t v = 0; fr.Read(&v, 1); return v; } diff --git a/src/common/rendering/vulkan/shaders/vk_shadercache.h b/src/common/rendering/vulkan/shaders/vk_shadercache.h index e72c7652f..f5e4b9a56 100644 --- a/src/common/rendering/vulkan/shaders/vk_shadercache.h +++ b/src/common/rendering/vulkan/shaders/vk_shadercache.h @@ -50,8 +50,8 @@ public: std::vector Compile(ShaderType type, const TArrayView& sources, const std::function& includeFilter = {}); - const VkCachedShaderLump* GetPublicFile(const FString& lumpname); - const VkCachedShaderLump* GetPrivateFile(const FString& lumpname); + const VkCachedShaderLump& GetPublicFile(const FString& lumpname); + const VkCachedShaderLump& GetPrivateFile(const FString& lumpname); private: void Load(); @@ -70,12 +70,9 @@ private: VulkanRenderDevice* fb = nullptr; - std::mutex CacheMutex; + std::map PublicFiles; + std::map PrivateFiles; std::map CodeCache; - - std::mutex FileMutex; - std::map> PublicFiles; - std::map> PrivateFiles; }; class CachedGLSLCompiler diff --git a/src/common/rendering/vulkan/vk_levelmesh.cpp b/src/common/rendering/vulkan/vk_levelmesh.cpp index 2edcfedf8..b6012f981 100644 --- a/src/common/rendering/vulkan/vk_levelmesh.cpp +++ b/src/common/rendering/vulkan/vk_levelmesh.cpp @@ -651,12 +651,12 @@ void VkLevelMesh::CreateViewerObjects() FString VkLevelMesh::LoadPrivateShaderLump(const char* lumpname) { - return fb->GetShaderCache()->GetPrivateFile(lumpname)->Code; + return fb->GetShaderCache()->GetPrivateFile(lumpname).Code; } FString VkLevelMesh::LoadPublicShaderLump(const char* lumpname) { - return fb->GetShaderCache()->GetPublicFile(lumpname)->Code; + return fb->GetShaderCache()->GetPublicFile(lumpname).Code; } ///////////////////////////////////////////////////////////////////////////// diff --git a/src/common/rendering/vulkan/vk_lightmapper.cpp b/src/common/rendering/vulkan/vk_lightmapper.cpp index d201cfb78..faf14a489 100644 --- a/src/common/rendering/vulkan/vk_lightmapper.cpp +++ b/src/common/rendering/vulkan/vk_lightmapper.cpp @@ -610,12 +610,12 @@ int VkLightmapper::GetRaytracePipelineIndex() FString VkLightmapper::LoadPrivateShaderLump(const char* lumpname) { - return fb->GetShaderCache()->GetPrivateFile(lumpname)->Code; + return fb->GetShaderCache()->GetPrivateFile(lumpname).Code; } FString VkLightmapper::LoadPublicShaderLump(const char* lumpname) { - return fb->GetShaderCache()->GetPublicFile(lumpname)->Code; + return fb->GetShaderCache()->GetPublicFile(lumpname).Code; } void VkLightmapper::CreateRaytracePipeline()