From cd933d5d204026689d37c35d406da9e8066ee565 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 11 Oct 2023 02:59:57 -0400 Subject: [PATCH 1/9] Timeline semaphores This CL adds timeline semaphore support for both APIs. grfx::Semaphore is modified to support both binary and timeline semaphores similar to how Vulkan does it. This CL also makes the Vulkan queues thread safe. - Modified vk::Queue to be thread safe. - Added timeline semaphore support for Vulkan and D3D12. - Added sample demonstrating timeline semaphore. --- include/ppx/config.h | 1 + include/ppx/grfx/dx12/dx12_queue.h | 3 + include/ppx/grfx/dx12/dx12_sync.h | 8 +- include/ppx/grfx/grfx_enums.h | 6 + include/ppx/grfx/grfx_queue.h | 6 + include/ppx/grfx/grfx_sync.h | 33 +++ include/ppx/grfx/vk/vk_device.h | 35 ++- include/ppx/grfx/vk/vk_queue.h | 5 + include/ppx/grfx/vk/vk_sync.h | 25 +- projects/CMakeLists.txt | 1 + projects/timeline_semaphore/CMakeLists.txt | 22 ++ projects/timeline_semaphore/README.md | 9 + projects/timeline_semaphore/main.cpp | 325 +++++++++++++++++++++ src/ppx/grfx/dx12/dx12_queue.cpp | 58 +++- src/ppx/grfx/dx12/dx12_sync.cpp | 67 ++++- src/ppx/grfx/grfx_sync.cpp | 55 ++++ src/ppx/grfx/vk/vk_device.cpp | 24 ++ src/ppx/grfx/vk/vk_queue.cpp | 164 +++++++++-- src/ppx/grfx/vk/vk_sync.cpp | 64 ++++ 19 files changed, 862 insertions(+), 49 deletions(-) create mode 100644 projects/timeline_semaphore/CMakeLists.txt create mode 100644 projects/timeline_semaphore/README.md create mode 100644 projects/timeline_semaphore/main.cpp diff --git a/include/ppx/config.h b/include/ppx/config.h index a51562cfc..25922637a 100644 --- a/include/ppx/config.h +++ b/include/ppx/config.h @@ -146,6 +146,7 @@ enum Result ERROR_GRFX_INVALID_BINDING_NUMBER = -1024, ERROR_GRFX_INVALID_SET_NUMBER = -1025, ERROR_GRFX_OPERATION_NOT_PERMITTED = -1026, + ERROR_GRFX_INVALID_SEMAPHORE_TYPE = -1027, ERROR_IMAGE_FILE_LOAD_FAILED = -2000, ERROR_IMAGE_FILE_SAVE_FAILED = -2001, diff --git a/include/ppx/grfx/dx12/dx12_queue.h b/include/ppx/grfx/dx12/dx12_queue.h index a46329e50..ad8549793 100644 --- a/include/ppx/grfx/dx12/dx12_queue.h +++ b/include/ppx/grfx/dx12/dx12_queue.h @@ -35,6 +35,9 @@ class Queue virtual Result Submit(const grfx::SubmitInfo* pSubmitInfo) override; + virtual Result QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result GetTimestampFrequency(uint64_t* pFrequency) const override; protected: diff --git a/include/ppx/grfx/dx12/dx12_sync.h b/include/ppx/grfx/dx12/dx12_sync.h index dc402945a..1f2595453 100644 --- a/include/ppx/grfx/dx12/dx12_sync.h +++ b/include/ppx/grfx/dx12/dx12_sync.h @@ -61,13 +61,19 @@ class Semaphore UINT64 GetNextSignalValue(); UINT64 GetWaitForValue() const; +private: + virtual Result TimelineWait(uint64_t value, uint64_t timeout) const override; + virtual Result TimelineSignal(uint64_t value) const override; + virtual uint64_t TimelineCounterValue() const override; + protected: virtual Result CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) override; virtual void DestroyApiObjects() override; private: D3D12FencePtr mFence; - UINT64 mValue = 0; + HANDLE mWaitEventHandle = nullptr; + UINT64 mValue = 0; }; } // namespace dx12 diff --git a/include/ppx/grfx/grfx_enums.h b/include/ppx/grfx/grfx_enums.h index 1dbdf7654..9bbbcf67c 100644 --- a/include/ppx/grfx/grfx_enums.h +++ b/include/ppx/grfx/grfx_enums.h @@ -432,6 +432,12 @@ enum SampleCount SAMPLE_COUNT_64 = 64, }; +enum SemaphoreType +{ + SEMAPHORE_TYPE_BINARY = 0, + SEMAPHORE_TYPE_TIMELINE = 1, +}; + enum ShaderStageBits { SHADER_STAGE_UNDEFINED = 0x00000000, diff --git a/include/ppx/grfx/grfx_queue.h b/include/ppx/grfx/grfx_queue.h index 048ec8cc1..ef8939d87 100644 --- a/include/ppx/grfx/grfx_queue.h +++ b/include/ppx/grfx/grfx_queue.h @@ -27,8 +27,10 @@ struct SubmitInfo const grfx::CommandBuffer* const* ppCommandBuffers = nullptr; uint32_t waitSemaphoreCount = 0; const grfx::Semaphore* const* ppWaitSemaphores = nullptr; + std::vector waitValues = {}; // Use 0 if index is binary semaphore uint32_t signalSemaphoreCount = 0; grfx::Semaphore** ppSignalSemaphores = nullptr; + std::vector signalValues = {}; // Use 0 if index is binary smeaphore grfx::Fence* pFence = nullptr; }; @@ -63,6 +65,10 @@ class Queue virtual Result Submit(const grfx::SubmitInfo* pSubmitInfo) = 0; + // Timeline semaphore functions + virtual Result QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) = 0; + virtual Result QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) = 0; + // GPU timestamp frequency counter in ticks per second virtual Result GetTimestampFrequency(uint64_t* pFrequency) const = 0; diff --git a/include/ppx/grfx/grfx_sync.h b/include/ppx/grfx/grfx_sync.h index b2327d167..a0e1f674e 100644 --- a/include/ppx/grfx/grfx_sync.h +++ b/include/ppx/grfx/grfx_sync.h @@ -56,6 +56,8 @@ class Fence //! struct SemaphoreCreateInfo { + grfx::SemaphoreType semaphoreType = grfx::SEMAPHORE_TYPE_BINARY; + uint64_t initialValue = 0; // Timeline semaphore only }; //! @class Semaphore @@ -68,12 +70,43 @@ class Semaphore Semaphore() {} virtual ~Semaphore() {} + grfx::SemaphoreType GetSemaphoreType() const { return mCreateInfo.semaphoreType; } + bool IsBinary() const { return mCreateInfo.semaphoreType == grfx::SEMAPHORE_TYPE_BINARY; } + bool IsTimeline() const { return mCreateInfo.semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE; } + + // Timeline semaphore wait + Result Wait(uint64_t value, uint64_t timeout = UINT64_MAX) const; + + // Timeline semaphore signal + // + // WARNING: Signaling a value less that's less than what's already been signaled + // can cause a block or a race condition. + // + // Use forceMonotonicValue=true to use the current timeline semaphore value + // if it's greater than the passed in value. This is useful when signaling + // from threads where ordering is not guaranteed. + // + Result Signal(uint64_t value, bool forceMonotonicValue = false) const; + + // Returns current timeline semaphore value + uint64_t GetCounterValue() const; + +private: + virtual Result TimelineWait(uint64_t value, uint64_t timeout) const = 0; + virtual Result TimelineSignal(uint64_t value) const = 0; + virtual uint64_t TimelineCounterValue() const = 0; + protected: virtual Result CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) = 0; virtual void DestroyApiObjects() = 0; friend class grfx::Device; + +private: + mutable std::mutex mTimelineMutex; }; +// ------------------------------------------------------------------------------------------------- + } // namespace grfx } // namespace ppx diff --git a/include/ppx/grfx/vk/vk_device.h b/include/ppx/grfx/vk/vk_device.h index 6ebab754b..c2b4de298 100644 --- a/include/ppx/grfx/vk/vk_device.h +++ b/include/ppx/grfx/vk/vk_device.h @@ -50,6 +50,10 @@ class Device uint32_t firstQuery, uint32_t queryCount) const; + VkResult WaitSemaphores(const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout) const; + VkResult SignalSemaphore(const VkSemaphoreSignalInfo* pSignalInfo); + VkResult GetSemaphoreCounterValue(VkSemaphore semaphore, uint64_t* pValue); + uint32_t GetGraphicsQueueFamilyIndex() const { return mGraphicsQueueFamilyIndex; } uint32_t GetComputeQueueFamilyIndex() const { return mComputeQueueFamilyIndex; } uint32_t GetTransferQueueFamilyIndex() const { return mTransferQueueFamilyIndex; } @@ -93,20 +97,23 @@ class Device Result CreateQueues(const grfx::DeviceCreateInfo* pCreateInfo); private: - std::vector mFoundExtensions; - std::vector mExtensions; - VkDevicePtr mDevice; - VkPhysicalDeviceFeatures mDeviceFeatures = {}; - VmaAllocatorPtr mVmaAllocator; - bool mHasTimelineSemaphore = false; - bool mHasExtendedDynamicState = false; - bool mHasUnrestrictedDepthRange = false; - bool mHasDynamicRendering = false; - PFN_vkResetQueryPoolEXT mFnResetQueryPoolEXT = nullptr; - uint32_t mGraphicsQueueFamilyIndex = 0; - uint32_t mComputeQueueFamilyIndex = 0; - uint32_t mTransferQueueFamilyIndex = 0; - uint32_t mMaxPushDescriptors = 0; + std::vector mFoundExtensions; + std::vector mExtensions; + VkDevicePtr mDevice; + VkPhysicalDeviceFeatures mDeviceFeatures = {}; + VmaAllocatorPtr mVmaAllocator; + bool mHasTimelineSemaphore = false; + bool mHasExtendedDynamicState = false; + bool mHasUnrestrictedDepthRange = false; + bool mHasDynamicRendering = false; + PFN_vkResetQueryPoolEXT mFnResetQueryPoolEXT = nullptr; + PFN_vkWaitSemaphores mFnWaitSemaphores = nullptr; + PFN_vkSignalSemaphore mFnSignalSemaphore = nullptr; + PFN_vkGetSemaphoreCounterValue mFnGetSemaphoreCounterValue = nullptr; + uint32_t mGraphicsQueueFamilyIndex = 0; + uint32_t mComputeQueueFamilyIndex = 0; + uint32_t mTransferQueueFamilyIndex = 0; + uint32_t mMaxPushDescriptors = 0; }; extern PFN_vkCmdPushDescriptorSetKHR CmdPushDescriptorSetKHR; diff --git a/include/ppx/grfx/vk/vk_queue.h b/include/ppx/grfx/vk/vk_queue.h index 8e9c38797..f6fc6163c 100644 --- a/include/ppx/grfx/vk/vk_queue.h +++ b/include/ppx/grfx/vk/vk_queue.h @@ -37,6 +37,9 @@ class Queue virtual Result Submit(const grfx::SubmitInfo* pSubmitInfo) override; + virtual Result QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result GetTimestampFrequency(uint64_t* pFrequency) const override; VkResult TransitionImageLayout( @@ -57,6 +60,8 @@ class Queue private: VkQueuePtr mQueue; VkCommandPoolPtr mTransientPool; + std::mutex mQueueMutex; + std::mutex mCommandPoolMutex; }; } // namespace vk diff --git a/include/ppx/grfx/vk/vk_sync.h b/include/ppx/grfx/vk/vk_sync.h index cfea57117..16b68f34c 100644 --- a/include/ppx/grfx/vk/vk_sync.h +++ b/include/ppx/grfx/vk/vk_sync.h @@ -59,12 +59,35 @@ class Semaphore VkSemaphorePtr GetVkSemaphore() const { return mSemaphore; } +private: + virtual Result TimelineWait(uint64_t value, uint64_t timeout) const override; + virtual Result TimelineSignal(uint64_t value) const override; + virtual uint64_t TimelineCounterValue() const override; + protected: virtual Result CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) override; virtual void DestroyApiObjects() override; private: - VkSemaphorePtr mSemaphore; + // + // Why are we storing timeline semaphore signaled values? + // + // Direct3D allows fence objects to signal a value if the value is + // equal to or greater than what's already been signaled. + // + // Vulkan does not: + // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkSemaphoreSignalInfo.html#VUID-VkSemaphoreSignalInfo-value-03258 + // + // This is unfortunate, because there are cases where an application + // may need to signal a valuet hat is equal to what's been signaled. + // + // Even though it's possible to get the current value, add 1 to it, + // and then signal it - this creates can create a different problem + // where a value is signaled too soon and a write-after-read hazard + // possibly gets introduced. + // + mutable uint64_t mTimelineSignaledValue = 0; + VkSemaphorePtr mSemaphore; }; } // namespace vk diff --git a/projects/CMakeLists.txt b/projects/CMakeLists.txt index 6df6d89e7..a9272d426 100644 --- a/projects/CMakeLists.txt +++ b/projects/CMakeLists.txt @@ -48,6 +48,7 @@ add_subdirectory(alloc) add_subdirectory(fishtornado) add_subdirectory(fluid_simulation) add_subdirectory(oit_demo) +add_subdirectory(timeline_semaphore) if (PPX_BUILD_XR) add_subdirectory(04_cube_xr) diff --git a/projects/timeline_semaphore/CMakeLists.txt b/projects/timeline_semaphore/CMakeLists.txt new file mode 100644 index 000000000..52ae0efb1 --- /dev/null +++ b/projects/timeline_semaphore/CMakeLists.txt @@ -0,0 +1,22 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +cmake_minimum_required(VERSION 3.0 FATAL_ERROR) + +project(timeline_semaphore) + +add_samples_for_all_apis( + NAME ${PROJECT_NAME} + SOURCES "main.cpp" + SHADER_DEPENDENCIES + "shader_text_draw") diff --git a/projects/timeline_semaphore/README.md b/projects/timeline_semaphore/README.md new file mode 100644 index 000000000..03df03e68 --- /dev/null +++ b/projects/timeline_semaphore/README.md @@ -0,0 +1,9 @@ +# Timeline semaphore + +Example of how to use timeline semaphores. + +## Shaders + +Shader | Purpose for this project +--------------- | ---------------------------- +`TextDraw.hlsl` | Draw colored text. diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp new file mode 100644 index 000000000..629b43f93 --- /dev/null +++ b/projects/timeline_semaphore/main.cpp @@ -0,0 +1,325 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "ppx/ppx.h" +#include "ppx/camera.h" +using namespace ppx; + +#if defined(USE_DX12) +const grfx::Api kApi = grfx::API_DX_12_0; +#elif defined(USE_VK) +const grfx::Api kApi = grfx::API_VK_1_1; +#endif + +class ProjApp + : public ppx::Application +{ +public: + virtual void Config(ppx::ApplicationSettings& settings) override; + virtual void Setup() override; + virtual void Render() override; + +private: + struct PerFrame + { + grfx::CommandBufferPtr drawTextCmd; + grfx::CommandBufferPtr drawImGuiCmd; + grfx::SemaphorePtr imageAcquiredSemaphore; + grfx::FencePtr imageAcquiredFence; + grfx::SemaphorePtr presentReadySemaphore; // Binary semaphore + + grfx::SemaphorePtr timelineSemaphore; + uint64_t timelineValue = 0; + }; + std::vector mPerFrame; + grfx::TextureFontPtr mRoboto; + grfx::TextDrawPtr mDynamicText; + PerspCamera mCamera; +}; + +void ProjApp::Config(ppx::ApplicationSettings& settings) +{ + settings.appName = "timeline semaphore"; + settings.enableImGui = true; + settings.grfx.api = kApi; + settings.grfx.enableDebug = true; +} + +void ProjApp::Setup() +{ + mCamera = PerspCamera(GetWindowWidth(), GetWindowHeight()); + + // Per frame data + { + PerFrame frame = {}; + + // Command buffers + PPX_CHECKED_CALL(GetGraphicsQueue()->CreateCommandBuffer(&frame.drawTextCmd)); + PPX_CHECKED_CALL(GetGraphicsQueue()->CreateCommandBuffer(&frame.drawImGuiCmd)); + + // Defaults to binary semaphore + grfx::SemaphoreCreateInfo semaCreateInfo = {}; + PPX_CHECKED_CALL(GetDevice()->CreateSemaphore(&semaCreateInfo, &frame.imageAcquiredSemaphore)); + + grfx::FenceCreateInfo fenceCreateInfo = {}; + PPX_CHECKED_CALL(GetDevice()->CreateFence(&fenceCreateInfo, &frame.imageAcquiredFence)); + + PPX_CHECKED_CALL(GetDevice()->CreateSemaphore(&semaCreateInfo, &frame.presentReadySemaphore)); + + // Change create info for timeline semaphore + semaCreateInfo = {}; + semaCreateInfo.semaphoreType = grfx::SEMAPHORE_TYPE_TIMELINE; + semaCreateInfo.initialValue = 0; + PPX_CHECKED_CALL(GetDevice()->CreateSemaphore(&semaCreateInfo, &frame.timelineSemaphore)); + + mPerFrame.push_back(frame); + } + + // Texture font + { + ppx::Font font; + PPX_CHECKED_CALL(ppx::Font::CreateFromFile(GetAssetPath("basic/fonts/Roboto/Roboto-Regular.ttf"), &font)); + + grfx::TextureFontCreateInfo createInfo = {}; + createInfo.font = font; + createInfo.size = 48.0f; + createInfo.characters = grfx::TextureFont::GetDefaultCharacters(); + + PPX_CHECKED_CALL(GetDevice()->CreateTextureFont(&createInfo, &mRoboto)); + } + + // Text draw + { + grfx::ShaderModulePtr VS; + grfx::ShaderModulePtr PS; + + std::vector bytecode = LoadShader("basic/shaders", "TextDraw.vs"); + PPX_ASSERT_MSG(!bytecode.empty(), "VS shader bytecode load failed"); + grfx::ShaderModuleCreateInfo shaderCreateInfo = {static_cast(bytecode.size()), bytecode.data()}; + PPX_CHECKED_CALL(GetDevice()->CreateShaderModule(&shaderCreateInfo, &VS)); + + bytecode = LoadShader("basic/shaders", "TextDraw.ps"); + PPX_ASSERT_MSG(!bytecode.empty(), "PS shader bytecode load failed"); + shaderCreateInfo = {static_cast(bytecode.size()), bytecode.data()}; + PPX_CHECKED_CALL(GetDevice()->CreateShaderModule(&shaderCreateInfo, &PS)); + + grfx::TextDrawCreateInfo createInfo = {}; + createInfo.pFont = mRoboto; + createInfo.maxTextLength = 4096; + createInfo.VS = {VS.Get(), "vsmain"}; + createInfo.PS = {PS.Get(), "psmain"}; + createInfo.renderTargetFormat = GetSwapchain()->GetColorFormat(); + + PPX_CHECKED_CALL(GetDevice()->CreateTextDraw(&createInfo, &mDynamicText)); + + GetDevice()->DestroyShaderModule(VS); + GetDevice()->DestroyShaderModule(PS); + } +} + +void ProjApp::Render() +{ + PerFrame& frame = mPerFrame[0]; + + grfx::SwapchainPtr swapchain = GetSwapchain(); + + uint32_t imageIndex = UINT32_MAX; + PPX_CHECKED_CALL(swapchain->AcquireNextImage(UINT64_MAX, frame.imageAcquiredSemaphore, frame.imageAcquiredFence, &imageIndex)); + + // Wait for and reset image acquired fence + PPX_CHECKED_CALL(frame.imageAcquiredFence->WaitAndReset()); + + // Wait for previous frame's render to complete (GPU signal to CPU wait) + PPX_CHECKED_CALL(frame.timelineSemaphore->Wait(frame.timelineValue)); + + // Spawn a thread that will spawn other threads to signal values on the CPU + std::unique_ptr spawnerThread; + { + const uint32_t kNumThreads = 4; + grfx::Semaphore* pSemaphore = frame.timelineSemaphore; + + // Normally, we increment after a wait and before the next signal so we need to add 1. + const uint64_t startSignalValue = frame.timelineValue + 1; + + spawnerThread = std::unique_ptr( + new std::thread([kNumThreads, pSemaphore, startSignalValue]() { + std::vector> signalThreads; + + // Create signaling threads + for (uint32_t i = 0; i < kNumThreads; ++i) { + const uint64_t signalValue = startSignalValue + i; + + auto signalThread = std::unique_ptr( + new std::thread([pSemaphore, signalValue] { + PPX_CHECKED_CALL(pSemaphore->Signal(signalValue, true)); + })); + + signalThreads.push_back(std::move(signalThread)); + } + + // Join threads + for (auto& thread : signalThreads) { + thread->join(); + } + signalThreads.clear(); + })); + + // Increment to account signaling thread values + frame.timelineValue += kNumThreads; + } + + // Wait on primary for secondary threads to signal on the CPU (CPU signals to CPU wait) + PPX_CHECKED_CALL(frame.timelineSemaphore->Wait(frame.timelineValue)); + + // Join spawner thread + spawnerThread->join(); + spawnerThread.reset(); + + // Signal values for text draw start and finish + const uint64_t drawTextStartSignalValue = ++frame.timelineValue; + const uint64_t drawTextFinishSignalValue = ++frame.timelineValue; + + // Queue the text draw but don't start until the CPU signals (CPU signal to GPU wait) + { + PPX_CHECKED_CALL(frame.drawTextCmd->Begin()); + { + // Prepare string outside of render pass + { + std::stringstream ss; + ss << "Frame: " << GetFrameCount() << "\n"; + ss << "FPS: " << std::setw(6) << std::setprecision(6) << GetAverageFPS() << "\n"; + ss << "Timeline semaphores FTW!" << "\n"; + ss << "Timeline value: " << frame.timelineValue; + + mDynamicText->Clear(); + mDynamicText->AddString(float2(15, 50), ss.str()); + mDynamicText->UploadToGpu(frame.drawTextCmd); + mDynamicText->PrepareDraw(mCamera.GetViewProjectionMatrix(), frame.drawTextCmd); + } + + // ------------------------------------------------------------------------------------- + + grfx::RenderPassPtr renderPass = swapchain->GetRenderPass(imageIndex); + PPX_ASSERT_MSG(!renderPass.IsNull(), "render pass object is null"); + + grfx::RenderPassBeginInfo beginInfo = {}; + beginInfo.pRenderPass = renderPass; + beginInfo.renderArea = renderPass->GetRenderArea(); + beginInfo.RTVClearCount = 1; + beginInfo.RTVClearValues[0] = {{0.25f, 0.3f, 0.33f, 1}}; + + frame.drawTextCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_PRESENT, grfx::RESOURCE_STATE_RENDER_TARGET); + frame.drawTextCmd->BeginRenderPass(&beginInfo); + { + grfx::Rect scissorRect = renderPass->GetScissor(); + grfx::Viewport viewport = renderPass->GetViewport(); + frame.drawTextCmd->SetScissors(1, &scissorRect); + frame.drawTextCmd->SetViewports(1, &viewport); + + mDynamicText->Draw(frame.drawTextCmd); + } + frame.drawTextCmd->EndRenderPass(); + frame.drawTextCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_RENDER_TARGET, grfx::RESOURCE_STATE_PRESENT); + } + PPX_CHECKED_CALL(frame.drawTextCmd->End()); + + std::vector waitSemaphores = {frame.imageAcquiredSemaphore, frame.timelineSemaphore}; + std::vector waitValues = {0, drawTextStartSignalValue}; + + std::vector signalSemaphores = {frame.timelineSemaphore}; + std::vector signalValues = {drawTextFinishSignalValue}; + + grfx::SubmitInfo submitInfo = {}; + submitInfo.commandBufferCount = 1; + submitInfo.ppCommandBuffers = &frame.drawTextCmd; + submitInfo.waitSemaphoreCount = CountU32(waitSemaphores); + submitInfo.ppWaitSemaphores = DataPtr(waitSemaphores); + submitInfo.waitValues = waitValues; + submitInfo.signalSemaphoreCount = CountU32(signalSemaphores); + submitInfo.ppSignalSemaphores = DataPtr(signalSemaphores); + submitInfo.signalValues = signalValues; + submitInfo.pFence = nullptr; + + PPX_CHECKED_CALL(GetGraphicsQueue()->Submit(&submitInfo)); + } + + // Spawn a thread to signal on the CPU to kick off the text draw + { + grfx::Semaphore* pSemaphore = frame.timelineSemaphore; + + spawnerThread = std::unique_ptr( + new std::thread([pSemaphore, drawTextStartSignalValue]() { + PPX_CHECKED_CALL(pSemaphore->Signal(drawTextStartSignalValue)); + })); + + spawnerThread->join(); + spawnerThread.reset(); + } + + // Queue ImGui draw but wait on the text draw to finish (GPU signal to GPU wait) + { + PPX_CHECKED_CALL(frame.drawImGuiCmd->Begin()); + { + grfx::RenderPassPtr renderPass = swapchain->GetRenderPass(imageIndex, grfx::ATTACHMENT_LOAD_OP_LOAD); + PPX_ASSERT_MSG(!renderPass.IsNull(), "render pass object is null"); + + grfx::RenderPassBeginInfo beginInfo = {}; + beginInfo.pRenderPass = renderPass; + beginInfo.renderArea = renderPass->GetRenderArea(); + + frame.drawImGuiCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_PRESENT, grfx::RESOURCE_STATE_RENDER_TARGET); + frame.drawImGuiCmd->BeginRenderPass(&beginInfo); + { + grfx::Rect scissorRect = renderPass->GetScissor(); + grfx::Viewport viewport = renderPass->GetViewport(); + frame.drawImGuiCmd->SetScissors(1, &scissorRect); + frame.drawImGuiCmd->SetViewports(1, &viewport); + + // Draw ImGui + DrawDebugInfo(); + DrawImGui(frame.drawImGuiCmd); + } + frame.drawImGuiCmd->EndRenderPass(); + frame.drawImGuiCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_RENDER_TARGET, grfx::RESOURCE_STATE_PRESENT); + } + PPX_CHECKED_CALL(frame.drawImGuiCmd->End()); + + // Wait for text daw to finish + std::vector waitSemaphores = {frame.timelineSemaphore}; + std::vector waitValues = {drawTextFinishSignalValue}; + + // Signal value for render work complete + ++frame.timelineValue; + + std::vector signalSemaphores = {frame.presentReadySemaphore, frame.timelineSemaphore}; + std::vector signalValues = {0, frame.timelineValue}; + + grfx::SubmitInfo submitInfo = {}; + submitInfo.commandBufferCount = 1; + submitInfo.ppCommandBuffers = &frame.drawImGuiCmd; + submitInfo.waitSemaphoreCount = CountU32(waitSemaphores); + submitInfo.ppWaitSemaphores = DataPtr(waitSemaphores); + submitInfo.waitValues = waitValues; + submitInfo.signalSemaphoreCount = CountU32(signalSemaphores); + submitInfo.ppSignalSemaphores = DataPtr(signalSemaphores); + submitInfo.signalValues = signalValues; + submitInfo.pFence = nullptr; + + PPX_CHECKED_CALL(GetGraphicsQueue()->Submit(&submitInfo)); + } + + PPX_CHECKED_CALL(swapchain->Present(imageIndex, 1, &frame.presentReadySemaphore)); +} + +SETUP_APPLICATION(ProjApp) \ No newline at end of file diff --git a/src/ppx/grfx/dx12/dx12_queue.cpp b/src/ppx/grfx/dx12/dx12_queue.cpp index 46255de7e..99e24ed34 100644 --- a/src/ppx/grfx/dx12/dx12_queue.cpp +++ b/src/ppx/grfx/dx12/dx12_queue.cpp @@ -82,10 +82,13 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) mListBuffer[i] = ToApi(pSubmitInfo->ppCommandBuffers[i])->GetDxCommandList(); } + // Wait semaphores for (uint32_t i = 0; i < pSubmitInfo->waitSemaphoreCount; ++i) { - ID3D12Fence* pDxFence = ToApi(pSubmitInfo->ppWaitSemaphores[i])->GetDxFence(); - UINT64 value = ToApi(pSubmitInfo->ppWaitSemaphores[i])->GetWaitForValue(); - HRESULT hr = mCommandQueue->Wait(pDxFence, value); + auto pSemaphore = ToApi(pSubmitInfo->ppWaitSemaphores[i]); + ID3D12Fence* pDxFence = pSemaphore->GetDxFence(); + UINT64 value = pSemaphore->IsTimeline() ? pSubmitInfo->waitValues[i] : pSemaphore->GetWaitForValue(); + + HRESULT hr = mCommandQueue->Wait(pDxFence, value); if (FAILED(hr)) { PPX_ASSERT_MSG(false, "ID3D12CommandQueue::Wait failed"); return ppx::ERROR_API_FAILURE; @@ -96,10 +99,13 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) static_cast(pSubmitInfo->commandBufferCount), mListBuffer.data()); + // Signal semaphores for (uint32_t i = 0; i < pSubmitInfo->signalSemaphoreCount; ++i) { - ID3D12Fence* pDxFence = ToApi(pSubmitInfo->ppSignalSemaphores[i])->GetDxFence(); - UINT64 value = ToApi(pSubmitInfo->ppSignalSemaphores[i])->GetNextSignalValue(); - HRESULT hr = mCommandQueue->Signal(pDxFence, value); + auto pSemaphore = ToApi(pSubmitInfo->ppSignalSemaphores[i]); + ID3D12Fence* pDxFence = pSemaphore->GetDxFence(); + UINT64 value = pSemaphore->IsTimeline() ? pSubmitInfo->signalValues[i] : pSemaphore->GetNextSignalValue(); + + HRESULT hr = mCommandQueue->Signal(pDxFence, value); if (FAILED(hr)) { PPX_ASSERT_MSG(false, "ID3D12CommandQueue::Signal failed"); return ppx::ERROR_API_FAILURE; @@ -119,6 +125,46 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) return ppx::SUCCESS; } +Result Queue::QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + HRESULT hr = mCommandQueue->Wait( + ToApi(pSemaphore)->GetDxFence(), + static_cast(value)); + if (FAILED(hr)) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + +Result Queue::QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + HRESULT hr = mCommandQueue->Signal( + ToApi(pSemaphore)->GetDxFence(), + static_cast(value)); + if (FAILED(hr)) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + Result Queue::GetTimestampFrequency(uint64_t* pFrequency) const { if (IsNull(pFrequency)) { diff --git a/src/ppx/grfx/dx12/dx12_sync.cpp b/src/ppx/grfx/dx12/dx12_sync.cpp index f3676a4c7..535e4c799 100644 --- a/src/ppx/grfx/dx12/dx12_sync.cpp +++ b/src/ppx/grfx/dx12/dx12_sync.cpp @@ -15,6 +15,9 @@ #include "ppx/grfx/dx12/dx12_sync.h" #include "ppx/grfx/dx12/dx12_device.h" +#define REQUIRES_BINARY_MSG "invalid semaphore type: operation requires binary semaphore" +#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" + namespace ppx { namespace grfx { namespace dx12 { @@ -83,20 +86,37 @@ Result Fence::Reset() // ------------------------------------------------------------------------------------------------- Result Semaphore::CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) { + UINT64 initialValue = mValue; + if (pCreateInfo->semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE) { + initialValue = static_cast(pCreateInfo->initialValue); + } + D3D12_FENCE_FLAGS flags = D3D12_FENCE_FLAG_NONE; - HRESULT hr = ToApi(GetDevice())->GetDxDevice()->CreateFence(mValue, flags, IID_PPV_ARGS(&mFence)); + HRESULT hr = ToApi(GetDevice())->GetDxDevice()->CreateFence(initialValue, flags, IID_PPV_ARGS(&mFence)); if (FAILED(hr)) { PPX_ASSERT_MSG(false, "ID3D12Device::CreateFence(fence) failed"); return ppx::ERROR_API_FAILURE; } PPX_LOG_OBJECT_CREATION(D3D12Fence(Semaphore), mFence.Get()); + // Wait event handle + if (pCreateInfo->semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE) { + mWaitEventHandle = CreateEventEx(NULL, NULL, false, EVENT_ALL_ACCESS); + if (mWaitEventHandle == INVALID_HANDLE_VALUE) { + return ppx::ERROR_API_FAILURE; + } + } + return ppx::SUCCESS; } void Semaphore::DestroyApiObjects() { + if (mWaitEventHandle != nullptr) { + CloseHandle(mWaitEventHandle); + } + if (mFence) { mFence.Reset(); } @@ -104,15 +124,60 @@ void Semaphore::DestroyApiObjects() UINT64 Semaphore::GetNextSignalValue() { + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_BINARY) { + PPX_ASSERT_MSG(false, REQUIRES_BINARY_MSG); + return UINT64_MAX; + } + mValue += 1; return mValue; } UINT64 Semaphore::GetWaitForValue() const { + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_BINARY) { + PPX_ASSERT_MSG(false, REQUIRES_BINARY_MSG); + return UINT64_MAX; + } + return mValue; } +Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + UINT64 completedValue = mFence->GetCompletedValue(); + if (completedValue < value) { + mFence->SetEventOnCompletion(value, mWaitEventHandle); + + DWORD dwMillis = (timeout == UINT64_MAX) ? INFINITE : static_cast(timeout / 1000000ULL); + WaitForSingleObjectEx(mWaitEventHandle, dwMillis, false); + } + + return ppx::SUCCESS; +} + +Result Semaphore::TimelineSignal(uint64_t value) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + HRESULT hr = mFence->Signal(static_cast(value)); + if (FAILED(hr)) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + +uint64_t Semaphore::TimelineCounterValue() const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + UINT64 value = mFence->GetCompletedValue(); + return static_cast(value); +} + } // namespace dx12 } // namespace grfx } // namespace ppx diff --git a/src/ppx/grfx/grfx_sync.cpp b/src/ppx/grfx/grfx_sync.cpp index f78046967..f58b5f894 100644 --- a/src/ppx/grfx/grfx_sync.cpp +++ b/src/ppx/grfx/grfx_sync.cpp @@ -17,6 +17,9 @@ namespace ppx { namespace grfx { +// ------------------------------------------------------------------------------------------------- +// Fence +// ------------------------------------------------------------------------------------------------- Result Fence::WaitAndReset(uint64_t timeout) { Result ppxres = Wait(timeout); @@ -32,5 +35,57 @@ Result Fence::WaitAndReset(uint64_t timeout) return ppx::SUCCESS; } +// ------------------------------------------------------------------------------------------------- +// Semaphore +// ------------------------------------------------------------------------------------------------- +Result Semaphore::Wait(uint64_t value, uint64_t timeout) const +{ + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + auto ppxres = this->TimelineWait(value, timeout); + if (Failed(ppxres)) { + return ppxres; + } + + return ppx::SUCCESS; +} + +Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const +{ + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + // Synchronize access to API semaphore object + std::lock_guard lock(mTimelineMutex); + + if (forceMonotonicValue) { + uint64_t currentValue = this->TimelineCounterValue(); + value = std::max(value, currentValue); + } + + auto ppxres = this->TimelineSignal(value); + if (Failed(ppxres)) { + return ppxres; + } + + return ppx::SUCCESS; +} + +uint64_t Semaphore::GetCounterValue() const +{ + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return UINT64_MAX; + } + + // Synchronize access to API semaphore object + std::lock_guard lock(mTimelineMutex); + + uint64_t value = this->TimelineCounterValue(); + return value; +} + } // namespace grfx } // namespace ppx diff --git a/src/ppx/grfx/vk/vk_device.cpp b/src/ppx/grfx/vk/vk_device.cpp index 042062010..a8fe4bf58 100644 --- a/src/ppx/grfx/vk/vk_device.cpp +++ b/src/ppx/grfx/vk/vk_device.cpp @@ -420,6 +420,12 @@ Result Device::CreateApiObjects(const grfx::DeviceCreateInfo* pCreateInfo) else { mHasTimelineSemaphore = true; } + if (mHasTimelineSemaphore) { + // Load in KHR versions of functions since they'll cover Vulkan 1.1 and later versions + mFnWaitSemaphores = (PFN_vkWaitSemaphoresKHR)vkGetDeviceProcAddr(mDevice, "vkWaitSemaphoresKHR"); + mFnSignalSemaphore = (PFN_vkSignalSemaphoreKHR)vkGetDeviceProcAddr(mDevice, "vkSignalSemaphoreKHR"); + mFnGetSemaphoreCounterValue = (PFN_vkGetSemaphoreCounterValueKHR)vkGetDeviceProcAddr(mDevice, "vkGetSemaphoreCounterValueKHR"); + } PPX_LOG_INFO("Vulkan timeline semaphore is present: " << mHasTimelineSemaphore); #if defined(PPX_VK_EXTENDED_DYNAMIC_STATE) @@ -742,6 +748,24 @@ void Device::ResetQueryPoolEXT( mFnResetQueryPoolEXT(mDevice, queryPool, firstQuery, queryCount); } +VkResult Device::WaitSemaphores(const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout) const +{ + PPX_ASSERT_NULL_ARG(mFnWaitSemaphores); + return mFnWaitSemaphores(mDevice, pWaitInfo, timeout); +} + +VkResult Device::SignalSemaphore(const VkSemaphoreSignalInfo* pSignalInfo) +{ + PPX_ASSERT_NULL_ARG(mFnSignalSemaphore); + return mFnSignalSemaphore(mDevice, pSignalInfo); +} + +VkResult Device::GetSemaphoreCounterValue(VkSemaphore semaphore, uint64_t* pValue) +{ + PPX_ASSERT_NULL_ARG(mFnGetSemaphoreCounterValue); + return mFnGetSemaphoreCounterValue(mDevice, semaphore, pValue); +} + std::array Device::GetAllQueueFamilyIndices() const { return {mGraphicsQueueFamilyIndex, mComputeQueueFamilyIndex, mTransferQueueFamilyIndex}; diff --git a/src/ppx/grfx/vk/vk_queue.cpp b/src/ppx/grfx/vk/vk_queue.cpp index a3887f0b1..d3fe7ca4c 100644 --- a/src/ppx/grfx/vk/vk_queue.cpp +++ b/src/ppx/grfx/vk/vk_queue.cpp @@ -68,6 +68,9 @@ void Queue::DestroyApiObjects() Result Queue::WaitIdle() { + // Synchronized queue access + std::lock_guard lock(mQueueMutex); + VkResult vkres = vkQueueWaitIdle(mQueue); if (vkres != VK_SUCCESS) { PPX_ASSERT_MSG(false, "vkQueueWaitIdle failed" << ToString(vkres)); @@ -99,7 +102,15 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) signalSemaphores.push_back(ToApi(pSubmitInfo->ppSignalSemaphores[i])->GetVkSemaphore()); } + VkTimelineSemaphoreSubmitInfo timelineSubmitInfo = {VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO}; + timelineSubmitInfo.pNext = nullptr; + timelineSubmitInfo.waitSemaphoreValueCount = CountU32(pSubmitInfo->waitValues); + timelineSubmitInfo.pWaitSemaphoreValues = DataPtr(pSubmitInfo->waitValues); + timelineSubmitInfo.signalSemaphoreValueCount = CountU32(pSubmitInfo->signalValues); + timelineSubmitInfo.pSignalSemaphoreValues = DataPtr(pSubmitInfo->signalValues); + VkSubmitInfo vksi = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + vksi.pNext = &timelineSubmitInfo; vksi.waitSemaphoreCount = CountU32(waitSemaphores); vksi.pWaitSemaphores = DataPtr(waitSemaphores); vksi.pWaitDstStageMask = DataPtr(waitDstStageMasks); @@ -114,13 +125,96 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) fence = ToApi(pSubmitInfo->pFence)->GetVkFence(); } - VkResult vkres = vk::QueueSubmit( - mQueue, - 1, - &vksi, - fence); - if (vkres != VK_SUCCESS) { - return ppx::ERROR_API_FAILURE; + // Synchronized queue access + { + std::lock_guard lock(mQueueMutex); + + VkResult vkres = vk::QueueSubmit( + mQueue, + 1, + &vksi, + fence); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + } + + return ppx::SUCCESS; +} + +Result Queue::QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + VkSemaphore semaphoreHandle = ToApi(pSemaphore)->GetVkSemaphore(); + + VkTimelineSemaphoreSubmitInfo timelineSubmitInfo = {VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO}; + timelineSubmitInfo.pNext = nullptr; + timelineSubmitInfo.waitSemaphoreValueCount = 1; + timelineSubmitInfo.pWaitSemaphoreValues = &value; + + VkSubmitInfo vksi = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + vksi.pNext = &timelineSubmitInfo; + vksi.waitSemaphoreCount = 1; + vksi.pWaitSemaphores = &semaphoreHandle; + + // Synchronized queue access + { + std::lock_guard lock(mQueueMutex); + + VkResult vkres = vk::QueueSubmit( + mQueue, + 1, + &vksi, + VK_NULL_HANDLE); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + } + + return ppx::SUCCESS; +} + +Result Queue::QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + VkSemaphore semaphoreHandle = ToApi(pSemaphore)->GetVkSemaphore(); + + VkTimelineSemaphoreSubmitInfo timelineSubmitInfo = {VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO}; + timelineSubmitInfo.pNext = nullptr; + timelineSubmitInfo.signalSemaphoreValueCount = 1; + timelineSubmitInfo.pSignalSemaphoreValues = &value; + + VkSubmitInfo vksi = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + vksi.pNext = &timelineSubmitInfo; + vksi.signalSemaphoreCount = 1; + vksi.pSignalSemaphores = &semaphoreHandle; + + // Synchronized queue access + { + std::lock_guard lock(mQueueMutex); + + VkResult vkres = vk::QueueSubmit( + mQueue, + 1, + &vksi, + VK_NULL_HANDLE); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } } return ppx::SUCCESS; @@ -332,22 +426,34 @@ VkResult Queue::TransitionImageLayout( vkai.commandBufferCount = 1; VkCommandBufferPtr commandBuffer; - VkResult vkres = vkAllocateCommandBuffers( - ToApi(GetDevice())->GetVkDevice(), - &vkai, - &commandBuffer); - if (vkres != VK_SUCCESS) { - PPX_ASSERT_MSG(false, "vkAllocateCommandBuffers failed" << ToString(vkres)); - return vkres; + // Synchronize command pool access + { + std::lock_guard lock(mCommandPoolMutex); + + VkResult vkres = vkAllocateCommandBuffers( + ToApi(GetDevice())->GetVkDevice(), + &vkai, + &commandBuffer); + if (vkres != VK_SUCCESS) { + PPX_ASSERT_MSG(false, "vkAllocateCommandBuffers failed" << ToString(vkres)); + return vkres; + } } + // Save ourselves from having to write a bunch of mutex locks + auto FreeCommandBuffer = [this, &commandBuffer] { + std::lock_guard lock(mCommandPoolMutex); + + vkFreeCommandBuffers(ToApi(this->GetDevice())->GetVkDevice(), this->mTransientPool, 1, commandBuffer); + }; + VkCommandBufferBeginInfo beginInfo = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; beginInfo.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; beginInfo.pInheritanceInfo = nullptr; - vkres = vkBeginCommandBuffer(commandBuffer, &beginInfo); + VkResult vkres = vkBeginCommandBuffer(commandBuffer, &beginInfo); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "vkBeginCommandBuffer failed" << ToString(vkres)); return vkres; } @@ -364,14 +470,14 @@ VkResult Queue::TransitionImageLayout( newLayout, newPipelineStage); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "CmdTransitionImageLayout failed" << ToString(vkres)); return vkres; } vkres = vkEndCommandBuffer(commandBuffer); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "vkEndCommandBuffer failed" << ToString(vkres)); return vkres; } @@ -385,21 +491,27 @@ VkResult Queue::TransitionImageLayout( submitInfo.signalSemaphoreCount = 0; submitInfo.pSignalSemaphores = nullptr; - vkres = vkQueueSubmit(mQueue, 1, &submitInfo, VK_NULL_HANDLE); - if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); - PPX_ASSERT_MSG(false, "vkQueueSubmit failed" << ToString(vkres)); - return vkres; + // Sychronized queue access + { + std::lock_guard lock(mQueueMutex); + + vkres = vkQueueSubmit(mQueue, 1, &submitInfo, VK_NULL_HANDLE); + if (vkres != VK_SUCCESS) { + FreeCommandBuffer(); + PPX_ASSERT_MSG(false, "vkQueueSubmit failed" << ToString(vkres)); + return vkres; + } + + vkres = vkQueueWaitIdle(mQueue); } - vkres = vkQueueWaitIdle(mQueue); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "vkQueueWaitIdle failed" << ToString(vkres)); return vkres; } - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); return VK_SUCCESS; } diff --git a/src/ppx/grfx/vk/vk_sync.cpp b/src/ppx/grfx/vk/vk_sync.cpp index cab2c1257..c2a9dbcdc 100644 --- a/src/ppx/grfx/vk/vk_sync.cpp +++ b/src/ppx/grfx/vk/vk_sync.cpp @@ -16,6 +16,8 @@ #include "ppx/grfx/vk/vk_device.h" #include "ppx/grfx/vk/vk_queue.h" +#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" + namespace ppx { namespace grfx { namespace vk { @@ -86,7 +88,13 @@ Result Fence::Reset() // ------------------------------------------------------------------------------------------------- Result Semaphore::CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) { + VkSemaphoreTypeCreateInfo timelineCreateInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_TYPE_CREATE_INFO}; + timelineCreateInfo.pNext = nullptr; + timelineCreateInfo.semaphoreType = VK_SEMAPHORE_TYPE_TIMELINE; + timelineCreateInfo.initialValue = pCreateInfo->initialValue; + VkSemaphoreCreateInfo vkci = {VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO}; + vkci.pNext = (pCreateInfo->semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE) ? &timelineCreateInfo : nullptr; vkci.flags = 0; VkResult vkres = vkCreateSemaphore( @@ -114,6 +122,62 @@ void Semaphore::DestroyApiObjects() } } +Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + VkSemaphore semaphoreHandle = mSemaphore; + + VkSemaphoreWaitInfo waitInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO}; + waitInfo.pNext = nullptr; + waitInfo.flags = 0; + waitInfo.semaphoreCount = 1; + waitInfo.pSemaphores = &semaphoreHandle; + waitInfo.pValues = &value; + + VkResult vkres = ToApi(GetDevice())->WaitSemaphores(&waitInfo, timeout); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + +Result Semaphore::TimelineSignal(uint64_t value) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + // See header for explanation + if (value > mTimelineSignaledValue) { + VkSemaphore semaphoreHandle = mSemaphore; + + VkSemaphoreSignalInfo signalInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_SIGNAL_INFO}; + signalInfo.pNext = nullptr; + signalInfo.semaphore = mSemaphore; + signalInfo.value = value; + + VkResult vkres = ToApi(GetDevice())->SignalSemaphore(&signalInfo); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + + mTimelineSignaledValue = value; + } + + return ppx::SUCCESS; +} + +uint64_t Semaphore::TimelineCounterValue() const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + uint64_t value = UINT64_MAX; + VkResult vkres = ToApi(GetDevice())->GetSemaphoreCounterValue(mSemaphore, &value); + + // Not clear if value is written to upon failure so prefer safety. + return (vkres == VK_SUCCESS) ? value : UINT64_MAX; +} + } // namespace vk } // namespace grfx } // namespace ppx From d9ee1127be6893b5bb8d57a751d2ec9d30a717f7 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 11 Oct 2023 03:02:58 -0400 Subject: [PATCH 2/9] Fixed formatting --- projects/timeline_semaphore/main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index 629b43f93..dd748f7a8 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -199,7 +199,8 @@ void ProjApp::Render() std::stringstream ss; ss << "Frame: " << GetFrameCount() << "\n"; ss << "FPS: " << std::setw(6) << std::setprecision(6) << GetAverageFPS() << "\n"; - ss << "Timeline semaphores FTW!" << "\n"; + ss << "Timeline semaphores FTW!" + << "\n"; ss << "Timeline value: " << frame.timelineValue; mDynamicText->Clear(); From 7a37b7e5ecd6086cf27e05a33b12ed86bc35830a Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 11 Oct 2023 03:11:33 -0400 Subject: [PATCH 3/9] Added missing header --- projects/timeline_semaphore/main.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index dd748f7a8..ff915f055 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -16,6 +16,8 @@ #include "ppx/camera.h" using namespace ppx; +#include + #if defined(USE_DX12) const grfx::Api kApi = grfx::API_DX_12_0; #elif defined(USE_VK) From 10b3a0feedba63bf537a344337c1f4546f67f1ab Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Sun, 15 Oct 2023 22:55:30 -0400 Subject: [PATCH 4/9] Addressed feed back in pr#315 - Fixed typos and errors in comments - Fixed copyright year - Moved timeline asserts from API level to grfx level --- include/ppx/grfx/grfx_sync.h | 4 ++-- include/ppx/grfx/vk/vk_sync.h | 6 +++--- projects/timeline_semaphore/CMakeLists.txt | 2 +- projects/timeline_semaphore/main.cpp | 4 ++-- src/ppx/grfx/dx12/dx12_sync.cpp | 9 +-------- src/ppx/grfx/grfx_sync.cpp | 5 +++++ src/ppx/grfx/vk/vk_sync.cpp | 8 -------- 7 files changed, 14 insertions(+), 24 deletions(-) diff --git a/include/ppx/grfx/grfx_sync.h b/include/ppx/grfx/grfx_sync.h index a0e1f674e..76d90c561 100644 --- a/include/ppx/grfx/grfx_sync.h +++ b/include/ppx/grfx/grfx_sync.h @@ -79,8 +79,8 @@ class Semaphore // Timeline semaphore signal // - // WARNING: Signaling a value less that's less than what's already been signaled - // can cause a block or a race condition. + // WARNING: Signaling a value less than what's already been signaled can + // cause a block or a race condition. // // Use forceMonotonicValue=true to use the current timeline semaphore value // if it's greater than the passed in value. This is useful when signaling diff --git a/include/ppx/grfx/vk/vk_sync.h b/include/ppx/grfx/vk/vk_sync.h index 16b68f34c..23d5616d0 100644 --- a/include/ppx/grfx/vk/vk_sync.h +++ b/include/ppx/grfx/vk/vk_sync.h @@ -79,11 +79,11 @@ class Semaphore // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkSemaphoreSignalInfo.html#VUID-VkSemaphoreSignalInfo-value-03258 // // This is unfortunate, because there are cases where an application - // may need to signal a valuet hat is equal to what's been signaled. + // may need to signal a value that is equal to what's been signaled. // // Even though it's possible to get the current value, add 1 to it, - // and then signal it - this creates can create a different problem - // where a value is signaled too soon and a write-after-read hazard + // and then signal it - this can create a different problem where a value + // is signaled too soon and a write-after-read hazard // possibly gets introduced. // mutable uint64_t mTimelineSignaledValue = 0; diff --git a/projects/timeline_semaphore/CMakeLists.txt b/projects/timeline_semaphore/CMakeLists.txt index 52ae0efb1..1d8278655 100644 --- a/projects/timeline_semaphore/CMakeLists.txt +++ b/projects/timeline_semaphore/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright 2022 Google LLC +# Copyright 2023 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index ff915f055..841f60388 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -1,4 +1,4 @@ -// Copyright 2022 Google LLC +// Copyright 2023 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -55,7 +55,7 @@ void ProjApp::Config(ppx::ApplicationSettings& settings) settings.appName = "timeline semaphore"; settings.enableImGui = true; settings.grfx.api = kApi; - settings.grfx.enableDebug = true; + settings.grfx.enableDebug = false; } void ProjApp::Setup() diff --git a/src/ppx/grfx/dx12/dx12_sync.cpp b/src/ppx/grfx/dx12/dx12_sync.cpp index 535e4c799..6475ee65f 100644 --- a/src/ppx/grfx/dx12/dx12_sync.cpp +++ b/src/ppx/grfx/dx12/dx12_sync.cpp @@ -15,8 +15,7 @@ #include "ppx/grfx/dx12/dx12_sync.h" #include "ppx/grfx/dx12/dx12_device.h" -#define REQUIRES_BINARY_MSG "invalid semaphore type: operation requires binary semaphore" -#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" +#define REQUIRES_BINARY_MSG "invalid semaphore type: operation requires binary semaphore" namespace ppx { namespace grfx { @@ -145,8 +144,6 @@ UINT64 Semaphore::GetWaitForValue() const Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - UINT64 completedValue = mFence->GetCompletedValue(); if (completedValue < value) { mFence->SetEventOnCompletion(value, mWaitEventHandle); @@ -160,8 +157,6 @@ Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const Result Semaphore::TimelineSignal(uint64_t value) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - HRESULT hr = mFence->Signal(static_cast(value)); if (FAILED(hr)) { return ppx::ERROR_API_FAILURE; @@ -172,8 +167,6 @@ Result Semaphore::TimelineSignal(uint64_t value) const uint64_t Semaphore::TimelineCounterValue() const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - UINT64 value = mFence->GetCompletedValue(); return static_cast(value); } diff --git a/src/ppx/grfx/grfx_sync.cpp b/src/ppx/grfx/grfx_sync.cpp index f58b5f894..8f89d77b1 100644 --- a/src/ppx/grfx/grfx_sync.cpp +++ b/src/ppx/grfx/grfx_sync.cpp @@ -14,6 +14,8 @@ #include "ppx/grfx/grfx_sync.h" +#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" + namespace ppx { namespace grfx { @@ -41,6 +43,7 @@ Result Fence::WaitAndReset(uint64_t timeout) Result Semaphore::Wait(uint64_t value, uint64_t timeout) const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; } @@ -55,6 +58,7 @@ Result Semaphore::Wait(uint64_t value, uint64_t timeout) const Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; } @@ -77,6 +81,7 @@ Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const uint64_t Semaphore::GetCounterValue() const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return UINT64_MAX; } diff --git a/src/ppx/grfx/vk/vk_sync.cpp b/src/ppx/grfx/vk/vk_sync.cpp index c2a9dbcdc..72524b15f 100644 --- a/src/ppx/grfx/vk/vk_sync.cpp +++ b/src/ppx/grfx/vk/vk_sync.cpp @@ -16,8 +16,6 @@ #include "ppx/grfx/vk/vk_device.h" #include "ppx/grfx/vk/vk_queue.h" -#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" - namespace ppx { namespace grfx { namespace vk { @@ -124,8 +122,6 @@ void Semaphore::DestroyApiObjects() Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - VkSemaphore semaphoreHandle = mSemaphore; VkSemaphoreWaitInfo waitInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO}; @@ -145,8 +141,6 @@ Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const Result Semaphore::TimelineSignal(uint64_t value) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - // See header for explanation if (value > mTimelineSignaledValue) { VkSemaphore semaphoreHandle = mSemaphore; @@ -169,8 +163,6 @@ Result Semaphore::TimelineSignal(uint64_t value) const uint64_t Semaphore::TimelineCounterValue() const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - uint64_t value = UINT64_MAX; VkResult vkres = ToApi(GetDevice())->GetSemaphoreCounterValue(mSemaphore, &value); From ef8ad4f8b9630bcbc3217b35b9a738cb07750d21 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 11 Oct 2023 02:59:57 -0400 Subject: [PATCH 5/9] Timeline semaphores This CL adds timeline semaphore support for both APIs. grfx::Semaphore is modified to support both binary and timeline semaphores similar to how Vulkan does it. This CL also makes the Vulkan queues thread safe. - Modified vk::Queue to be thread safe. - Added timeline semaphore support for Vulkan and D3D12. - Added sample demonstrating timeline semaphore. --- include/ppx/config.h | 1 + include/ppx/grfx/dx12/dx12_queue.h | 3 + include/ppx/grfx/dx12/dx12_sync.h | 8 +- include/ppx/grfx/grfx_enums.h | 6 + include/ppx/grfx/grfx_queue.h | 6 + include/ppx/grfx/grfx_sync.h | 33 +++ include/ppx/grfx/vk/vk_device.h | 7 + include/ppx/grfx/vk/vk_queue.h | 5 + include/ppx/grfx/vk/vk_sync.h | 25 +- projects/CMakeLists.txt | 1 + projects/timeline_semaphore/CMakeLists.txt | 22 ++ projects/timeline_semaphore/README.md | 9 + projects/timeline_semaphore/main.cpp | 325 +++++++++++++++++++++ src/ppx/grfx/dx12/dx12_queue.cpp | 58 +++- src/ppx/grfx/dx12/dx12_sync.cpp | 67 ++++- src/ppx/grfx/grfx_sync.cpp | 55 ++++ src/ppx/grfx/vk/vk_device.cpp | 24 ++ src/ppx/grfx/vk/vk_queue.cpp | 164 +++++++++-- src/ppx/grfx/vk/vk_sync.cpp | 64 ++++ 19 files changed, 848 insertions(+), 35 deletions(-) create mode 100644 projects/timeline_semaphore/CMakeLists.txt create mode 100644 projects/timeline_semaphore/README.md create mode 100644 projects/timeline_semaphore/main.cpp diff --git a/include/ppx/config.h b/include/ppx/config.h index a51562cfc..25922637a 100644 --- a/include/ppx/config.h +++ b/include/ppx/config.h @@ -146,6 +146,7 @@ enum Result ERROR_GRFX_INVALID_BINDING_NUMBER = -1024, ERROR_GRFX_INVALID_SET_NUMBER = -1025, ERROR_GRFX_OPERATION_NOT_PERMITTED = -1026, + ERROR_GRFX_INVALID_SEMAPHORE_TYPE = -1027, ERROR_IMAGE_FILE_LOAD_FAILED = -2000, ERROR_IMAGE_FILE_SAVE_FAILED = -2001, diff --git a/include/ppx/grfx/dx12/dx12_queue.h b/include/ppx/grfx/dx12/dx12_queue.h index a46329e50..ad8549793 100644 --- a/include/ppx/grfx/dx12/dx12_queue.h +++ b/include/ppx/grfx/dx12/dx12_queue.h @@ -35,6 +35,9 @@ class Queue virtual Result Submit(const grfx::SubmitInfo* pSubmitInfo) override; + virtual Result QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result GetTimestampFrequency(uint64_t* pFrequency) const override; protected: diff --git a/include/ppx/grfx/dx12/dx12_sync.h b/include/ppx/grfx/dx12/dx12_sync.h index dc402945a..1f2595453 100644 --- a/include/ppx/grfx/dx12/dx12_sync.h +++ b/include/ppx/grfx/dx12/dx12_sync.h @@ -61,13 +61,19 @@ class Semaphore UINT64 GetNextSignalValue(); UINT64 GetWaitForValue() const; +private: + virtual Result TimelineWait(uint64_t value, uint64_t timeout) const override; + virtual Result TimelineSignal(uint64_t value) const override; + virtual uint64_t TimelineCounterValue() const override; + protected: virtual Result CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) override; virtual void DestroyApiObjects() override; private: D3D12FencePtr mFence; - UINT64 mValue = 0; + HANDLE mWaitEventHandle = nullptr; + UINT64 mValue = 0; }; } // namespace dx12 diff --git a/include/ppx/grfx/grfx_enums.h b/include/ppx/grfx/grfx_enums.h index 807b20352..e1d1b0f59 100644 --- a/include/ppx/grfx/grfx_enums.h +++ b/include/ppx/grfx/grfx_enums.h @@ -441,6 +441,12 @@ enum SampleCount SAMPLE_COUNT_64 = 64, }; +enum SemaphoreType +{ + SEMAPHORE_TYPE_BINARY = 0, + SEMAPHORE_TYPE_TIMELINE = 1, +}; + enum ShaderStageBits { SHADER_STAGE_UNDEFINED = 0x00000000, diff --git a/include/ppx/grfx/grfx_queue.h b/include/ppx/grfx/grfx_queue.h index 048ec8cc1..ef8939d87 100644 --- a/include/ppx/grfx/grfx_queue.h +++ b/include/ppx/grfx/grfx_queue.h @@ -27,8 +27,10 @@ struct SubmitInfo const grfx::CommandBuffer* const* ppCommandBuffers = nullptr; uint32_t waitSemaphoreCount = 0; const grfx::Semaphore* const* ppWaitSemaphores = nullptr; + std::vector waitValues = {}; // Use 0 if index is binary semaphore uint32_t signalSemaphoreCount = 0; grfx::Semaphore** ppSignalSemaphores = nullptr; + std::vector signalValues = {}; // Use 0 if index is binary smeaphore grfx::Fence* pFence = nullptr; }; @@ -63,6 +65,10 @@ class Queue virtual Result Submit(const grfx::SubmitInfo* pSubmitInfo) = 0; + // Timeline semaphore functions + virtual Result QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) = 0; + virtual Result QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) = 0; + // GPU timestamp frequency counter in ticks per second virtual Result GetTimestampFrequency(uint64_t* pFrequency) const = 0; diff --git a/include/ppx/grfx/grfx_sync.h b/include/ppx/grfx/grfx_sync.h index b2327d167..a0e1f674e 100644 --- a/include/ppx/grfx/grfx_sync.h +++ b/include/ppx/grfx/grfx_sync.h @@ -56,6 +56,8 @@ class Fence //! struct SemaphoreCreateInfo { + grfx::SemaphoreType semaphoreType = grfx::SEMAPHORE_TYPE_BINARY; + uint64_t initialValue = 0; // Timeline semaphore only }; //! @class Semaphore @@ -68,12 +70,43 @@ class Semaphore Semaphore() {} virtual ~Semaphore() {} + grfx::SemaphoreType GetSemaphoreType() const { return mCreateInfo.semaphoreType; } + bool IsBinary() const { return mCreateInfo.semaphoreType == grfx::SEMAPHORE_TYPE_BINARY; } + bool IsTimeline() const { return mCreateInfo.semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE; } + + // Timeline semaphore wait + Result Wait(uint64_t value, uint64_t timeout = UINT64_MAX) const; + + // Timeline semaphore signal + // + // WARNING: Signaling a value less that's less than what's already been signaled + // can cause a block or a race condition. + // + // Use forceMonotonicValue=true to use the current timeline semaphore value + // if it's greater than the passed in value. This is useful when signaling + // from threads where ordering is not guaranteed. + // + Result Signal(uint64_t value, bool forceMonotonicValue = false) const; + + // Returns current timeline semaphore value + uint64_t GetCounterValue() const; + +private: + virtual Result TimelineWait(uint64_t value, uint64_t timeout) const = 0; + virtual Result TimelineSignal(uint64_t value) const = 0; + virtual uint64_t TimelineCounterValue() const = 0; + protected: virtual Result CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) = 0; virtual void DestroyApiObjects() = 0; friend class grfx::Device; + +private: + mutable std::mutex mTimelineMutex; }; +// ------------------------------------------------------------------------------------------------- + } // namespace grfx } // namespace ppx diff --git a/include/ppx/grfx/vk/vk_device.h b/include/ppx/grfx/vk/vk_device.h index d16397d96..35069ca4c 100644 --- a/include/ppx/grfx/vk/vk_device.h +++ b/include/ppx/grfx/vk/vk_device.h @@ -50,6 +50,10 @@ class Device uint32_t firstQuery, uint32_t queryCount) const; + VkResult WaitSemaphores(const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout) const; + VkResult SignalSemaphore(const VkSemaphoreSignalInfo* pSignalInfo); + VkResult GetSemaphoreCounterValue(VkSemaphore semaphore, uint64_t* pValue); + uint32_t GetGraphicsQueueFamilyIndex() const { return mGraphicsQueueFamilyIndex; } uint32_t GetComputeQueueFamilyIndex() const { return mComputeQueueFamilyIndex; } uint32_t GetTransferQueueFamilyIndex() const { return mTransferQueueFamilyIndex; } @@ -113,6 +117,9 @@ class Device bool mHasUnrestrictedDepthRange = false; bool mHasDynamicRendering = false; PFN_vkResetQueryPoolEXT mFnResetQueryPoolEXT = nullptr; + PFN_vkWaitSemaphores mFnWaitSemaphores = nullptr; + PFN_vkSignalSemaphore mFnSignalSemaphore = nullptr; + PFN_vkGetSemaphoreCounterValue mFnGetSemaphoreCounterValue = nullptr; uint32_t mGraphicsQueueFamilyIndex = 0; uint32_t mComputeQueueFamilyIndex = 0; uint32_t mTransferQueueFamilyIndex = 0; diff --git a/include/ppx/grfx/vk/vk_queue.h b/include/ppx/grfx/vk/vk_queue.h index 8e9c38797..f6fc6163c 100644 --- a/include/ppx/grfx/vk/vk_queue.h +++ b/include/ppx/grfx/vk/vk_queue.h @@ -37,6 +37,9 @@ class Queue virtual Result Submit(const grfx::SubmitInfo* pSubmitInfo) override; + virtual Result QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) override; + virtual Result GetTimestampFrequency(uint64_t* pFrequency) const override; VkResult TransitionImageLayout( @@ -57,6 +60,8 @@ class Queue private: VkQueuePtr mQueue; VkCommandPoolPtr mTransientPool; + std::mutex mQueueMutex; + std::mutex mCommandPoolMutex; }; } // namespace vk diff --git a/include/ppx/grfx/vk/vk_sync.h b/include/ppx/grfx/vk/vk_sync.h index cfea57117..16b68f34c 100644 --- a/include/ppx/grfx/vk/vk_sync.h +++ b/include/ppx/grfx/vk/vk_sync.h @@ -59,12 +59,35 @@ class Semaphore VkSemaphorePtr GetVkSemaphore() const { return mSemaphore; } +private: + virtual Result TimelineWait(uint64_t value, uint64_t timeout) const override; + virtual Result TimelineSignal(uint64_t value) const override; + virtual uint64_t TimelineCounterValue() const override; + protected: virtual Result CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) override; virtual void DestroyApiObjects() override; private: - VkSemaphorePtr mSemaphore; + // + // Why are we storing timeline semaphore signaled values? + // + // Direct3D allows fence objects to signal a value if the value is + // equal to or greater than what's already been signaled. + // + // Vulkan does not: + // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkSemaphoreSignalInfo.html#VUID-VkSemaphoreSignalInfo-value-03258 + // + // This is unfortunate, because there are cases where an application + // may need to signal a valuet hat is equal to what's been signaled. + // + // Even though it's possible to get the current value, add 1 to it, + // and then signal it - this creates can create a different problem + // where a value is signaled too soon and a write-after-read hazard + // possibly gets introduced. + // + mutable uint64_t mTimelineSignaledValue = 0; + VkSemaphorePtr mSemaphore; }; } // namespace vk diff --git a/projects/CMakeLists.txt b/projects/CMakeLists.txt index 8a752c9fc..d99fbfb3a 100644 --- a/projects/CMakeLists.txt +++ b/projects/CMakeLists.txt @@ -49,6 +49,7 @@ add_subdirectory(alloc) add_subdirectory(fishtornado) add_subdirectory(fluid_simulation) add_subdirectory(oit_demo) +add_subdirectory(timeline_semaphore) if (PPX_BUILD_XR) add_subdirectory(cube_xr) diff --git a/projects/timeline_semaphore/CMakeLists.txt b/projects/timeline_semaphore/CMakeLists.txt new file mode 100644 index 000000000..52ae0efb1 --- /dev/null +++ b/projects/timeline_semaphore/CMakeLists.txt @@ -0,0 +1,22 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +cmake_minimum_required(VERSION 3.0 FATAL_ERROR) + +project(timeline_semaphore) + +add_samples_for_all_apis( + NAME ${PROJECT_NAME} + SOURCES "main.cpp" + SHADER_DEPENDENCIES + "shader_text_draw") diff --git a/projects/timeline_semaphore/README.md b/projects/timeline_semaphore/README.md new file mode 100644 index 000000000..03df03e68 --- /dev/null +++ b/projects/timeline_semaphore/README.md @@ -0,0 +1,9 @@ +# Timeline semaphore + +Example of how to use timeline semaphores. + +## Shaders + +Shader | Purpose for this project +--------------- | ---------------------------- +`TextDraw.hlsl` | Draw colored text. diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp new file mode 100644 index 000000000..629b43f93 --- /dev/null +++ b/projects/timeline_semaphore/main.cpp @@ -0,0 +1,325 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "ppx/ppx.h" +#include "ppx/camera.h" +using namespace ppx; + +#if defined(USE_DX12) +const grfx::Api kApi = grfx::API_DX_12_0; +#elif defined(USE_VK) +const grfx::Api kApi = grfx::API_VK_1_1; +#endif + +class ProjApp + : public ppx::Application +{ +public: + virtual void Config(ppx::ApplicationSettings& settings) override; + virtual void Setup() override; + virtual void Render() override; + +private: + struct PerFrame + { + grfx::CommandBufferPtr drawTextCmd; + grfx::CommandBufferPtr drawImGuiCmd; + grfx::SemaphorePtr imageAcquiredSemaphore; + grfx::FencePtr imageAcquiredFence; + grfx::SemaphorePtr presentReadySemaphore; // Binary semaphore + + grfx::SemaphorePtr timelineSemaphore; + uint64_t timelineValue = 0; + }; + std::vector mPerFrame; + grfx::TextureFontPtr mRoboto; + grfx::TextDrawPtr mDynamicText; + PerspCamera mCamera; +}; + +void ProjApp::Config(ppx::ApplicationSettings& settings) +{ + settings.appName = "timeline semaphore"; + settings.enableImGui = true; + settings.grfx.api = kApi; + settings.grfx.enableDebug = true; +} + +void ProjApp::Setup() +{ + mCamera = PerspCamera(GetWindowWidth(), GetWindowHeight()); + + // Per frame data + { + PerFrame frame = {}; + + // Command buffers + PPX_CHECKED_CALL(GetGraphicsQueue()->CreateCommandBuffer(&frame.drawTextCmd)); + PPX_CHECKED_CALL(GetGraphicsQueue()->CreateCommandBuffer(&frame.drawImGuiCmd)); + + // Defaults to binary semaphore + grfx::SemaphoreCreateInfo semaCreateInfo = {}; + PPX_CHECKED_CALL(GetDevice()->CreateSemaphore(&semaCreateInfo, &frame.imageAcquiredSemaphore)); + + grfx::FenceCreateInfo fenceCreateInfo = {}; + PPX_CHECKED_CALL(GetDevice()->CreateFence(&fenceCreateInfo, &frame.imageAcquiredFence)); + + PPX_CHECKED_CALL(GetDevice()->CreateSemaphore(&semaCreateInfo, &frame.presentReadySemaphore)); + + // Change create info for timeline semaphore + semaCreateInfo = {}; + semaCreateInfo.semaphoreType = grfx::SEMAPHORE_TYPE_TIMELINE; + semaCreateInfo.initialValue = 0; + PPX_CHECKED_CALL(GetDevice()->CreateSemaphore(&semaCreateInfo, &frame.timelineSemaphore)); + + mPerFrame.push_back(frame); + } + + // Texture font + { + ppx::Font font; + PPX_CHECKED_CALL(ppx::Font::CreateFromFile(GetAssetPath("basic/fonts/Roboto/Roboto-Regular.ttf"), &font)); + + grfx::TextureFontCreateInfo createInfo = {}; + createInfo.font = font; + createInfo.size = 48.0f; + createInfo.characters = grfx::TextureFont::GetDefaultCharacters(); + + PPX_CHECKED_CALL(GetDevice()->CreateTextureFont(&createInfo, &mRoboto)); + } + + // Text draw + { + grfx::ShaderModulePtr VS; + grfx::ShaderModulePtr PS; + + std::vector bytecode = LoadShader("basic/shaders", "TextDraw.vs"); + PPX_ASSERT_MSG(!bytecode.empty(), "VS shader bytecode load failed"); + grfx::ShaderModuleCreateInfo shaderCreateInfo = {static_cast(bytecode.size()), bytecode.data()}; + PPX_CHECKED_CALL(GetDevice()->CreateShaderModule(&shaderCreateInfo, &VS)); + + bytecode = LoadShader("basic/shaders", "TextDraw.ps"); + PPX_ASSERT_MSG(!bytecode.empty(), "PS shader bytecode load failed"); + shaderCreateInfo = {static_cast(bytecode.size()), bytecode.data()}; + PPX_CHECKED_CALL(GetDevice()->CreateShaderModule(&shaderCreateInfo, &PS)); + + grfx::TextDrawCreateInfo createInfo = {}; + createInfo.pFont = mRoboto; + createInfo.maxTextLength = 4096; + createInfo.VS = {VS.Get(), "vsmain"}; + createInfo.PS = {PS.Get(), "psmain"}; + createInfo.renderTargetFormat = GetSwapchain()->GetColorFormat(); + + PPX_CHECKED_CALL(GetDevice()->CreateTextDraw(&createInfo, &mDynamicText)); + + GetDevice()->DestroyShaderModule(VS); + GetDevice()->DestroyShaderModule(PS); + } +} + +void ProjApp::Render() +{ + PerFrame& frame = mPerFrame[0]; + + grfx::SwapchainPtr swapchain = GetSwapchain(); + + uint32_t imageIndex = UINT32_MAX; + PPX_CHECKED_CALL(swapchain->AcquireNextImage(UINT64_MAX, frame.imageAcquiredSemaphore, frame.imageAcquiredFence, &imageIndex)); + + // Wait for and reset image acquired fence + PPX_CHECKED_CALL(frame.imageAcquiredFence->WaitAndReset()); + + // Wait for previous frame's render to complete (GPU signal to CPU wait) + PPX_CHECKED_CALL(frame.timelineSemaphore->Wait(frame.timelineValue)); + + // Spawn a thread that will spawn other threads to signal values on the CPU + std::unique_ptr spawnerThread; + { + const uint32_t kNumThreads = 4; + grfx::Semaphore* pSemaphore = frame.timelineSemaphore; + + // Normally, we increment after a wait and before the next signal so we need to add 1. + const uint64_t startSignalValue = frame.timelineValue + 1; + + spawnerThread = std::unique_ptr( + new std::thread([kNumThreads, pSemaphore, startSignalValue]() { + std::vector> signalThreads; + + // Create signaling threads + for (uint32_t i = 0; i < kNumThreads; ++i) { + const uint64_t signalValue = startSignalValue + i; + + auto signalThread = std::unique_ptr( + new std::thread([pSemaphore, signalValue] { + PPX_CHECKED_CALL(pSemaphore->Signal(signalValue, true)); + })); + + signalThreads.push_back(std::move(signalThread)); + } + + // Join threads + for (auto& thread : signalThreads) { + thread->join(); + } + signalThreads.clear(); + })); + + // Increment to account signaling thread values + frame.timelineValue += kNumThreads; + } + + // Wait on primary for secondary threads to signal on the CPU (CPU signals to CPU wait) + PPX_CHECKED_CALL(frame.timelineSemaphore->Wait(frame.timelineValue)); + + // Join spawner thread + spawnerThread->join(); + spawnerThread.reset(); + + // Signal values for text draw start and finish + const uint64_t drawTextStartSignalValue = ++frame.timelineValue; + const uint64_t drawTextFinishSignalValue = ++frame.timelineValue; + + // Queue the text draw but don't start until the CPU signals (CPU signal to GPU wait) + { + PPX_CHECKED_CALL(frame.drawTextCmd->Begin()); + { + // Prepare string outside of render pass + { + std::stringstream ss; + ss << "Frame: " << GetFrameCount() << "\n"; + ss << "FPS: " << std::setw(6) << std::setprecision(6) << GetAverageFPS() << "\n"; + ss << "Timeline semaphores FTW!" << "\n"; + ss << "Timeline value: " << frame.timelineValue; + + mDynamicText->Clear(); + mDynamicText->AddString(float2(15, 50), ss.str()); + mDynamicText->UploadToGpu(frame.drawTextCmd); + mDynamicText->PrepareDraw(mCamera.GetViewProjectionMatrix(), frame.drawTextCmd); + } + + // ------------------------------------------------------------------------------------- + + grfx::RenderPassPtr renderPass = swapchain->GetRenderPass(imageIndex); + PPX_ASSERT_MSG(!renderPass.IsNull(), "render pass object is null"); + + grfx::RenderPassBeginInfo beginInfo = {}; + beginInfo.pRenderPass = renderPass; + beginInfo.renderArea = renderPass->GetRenderArea(); + beginInfo.RTVClearCount = 1; + beginInfo.RTVClearValues[0] = {{0.25f, 0.3f, 0.33f, 1}}; + + frame.drawTextCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_PRESENT, grfx::RESOURCE_STATE_RENDER_TARGET); + frame.drawTextCmd->BeginRenderPass(&beginInfo); + { + grfx::Rect scissorRect = renderPass->GetScissor(); + grfx::Viewport viewport = renderPass->GetViewport(); + frame.drawTextCmd->SetScissors(1, &scissorRect); + frame.drawTextCmd->SetViewports(1, &viewport); + + mDynamicText->Draw(frame.drawTextCmd); + } + frame.drawTextCmd->EndRenderPass(); + frame.drawTextCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_RENDER_TARGET, grfx::RESOURCE_STATE_PRESENT); + } + PPX_CHECKED_CALL(frame.drawTextCmd->End()); + + std::vector waitSemaphores = {frame.imageAcquiredSemaphore, frame.timelineSemaphore}; + std::vector waitValues = {0, drawTextStartSignalValue}; + + std::vector signalSemaphores = {frame.timelineSemaphore}; + std::vector signalValues = {drawTextFinishSignalValue}; + + grfx::SubmitInfo submitInfo = {}; + submitInfo.commandBufferCount = 1; + submitInfo.ppCommandBuffers = &frame.drawTextCmd; + submitInfo.waitSemaphoreCount = CountU32(waitSemaphores); + submitInfo.ppWaitSemaphores = DataPtr(waitSemaphores); + submitInfo.waitValues = waitValues; + submitInfo.signalSemaphoreCount = CountU32(signalSemaphores); + submitInfo.ppSignalSemaphores = DataPtr(signalSemaphores); + submitInfo.signalValues = signalValues; + submitInfo.pFence = nullptr; + + PPX_CHECKED_CALL(GetGraphicsQueue()->Submit(&submitInfo)); + } + + // Spawn a thread to signal on the CPU to kick off the text draw + { + grfx::Semaphore* pSemaphore = frame.timelineSemaphore; + + spawnerThread = std::unique_ptr( + new std::thread([pSemaphore, drawTextStartSignalValue]() { + PPX_CHECKED_CALL(pSemaphore->Signal(drawTextStartSignalValue)); + })); + + spawnerThread->join(); + spawnerThread.reset(); + } + + // Queue ImGui draw but wait on the text draw to finish (GPU signal to GPU wait) + { + PPX_CHECKED_CALL(frame.drawImGuiCmd->Begin()); + { + grfx::RenderPassPtr renderPass = swapchain->GetRenderPass(imageIndex, grfx::ATTACHMENT_LOAD_OP_LOAD); + PPX_ASSERT_MSG(!renderPass.IsNull(), "render pass object is null"); + + grfx::RenderPassBeginInfo beginInfo = {}; + beginInfo.pRenderPass = renderPass; + beginInfo.renderArea = renderPass->GetRenderArea(); + + frame.drawImGuiCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_PRESENT, grfx::RESOURCE_STATE_RENDER_TARGET); + frame.drawImGuiCmd->BeginRenderPass(&beginInfo); + { + grfx::Rect scissorRect = renderPass->GetScissor(); + grfx::Viewport viewport = renderPass->GetViewport(); + frame.drawImGuiCmd->SetScissors(1, &scissorRect); + frame.drawImGuiCmd->SetViewports(1, &viewport); + + // Draw ImGui + DrawDebugInfo(); + DrawImGui(frame.drawImGuiCmd); + } + frame.drawImGuiCmd->EndRenderPass(); + frame.drawImGuiCmd->TransitionImageLayout(renderPass->GetRenderTargetImage(0), PPX_ALL_SUBRESOURCES, grfx::RESOURCE_STATE_RENDER_TARGET, grfx::RESOURCE_STATE_PRESENT); + } + PPX_CHECKED_CALL(frame.drawImGuiCmd->End()); + + // Wait for text daw to finish + std::vector waitSemaphores = {frame.timelineSemaphore}; + std::vector waitValues = {drawTextFinishSignalValue}; + + // Signal value for render work complete + ++frame.timelineValue; + + std::vector signalSemaphores = {frame.presentReadySemaphore, frame.timelineSemaphore}; + std::vector signalValues = {0, frame.timelineValue}; + + grfx::SubmitInfo submitInfo = {}; + submitInfo.commandBufferCount = 1; + submitInfo.ppCommandBuffers = &frame.drawImGuiCmd; + submitInfo.waitSemaphoreCount = CountU32(waitSemaphores); + submitInfo.ppWaitSemaphores = DataPtr(waitSemaphores); + submitInfo.waitValues = waitValues; + submitInfo.signalSemaphoreCount = CountU32(signalSemaphores); + submitInfo.ppSignalSemaphores = DataPtr(signalSemaphores); + submitInfo.signalValues = signalValues; + submitInfo.pFence = nullptr; + + PPX_CHECKED_CALL(GetGraphicsQueue()->Submit(&submitInfo)); + } + + PPX_CHECKED_CALL(swapchain->Present(imageIndex, 1, &frame.presentReadySemaphore)); +} + +SETUP_APPLICATION(ProjApp) \ No newline at end of file diff --git a/src/ppx/grfx/dx12/dx12_queue.cpp b/src/ppx/grfx/dx12/dx12_queue.cpp index 46255de7e..99e24ed34 100644 --- a/src/ppx/grfx/dx12/dx12_queue.cpp +++ b/src/ppx/grfx/dx12/dx12_queue.cpp @@ -82,10 +82,13 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) mListBuffer[i] = ToApi(pSubmitInfo->ppCommandBuffers[i])->GetDxCommandList(); } + // Wait semaphores for (uint32_t i = 0; i < pSubmitInfo->waitSemaphoreCount; ++i) { - ID3D12Fence* pDxFence = ToApi(pSubmitInfo->ppWaitSemaphores[i])->GetDxFence(); - UINT64 value = ToApi(pSubmitInfo->ppWaitSemaphores[i])->GetWaitForValue(); - HRESULT hr = mCommandQueue->Wait(pDxFence, value); + auto pSemaphore = ToApi(pSubmitInfo->ppWaitSemaphores[i]); + ID3D12Fence* pDxFence = pSemaphore->GetDxFence(); + UINT64 value = pSemaphore->IsTimeline() ? pSubmitInfo->waitValues[i] : pSemaphore->GetWaitForValue(); + + HRESULT hr = mCommandQueue->Wait(pDxFence, value); if (FAILED(hr)) { PPX_ASSERT_MSG(false, "ID3D12CommandQueue::Wait failed"); return ppx::ERROR_API_FAILURE; @@ -96,10 +99,13 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) static_cast(pSubmitInfo->commandBufferCount), mListBuffer.data()); + // Signal semaphores for (uint32_t i = 0; i < pSubmitInfo->signalSemaphoreCount; ++i) { - ID3D12Fence* pDxFence = ToApi(pSubmitInfo->ppSignalSemaphores[i])->GetDxFence(); - UINT64 value = ToApi(pSubmitInfo->ppSignalSemaphores[i])->GetNextSignalValue(); - HRESULT hr = mCommandQueue->Signal(pDxFence, value); + auto pSemaphore = ToApi(pSubmitInfo->ppSignalSemaphores[i]); + ID3D12Fence* pDxFence = pSemaphore->GetDxFence(); + UINT64 value = pSemaphore->IsTimeline() ? pSubmitInfo->signalValues[i] : pSemaphore->GetNextSignalValue(); + + HRESULT hr = mCommandQueue->Signal(pDxFence, value); if (FAILED(hr)) { PPX_ASSERT_MSG(false, "ID3D12CommandQueue::Signal failed"); return ppx::ERROR_API_FAILURE; @@ -119,6 +125,46 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) return ppx::SUCCESS; } +Result Queue::QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + HRESULT hr = mCommandQueue->Wait( + ToApi(pSemaphore)->GetDxFence(), + static_cast(value)); + if (FAILED(hr)) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + +Result Queue::QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + HRESULT hr = mCommandQueue->Signal( + ToApi(pSemaphore)->GetDxFence(), + static_cast(value)); + if (FAILED(hr)) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + Result Queue::GetTimestampFrequency(uint64_t* pFrequency) const { if (IsNull(pFrequency)) { diff --git a/src/ppx/grfx/dx12/dx12_sync.cpp b/src/ppx/grfx/dx12/dx12_sync.cpp index f3676a4c7..535e4c799 100644 --- a/src/ppx/grfx/dx12/dx12_sync.cpp +++ b/src/ppx/grfx/dx12/dx12_sync.cpp @@ -15,6 +15,9 @@ #include "ppx/grfx/dx12/dx12_sync.h" #include "ppx/grfx/dx12/dx12_device.h" +#define REQUIRES_BINARY_MSG "invalid semaphore type: operation requires binary semaphore" +#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" + namespace ppx { namespace grfx { namespace dx12 { @@ -83,20 +86,37 @@ Result Fence::Reset() // ------------------------------------------------------------------------------------------------- Result Semaphore::CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) { + UINT64 initialValue = mValue; + if (pCreateInfo->semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE) { + initialValue = static_cast(pCreateInfo->initialValue); + } + D3D12_FENCE_FLAGS flags = D3D12_FENCE_FLAG_NONE; - HRESULT hr = ToApi(GetDevice())->GetDxDevice()->CreateFence(mValue, flags, IID_PPV_ARGS(&mFence)); + HRESULT hr = ToApi(GetDevice())->GetDxDevice()->CreateFence(initialValue, flags, IID_PPV_ARGS(&mFence)); if (FAILED(hr)) { PPX_ASSERT_MSG(false, "ID3D12Device::CreateFence(fence) failed"); return ppx::ERROR_API_FAILURE; } PPX_LOG_OBJECT_CREATION(D3D12Fence(Semaphore), mFence.Get()); + // Wait event handle + if (pCreateInfo->semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE) { + mWaitEventHandle = CreateEventEx(NULL, NULL, false, EVENT_ALL_ACCESS); + if (mWaitEventHandle == INVALID_HANDLE_VALUE) { + return ppx::ERROR_API_FAILURE; + } + } + return ppx::SUCCESS; } void Semaphore::DestroyApiObjects() { + if (mWaitEventHandle != nullptr) { + CloseHandle(mWaitEventHandle); + } + if (mFence) { mFence.Reset(); } @@ -104,15 +124,60 @@ void Semaphore::DestroyApiObjects() UINT64 Semaphore::GetNextSignalValue() { + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_BINARY) { + PPX_ASSERT_MSG(false, REQUIRES_BINARY_MSG); + return UINT64_MAX; + } + mValue += 1; return mValue; } UINT64 Semaphore::GetWaitForValue() const { + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_BINARY) { + PPX_ASSERT_MSG(false, REQUIRES_BINARY_MSG); + return UINT64_MAX; + } + return mValue; } +Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + UINT64 completedValue = mFence->GetCompletedValue(); + if (completedValue < value) { + mFence->SetEventOnCompletion(value, mWaitEventHandle); + + DWORD dwMillis = (timeout == UINT64_MAX) ? INFINITE : static_cast(timeout / 1000000ULL); + WaitForSingleObjectEx(mWaitEventHandle, dwMillis, false); + } + + return ppx::SUCCESS; +} + +Result Semaphore::TimelineSignal(uint64_t value) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + HRESULT hr = mFence->Signal(static_cast(value)); + if (FAILED(hr)) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + +uint64_t Semaphore::TimelineCounterValue() const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + UINT64 value = mFence->GetCompletedValue(); + return static_cast(value); +} + } // namespace dx12 } // namespace grfx } // namespace ppx diff --git a/src/ppx/grfx/grfx_sync.cpp b/src/ppx/grfx/grfx_sync.cpp index f78046967..f58b5f894 100644 --- a/src/ppx/grfx/grfx_sync.cpp +++ b/src/ppx/grfx/grfx_sync.cpp @@ -17,6 +17,9 @@ namespace ppx { namespace grfx { +// ------------------------------------------------------------------------------------------------- +// Fence +// ------------------------------------------------------------------------------------------------- Result Fence::WaitAndReset(uint64_t timeout) { Result ppxres = Wait(timeout); @@ -32,5 +35,57 @@ Result Fence::WaitAndReset(uint64_t timeout) return ppx::SUCCESS; } +// ------------------------------------------------------------------------------------------------- +// Semaphore +// ------------------------------------------------------------------------------------------------- +Result Semaphore::Wait(uint64_t value, uint64_t timeout) const +{ + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + auto ppxres = this->TimelineWait(value, timeout); + if (Failed(ppxres)) { + return ppxres; + } + + return ppx::SUCCESS; +} + +Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const +{ + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + // Synchronize access to API semaphore object + std::lock_guard lock(mTimelineMutex); + + if (forceMonotonicValue) { + uint64_t currentValue = this->TimelineCounterValue(); + value = std::max(value, currentValue); + } + + auto ppxres = this->TimelineSignal(value); + if (Failed(ppxres)) { + return ppxres; + } + + return ppx::SUCCESS; +} + +uint64_t Semaphore::GetCounterValue() const +{ + if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return UINT64_MAX; + } + + // Synchronize access to API semaphore object + std::lock_guard lock(mTimelineMutex); + + uint64_t value = this->TimelineCounterValue(); + return value; +} + } // namespace grfx } // namespace ppx diff --git a/src/ppx/grfx/vk/vk_device.cpp b/src/ppx/grfx/vk/vk_device.cpp index f62b54df3..e676c7608 100644 --- a/src/ppx/grfx/vk/vk_device.cpp +++ b/src/ppx/grfx/vk/vk_device.cpp @@ -640,6 +640,12 @@ Result Device::CreateApiObjects(const grfx::DeviceCreateInfo* pCreateInfo) else { mHasTimelineSemaphore = true; } + if (mHasTimelineSemaphore) { + // Load in KHR versions of functions since they'll cover Vulkan 1.1 and later versions + mFnWaitSemaphores = (PFN_vkWaitSemaphoresKHR)vkGetDeviceProcAddr(mDevice, "vkWaitSemaphoresKHR"); + mFnSignalSemaphore = (PFN_vkSignalSemaphoreKHR)vkGetDeviceProcAddr(mDevice, "vkSignalSemaphoreKHR"); + mFnGetSemaphoreCounterValue = (PFN_vkGetSemaphoreCounterValueKHR)vkGetDeviceProcAddr(mDevice, "vkGetSemaphoreCounterValueKHR"); + } PPX_LOG_INFO("Vulkan timeline semaphore is present: " << mHasTimelineSemaphore); #if defined(VK_KHR_dynamic_rendering) @@ -989,6 +995,24 @@ void Device::ResetQueryPoolEXT( mFnResetQueryPoolEXT(mDevice, queryPool, firstQuery, queryCount); } +VkResult Device::WaitSemaphores(const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout) const +{ + PPX_ASSERT_NULL_ARG(mFnWaitSemaphores); + return mFnWaitSemaphores(mDevice, pWaitInfo, timeout); +} + +VkResult Device::SignalSemaphore(const VkSemaphoreSignalInfo* pSignalInfo) +{ + PPX_ASSERT_NULL_ARG(mFnSignalSemaphore); + return mFnSignalSemaphore(mDevice, pSignalInfo); +} + +VkResult Device::GetSemaphoreCounterValue(VkSemaphore semaphore, uint64_t* pValue) +{ + PPX_ASSERT_NULL_ARG(mFnGetSemaphoreCounterValue); + return mFnGetSemaphoreCounterValue(mDevice, semaphore, pValue); +} + std::array Device::GetAllQueueFamilyIndices() const { return {mGraphicsQueueFamilyIndex, mComputeQueueFamilyIndex, mTransferQueueFamilyIndex}; diff --git a/src/ppx/grfx/vk/vk_queue.cpp b/src/ppx/grfx/vk/vk_queue.cpp index aba6ca7aa..ff5f12012 100644 --- a/src/ppx/grfx/vk/vk_queue.cpp +++ b/src/ppx/grfx/vk/vk_queue.cpp @@ -68,6 +68,9 @@ void Queue::DestroyApiObjects() Result Queue::WaitIdle() { + // Synchronized queue access + std::lock_guard lock(mQueueMutex); + VkResult vkres = vkQueueWaitIdle(mQueue); if (vkres != VK_SUCCESS) { PPX_ASSERT_MSG(false, "vkQueueWaitIdle failed" << ToString(vkres)); @@ -99,7 +102,15 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) signalSemaphores.push_back(ToApi(pSubmitInfo->ppSignalSemaphores[i])->GetVkSemaphore()); } + VkTimelineSemaphoreSubmitInfo timelineSubmitInfo = {VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO}; + timelineSubmitInfo.pNext = nullptr; + timelineSubmitInfo.waitSemaphoreValueCount = CountU32(pSubmitInfo->waitValues); + timelineSubmitInfo.pWaitSemaphoreValues = DataPtr(pSubmitInfo->waitValues); + timelineSubmitInfo.signalSemaphoreValueCount = CountU32(pSubmitInfo->signalValues); + timelineSubmitInfo.pSignalSemaphoreValues = DataPtr(pSubmitInfo->signalValues); + VkSubmitInfo vksi = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + vksi.pNext = &timelineSubmitInfo; vksi.waitSemaphoreCount = CountU32(waitSemaphores); vksi.pWaitSemaphores = DataPtr(waitSemaphores); vksi.pWaitDstStageMask = DataPtr(waitDstStageMasks); @@ -114,13 +125,96 @@ Result Queue::Submit(const grfx::SubmitInfo* pSubmitInfo) fence = ToApi(pSubmitInfo->pFence)->GetVkFence(); } - VkResult vkres = vk::QueueSubmit( - mQueue, - 1, - &vksi, - fence); - if (vkres != VK_SUCCESS) { - return ppx::ERROR_API_FAILURE; + // Synchronized queue access + { + std::lock_guard lock(mQueueMutex); + + VkResult vkres = vk::QueueSubmit( + mQueue, + 1, + &vksi, + fence); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + } + + return ppx::SUCCESS; +} + +Result Queue::QueueWait(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + VkSemaphore semaphoreHandle = ToApi(pSemaphore)->GetVkSemaphore(); + + VkTimelineSemaphoreSubmitInfo timelineSubmitInfo = {VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO}; + timelineSubmitInfo.pNext = nullptr; + timelineSubmitInfo.waitSemaphoreValueCount = 1; + timelineSubmitInfo.pWaitSemaphoreValues = &value; + + VkSubmitInfo vksi = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + vksi.pNext = &timelineSubmitInfo; + vksi.waitSemaphoreCount = 1; + vksi.pWaitSemaphores = &semaphoreHandle; + + // Synchronized queue access + { + std::lock_guard lock(mQueueMutex); + + VkResult vkres = vk::QueueSubmit( + mQueue, + 1, + &vksi, + VK_NULL_HANDLE); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + } + + return ppx::SUCCESS; +} + +Result Queue::QueueSignal(grfx::Semaphore* pSemaphore, uint64_t value) +{ + if (IsNull(pSemaphore)) { + return ppx::ERROR_UNEXPECTED_NULL_ARGUMENT; + } + + if (pSemaphore->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; + } + + VkSemaphore semaphoreHandle = ToApi(pSemaphore)->GetVkSemaphore(); + + VkTimelineSemaphoreSubmitInfo timelineSubmitInfo = {VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO}; + timelineSubmitInfo.pNext = nullptr; + timelineSubmitInfo.signalSemaphoreValueCount = 1; + timelineSubmitInfo.pSignalSemaphoreValues = &value; + + VkSubmitInfo vksi = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + vksi.pNext = &timelineSubmitInfo; + vksi.signalSemaphoreCount = 1; + vksi.pSignalSemaphores = &semaphoreHandle; + + // Synchronized queue access + { + std::lock_guard lock(mQueueMutex); + + VkResult vkres = vk::QueueSubmit( + mQueue, + 1, + &vksi, + VK_NULL_HANDLE); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } } return ppx::SUCCESS; @@ -352,22 +446,34 @@ VkResult Queue::TransitionImageLayout( vkai.commandBufferCount = 1; VkCommandBufferPtr commandBuffer; - VkResult vkres = vkAllocateCommandBuffers( - ToApi(GetDevice())->GetVkDevice(), - &vkai, - &commandBuffer); - if (vkres != VK_SUCCESS) { - PPX_ASSERT_MSG(false, "vkAllocateCommandBuffers failed" << ToString(vkres)); - return vkres; + // Synchronize command pool access + { + std::lock_guard lock(mCommandPoolMutex); + + VkResult vkres = vkAllocateCommandBuffers( + ToApi(GetDevice())->GetVkDevice(), + &vkai, + &commandBuffer); + if (vkres != VK_SUCCESS) { + PPX_ASSERT_MSG(false, "vkAllocateCommandBuffers failed" << ToString(vkres)); + return vkres; + } } + // Save ourselves from having to write a bunch of mutex locks + auto FreeCommandBuffer = [this, &commandBuffer] { + std::lock_guard lock(mCommandPoolMutex); + + vkFreeCommandBuffers(ToApi(this->GetDevice())->GetVkDevice(), this->mTransientPool, 1, commandBuffer); + }; + VkCommandBufferBeginInfo beginInfo = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; beginInfo.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; beginInfo.pInheritanceInfo = nullptr; - vkres = vkBeginCommandBuffer(commandBuffer, &beginInfo); + VkResult vkres = vkBeginCommandBuffer(commandBuffer, &beginInfo); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "vkBeginCommandBuffer failed" << ToString(vkres)); return vkres; } @@ -384,14 +490,14 @@ VkResult Queue::TransitionImageLayout( newLayout, newPipelineStage); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "CmdTransitionImageLayout failed" << ToString(vkres)); return vkres; } vkres = vkEndCommandBuffer(commandBuffer); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "vkEndCommandBuffer failed" << ToString(vkres)); return vkres; } @@ -405,21 +511,27 @@ VkResult Queue::TransitionImageLayout( submitInfo.signalSemaphoreCount = 0; submitInfo.pSignalSemaphores = nullptr; - vkres = vkQueueSubmit(mQueue, 1, &submitInfo, VK_NULL_HANDLE); - if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); - PPX_ASSERT_MSG(false, "vkQueueSubmit failed" << ToString(vkres)); - return vkres; + // Sychronized queue access + { + std::lock_guard lock(mQueueMutex); + + vkres = vkQueueSubmit(mQueue, 1, &submitInfo, VK_NULL_HANDLE); + if (vkres != VK_SUCCESS) { + FreeCommandBuffer(); + PPX_ASSERT_MSG(false, "vkQueueSubmit failed" << ToString(vkres)); + return vkres; + } + + vkres = vkQueueWaitIdle(mQueue); } - vkres = vkQueueWaitIdle(mQueue); if (vkres != VK_SUCCESS) { - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); PPX_ASSERT_MSG(false, "vkQueueWaitIdle failed" << ToString(vkres)); return vkres; } - vkFreeCommandBuffers(ToApi(GetDevice())->GetVkDevice(), mTransientPool, 1, commandBuffer); + FreeCommandBuffer(); return VK_SUCCESS; } diff --git a/src/ppx/grfx/vk/vk_sync.cpp b/src/ppx/grfx/vk/vk_sync.cpp index cab2c1257..c2a9dbcdc 100644 --- a/src/ppx/grfx/vk/vk_sync.cpp +++ b/src/ppx/grfx/vk/vk_sync.cpp @@ -16,6 +16,8 @@ #include "ppx/grfx/vk/vk_device.h" #include "ppx/grfx/vk/vk_queue.h" +#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" + namespace ppx { namespace grfx { namespace vk { @@ -86,7 +88,13 @@ Result Fence::Reset() // ------------------------------------------------------------------------------------------------- Result Semaphore::CreateApiObjects(const grfx::SemaphoreCreateInfo* pCreateInfo) { + VkSemaphoreTypeCreateInfo timelineCreateInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_TYPE_CREATE_INFO}; + timelineCreateInfo.pNext = nullptr; + timelineCreateInfo.semaphoreType = VK_SEMAPHORE_TYPE_TIMELINE; + timelineCreateInfo.initialValue = pCreateInfo->initialValue; + VkSemaphoreCreateInfo vkci = {VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO}; + vkci.pNext = (pCreateInfo->semaphoreType == grfx::SEMAPHORE_TYPE_TIMELINE) ? &timelineCreateInfo : nullptr; vkci.flags = 0; VkResult vkres = vkCreateSemaphore( @@ -114,6 +122,62 @@ void Semaphore::DestroyApiObjects() } } +Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + VkSemaphore semaphoreHandle = mSemaphore; + + VkSemaphoreWaitInfo waitInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO}; + waitInfo.pNext = nullptr; + waitInfo.flags = 0; + waitInfo.semaphoreCount = 1; + waitInfo.pSemaphores = &semaphoreHandle; + waitInfo.pValues = &value; + + VkResult vkres = ToApi(GetDevice())->WaitSemaphores(&waitInfo, timeout); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + + return ppx::SUCCESS; +} + +Result Semaphore::TimelineSignal(uint64_t value) const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + // See header for explanation + if (value > mTimelineSignaledValue) { + VkSemaphore semaphoreHandle = mSemaphore; + + VkSemaphoreSignalInfo signalInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_SIGNAL_INFO}; + signalInfo.pNext = nullptr; + signalInfo.semaphore = mSemaphore; + signalInfo.value = value; + + VkResult vkres = ToApi(GetDevice())->SignalSemaphore(&signalInfo); + if (vkres != VK_SUCCESS) { + return ppx::ERROR_API_FAILURE; + } + + mTimelineSignaledValue = value; + } + + return ppx::SUCCESS; +} + +uint64_t Semaphore::TimelineCounterValue() const +{ + PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); + + uint64_t value = UINT64_MAX; + VkResult vkres = ToApi(GetDevice())->GetSemaphoreCounterValue(mSemaphore, &value); + + // Not clear if value is written to upon failure so prefer safety. + return (vkres == VK_SUCCESS) ? value : UINT64_MAX; +} + } // namespace vk } // namespace grfx } // namespace ppx From df808fb8984ca61490a007bb8632b8117a09f5b6 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 11 Oct 2023 03:02:58 -0400 Subject: [PATCH 6/9] Fixed formatting --- projects/timeline_semaphore/main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index 629b43f93..dd748f7a8 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -199,7 +199,8 @@ void ProjApp::Render() std::stringstream ss; ss << "Frame: " << GetFrameCount() << "\n"; ss << "FPS: " << std::setw(6) << std::setprecision(6) << GetAverageFPS() << "\n"; - ss << "Timeline semaphores FTW!" << "\n"; + ss << "Timeline semaphores FTW!" + << "\n"; ss << "Timeline value: " << frame.timelineValue; mDynamicText->Clear(); From 5817257395b96929061fcb29b5737c81d8667d5f Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Wed, 11 Oct 2023 03:11:33 -0400 Subject: [PATCH 7/9] Added missing header --- projects/timeline_semaphore/main.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index dd748f7a8..ff915f055 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -16,6 +16,8 @@ #include "ppx/camera.h" using namespace ppx; +#include + #if defined(USE_DX12) const grfx::Api kApi = grfx::API_DX_12_0; #elif defined(USE_VK) From 1203b43acc3df8997311d55c7d84d32ef63cc519 Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Sun, 15 Oct 2023 22:55:30 -0400 Subject: [PATCH 8/9] Addressed feed back in pr#315 - Fixed typos and errors in comments - Fixed copyright year - Moved timeline asserts from API level to grfx level --- include/ppx/grfx/grfx_sync.h | 4 ++-- include/ppx/grfx/vk/vk_sync.h | 6 +++--- projects/timeline_semaphore/CMakeLists.txt | 2 +- projects/timeline_semaphore/main.cpp | 4 ++-- src/ppx/grfx/dx12/dx12_sync.cpp | 9 +-------- src/ppx/grfx/grfx_sync.cpp | 5 +++++ src/ppx/grfx/vk/vk_sync.cpp | 8 -------- 7 files changed, 14 insertions(+), 24 deletions(-) diff --git a/include/ppx/grfx/grfx_sync.h b/include/ppx/grfx/grfx_sync.h index a0e1f674e..76d90c561 100644 --- a/include/ppx/grfx/grfx_sync.h +++ b/include/ppx/grfx/grfx_sync.h @@ -79,8 +79,8 @@ class Semaphore // Timeline semaphore signal // - // WARNING: Signaling a value less that's less than what's already been signaled - // can cause a block or a race condition. + // WARNING: Signaling a value less than what's already been signaled can + // cause a block or a race condition. // // Use forceMonotonicValue=true to use the current timeline semaphore value // if it's greater than the passed in value. This is useful when signaling diff --git a/include/ppx/grfx/vk/vk_sync.h b/include/ppx/grfx/vk/vk_sync.h index 16b68f34c..23d5616d0 100644 --- a/include/ppx/grfx/vk/vk_sync.h +++ b/include/ppx/grfx/vk/vk_sync.h @@ -79,11 +79,11 @@ class Semaphore // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkSemaphoreSignalInfo.html#VUID-VkSemaphoreSignalInfo-value-03258 // // This is unfortunate, because there are cases where an application - // may need to signal a valuet hat is equal to what's been signaled. + // may need to signal a value that is equal to what's been signaled. // // Even though it's possible to get the current value, add 1 to it, - // and then signal it - this creates can create a different problem - // where a value is signaled too soon and a write-after-read hazard + // and then signal it - this can create a different problem where a value + // is signaled too soon and a write-after-read hazard // possibly gets introduced. // mutable uint64_t mTimelineSignaledValue = 0; diff --git a/projects/timeline_semaphore/CMakeLists.txt b/projects/timeline_semaphore/CMakeLists.txt index 52ae0efb1..1d8278655 100644 --- a/projects/timeline_semaphore/CMakeLists.txt +++ b/projects/timeline_semaphore/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright 2022 Google LLC +# Copyright 2023 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index ff915f055..841f60388 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -1,4 +1,4 @@ -// Copyright 2022 Google LLC +// Copyright 2023 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -55,7 +55,7 @@ void ProjApp::Config(ppx::ApplicationSettings& settings) settings.appName = "timeline semaphore"; settings.enableImGui = true; settings.grfx.api = kApi; - settings.grfx.enableDebug = true; + settings.grfx.enableDebug = false; } void ProjApp::Setup() diff --git a/src/ppx/grfx/dx12/dx12_sync.cpp b/src/ppx/grfx/dx12/dx12_sync.cpp index 535e4c799..6475ee65f 100644 --- a/src/ppx/grfx/dx12/dx12_sync.cpp +++ b/src/ppx/grfx/dx12/dx12_sync.cpp @@ -15,8 +15,7 @@ #include "ppx/grfx/dx12/dx12_sync.h" #include "ppx/grfx/dx12/dx12_device.h" -#define REQUIRES_BINARY_MSG "invalid semaphore type: operation requires binary semaphore" -#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" +#define REQUIRES_BINARY_MSG "invalid semaphore type: operation requires binary semaphore" namespace ppx { namespace grfx { @@ -145,8 +144,6 @@ UINT64 Semaphore::GetWaitForValue() const Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - UINT64 completedValue = mFence->GetCompletedValue(); if (completedValue < value) { mFence->SetEventOnCompletion(value, mWaitEventHandle); @@ -160,8 +157,6 @@ Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const Result Semaphore::TimelineSignal(uint64_t value) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - HRESULT hr = mFence->Signal(static_cast(value)); if (FAILED(hr)) { return ppx::ERROR_API_FAILURE; @@ -172,8 +167,6 @@ Result Semaphore::TimelineSignal(uint64_t value) const uint64_t Semaphore::TimelineCounterValue() const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - UINT64 value = mFence->GetCompletedValue(); return static_cast(value); } diff --git a/src/ppx/grfx/grfx_sync.cpp b/src/ppx/grfx/grfx_sync.cpp index f58b5f894..8f89d77b1 100644 --- a/src/ppx/grfx/grfx_sync.cpp +++ b/src/ppx/grfx/grfx_sync.cpp @@ -14,6 +14,8 @@ #include "ppx/grfx/grfx_sync.h" +#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" + namespace ppx { namespace grfx { @@ -41,6 +43,7 @@ Result Fence::WaitAndReset(uint64_t timeout) Result Semaphore::Wait(uint64_t value, uint64_t timeout) const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; } @@ -55,6 +58,7 @@ Result Semaphore::Wait(uint64_t value, uint64_t timeout) const Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; } @@ -77,6 +81,7 @@ Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const uint64_t Semaphore::GetCounterValue() const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { + PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return UINT64_MAX; } diff --git a/src/ppx/grfx/vk/vk_sync.cpp b/src/ppx/grfx/vk/vk_sync.cpp index c2a9dbcdc..72524b15f 100644 --- a/src/ppx/grfx/vk/vk_sync.cpp +++ b/src/ppx/grfx/vk/vk_sync.cpp @@ -16,8 +16,6 @@ #include "ppx/grfx/vk/vk_device.h" #include "ppx/grfx/vk/vk_queue.h" -#define REQUIRES_TIMELINE_MSG "invalid semaphore type: operation requires timeline semaphore" - namespace ppx { namespace grfx { namespace vk { @@ -124,8 +122,6 @@ void Semaphore::DestroyApiObjects() Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - VkSemaphore semaphoreHandle = mSemaphore; VkSemaphoreWaitInfo waitInfo = {VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO}; @@ -145,8 +141,6 @@ Result Semaphore::TimelineWait(uint64_t value, uint64_t timeout) const Result Semaphore::TimelineSignal(uint64_t value) const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - // See header for explanation if (value > mTimelineSignaledValue) { VkSemaphore semaphoreHandle = mSemaphore; @@ -169,8 +163,6 @@ Result Semaphore::TimelineSignal(uint64_t value) const uint64_t Semaphore::TimelineCounterValue() const { - PPX_ASSERT_MSG((this->GetSemaphoreType() == grfx::SEMAPHORE_TYPE_TIMELINE), REQUIRES_TIMELINE_MSG); - uint64_t value = UINT64_MAX; VkResult vkres = ToApi(GetDevice())->GetSemaphoreCounterValue(mSemaphore, &value); From 06f6f9c87de28bfa180f4b92e1a8be817998bfab Mon Sep 17 00:00:00 2001 From: Hai Nguyen Date: Tue, 20 Feb 2024 13:38:01 -0500 Subject: [PATCH 9/9] Subject timeline semaphore to behavior of underlying API - Removed the CPU to CPU sync test cases since there's not really any practical use case for this. - Removed forceMonotonic flag from grfx::Semaphore::Signal since it obfuscates the intentions of the underlying APIs. This unfortunately subjects an app to UB, but doing otherwise might lead the app to believing it's handling its semaphores correctly when it's not. --- include/ppx/grfx/grfx_sync.h | 6 +--- projects/timeline_semaphore/main.cpp | 45 +--------------------------- src/ppx/grfx/grfx_sync.cpp | 13 +------- 3 files changed, 3 insertions(+), 61 deletions(-) diff --git a/include/ppx/grfx/grfx_sync.h b/include/ppx/grfx/grfx_sync.h index 76d90c561..ffee81849 100644 --- a/include/ppx/grfx/grfx_sync.h +++ b/include/ppx/grfx/grfx_sync.h @@ -82,11 +82,7 @@ class Semaphore // WARNING: Signaling a value less than what's already been signaled can // cause a block or a race condition. // - // Use forceMonotonicValue=true to use the current timeline semaphore value - // if it's greater than the passed in value. This is useful when signaling - // from threads where ordering is not guaranteed. - // - Result Signal(uint64_t value, bool forceMonotonicValue = false) const; + Result Signal(uint64_t value) const; // Returns current timeline semaphore value uint64_t GetCounterValue() const; diff --git a/projects/timeline_semaphore/main.cpp b/projects/timeline_semaphore/main.cpp index 841f60388..76c6a8ff3 100644 --- a/projects/timeline_semaphore/main.cpp +++ b/projects/timeline_semaphore/main.cpp @@ -145,49 +145,6 @@ void ProjApp::Render() // Wait for previous frame's render to complete (GPU signal to CPU wait) PPX_CHECKED_CALL(frame.timelineSemaphore->Wait(frame.timelineValue)); - // Spawn a thread that will spawn other threads to signal values on the CPU - std::unique_ptr spawnerThread; - { - const uint32_t kNumThreads = 4; - grfx::Semaphore* pSemaphore = frame.timelineSemaphore; - - // Normally, we increment after a wait and before the next signal so we need to add 1. - const uint64_t startSignalValue = frame.timelineValue + 1; - - spawnerThread = std::unique_ptr( - new std::thread([kNumThreads, pSemaphore, startSignalValue]() { - std::vector> signalThreads; - - // Create signaling threads - for (uint32_t i = 0; i < kNumThreads; ++i) { - const uint64_t signalValue = startSignalValue + i; - - auto signalThread = std::unique_ptr( - new std::thread([pSemaphore, signalValue] { - PPX_CHECKED_CALL(pSemaphore->Signal(signalValue, true)); - })); - - signalThreads.push_back(std::move(signalThread)); - } - - // Join threads - for (auto& thread : signalThreads) { - thread->join(); - } - signalThreads.clear(); - })); - - // Increment to account signaling thread values - frame.timelineValue += kNumThreads; - } - - // Wait on primary for secondary threads to signal on the CPU (CPU signals to CPU wait) - PPX_CHECKED_CALL(frame.timelineSemaphore->Wait(frame.timelineValue)); - - // Join spawner thread - spawnerThread->join(); - spawnerThread.reset(); - // Signal values for text draw start and finish const uint64_t drawTextStartSignalValue = ++frame.timelineValue; const uint64_t drawTextFinishSignalValue = ++frame.timelineValue; @@ -261,7 +218,7 @@ void ProjApp::Render() { grfx::Semaphore* pSemaphore = frame.timelineSemaphore; - spawnerThread = std::unique_ptr( + auto spawnerThread = std::unique_ptr( new std::thread([pSemaphore, drawTextStartSignalValue]() { PPX_CHECKED_CALL(pSemaphore->Signal(drawTextStartSignalValue)); })); diff --git a/src/ppx/grfx/grfx_sync.cpp b/src/ppx/grfx/grfx_sync.cpp index 8f89d77b1..7c9e406df 100644 --- a/src/ppx/grfx/grfx_sync.cpp +++ b/src/ppx/grfx/grfx_sync.cpp @@ -55,21 +55,13 @@ Result Semaphore::Wait(uint64_t value, uint64_t timeout) const return ppx::SUCCESS; } -Result Semaphore::Signal(uint64_t value, bool forceMonotonicValue) const +Result Semaphore::Signal(uint64_t value) const { if (this->GetSemaphoreType() != grfx::SEMAPHORE_TYPE_TIMELINE) { PPX_ASSERT_MSG(false, REQUIRES_TIMELINE_MSG); return ppx::ERROR_GRFX_INVALID_SEMAPHORE_TYPE; } - // Synchronize access to API semaphore object - std::lock_guard lock(mTimelineMutex); - - if (forceMonotonicValue) { - uint64_t currentValue = this->TimelineCounterValue(); - value = std::max(value, currentValue); - } - auto ppxres = this->TimelineSignal(value); if (Failed(ppxres)) { return ppxres; @@ -85,9 +77,6 @@ uint64_t Semaphore::GetCounterValue() const return UINT64_MAX; } - // Synchronize access to API semaphore object - std::lock_guard lock(mTimelineMutex); - uint64_t value = this->TimelineCounterValue(); return value; }