Fixed issue with synchronous pipelines not waiting for asynchronous shaders

Author: TheMostDiligent

Common/interface/AsyncInitializer.hpp

@@ -28,9 +28,11 @@
 
 #include <atomic>
 #include <memory>
+#include <vector>
 
 #include "SpinLock.hpp"
 #include "ThreadPool.hpp"
+#include "RefCntAutoPtr.hpp"
 
 namespace Diligent
 {
@@ -107,6 +109,15 @@ class AsyncInitializer
         };
     }
 
+    template <typename HanlderType>
+    static std::unique_ptr<AsyncInitializer> Start(IThreadPool*                                  pThreadPool,
+                                                   const std::vector<RefCntAutoPtr<IAsyncTask>>& Prerequisites,
+                                                   HanlderType&&                                 Handler)
+    {
+        std::vector<IAsyncTask*> PrerequisitePtrs{Prerequisites.begin(), Prerequisites.end()};
+        return Start(pThreadPool, PrerequisitePtrs.data(), static_cast<Uint32>(PrerequisitePtrs.size()), std::forward<HanlderType>(Handler));
+    }
+
     template <typename HanlderType>
     static std::unique_ptr<AsyncInitializer> Start(IThreadPool*  pThreadPool,
                                                    HanlderType&& Handler)

Graphics/Archiver/src/SerializedPipelineStateImpl.cpp

@@ -382,26 +382,36 @@ SerializedPipelineStateImpl::SerializedPipelineStateImpl(IReferenceCounters*
         constexpr bool                         WaitUntilShadersReady = false;
         PipelineStateUtils::ExtractShaders<SerializedShaderImpl>(CreateInfo, ShaderStages, WaitUntilShadersReady, ActiveShaderStages);
 
-        std::vector<RefCntAutoPtr<IAsyncTask>> ShaderCompileTasks;
+        std::vector<const SerializedShaderImpl*> Shaders;
         for (const SerializedShaderStageInfo& Stage : ShaderStages)
         {
-            for (const SerializedShaderImpl* pShader : Stage.Shaders)
-            {
-                for (RefCntAutoPtr<IAsyncTask> pCompileTask : pShader->GetCompileTasks())
-                    ShaderCompileTasks.emplace_back(std::move(pCompileTask));
-            }
+            Shaders.insert(Shaders.end(), Stage.Shaders.begin(), Stage.Shaders.end());
         }
 
-        std::vector<IAsyncTask*> Prerequisites{ShaderCompileTasks.begin(), ShaderCompileTasks.end()};
+        std::vector<RefCntAutoPtr<IAsyncTask>> ShaderCompileTasks;
+        for (const SerializedShaderImpl* pShader : Shaders)
+        {
+            for (RefCntAutoPtr<IAsyncTask> pCompileTask : pShader->GetCompileTasks())
+                ShaderCompileTasks.emplace_back(std::move(pCompileTask));
+        }
 
         m_AsyncInitializer = AsyncInitializer::Start(
             pDevice->GetShaderCompilationThreadPool(),
-            Prerequisites.data(),                      // Make sure that all asynchronous shader compile tasks are
-            static_cast<Uint32>(Prerequisites.size()), // completed before the pipeline initialization task starts
+            ShaderCompileTasks, // Make sure that all asynchronous shader compile tasks are completed first
             [this,
+#ifdef DILIGENT_DEBUG
+             Shaders,
+#endif
              CreateInfo = typename PipelineStateCreateInfoXTraits<PSOCreateInfoType>::CreateInfoXType{CreateInfo},
              ArchiveInfo](Uint32 ThreadId) mutable //
             {
+#ifdef DILIGENT_DEBUG
+                for (const SerializedShaderImpl* pShader : Shaders)
+                {
+                    VERIFY(!pShader->IsCompiling(), "All shader compile tasks must have been completed since we used them as "
+                                                    "prerequisites for the pipeline initialization task. This appears to be a bug.");
+                }
+#endif
                 try
                 {
                     Initialize(static_cast<const PSOCreateInfoType&>(CreateInfo), ArchiveInfo);

Graphics/GraphicsEngine/include/PipelineStateBase.hpp

@@ -181,7 +181,6 @@ void WaitUntilShaderReadyIfRequested(RefCntAutoPtr<ShaderImplType>& pShader, boo
 {
     if (WaitForCompletion)
     {
-        VERIFY(!pShader->IsCompiling(), "All shader compile tasks must have been completed since we used them as prerequisites for the pipeline initialization task. This appears to be a bug.");
         const SHADER_STATUS ShaderStatus = pShader->GetStatus(/*WaitForCompletion = */ true);
         if (ShaderStatus != SHADER_STATUS_READY)
         {
@@ -864,28 +863,40 @@ class PipelineStateBase : public DeviceObjectBase<typename EngineImplTraits::Pip
         {
             // Collect all asynchronous shader compile tasks
             typename PipelineStateImplType::TShaderStages ShaderStages;
-            ExtractShaders<ShaderImplType>(CreateInfo, ShaderStages);
+            SHADER_TYPE                                   ActiveShaderStages    = SHADER_TYPE_UNKNOWN;
+            constexpr bool                                WaitUntilShadersReady = false;
+            PipelineStateUtils::ExtractShaders<ShaderImplType>(CreateInfo, ShaderStages, WaitUntilShadersReady, ActiveShaderStages);
 
-            std::vector<RefCntAutoPtr<IAsyncTask>> ShaderCompileTasks;
+            std::vector<const ShaderImplType*> Shaders;
             for (const auto& Stage : ShaderStages)
             {
-                std::vector<const ShaderImplType*> Shaders = GetStageShaders(Stage);
-                for (const ShaderImplType* pShader : Shaders)
-                {
-                    if (RefCntAutoPtr<IAsyncTask> pCompileTask = pShader->GetCompileTask())
-                        ShaderCompileTasks.emplace_back(std::move(pCompileTask));
-                }
+                std::vector<const ShaderImplType*> StageShaders = GetStageShaders(Stage);
+                Shaders.insert(Shaders.end(), StageShaders.begin(), StageShaders.end());
             }
 
-            std::vector<IAsyncTask*> Prerequisites{ShaderCompileTasks.begin(), ShaderCompileTasks.end()};
+            std::vector<RefCntAutoPtr<IAsyncTask>> ShaderCompileTasks;
+            for (const ShaderImplType* pShader : Shaders)
+            {
+                if (RefCntAutoPtr<IAsyncTask> pCompileTask = pShader->GetCompileTask())
+                    ShaderCompileTasks.emplace_back(std::move(pCompileTask));
+            }
 
             m_AsyncInitializer = AsyncInitializer::Start(
                 this->m_pDevice->GetShaderCompilationThreadPool(),
-                Prerequisites.data(),                      // Make sure that all asynchronous shader compile tasks are
-                static_cast<Uint32>(Prerequisites.size()), // completed before the pipeline initialization task starts
+                ShaderCompileTasks, // Make sure that all asynchronous shader compile tasks are completed first
                 [pThisImpl,
+#ifdef DILIGENT_DEBUG
+                 Shaders,
+#endif
                  CreateInfo = typename PipelineStateCreateInfoXTraits<PSOCreateInfoType>::CreateInfoXType{CreateInfo}](Uint32 ThreadId) mutable //
                 {
+#ifdef DILIGENT_DEBUG
+                    for (const ShaderImplType* pShader : Shaders)
+                    {
+                        VERIFY(!pShader->IsCompiling(), "All shader compile tasks must have been completed since we used them as "
+                                                        "prerequisites for the pipeline initialization task. This appears to be a bug.");
+                    }
+#endif
                     try
                     {
                         pThisImpl->InitializePipeline(CreateInfo);

Tests/DiligentCoreAPITest/src/AsyncShaderCompilationTest.cpp

@@ -143,7 +143,7 @@ TEST(Shader, ReleaseWhileCompiling)
     pShader.Release();
 }
 
-TEST(Shader, AsyncPipeline)
+void TestAsyncPipeline(SHADER_COMPILE_FLAGS ShaderFlags, PSO_CREATE_FLAGS PSOFlags)
 {
     GPUTestingEnvironment::ScopedReset EnvironmentAutoReset;
 
@@ -158,12 +158,12 @@ TEST(Shader, AsyncPipeline)
     {
         constexpr bool SimplifiedShader = true;
 
-        auto pVS = CreateShader("AsyncShaderCompilationTest.vsh", "Async pipeline test VS", SHADER_TYPE_VERTEX, SHADER_COMPILE_FLAG_ASYNCHRONOUS, SimplifiedShader);
+        auto pVS = CreateShader("AsyncShaderCompilationTest.vsh", "Async pipeline test VS", SHADER_TYPE_VERTEX, ShaderFlags, SimplifiedShader);
         ASSERT_NE(pVS, nullptr);
 
         for (size_t i = 0; i < 16; ++i)
         {
-            auto pPS = CreateShader("AsyncShaderCompilationTest.psh", "Async pipeline test PS", SHADER_TYPE_PIXEL, SHADER_COMPILE_FLAG_ASYNCHRONOUS, SimplifiedShader);
+            auto pPS = CreateShader("AsyncShaderCompilationTest.psh", "Async pipeline test PS", SHADER_TYPE_PIXEL, ShaderFlags, SimplifiedShader);
             ASSERT_NE(pPS, nullptr);
 
             // Allocate pipeline CI on the heap to check that all data is copied correctly
@@ -183,7 +183,7 @@ TEST(Shader, AsyncPipeline)
                 .AddRenderTarget(TEX_FORMAT_RGBA8_UNORM)
                 .SetInputLayout(InputLayout)
                 .SetResourceLayout(ResourceLayout)
-                .SetFlags(PSO_CREATE_FLAG_ASYNCHRONOUS);
+                .SetFlags(PSOFlags);
 
             // Create multiple pipelines that use the same shaders.
             // In particular this reproduces the problem with the non-thread-safe ID3DBlob in D3D12.
@@ -217,4 +217,19 @@ TEST(Shader, AsyncPipeline)
     LOG_INFO_MESSAGE(pPSOs.size(), " PSOs were compiled after ", Iter, " iterations (", (T.GetElapsedTime() - StartTime) * 1000, " ms)");
 }
 
+TEST(Shader, AsyncPipeline_SyncShaders)
+{
+    TestAsyncPipeline(SHADER_COMPILE_FLAG_NONE, PSO_CREATE_FLAG_ASYNCHRONOUS);
+}
+
+TEST(Shader, SyncPipeline_AsyncShaders)
+{
+    TestAsyncPipeline(SHADER_COMPILE_FLAG_ASYNCHRONOUS, PSO_CREATE_FLAG_NONE);
+}
+
+TEST(Shader, AsyncPipeline_AsyncShaders)
+{
+    TestAsyncPipeline(SHADER_COMPILE_FLAG_ASYNCHRONOUS, PSO_CREATE_FLAG_ASYNCHRONOUS);
+}
+
 } // namespace