diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index 232fa12ec..39d4d7fee 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -36,7 +36,7 @@ env: METHANE_VERSION_MINOR: 8 METHANE_VERSION_PATCH: 2 METHANE_VERSION_BUILD: ${{ github.run_number }} - TRACY_RELEASE_VERSION: "0.14.0.1" + TRACY_RELEASE_VERSION: "0.14.1" VULKAN_SDK_VERSION: "1.4.350.1" ORIGIN_REPOSITORY: 'MethanePowered/MethaneKit' diff --git a/.gitpod.dockerfile b/.gitpod.dockerfile deleted file mode 100644 index 0f0af6023..000000000 --- a/.gitpod.dockerfile +++ /dev/null @@ -1,16 +0,0 @@ -FROM gitpod/workspace-full-vnc - -# Install Linux dependencies -USER root -RUN apt-get update \ - && apt-get -y install build-essential ninja-build xcb libx11-dev libx11-xcb-dev libxcb-sync-dev libxcb-randr0-dev \ - && wget https://github.com/Kitware/CMake/releases/download/v3.25.2/cmake-3.25.2-linux-x86_64.sh \ - -q -O /tmp/cmake-install.sh \ - && chmod u+x /tmp/cmake-install.sh \ - && mkdir /opt/cmake-3.25.2 \ - && /tmp/cmake-install.sh --skip-license --prefix=/opt/cmake-3.25.2 \ - && rm /tmp/cmake-install.sh \ - && ln -s /opt/cmake-3.25.2/bin/* /usr/local/bin \ - && apt-get clean && rm -rf /var/cache/apt/* && rm -rf /var/lib/apt/lists/* && rm -rf /tmp/* - -USER gitpod diff --git a/.gitpod.yml b/.gitpod.yml deleted file mode 100644 index 7fca1fd2f..000000000 --- a/.gitpod.yml +++ /dev/null @@ -1,23 +0,0 @@ -github: - prebuilds: - master: true - branches: true - pullRequests: true - pullRequestsFromForks: true - addCheck: true - addComment: true - addBadge: false - addLabel: false - -image: - file: .gitpod.dockerfile - -ports: - - port: 6080 - onOpen: open-preview - -vscode: - extensions: - - twxs.cmake - - ms-vscode.cmake-tools - - hbenl.vscode-test-explorer \ No newline at end of file diff --git a/Apps/04-ShadowCube/ShadowCubeApp.cpp b/Apps/04-ShadowCube/ShadowCubeApp.cpp index 19a91c7bc..dc8602140 100644 --- a/Apps/04-ShadowCube/ShadowCubeApp.cpp +++ b/Apps/04-ShadowCube/ShadowCubeApp.cpp @@ -245,7 +245,7 @@ void ShadowCubeApp::Init() // Create shadow pass configuration with depth attachment frame.shadow_pass.render_pass = m_shadow_pass_pattern.CreateRenderPass({ - { frame.shadow_pass.rt_texture.GetInterface() }, + { rhi::TextureView(frame.shadow_pass.rt_texture.GetInterface()) }, shadow_texture_settings.dimensions.AsRectSize() }); diff --git a/Apps/05-Typography/TypographyApp.cpp b/Apps/05-Typography/TypographyApp.cpp index 341710ab9..f7399e51b 100644 --- a/Apps/05-Typography/TypographyApp.cpp +++ b/Apps/05-Typography/TypographyApp.cpp @@ -308,6 +308,12 @@ bool TypographyApp::Resize(const gfx::FrameSize& frame_size, bool is_minimized) if (!UserInterfaceApp::Resize(frame_size, is_minimized)) return false; + // Text items and font atlas badges are created by Init() only after the base application was + // initialized, so a resize event received in between (for example, when Init() was interrupted + // by an exception) has nothing to lay out yet and must not index the incomplete text items. + if (m_texts.size() < g_text_blocks_count) + return false; + const gfx::FrameSize frame_size_in_dots = GetFrameSizeInDots(); int32_t vertical_text_pos_in_dots = g_top_text_pos_in_dots; diff --git a/Externals/README.md b/Externals/README.md index 6a4bcf776..8fb164592 100644 --- a/Externals/README.md +++ b/Externals/README.md @@ -27,7 +27,7 @@ to support parallel cmake configurations processing. | [SPIRV-Cross](https://github.com/KhronosGroup/SPIRV-Cross) | 1.3.296.0 | Static | [Apache 2.0](https://github.com/KhronosGroup/SPIRV-Cross/blob/master/LICENSE) | A library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages. | | [STB](https://github.com/nothings/stb) | 2023-01-29 | Header-only | [MIT, Public Domain](https://github.com/nothings/stb/blob/master/LICENSE) | Single-file public domain libraries for C/C++. | | [TaskFlow](https://github.com/taskflow/taskflow) | 4.1.0 | Header-only | [MIT](https://github.com/taskflow/taskflow/blob/master/LICENSE) | A General-purpose Parallel and Heterogeneous Task Programming System. | -| [Tracy](https://github.com/wolfpld/tracy) | 0.13.1 | Static | [BSD 3.0](https://github.com/wolfpld/tracy/blob/master/LICENSE) | A real-time, nanosecond-resolution, remote telemetry, hybrid frame and sampling profiler for games and other applications. | +| [Tracy](https://github.com/wolfpld/tracy) | 0.14.1 | Static | [BSD 3.0](https://github.com/wolfpld/tracy/blob/master/LICENSE) | A real-time, nanosecond-resolution, remote telemetry, hybrid frame and sampling profiler for games and other applications. | | [Vulkan Headers](https://github.com/MethanePowered/VulkanHeaders) | 1.4.357.0 | Header-only | [Apache 2.0](https://github.com/KhronosGroup/Vulkan-Headers/blob/main/LICENSE.txt) | Vulkan Header files and API registry. | ## Build Tools diff --git a/Externals/Tracy.cmake b/Externals/Tracy.cmake index 41ec6dc0b..86b5ca6b9 100644 --- a/Externals/Tracy.cmake +++ b/Externals/Tracy.cmake @@ -1,7 +1,7 @@ CPMAddPackage( NAME Tracy GITHUB_REPOSITORY MethanePowered/Tracy - VERSION 0.14.0.1 + VERSION 0.14.1 OPTIONS "TRACY_STATIC ON" "TRACY_ENABLE ${METHANE_TRACY_PROFILING_ENABLED}" diff --git a/Modules/Common/Instrumentation/Include/Methane/TracyGpu.hpp b/Modules/Common/Instrumentation/Include/Methane/TracyGpu.hpp index d89600b9e..e75238ac3 100644 --- a/Modules/Common/Instrumentation/Include/Methane/TracyGpu.hpp +++ b/Modules/Common/Instrumentation/Include/Methane/TracyGpu.hpp @@ -126,12 +126,17 @@ class GpuContext } } + static uint8_t GetGpuCtxCounter() + { + const uint32_t ctx_id = tracy::GetGpuCtxCounter().fetch_add(1, std::memory_order_relaxed); + META_CHECK_LESS_DESCR(ctx_id, 255, "Tracy GPU context count is exceeding the maximum 255."); + return static_cast(ctx_id); + } + explicit GpuContext(const Settings& settings) - : m_id(tracy::GetGpuCtxCounter().fetch_add( 1, std::memory_order_relaxed )) + : m_id(GetGpuCtxCounter()) , m_prev_calibration_cpu_timestamp(settings.cpu_timestamp) { - META_CHECK_LESS_DESCR(m_id, 255, "Tracy GPU context count is exceeding the maximum 255."); - auto item = tracy::Profiler::QueueSerial(); tracy::MemWrite(&item->hdr.type, tracy::QueueType::GpuNewContext); tracy::MemWrite(&item->gpuNewContext.cpuTime, settings.cpu_ref_timestamp); diff --git a/Modules/Common/Primitives/Include/Methane/Exceptions.hpp b/Modules/Common/Primitives/Include/Methane/Exceptions.hpp index d9c0b2f91..b86e8e09e 100644 --- a/Modules/Common/Primitives/Include/Methane/Exceptions.hpp +++ b/Modules/Common/Primitives/Include/Methane/Exceptions.hpp @@ -206,7 +206,7 @@ class UnexpectedArgumentException : public ArgumentExceptionBase::max()) + explicit TimeAnimation(const FunctorType& update_function, double duration_sec = std::numeric_limits::max()) : Animation(duration_sec) , m_update_function(update_function) { } diff --git a/Modules/Data/Events/Include/Methane/Data/Emitter.hpp b/Modules/Data/Events/Include/Methane/Data/Emitter.hpp index 8babe8c93..d89b5fcc8 100644 --- a/Modules/Data/Events/Include/Methane/Data/Emitter.hpp +++ b/Modules/Data/Events/Include/Methane/Data/Emitter.hpp @@ -91,7 +91,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap void Connect(Receiver& receiver, int32_t priority = 0) noexcept final { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_receivers_mutex); + std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration if (FindConnectedReceiver(receiver) != m_connected_receivers.end()) return; @@ -109,7 +109,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap void Disconnect(Receiver& receiver) noexcept final { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_receivers_mutex); + std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration const auto connected_receiver_it = FindConnectedReceiver(receiver); if (connected_receiver_it == m_connected_receivers.end()) @@ -142,7 +142,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap void Emit(FuncType&& func_ptr, ArgTypes&&... args) { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_receivers_mutex); + std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration // Additional receivers may be non-empty before emitting connected receiver calls // only in case when current emit is called during another emitted callback for the same emitter @@ -216,7 +216,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap inline void CleanupConnectedReceivers() noexcept { // Erase receivers disconnected during emit cycle from the connected receivers - std::lock_guard lock(m_connected_receivers_mutex); + std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration for(auto connected_receiver_it = m_connected_receivers.begin(); connected_receiver_it != m_connected_receivers.end();) { if (connected_receiver_it->first) @@ -228,7 +228,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap inline void ConnectReceivers() noexcept { - std::lock_guard lock(m_connected_receivers_mutex); + std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration for(const ReceiverAndPriority& receiver_and_priority : m_connected_receivers) { if (receiver_and_priority.first) @@ -239,7 +239,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap inline auto DisconnectReceivers() noexcept { // Move connected receivers so that OnDisconnected callbacks are not processed (m_connected_receivers would be empty) - std::lock_guard lock(m_connected_receivers_mutex); + std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration const auto connected_receivers = std::move(m_connected_receivers); for(const ReceiverAndPriority& receiver_and_priority : connected_receivers) { @@ -254,6 +254,9 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap bool m_is_emitting = false; std::vector m_connected_receivers; std::vector m_additional_connected_receivers; + // Mutex has to be recursive: Emit() holds this lock while calling receiver callbacks, + // which may re-enter Connect(), Disconnect() or a nested Emit() on the same emitter, + // and Emit() itself re-locks it in CleanupConnectedReceivers(). #if defined(__GNUG__) && !defined(__clang__) // GCC fails with internal compiler error: Segmentation fault std::recursive_mutex m_connected_receivers_mutex; diff --git a/Modules/Data/Events/Include/Methane/Data/Receiver.hpp b/Modules/Data/Events/Include/Methane/Data/Receiver.hpp index 58c095deb..1c230112b 100644 --- a/Modules/Data/Events/Include/Methane/Data/Receiver.hpp +++ b/Modules/Data/Events/Include/Methane/Data/Receiver.hpp @@ -44,7 +44,7 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru : m_connected_emitter_refs(other.m_connected_emitter_refs) { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration ConnectEmitters(); } @@ -52,14 +52,14 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru : m_connected_emitter_refs(other.DisconnectEmitters()) { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration ConnectEmitters(); } ~Receiver() override // NOSONAR { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration DisconnectEmitters(); } @@ -69,7 +69,7 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru if (this == std::addressof(other)) return *this; - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration DisconnectEmitters(); m_connected_emitter_refs = other.m_connected_emitter_refs; ConnectEmitters(); @@ -82,7 +82,7 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru if (this == std::addressof(other)) return *this; - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration DisconnectEmitters(); m_connected_emitter_refs = std::move(other.m_connected_emitter_refs); ConnectEmitters(); @@ -96,7 +96,7 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru void OnConnected(IEmitter& emitter) noexcept { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration if (FindConnectedEmitter(emitter) != m_connected_emitter_refs.end()) return; @@ -106,7 +106,7 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru void OnDisconnected(IEmitter& emitter) noexcept { META_FUNCTION_TASK(); - std::lock_guard lock(m_connected_emitter_refs_mutex); + std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration const auto connected_emitter_ref_it = FindConnectedEmitter(emitter); if (connected_emitter_ref_it == m_connected_emitter_refs.end()) return; @@ -148,6 +148,9 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru } Refs> m_connected_emitter_refs; + // Mutex has to be recursive: Connect/DisconnectEmitters() are called with this lock held and + // call back into Emitter::Connect()/Disconnect(), which re-enter OnConnected()/OnDisconnected() + // on this receiver and lock it again on the same thread. TracyLockable(std::recursive_mutex, m_connected_emitter_refs_mutex); }; diff --git a/Modules/Data/Types/Include/Methane/Data/EnumMask.hpp b/Modules/Data/Types/Include/Methane/Data/EnumMask.hpp index 0a3cf6679..2b732b941 100644 --- a/Modules/Data/Types/Include/Methane/Data/EnumMask.hpp +++ b/Modules/Data/Types/Include/Methane/Data/EnumMask.hpp @@ -97,8 +97,8 @@ class EnumMask constexpr explicit operator bool() const noexcept { return m_value != M{}; } constexpr explicit operator M() const noexcept { return m_value; } - constexpr EnumMask& SetBitOn(Bit bit) noexcept { return *this |= bit; } - constexpr EnumMask& SetBitOff(Bit bit) noexcept { return *this &= ~EnumMask(bit); } + constexpr EnumMask& SetBitOn(Bit bit) noexcept { *this |= bit; return *this; } + constexpr EnumMask& SetBitOff(Bit bit) noexcept { *this &= ~EnumMask(bit); return *this; } constexpr EnumMask& SetBit(Bit bit, bool on) noexcept { return on ? SetBitOn(bit) : SetBitOff(bit); } constexpr bool HasBits(EnumMask mask) const noexcept { return mask.m_value ? ((m_value & mask.GetValue()) == mask.GetValue()) : true; } constexpr bool HasBit(Bit bit) const noexcept { return HasBits(EnumMask(bit)); } diff --git a/Modules/Graphics/App/Include/Methane/Graphics/AppContextController.h b/Modules/Graphics/App/Include/Methane/Graphics/AppContextController.h index fc2f0e6ad..39d4a74af 100644 --- a/Modules/Graphics/App/Include/Methane/Graphics/AppContextController.h +++ b/Modules/Graphics/App/Include/Methane/Graphics/AppContextController.h @@ -61,7 +61,7 @@ class AppContextController final { { pin::Keyboard::Key::LeftControl, pin::Keyboard::Key::Minus }, AppContextAction::RemoveFrameBufferFromSwapChain }, }; - AppContextController(Rhi::IRenderContext& context, const ActionByKeyboardState& action_by_keyboard_state = default_action_by_keyboard_state); + explicit AppContextController(Rhi::IRenderContext& context, const ActionByKeyboardState& action_by_keyboard_state = default_action_by_keyboard_state); // Input::Controller implementation void OnKeyboardChanged(pin::Keyboard::Key, pin::Keyboard::KeyState, const pin::Keyboard::StateChange& state_change) override; diff --git a/Modules/Graphics/App/Include/Methane/Graphics/AppController.h b/Modules/Graphics/App/Include/Methane/Graphics/AppController.h index 6e5ecfc64..533741f7a 100644 --- a/Modules/Graphics/App/Include/Methane/Graphics/AppController.h +++ b/Modules/Graphics/App/Include/Methane/Graphics/AppController.h @@ -45,7 +45,7 @@ class AppController { public: using ActionByKeyboardState = pin::Keyboard::ActionControllerBase::ActionByKeyboardState; - inline static const ActionByKeyboardState default_action_by_keyboard_state{ + inline static const ActionByKeyboardState default_action_by_keyboard_state{ // NOSONAR - per-level default key map is intentional, always referenced fully qualified { { pin::Keyboard::Key::LeftControl, pin::Keyboard::Key::P }, AppAction::SwitchAnimations } }; diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h index 2e1f1e739..fcb3c5f65 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h @@ -26,8 +26,10 @@ Methane command kit implementation. #include "Object.h" #include +#include #include +#include namespace Methane::Graphics::Base { @@ -62,11 +64,21 @@ class CommandKit final using CommandListIndexById = std::map; using CommandListSetById = std::map>; - CommandListIndex GetCommandListIndexById(Rhi::CommandListId cmd_list_id) const noexcept; - CommandListSetId GetCommandListSetId(Rhi::CommandListIdSpan cmd_list_ids, Opt frame_index_opt) const; + // NOTE: methods below require m_mutex to be locked by the caller + CommandListIndex GetCommandListIndexById(Rhi::CommandListId cmd_list_id) const noexcept; + CommandListSetId GetCommandListSetId(Rhi::CommandListIdSpan cmd_list_ids, Opt frame_index_opt) const; + Rhi::ICommandQueue& GetQueueUnlocked() const; + Rhi::ICommandList& GetListUnlocked(Rhi::CommandListId cmd_list_id) const; const Rhi::IContext& m_context; Rhi::CommandListType m_cmd_list_type; + + // Command queue, lists, fences and list sets are lazily created by the const getters above, + // which are reachable from resource upload paths running on parallel-rendering worker threads. + // All accesses to the mutable members below are made with m_mutex locked, either by the public + // getter itself or by its caller when the access is made from a *Unlocked() helper. Static + // analysis can not follow the latter, so those accesses are marked with NOSONAR in CommandKit.cpp. + mutable TracyLockable(std::mutex, m_mutex); mutable Ptr m_cmd_queue_ptr; mutable Ptrs m_cmd_list_ptrs; mutable CommandListIndexById m_cmd_list_index_by_id; diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h index 555dc6439..11cd799e3 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h @@ -140,7 +140,9 @@ class CommandList // NOSONAR - custom destructor is used for logging, class has mutable TracyLockable(std::recursive_mutex, m_state_mutex); TracyLockable(std::mutex, m_state_change_mutex); - std::condition_variable_any m_state_change_condition_var; + // condition_variable_any is required because m_state_change_mutex is tracy::Lockable + // in profiling builds, which std::condition_variable can not wait on. + std::condition_variable_any m_state_change_condition_var; // NOSONAR - condition_variable_any is required, see comment above TRACY_GPU_SCOPE_TYPE m_tracy_gpu_scope; #ifdef METHANE_GPU_INSTRUMENTATION_ENABLED diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandQueueTracking.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandQueueTracking.h index 26468b4a6..cb427ac42 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandQueueTracking.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandQueueTracking.h @@ -101,6 +101,7 @@ class CommandQueueTracking // NOSONAR - destructor is required Ptr GetInitializedTimestampQueryPoolPtr() const; void CompleteExecutionSafely(); void WaitForExecution() noexcept; + bool IsExecutingCommandListsQueueEmpty() const; bool IsFrontListExecutingOnFrameIndex(const Opt& frame_index) const noexcept; bool IsExecutingOnFrameIndex(const Opt& frame_index) const noexcept; diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h index 330d6c4c8..11c8a1b7f 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h @@ -29,9 +29,12 @@ Base implementation of the context interface. #include #include #include +#include #include #include +#include +#include #include namespace tf @@ -75,7 +78,7 @@ class Context const Rhi::IObjectRegistry& GetObjectRegistry() const noexcept override { return m_objects_cache; } void RequestDeferredAction(DeferredAction action) const noexcept override; void CompleteInitialization() override; - bool IsCompletingInitialization() const noexcept override { return m_is_completing_initialization; } + bool IsCompletingInitialization() const noexcept override { return m_is_completing_initialization.load(); } void WaitForGpu(WaitFor wait_for) override; void Reset(Rhi::IDevice& device) override; void Reset() override; @@ -91,7 +94,7 @@ class Context // IObject interface bool SetName(std::string_view name) override; - DeferredAction GetRequestedAction() const noexcept { return m_requested_action; } + DeferredAction GetRequestedAction() const noexcept { return m_requested_action.load(); } Ptr GetBaseDevicePtr() const noexcept { return m_device_ptr; } Device& GetBaseDevice(); const Device& GetBaseDevice() const; @@ -119,10 +122,27 @@ class Context UniquePtr m_descriptor_manager_ptr; tf::Executor& m_parallel_executor; ObjectRegistry m_objects_cache; + + // Default command kits are lazily created by the const GetDefaultCommandKit() getters, which are + // reachable from resource upload paths running on parallel-rendering worker threads. + mutable TracyLockable(std::mutex, m_default_command_kits_mutex); mutable CommandKitPtrByType m_default_command_kit_ptrs; mutable CommandKitByQueue m_default_command_kit_ptr_by_queue; - mutable DeferredAction m_requested_action = DeferredAction::None; - mutable bool m_is_completing_initialization = false; + + // Both members below are accessed from multiple threads and use the default sequentially + // consistent memory ordering: every access happens either once per frame or on a cold + // initialization path, so the stronger ordering costs nothing measurable here while keeping + // the synchronization trivial to reason about. + + // RequestDeferredAction() escalates the pending action from any thread: IBuffer/ITexture::SetData() + // and IProgramArgumentBinding::SetRootConstant() are called concurrently from parallel-rendering + // worker threads. + mutable std::atomic m_requested_action{ DeferredAction::None }; + + // CompleteInitialization() writes this re-entrancy guard on the render thread, while the const + // IsCompletingInitialization() getter is a part of the public IContext API and may be queried + // from resource update callbacks running on other threads. + mutable std::atomic_bool m_is_completing_initialization{ false }; }; } // namespace Methane::Graphics::Base diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/ProgramBindings.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/ProgramBindings.h index 96fb0dd3a..34f57a3b3 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/ProgramBindings.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/ProgramBindings.h @@ -133,8 +133,8 @@ class ProgramBindings ArgumentBindings m_binding_by_argument; ResourceStatesByAccess m_transition_resource_states_by_access; ResourceRefsByAccess m_resource_refs_by_access; - mutable Ptr m_resource_state_transition_barriers_ptr; - mutable Ptrs m_retained_root_constant_buffer_ptrs; + mutable Ptr m_resource_state_transition_barriers_ptr; // NOSONAR - per-bindings instance, applied by the single command list which owns it + mutable Ptrs m_retained_root_constant_buffer_ptrs; // NOSONAR - per-bindings instance, applied by the single command list which owns it Data::Index m_bindings_index = 0u; // index of this program bindings object between all program bindings of the program }; diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/RootConstantBuffer.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/RootConstantBuffer.h index 32476dff7..2865aecb7 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/RootConstantBuffer.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/RootConstantBuffer.h @@ -71,7 +71,7 @@ class RootConstantAccessor // NOSONAR - custom destructor is required Ref m_storage_ref; // storage reference Range m_buffer_range; // aligned memory range Data::Size m_data_size; // unaligned original size - mutable bool m_is_initialized = false; + mutable bool m_is_initialized = false; // NOSONAR - one accessor per argument binding, never shared between threads }; class RootConstantStorage diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Shader.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Shader.h index 8666bf98b..7953de342 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Shader.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Shader.h @@ -74,7 +74,7 @@ class Shader const Type m_type; const Context& m_context; const Settings m_settings; - mutable ArgNamesSet m_cached_arg_names; + mutable ArgNamesSet m_cached_arg_names; // NOSONAR - filled only by shader reflection during single-threaded program creation }; } // namespace Methane::Graphics::Base diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp index 01d063f6c..e0b0c8274 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp @@ -87,6 +87,8 @@ bool CommandKit::SetName(std::string_view name) if (!Object::SetName(name)) return false; + std::scoped_lock lock_guard(m_mutex); + if (m_cmd_queue_ptr) m_cmd_queue_ptr->SetName(fmt::format("{} Command Queue", GetName())); @@ -110,17 +112,26 @@ bool CommandKit::SetName(std::string_view name) Rhi::ICommandQueue& CommandKit::GetQueue() const { META_FUNCTION_TASK(); - if (m_cmd_queue_ptr) - return *m_cmd_queue_ptr; + std::scoped_lock lock_guard(m_mutex); + return GetQueueUnlocked(); +} + +Rhi::ICommandQueue& CommandKit::GetQueueUnlocked() const +{ + META_FUNCTION_TASK(); + // NOSONAR - m_mutex is locked by the caller (see note in CommandKit.h) + if (m_cmd_queue_ptr) // NOSONAR + return *m_cmd_queue_ptr; // NOSONAR - m_cmd_queue_ptr = Rhi::ICommandQueue::Create(m_context, m_cmd_list_type); - m_cmd_queue_ptr->SetName(fmt::format("{} Command Queue", GetName())); - return *m_cmd_queue_ptr; + m_cmd_queue_ptr = Rhi::ICommandQueue::Create(m_context, m_cmd_list_type); // NOSONAR + m_cmd_queue_ptr->SetName(fmt::format("{} Command Queue", GetName())); // NOSONAR + return *m_cmd_queue_ptr; // NOSONAR } bool CommandKit::HasList(Rhi::CommandListId cmd_list_id) const noexcept { META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mutex); const CommandListIndex cmd_list_index = GetCommandListIndexById(cmd_list_id); return cmd_list_index < m_cmd_list_ptrs.size() && m_cmd_list_ptrs[cmd_list_index]; } @@ -128,28 +139,37 @@ bool CommandKit::HasList(Rhi::CommandListId cmd_list_id) const noexcept bool CommandKit::HasListWithState(Rhi::CommandListState cmd_list_state, Rhi::CommandListId cmd_list_id) const noexcept { META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mutex); const CommandListIndex cmd_list_index = GetCommandListIndexById(cmd_list_id); return cmd_list_index < m_cmd_list_ptrs.size() && m_cmd_list_ptrs[cmd_list_index] && m_cmd_list_ptrs[cmd_list_index]->GetState() == cmd_list_state; } Rhi::ICommandList& CommandKit::GetList(Rhi::CommandListId cmd_list_id = 0U) const +{ + META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mutex); + return GetListUnlocked(cmd_list_id); +} + +Rhi::ICommandList& CommandKit::GetListUnlocked(Rhi::CommandListId cmd_list_id) const { META_FUNCTION_TASK(); const CommandListIndex cmd_list_index = GetCommandListIndexById(cmd_list_id); META_CHECK_LESS_DESCR(cmd_list_index, g_max_cmd_lists_count, "no more than 32 command lists are supported in one command kit"); - if (cmd_list_index >= m_cmd_list_ptrs.size()) - m_cmd_list_ptrs.resize(cmd_list_index + 1); + // NOSONAR - m_mutex is locked by the caller (see note in CommandKit.h) + if (cmd_list_index >= m_cmd_list_ptrs.size()) // NOSONAR + m_cmd_list_ptrs.resize(cmd_list_index + 1); // NOSONAR - Ptr& cmd_list_ptr = m_cmd_list_ptrs[cmd_list_index]; + Ptr& cmd_list_ptr = m_cmd_list_ptrs[cmd_list_index]; // NOSONAR if (cmd_list_ptr) return *cmd_list_ptr; switch (m_cmd_list_type) { using enum Rhi::CommandListType; - case Transfer: cmd_list_ptr = GetQueue().CreateTransferCommandList(); break; - case Render: cmd_list_ptr = RenderCommandList::CreateForSynchronization(GetQueue()); break; - case Compute: cmd_list_ptr = GetQueue().CreateComputeCommandList(); break; + case Transfer: cmd_list_ptr = GetQueueUnlocked().CreateTransferCommandList(); break; + case Render: cmd_list_ptr = RenderCommandList::CreateForSynchronization(GetQueueUnlocked()); break; + case Compute: cmd_list_ptr = GetQueueUnlocked().CreateComputeCommandList(); break; default: META_UNEXPECTED(m_cmd_list_type); } @@ -187,6 +207,8 @@ Rhi::ICommandListSet& CommandKit::GetListSet(Rhi::CommandListIdSpan cmd_list_ids { META_FUNCTION_TASK(); META_CHECK_NOT_EMPTY(cmd_list_ids); + std::scoped_lock lock_guard(m_mutex); + const CommandListSetId cmd_list_set_id = GetCommandListSetId(cmd_list_ids, frame_index_opt); Ptr& cmd_list_set_ptr = m_cmd_list_set_by_id[cmd_list_set_id]; @@ -196,7 +218,7 @@ Rhi::ICommandListSet& CommandKit::GetListSet(Rhi::CommandListIdSpan cmd_list_ids Refs command_list_refs; for(Rhi::CommandListId cmd_list_id : cmd_list_ids) { - command_list_refs.emplace_back(GetList(cmd_list_id)); + command_list_refs.emplace_back(GetListUnlocked(cmd_list_id)); } cmd_list_set_ptr = Rhi::ICommandListSet::Create(command_list_refs, frame_index_opt); @@ -206,6 +228,8 @@ Rhi::ICommandListSet& CommandKit::GetListSet(Rhi::CommandListIdSpan cmd_list_ids Rhi::IFence& CommandKit::GetFence(Rhi::CommandListId fence_id) const { META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mutex); + const uint32_t fence_index = GetCommandListIndexById(fence_id); if (fence_index >= m_fence_ptrs.size()) m_fence_ptrs.resize(fence_index + 1); @@ -215,7 +239,7 @@ Rhi::IFence& CommandKit::GetFence(Rhi::CommandListId fence_id) const if (fence_ptr) return *fence_ptr; - fence_ptr = Rhi::IFence::Create(GetQueue()); + fence_ptr = Rhi::IFence::Create(GetQueueUnlocked()); fence_ptr->SetName(fmt::format("{} Fence {}", GetName(), fence_id)); return *fence_ptr; } @@ -229,9 +253,9 @@ void CommandKit::ExecuteListSet(Rhi::CommandListIdSpan cmd_list_ids, Opt frame_index_opt) const { META_FUNCTION_TASK(); - std::mutex execution_wait_mutex; - size_t executing_cmd_list_count = cmd_list_ids.size(); - std::condition_variable_any executing_cmd_list_set_condition_var; + std::mutex execution_wait_mutex; + size_t executing_cmd_list_count = cmd_list_ids.size(); + std::condition_variable executing_cmd_list_set_condition_var; GetQueue().Execute(GetListSet(cmd_list_ids, frame_index_opt), [&executing_cmd_list_count, &execution_wait_mutex, &executing_cmd_list_set_condition_var](const Rhi::ICommandList&) { @@ -247,7 +271,7 @@ void CommandKit::ExecuteListSetAndWaitForCompletion(Rhi::CommandListIdSpan cmd_l CommandKit::CommandListIndex CommandKit::GetCommandListIndexById(Rhi::CommandListId cmd_list_id) const noexcept { META_FUNCTION_TASK(); - const auto [it, success] = m_cmd_list_index_by_id.try_emplace(cmd_list_id, static_cast(m_cmd_list_index_by_id.size())); + const auto [it, success] = m_cmd_list_index_by_id.try_emplace(cmd_list_id, static_cast(m_cmd_list_index_by_id.size())); // NOSONAR - m_mutex is locked by the caller (see note in CommandKit.h) return it->second; } diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp index 8c9d3baed..deb633b32 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp @@ -67,18 +67,18 @@ void CommandQueueTracking::InitializeTimestampQueryPool() // NOSONAR - m_timesta { META_FUNCTION_TASK(); constexpr uint32_t g_max_timestamp_queries_count_per_frame = 1000; - m_timestamp_query_pool_ptr = Rhi::ITimestampQueryPool::Create(*this, g_max_timestamp_queries_count_per_frame); - if (!m_timestamp_query_pool_ptr) + m_timestamp_query_pool_ptr = Rhi::ITimestampQueryPool::Create(*this, g_max_timestamp_queries_count_per_frame); // NOSONAR - guarded by m_timestamp_query_pool_mutex in the caller + if (!m_timestamp_query_pool_ptr) // NOSONAR - guarded by m_timestamp_query_pool_mutex in the caller return; - const Rhi::ITimestampQueryPool::CalibratedTimestamps& calibrated_timestamps = m_timestamp_query_pool_ptr-> + const Rhi::ITimestampQueryPool::CalibratedTimestamps& calibrated_timestamps = m_timestamp_query_pool_ptr-> // NOSONAR - guarded by m_timestamp_query_pool_mutex in the caller GetCalibratedTimestamps(); InitializeTracyGpuContext( Tracy::GpuContext::Settings( ConvertSystemGraphicsApiToTracyGpuContextType(Rhi::ISystem::GetNativeApi()), calibrated_timestamps.cpu_ts, calibrated_timestamps.gpu_ts, - Data::ConvertFrequencyToTickPeriod(m_timestamp_query_pool_ptr->GetGpuFrequency()) + Data::ConvertFrequencyToTickPeriod(m_timestamp_query_pool_ptr->GetGpuFrequency()) // NOSONAR - guarded by m_timestamp_query_pool_mutex in the caller ) ); } @@ -142,8 +142,10 @@ void CommandQueueTracking::WaitForExecution() noexcept m_execution_waiting_condition_var.wait_for(lock, std::chrono::milliseconds(32), [this] { - return !m_execution_waiting || !m_executing_command_lists. - empty(); + // The executing command lists queue is modified by other threads + // (see Execute() and ProcessExecutingCommandListSet()), so it is + // read here under its own mutex rather than under the waiting mutex. + return !m_execution_waiting || !IsExecutingCommandListsQueueEmpty(); } ); @@ -154,12 +156,10 @@ void CommandQueueTracking::WaitForExecution() noexcept m_name_changed = false; } - while (!m_executing_command_lists.empty()) + // The loop is driven by the pop result only: testing m_executing_command_lists.empty() here + // would be an unsynchronized read of the queue concurrently modified by the other threads. + while (const Ptr command_list_set_ptr = PopNextExecutingCommandListSet()) { - Ptr command_list_set_ptr = PopNextExecutingCommandListSet(); - if (!command_list_set_ptr) - break; - CompleteCommandListSetExecution(*command_list_set_ptr); } @@ -182,6 +182,13 @@ void CommandQueueTracking::WaitForExecution() noexcept } } +bool CommandQueueTracking::IsExecutingCommandListsQueueEmpty() const +{ + META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_executing_command_lists_mutex); + return m_executing_command_lists.empty(); +} + bool CommandQueueTracking::IsFrontListExecutingOnFrameIndex(const Opt& frame_index) const noexcept { const Ptr command_list_set_ptr = m_executing_command_lists.front(); @@ -251,15 +258,19 @@ Ptr CommandQueueTracking::PopNextExecutingCommandListSet() return command_list_set_ptr; } -void CommandQueueTracking::CompleteCommandListSetExecution(CommandListSet& executing_command_list_set) +void CommandQueueTracking::CompleteCommandListSetExecution(CommandListSet&) { META_FUNCTION_TASK(); - std::unique_lock lock_guard(m_executing_command_lists_mutex); - if (!m_executing_command_lists.empty() && - m_executing_command_lists.front().get() == std::addressof(executing_command_list_set)) - { - m_executing_command_lists.pop(); - } + // NOTE: the completed command list set was already popped out of the executing queue by + // PopNextExecutingCommandListSet(), atomically with waiting for its completion, and it must NOT be + // popped here once more. The executing command lists mutex is released by the time this method is + // called, so the main thread may have already re-executed the very same command list set object for + // the next frame with the same frame buffer index, pushing it back to the front of the queue. + // Popping the queue front here used to drop that new execution out of tracking, after which + // WaitUntilCompleted() did not wait for it (IsExecutingOnFrameIndex() found nothing to wait for) + // and the following Reset() of its command lists threw an exception on the 'Executing' state. + // This method is kept as an extension point for the native command queue implementations, + // which need to release per-execution resources of the completed command list set. } void CommandQueueTracking::ShutdownQueueExecution() diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp index abf7a3037..af6c844c2 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp @@ -72,23 +72,30 @@ Ptr Context::CreateCommandKit(Rhi::CommandListType type) const void Context::RequestDeferredAction(DeferredAction action) const noexcept { META_FUNCTION_TASK(); - m_requested_action = std::max(m_requested_action, action); + // Escalate the pending action to the strongest one requested by any thread. + // DeferredAction values are ordered None < UploadResources < CompleteInitialization. + DeferredAction prev_action = m_requested_action.load(); + while (prev_action < action && !m_requested_action.compare_exchange_weak(prev_action, action)) + { + // Retry with the action escalated by a concurrent thread, which + // compare_exchange_weak() has just reloaded into 'prev_action'. + } } void Context::CompleteInitialization() { META_FUNCTION_TASK(); - if (m_is_completing_initialization) + if (m_is_completing_initialization.load()) return; - m_is_completing_initialization = true; + m_is_completing_initialization.store(true); META_LOG("Complete initialization of context '{}'", GetName()); UploadResourcesAndNotify(); GetDescriptorManager().CompleteInitialization(); - m_requested_action = DeferredAction::None; - m_is_completing_initialization = false; + m_requested_action.store(DeferredAction::None); + m_is_completing_initialization.store(false); } void Context::WaitForGpu(WaitFor wait_for) @@ -148,8 +155,17 @@ void Context::Release() m_device_ptr.reset(); - m_default_command_kit_ptr_by_queue.clear(); - for (Ptr& cmd_kit_ptr : m_default_command_kit_ptrs) + // Move the command kits out under the lock and destroy them after releasing it, so that + // command queue shutdown (which joins the execution waiting thread) is not done while holding it. + CommandKitByQueue released_command_kit_ptr_by_queue; + CommandKitPtrByType released_command_kit_ptrs; + { + std::scoped_lock lock_guard(m_default_command_kits_mutex); + released_command_kit_ptr_by_queue.swap(m_default_command_kit_ptr_by_queue); + released_command_kit_ptrs.swap(m_default_command_kit_ptrs); + } + released_command_kit_ptr_by_queue.clear(); + for (Ptr& cmd_kit_ptr : released_command_kit_ptrs) cmd_kit_ptr.reset(); Data::Emitter::Emit(&Rhi::IContextCallback::OnContextReleased, std::ref(*this)); @@ -176,6 +192,8 @@ void Context::Initialize(Device& device, bool is_callback_emitted) Rhi::ICommandKit& Context::GetDefaultCommandKit(Rhi::CommandListType type) const { META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_default_command_kits_mutex); + Ptr& cmd_kit_ptr = m_default_command_kit_ptrs[magic_enum::enum_index(type).value()]; if (cmd_kit_ptr) return *cmd_kit_ptr; @@ -190,6 +208,8 @@ Rhi::ICommandKit& Context::GetDefaultCommandKit(Rhi::CommandListType type) const Rhi::ICommandKit& Context::GetDefaultCommandKit(Rhi::ICommandQueue& cmd_queue) const { META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_default_command_kits_mutex); + Ptr& cmd_kit_ptr = m_default_command_kit_ptr_by_queue[std::addressof(cmd_queue)]; if (cmd_kit_ptr) return *cmd_kit_ptr; @@ -233,10 +253,13 @@ bool Context::SetName(std::string_view name) return false; GetBaseDevice().SetName(fmt::format("{} Device", name)); - for(const Ptr& cmd_kit_ptr : m_default_command_kit_ptrs) { - if (cmd_kit_ptr) - cmd_kit_ptr->SetName(fmt::format("{} {}", name, g_default_command_kit_names[magic_enum::enum_index(cmd_kit_ptr->GetListType()).value()])); + std::scoped_lock lock_guard(m_default_command_kits_mutex); + for(const Ptr& cmd_kit_ptr : m_default_command_kit_ptrs) + { + if (cmd_kit_ptr) + cmd_kit_ptr->SetName(fmt::format("{} {}", name, g_default_command_kit_names[magic_enum::enum_index(cmd_kit_ptr->GetListType()).value()])); + } } return true; } @@ -246,9 +269,17 @@ void Context::ExecuteSyncCommandLists(const Rhi::ICommandKit& upload_cmd_kit) co { META_FUNCTION_TASK(); constexpr auto cmd_list_id = static_cast(cmd_list_purpose); - const std::vector cmd_list_ids = { cmd_list_id }; + const std::vector cmd_list_ids = { cmd_list_id }; + + // Iterate over a snapshot of the command kits: the command list execution and fence waits below + // must not hold the lock, or they would block a concurrent GetDefaultCommandKit() call. + CommandKitByQueue default_command_kit_ptr_by_queue; + { + std::scoped_lock lock_guard(m_default_command_kits_mutex); + default_command_kit_ptr_by_queue = m_default_command_kit_ptr_by_queue; + } - for (const auto& [cmd_queue_ptr, cmd_kit_ptr] : m_default_command_kit_ptr_by_queue) + for (const auto& [cmd_queue_ptr, cmd_kit_ptr] : default_command_kit_ptr_by_queue) { if (cmd_kit_ptr.get() == std::addressof(upload_cmd_kit) || !cmd_kit_ptr->HasList(cmd_list_id)) continue; @@ -328,7 +359,8 @@ bool Context::UploadResourcesAndNotify() void Context::PerformRequestedAction() { META_FUNCTION_TASK(); - switch(m_requested_action) + DeferredAction requested_action = m_requested_action.load(); + switch(requested_action) { case DeferredAction::None: break; @@ -342,9 +374,11 @@ void Context::PerformRequestedAction() break; default: - META_UNEXPECTED(m_requested_action); + META_UNEXPECTED(requested_action); } - m_requested_action = DeferredAction::None; + // Clear only the action which was actually performed: another thread may have escalated the + // request while it was running, and that stronger request has to survive until the next frame. + m_requested_action.compare_exchange_strong(requested_action, DeferredAction::None); } void Context::SetDevice(Device& device) diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp index 5494d3c3f..bcf1e9479 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp @@ -44,6 +44,8 @@ RenderPass::RenderPass(RenderPattern& render_pattern, const Settings& settings, bool RenderPass::Update(const Rhi::RenderPassSettings& settings) { META_FUNCTION_TASK(); + + std::scoped_lock lock_guard(m_mutex); if (m_settings == settings) return false; @@ -52,6 +54,7 @@ bool RenderPass::Update(const Rhi::RenderPassSettings& settings) m_non_frame_buffer_attachment_textures.clear(); m_color_attachment_textures.clear(); m_depth_attachment_texture_ptr = nullptr; + m_stencil_attachment_texture_ptr = nullptr; InitAttachmentStates(); return true; @@ -60,6 +63,7 @@ bool RenderPass::Update(const Rhi::RenderPassSettings& settings) void RenderPass::ReleaseAttachmentTextures() { META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mutex); m_non_frame_buffer_attachment_textures.clear(); m_settings.attachments.clear(); } diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Resource.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Resource.cpp index fef17fcd5..262bf68ee 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Resource.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Resource.cpp @@ -115,7 +115,7 @@ bool Resource::SetOwnerQueueFamily(uint32_t family_index, Ptr& out_ba family_index, m_owner_queue_family_index_opt ? "with" : "without"); - if (m_owner_queue_family_index_opt) + if (m_owner_queue_family_index_opt.has_value()) { if (!out_barriers) out_barriers = Rhi::IResourceBarriers::Create(); diff --git a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Context.hpp b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Context.hpp index 847e1e117..9fd531b3e 100644 --- a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Context.hpp +++ b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Context.hpp @@ -185,7 +185,7 @@ class Context using NativeQueryHeaps = std::array, D3D12_QUERY_HEAP_TYPE_COPY_QUEUE_TIMESTAMP + 1>; DescriptorManager::Settings m_descriptor_manager_init_settings{ true, {}, {} }; - mutable NativeQueryHeaps m_query_heaps; + mutable NativeQueryHeaps m_query_heaps; // NOSONAR - lazily created per query heap type on the render thread }; } // namespace Methane::Graphics::DirectX diff --git a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Device.h b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Device.h index 5f0e7b197..bd7a29894 100644 --- a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Device.h +++ b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/Device.h @@ -62,7 +62,7 @@ class Device final : public Base::Device private: const wrl::ComPtr m_adapter_cptr; const D3D_FEATURE_LEVEL m_feature_level; - mutable NativeFeatureOptions5 m_feature_options_5; + mutable NativeFeatureOptions5 m_feature_options_5; // NOSONAR - device capabilities queried once during device initialization mutable wrl::ComPtr m_device_cptr; #ifdef _DEBUG mutable DWORD m_debug_message_callback_cookie = 0U; diff --git a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/ErrorHandling.h b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/ErrorHandling.h index 4e5679fde..a72950fbf 100644 --- a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/ErrorHandling.h +++ b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/ErrorHandling.h @@ -38,7 +38,7 @@ namespace wrl = Microsoft::WRL; class RuntimeException : public std::runtime_error { public: - RuntimeException(HRESULT hr, ID3D12Device* device = nullptr); + explicit RuntimeException(HRESULT hr, ID3D12Device* device = nullptr); RuntimeException(HRESULT hr, const wrl::ComPtr& error_blob); [[nodiscard]] HRESULT GetResult() const noexcept { return m_result; } diff --git a/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/BufferSet.h b/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/BufferSet.h index a6bc7f3a1..52041b35d 100644 --- a/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/BufferSet.h +++ b/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/BufferSet.h @@ -75,7 +75,7 @@ class BufferSet // NOSONAR - constructors and assignment operators are required using Impl = Methane::Graphics::META_GFX_NAME::BufferSet; Ptr m_impl_ptr; - mutable Buffers m_buffers; + mutable Buffers m_buffers; // NOSONAR - PIMPL-side wrapper cache rebuilt by GetRefs() on the render thread }; } // namespace Methane::Graphics::Rhi diff --git a/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/CommandListSet.h b/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/CommandListSet.h index 0eee3d548..256750983 100644 --- a/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/CommandListSet.h +++ b/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/CommandListSet.h @@ -45,7 +45,7 @@ class CommandListSet // NOSONAR - constructors and assignment operators are requ META_PIMPL_API explicit CommandListSet(const Ptr& interface_ptr); META_PIMPL_API explicit CommandListSet(ICommandListSet& interface_ref); - META_PIMPL_API CommandListSet(const Refs& command_list_refs, Opt frame_index_opt = {}); + META_PIMPL_API explicit CommandListSet(const Refs& command_list_refs, Opt frame_index_opt = {}); META_PIMPL_API bool IsInitialized() const META_PIMPL_NOEXCEPT; META_PIMPL_API ICommandListSet& GetInterface() const META_PIMPL_NOEXCEPT; diff --git a/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/ParallelRenderCommandList.h b/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/ParallelRenderCommandList.h index d0371da65..8315aafb6 100644 --- a/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/ParallelRenderCommandList.h +++ b/Modules/Graphics/RHI/Impl/Include/Methane/Graphics/RHI/ParallelRenderCommandList.h @@ -105,7 +105,7 @@ class ParallelRenderCommandList // NOSONAR - class has more than 35 methods, con using Impl = Methane::Graphics::META_GFX_NAME::ParallelRenderCommandList; Ptr m_impl_ptr; - mutable std::vector m_parallel_command_lists; + mutable std::vector m_parallel_command_lists; // NOSONAR - PIMPL-side wrapper cache rebuilt by GetParallelCommandLists() on the render thread }; } // namespace Methane::Graphics::Rhi diff --git a/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ISampler.h b/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ISampler.h index 5b33d048e..a202592af 100644 --- a/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ISampler.h +++ b/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ISampler.h @@ -51,7 +51,7 @@ struct SamplerFilter }; SamplerFilter(MinMag min, MinMag mag, Mip mip) : min(min), mag(mag), mip(mip) { } - SamplerFilter(MinMag min_mag, Mip mip) : SamplerFilter(min_mag, min_mag, mip) { } + SamplerFilter(MinMag min_mag, Mip mip_filter) : SamplerFilter(min_mag, min_mag, mip_filter) { } explicit SamplerFilter(MinMag min_mag) : SamplerFilter(min_mag, Mip::NotMipmapped) { } [[nodiscard]] friend bool operator==(const SamplerFilter& left, const SamplerFilter& right) @@ -94,7 +94,8 @@ struct SamplerAddress struct SamplerLevelOfDetail { - SamplerLevelOfDetail(float bias = 0.F, float min = 0.F, float max = std::numeric_limits::max()); + SamplerLevelOfDetail() = default; + explicit SamplerLevelOfDetail(float bias, float min = 0.F, float max = std::numeric_limits::max()); [[nodiscard]] friend bool operator==(const SamplerLevelOfDetail& left, const SamplerLevelOfDetail& right) { diff --git a/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ITexture.h b/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ITexture.h index 2b4a09f56..70503c8d8 100644 --- a/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ITexture.h +++ b/Modules/Graphics/RHI/Interface/Include/Methane/Graphics/RHI/ITexture.h @@ -46,10 +46,10 @@ class TextureView : public ResourceView { public: - TextureView(ITexture& texture, - const SubResource::Index& subresource_index = {}, - const SubResource::Count& subresource_count = {}, - Opt texture_dimension_type_opt = {}); + explicit TextureView(ITexture& texture, + const SubResource::Index& subresource_index = {}, + const SubResource::Count& subresource_count = {}, + Opt texture_dimension_type_opt = {}); using ResourceView::operator std::string; diff --git a/Modules/Graphics/RHI/Interface/Sources/Methane/Graphics/RHI/ITexture.cpp b/Modules/Graphics/RHI/Interface/Sources/Methane/Graphics/RHI/ITexture.cpp index b9388a80b..77a3f6f3a 100644 --- a/Modules/Graphics/RHI/Interface/Sources/Methane/Graphics/RHI/ITexture.cpp +++ b/Modules/Graphics/RHI/Interface/Sources/Methane/Graphics/RHI/ITexture.cpp @@ -49,9 +49,9 @@ TextureSettings TextureSettings::ForImage(const Dimensions& dimensions, const Op TextureSettings settings; if (dimensions.GetHeight() == 1) - settings.dimension_type = array_length_opt ? Tex1DArray : Tex1D; + settings.dimension_type = array_length_opt.has_value() ? Tex1DArray : Tex1D; else if (dimensions.GetDepth() == 1) - settings.dimension_type = array_length_opt ? Tex2DArray : Tex2D; + settings.dimension_type = array_length_opt.has_value() ? Tex2DArray : Tex2D; else settings.dimension_type = Tex3D; @@ -72,7 +72,7 @@ TextureSettings TextureSettings::ForCubeImage(uint32_t dimension_size, const Opt TextureSettings settings; settings.type = usage.HasAnyBit(ResourceUsage::RenderTarget) ? TextureType::RenderTarget : TextureType::Image; - settings.dimension_type = array_length_opt ? TextureDimensionType::CubeArray : TextureDimensionType::Cube; + settings.dimension_type = array_length_opt.has_value() ? TextureDimensionType::CubeArray : TextureDimensionType::Cube; settings.dimensions = Dimensions(dimension_size, dimension_size, 6U); settings.array_length = array_length_opt.value_or(1U); settings.pixel_format = pixel_format; diff --git a/Modules/Graphics/RHI/Metal/Include/Methane/Graphics/Metal/ProgramArgumentBinding.hh b/Modules/Graphics/RHI/Metal/Include/Methane/Graphics/Metal/ProgramArgumentBinding.hh index 913a7b3bb..7e7c93326 100644 --- a/Modules/Graphics/RHI/Metal/Include/Methane/Graphics/Metal/ProgramArgumentBinding.hh +++ b/Modules/Graphics/RHI/Metal/Include/Methane/Graphics/Metal/ProgramArgumentBinding.hh @@ -28,6 +28,8 @@ Metal implementation of the program argument binding interface. #import #include +#include +#include namespace Methane::Graphics::Metal { @@ -55,7 +57,20 @@ public: using NativeSamplerStates = std::vector<__unsafe_unretained id>; using NativeOffsets = std::vector; + struct NativeResourceViews + { + using Ptr = std::shared_ptr; + + MTLResourceUsage resource_usage = MTLResourceUsageRead; + NativeResources resources; + NativeSamplerStates sampler_states; + NativeTextures textures; + NativeBuffers buffers; + NativeOffsets buffer_offsets; + }; + ProgramArgumentBinding(const Base::Context& context, const Settings& settings); + ProgramArgumentBinding(const ProgramArgumentBinding& other); // Base::ProgramArgumentBinding interface [[nodiscard]] Ptr CreateCopy() const override; @@ -68,13 +83,8 @@ public: bool IsArgumentBufferMode() const noexcept { return !m_settings_mt.argument_buffer_offset_by_shader_type.empty(); } const Settings& GetMetalSettings() const noexcept { return m_settings_mt; } - MTLResourceUsage GetNativeResourceUsage() const noexcept { return m_mtl_resource_usage; } MTLRenderStages GetNativeRenderStages() const noexcept { return m_mtl_render_stages; } - const NativeResources& GetNativeResources() const noexcept { return m_mtl_resources; } - const NativeSamplerStates& GetNativeSamplerStates() const noexcept { return m_mtl_sampler_states; } - const NativeTextures& GetNativeTextures() const noexcept { return m_mtl_textures; } - const NativeBuffers& GetNativeBuffers() const noexcept { return m_mtl_buffers; } - const NativeOffsets& GetBufferOffsets() const noexcept { return m_mtl_buffer_offsets; } + NativeResourceViews::Ptr GetNativeResourceViews() const noexcept; protected: // Base::ProgramArgumentBinding overrides... @@ -83,14 +93,10 @@ protected: private: void SetMetalResourcesForViews(Rhi::ResourceViewSpan resource_views); - Settings m_settings_mt; - MTLResourceUsage m_mtl_resource_usage = MTLResourceUsageRead; - MTLRenderStages m_mtl_render_stages; - NativeResources m_mtl_resources; - NativeSamplerStates m_mtl_sampler_states; - NativeTextures m_mtl_textures; - NativeBuffers m_mtl_buffers; - NativeOffsets m_mtl_buffer_offsets; + Settings m_settings_mt; + MTLRenderStages m_mtl_render_stages; + NativeResourceViews::Ptr m_mtl_resource_views; + mutable TracyLockable(std::mutex, m_mtl_resource_views_mutex); }; } // namespace Methane::Graphics::Metal diff --git a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramArgumentBinding.mm b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramArgumentBinding.mm index 8784d83e1..f7696c2c2 100644 --- a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramArgumentBinding.mm +++ b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramArgumentBinding.mm @@ -73,10 +73,25 @@ static MTLRenderStages ConvertShaderTypeToMetalRenderStages(Rhi::ShaderType shad return mtl_render_stages; } +static const ProgramArgumentBinding::NativeResourceViews::Ptr& GetEmptyNativeResourceViews() +{ + static const auto s_empty_resource_views = std::make_shared(); + return s_empty_resource_views; +} + ProgramArgumentBinding::ProgramArgumentBinding(const Base::Context& context, const Settings& settings) : Base::ProgramArgumentBinding(context, settings) , m_settings_mt(settings) , m_mtl_render_stages(ConvertShaderTypeToMetalRenderStages(settings.argument.GetShaderType())) + , m_mtl_resource_views(GetEmptyNativeResourceViews()) +{ +} + +ProgramArgumentBinding::ProgramArgumentBinding(const ProgramArgumentBinding& other) + : Base::ProgramArgumentBinding(other) + , m_settings_mt(other.m_settings_mt) + , m_mtl_render_stages(other.m_mtl_render_stages) + , m_mtl_resource_views(other.GetNativeResourceViews()) { } @@ -161,16 +176,21 @@ static MTLRenderStages ConvertShaderTypeToMetalRenderStages(Rhi::ShaderType shad return true; } +ProgramArgumentBinding::NativeResourceViews::Ptr ProgramArgumentBinding::GetNativeResourceViews() const noexcept +{ + META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mtl_resource_views_mutex); + return m_mtl_resource_views; +} + void ProgramArgumentBinding::SetMetalResourcesForViews(Rhi::ResourceViewSpan resource_views) { META_FUNCTION_TASK(); - m_mtl_resource_usage = {}; - m_mtl_resources.clear(); - m_mtl_sampler_states.clear(); - m_mtl_textures.clear(); - m_mtl_buffers.clear(); - m_mtl_buffer_offsets.clear(); + // The new snapshot is built entirely in a local object, so that concurrent readers keep using + // the previous one until the fully built snapshot is published in one step at the end. + auto native_resource_views_ptr = std::make_shared(); + NativeResourceViews& native_resource_views = *native_resource_views_ptr; std::set> mtl_resource_set; @@ -178,40 +198,43 @@ static MTLRenderStages ConvertShaderTypeToMetalRenderStages(Rhi::ShaderType shad { using enum Rhi::ResourceType; case Sampler: - m_mtl_sampler_states.reserve(resource_views.size()); - std::ranges::transform(resource_views, std::back_inserter(m_mtl_sampler_states), + native_resource_views.sampler_states.reserve(resource_views.size()); + std::ranges::transform(resource_views, std::back_inserter(native_resource_views.sampler_states), [](const Rhi::ResourceView& resource_view) { return dynamic_cast(resource_view.GetResource()).GetNativeSamplerState(); }); break; case Texture: - m_mtl_textures.reserve(resource_views.size()); + native_resource_views.textures.reserve(resource_views.size()); for(const Rhi::ResourceView& resource_view : resource_views) { const auto& texture = dynamic_cast(resource_view.GetResource()); - m_mtl_resource_usage |= texture.GetNativeResourceUsage(); + native_resource_views.resource_usage |= texture.GetNativeResourceUsage(); mtl_resource_set.insert(static_cast>(texture.GetNativeTexture())); - m_mtl_textures.push_back(texture.GetNativeTexture()); + native_resource_views.textures.push_back(texture.GetNativeTexture()); } break; case Buffer: - m_mtl_buffers.reserve(resource_views.size()); - m_mtl_buffer_offsets.reserve(resource_views.size()); + native_resource_views.buffers.reserve(resource_views.size()); + native_resource_views.buffer_offsets.reserve(resource_views.size()); for (const Rhi::ResourceView& resource_view : resource_views) { const auto& buffer = dynamic_cast(resource_view.GetResource()); - m_mtl_resource_usage |= buffer.GetNativeResourceUsage(); + native_resource_views.resource_usage |= buffer.GetNativeResourceUsage(); mtl_resource_set.insert(static_cast>(buffer.GetNativeBuffer())); - m_mtl_buffers.push_back(buffer.GetNativeBuffer()); - m_mtl_buffer_offsets.push_back(static_cast(resource_view.GetOffset())); + native_resource_views.buffers.push_back(buffer.GetNativeBuffer()); + native_resource_views.buffer_offsets.push_back(static_cast(resource_view.GetOffset())); } break; default: META_UNEXPECTED(m_settings_mt.resource_type); } - std::copy(mtl_resource_set.begin(), mtl_resource_set.end(), std::back_inserter(m_mtl_resources)); + std::copy(mtl_resource_set.begin(), mtl_resource_set.end(), std::back_inserter(native_resource_views.resources)); + + std::scoped_lock lock_guard(m_mtl_resource_views_mutex); + m_mtl_resource_views = std::move(native_resource_views_ptr); } } // namespace Methane::Graphics::Metal diff --git a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramBindings.mm b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramBindings.mm index 53935ae32..40b01084b 100644 --- a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramBindings.mm +++ b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramBindings.mm @@ -495,20 +495,22 @@ static void WriteArgumentBindingResourceIds(const ProgramArgumentBinding& arg_bi { META_FUNCTION_TASK(); Rhi::ResourceType resource_type = arg_binding.GetSettings().resource_type; + const ProgramArgumentBinding::NativeResourceViews::Ptr resource_views_ptr = arg_binding.GetNativeResourceViews(); + META_CHECK_NOT_NULL(resource_views_ptr); switch(resource_type) { using enum Rhi::ResourceType; case Buffer: - WriteNativeBufferAddresses(arg_binding.GetNativeBuffers(), arg_binding.GetBufferOffsets(), buffer_ptr); + WriteNativeBufferAddresses(resource_views_ptr->buffers, resource_views_ptr->buffer_offsets, buffer_ptr); break; case Texture: - WriteNativeTextureIds(arg_binding.GetNativeTextures(), buffer_ptr); + WriteNativeTextureIds(resource_views_ptr->textures, buffer_ptr); break; case Sampler: - WriteNativeSamplerIds(arg_binding.GetNativeSamplerStates(), buffer_ptr); + WriteNativeSamplerIds(resource_views_ptr->sampler_states, buffer_ptr); break; default: @@ -706,27 +708,29 @@ static void WriteArgumentBindingResourceIds(const ProgramArgumentBinding& arg_bi } else if (!set_root_constant_values_only) { + const ProgramArgumentBinding::NativeResourceViews::Ptr resource_views_ptr = argument_binding.GetNativeResourceViews(); + META_CHECK_NOT_NULL(resource_views_ptr); switch (settings.resource_type) { using enum Rhi::ResourceType; case Buffer: SetMetalResourcesForAll(settings.argument.GetShaderType(), program, mtl_cmd_encoder, - argument_binding.GetNativeBuffers(), + resource_views_ptr->buffers, settings.argument_index, - argument_binding.GetBufferOffsets()); + resource_views_ptr->buffer_offsets); break; case Texture: SetMetalResourcesForAll(settings.argument.GetShaderType(), program, mtl_cmd_encoder, - argument_binding.GetNativeTextures(), + resource_views_ptr->textures, settings.argument_index); break; case Sampler: SetMetalResourcesForAll(settings.argument.GetShaderType(), program, mtl_cmd_encoder, - argument_binding.GetNativeSamplerStates(), + resource_views_ptr->sampler_states, settings.argument_index); break; @@ -855,8 +859,9 @@ static void WriteArgumentBindingResourceIds(const ProgramArgumentBinding& arg_bi ForEachArgumentBinding([this](const Rhi::ProgramArgument&, const ArgumentBinding& argument_binding) { m_has_root_constant_values |= argument_binding.GetSettings().argument.IsRootConstantValue(); - const ProgramArgumentBinding::NativeResources& argument_resources = argument_binding.GetNativeResources(); - std::copy(argument_resources.begin(), argument_resources.end(), + const ProgramArgumentBinding::NativeResourceViews::Ptr resource_views_ptr = argument_binding.GetNativeResourceViews(); + META_CHECK_NOT_NULL(resource_views_ptr); + std::copy(resource_views_ptr->resources.begin(), resource_views_ptr->resources.end(), std::inserter(m_mtl_used_resources, m_mtl_used_resources.begin())); }); } @@ -889,11 +894,13 @@ static void WriteArgumentBindingResourceIds(const ProgramArgumentBinding& arg_bi for (const auto& [program_arg, arg_binding_ptr]: GetArgumentBindings()) { const auto& metal_argument_binding = static_cast(*arg_binding_ptr); - const ProgramArgumentBinding::NativeResources& argument_resources = metal_argument_binding.GetNativeResources(); + const ProgramArgumentBinding::NativeResourceViews::Ptr resource_views_ptr = metal_argument_binding.GetNativeResourceViews(); + META_CHECK_NOT_NULL(resource_views_ptr); + const ProgramArgumentBinding::NativeResources& argument_resources = resource_views_ptr->resources; if (argument_resources.empty()) continue; - const NativeResourceUsageAndStage usage_and_stage(metal_argument_binding.GetNativeResourceUsage(), + const NativeResourceUsageAndStage usage_and_stage(resource_views_ptr->resource_usage, metal_argument_binding.GetNativeRenderStages()); if (metal_applied_program_bindings_ptr) diff --git a/Modules/Graphics/RHI/Vulkan/Include/Methane/Graphics/Vulkan/QueryPool.h b/Modules/Graphics/RHI/Vulkan/Include/Methane/Graphics/Vulkan/QueryPool.h index 345997ef9..6629c1467 100644 --- a/Modules/Graphics/RHI/Vulkan/Include/Methane/Graphics/Vulkan/QueryPool.h +++ b/Modules/Graphics/RHI/Vulkan/Include/Methane/Graphics/Vulkan/QueryPool.h @@ -106,6 +106,7 @@ class TimestampQueryPool final CalibratedTimestamps Calibrate() override; private: + const vk::TimeDomainEXT m_vk_cpu_time_domain; uint64_t m_deviation = 0U; }; diff --git a/Modules/Graphics/RHI/Vulkan/Sources/Methane/Graphics/Vulkan/QueryPool.cpp b/Modules/Graphics/RHI/Vulkan/Sources/Methane/Graphics/Vulkan/QueryPool.cpp index 63aa27d7a..4171bdc0f 100644 --- a/Modules/Graphics/RHI/Vulkan/Sources/Methane/Graphics/Vulkan/QueryPool.cpp +++ b/Modules/Graphics/RHI/Vulkan/Sources/Methane/Graphics/Vulkan/QueryPool.cpp @@ -33,18 +33,29 @@ Vulkan GPU query pool implementation. #include #include +#include +#include #include #include +#include +#include +#include +#include #include -static const vk::TimeDomainEXT g_vk_cpu_time_domain = +// CPU time domains which can be used to calibrate GPU timestamps, in the order of preference. +// The selected domain must return timestamps in the units which Data::GetQpcToNSecMultiplier() +// converts to nanoseconds: QueryPerformanceCounter ticks on Windows and nanoseconds of the POSIX +// monotonic clocks on all other platforms (where that multiplier is 1). +static constexpr std::array g_vk_cpu_time_domains{ #if defined(_WIN32) - vk::TimeDomainEXT::eQueryPerformanceCounter; -#elif defined(__linux__) && defined CLOCK_MONOTONIC_RAW - vk::TimeDomainEXT::eClockMonotonicRaw; + vk::TimeDomainEXT::eQueryPerformanceCounter #else - static_cast(-1); + // MoltenVK exposes CLOCK_MONOTONIC_RAW only, while other Vulkan drivers may expose either of these. + vk::TimeDomainEXT::eClockMonotonicRaw, + vk::TimeDomainEXT::eClockMonotonic #endif +}; namespace Methane::Graphics::Vulkan { @@ -70,6 +81,33 @@ static Data::Size GetMaxTimestampsCount(const Rhi::IContext& context, uint32_t m return frames_count * max_timestamps_per_frame; } +// Used only to build the description of the failed check below, which is compiled out with METHANE_CHECKS_ENABLED=OFF. +[[maybe_unused]] static std::vector GetTimeDomainNames(std::span time_domains) +{ + META_FUNCTION_TASK(); + std::vector time_domain_names; + time_domain_names.reserve(time_domains.size()); + std::ranges::transform(time_domains, std::back_inserter(time_domain_names), + [](vk::TimeDomainEXT time_domain) { return magic_enum::enum_name(time_domain); }); + return time_domain_names; +} + +static vk::TimeDomainEXT GetCalibrateableCpuTimeDomain(const vk::PhysicalDevice& vk_physical_device) +{ + META_FUNCTION_TASK(); + // Not every driver exposes every CPU time domain: MoltenVK, for example, exposes CLOCK_MONOTONIC_RAW only, + // so the first domain supported by the device is taken from the platform preference list instead of + // requiring one hard-coded domain to be present. + const std::vector calibrateable_time_domains = vk_physical_device.getCalibrateableTimeDomainsEXT(); + const auto cpu_time_domain_it = std::ranges::find_first_of(g_vk_cpu_time_domains, calibrateable_time_domains); + const bool is_cpu_time_domain_calibrateable = cpu_time_domain_it != g_vk_cpu_time_domains.end(); + META_CHECK_TRUE_DESCR(is_cpu_time_domain_calibrateable, + "Vulkan does not support calibration of any CPU time domain used on this platform ({}), device supports only ({})", + fmt::join(GetTimeDomainNames(g_vk_cpu_time_domains), ", "), + fmt::join(GetTimeDomainNames(calibrateable_time_domains), ", ")); + return is_cpu_time_domain_calibrateable ? *cpu_time_domain_it : g_vk_cpu_time_domains.front(); +} + Query::Query(Base::QueryPool& buffer, Base::CommandList& command_list, Index index, Range data_range) : Base::Query(buffer, command_list, index, data_range) , m_vk_device(GetVulkanQueryPool().GetVulkanContext().GetVulkanDevice().GetNativeDevice()) @@ -131,9 +169,14 @@ CommandQueue& QueryPool::GetVulkanCommandQueue() noexcept } TimestampQueryPool::TimestampQueryPool(CommandQueue& command_queue, uint32_t max_timestamps_per_frame) - : QueryPool(command_queue, Type::Timestamp, 1U << 15U, 1U, + // Query pool is created with exactly as many queries as its data buffer can hold timestamps: + // over-allocating queries makes MoltenVK fail to create the backing MTLCounterSampleBuffer + // (limited to 32768 bytes, i.e. 4096 timestamps) and silently fall back to emulated timestamps. + : QueryPool(command_queue, Type::Timestamp, + GetMaxTimestampsCount(command_queue.GetContext(), max_timestamps_per_frame), 1U, GetMaxTimestampsCount(command_queue.GetContext(), max_timestamps_per_frame) * sizeof(Timestamp), sizeof(Timestamp)) + , m_vk_cpu_time_domain(GetCalibrateableCpuTimeDomain(command_queue.GetVulkanDevice().GetNativePhysicalDevice())) { META_FUNCTION_TASK(); @@ -144,15 +187,8 @@ TimestampQueryPool::TimestampQueryPool(CommandQueue& command_queue, uint32_t max const float gpu_timestamp_period = vk_physical_device.getProperties().limits.timestampPeriod; SetGpuFrequency(static_cast(gpu_timestamp_period * std::chrono::nanoseconds(1s).count())); - // Check if Vulkan supports CPU time domains calibration - const auto calibrateable_time_domains = vk_physical_device.getCalibrateableTimeDomainsEXT(); - bool is_cpu_time_domain_calibrateable = std::ranges::find(calibrateable_time_domains, g_vk_cpu_time_domain) != calibrateable_time_domains.end(); - META_CHECK_TRUE_DESCR(is_cpu_time_domain_calibrateable, - "Vulkan does not support calibration of the CPU time domain {}", - magic_enum::enum_name(g_vk_cpu_time_domain)); - // Calculate the desired CPU-GPU timestamps deviation - const std::array timestamp_infos = {{ { vk::TimeDomainEXT::eDevice }, { g_vk_cpu_time_domain }, }}; + const std::array timestamp_infos = {{ { vk::TimeDomainEXT::eDevice }, { m_vk_cpu_time_domain }, }}; std::array timestamps{{}}; std::array probe_deviations{{}}; for(uint64_t& deviation : probe_deviations) @@ -189,7 +225,7 @@ Rhi::ITimestampQueryPool::CalibratedTimestamps TimestampQueryPool::Calibrate() constexpr uint32_t max_calibration_attempts = 16U; const vk::Device& vk_device = GetVulkanCommandQueue().GetVulkanDevice().GetNativeDevice(); - const std::array timestamp_infos = {{ { vk::TimeDomainEXT::eDevice }, { g_vk_cpu_time_domain }, }}; + const std::array timestamp_infos = {{ { vk::TimeDomainEXT::eDevice }, { m_vk_cpu_time_domain }, }}; std::array timestamps{{}}; std::array best_timestamps{{}}; uint64_t deviation = 0U; diff --git a/Modules/Graphics/Types/Sources/Methane/Graphics/Types.cpp b/Modules/Graphics/Types/Sources/Methane/Graphics/Types.cpp index 609e5f69b..c5b03fea1 100644 --- a/Modules/Graphics/Types/Sources/Methane/Graphics/Types.cpp +++ b/Modules/Graphics/Types/Sources/Methane/Graphics/Types.cpp @@ -28,21 +28,39 @@ Methane graphics type functions implementation. #include #include +#include +#include + namespace Methane::Graphics { +// Scissor rect of a frame rect is an intersection of the [offset, offset + dimension_size) range of that rect +// with the [0, render_attachment_size) range of the render attachment. Both bounds are computed in a signed +// 64-bit type, so that a frame rect moved partially or completely outside of the render attachment results in +// a clipped or empty scissor rect, instead of an unsigned integer underflow of its offset or extent. +// The underflowed extent used to be rejected by the graphics API validation, for example a UI text block +// laid out to the right of a narrow window produced a huge scissor extent close to 2^32. + +[[nodiscard]] +inline uint32_t GetNormalizedDimensionOffset(int32_t offset, uint32_t render_attachment_size) +{ + return static_cast(std::clamp(offset, 0, render_attachment_size)); +} + [[nodiscard]] inline uint32_t GetNormalizedDimensionSize(int32_t offset, uint32_t dimension_size, uint32_t render_attachment_size) { - return std::min(dimension_size + offset, render_attachment_size) - (offset >= 0 ? offset : 0); + const int64_t range_begin = std::max(offset, 0); + const int64_t range_end = std::min(static_cast(offset) + dimension_size, render_attachment_size); + return range_end > range_begin ? static_cast(range_end - range_begin) : 0U; } ScissorRect GetFrameScissorRect(const FrameRect& frame_rect, const FrameSize& render_attachment_size) { META_FUNCTION_TASK(); return { - ScissorRect::Point(static_cast(std::max(0, frame_rect.origin.GetX())), - static_cast(std::max(0, frame_rect.origin.GetY()))), + ScissorRect::Point(GetNormalizedDimensionOffset(frame_rect.origin.GetX(), render_attachment_size.GetWidth()), + GetNormalizedDimensionOffset(frame_rect.origin.GetY(), render_attachment_size.GetHeight())), ScissorRect::Size(GetNormalizedDimensionSize(frame_rect.origin.GetX(), frame_rect.size.GetWidth(), render_attachment_size.GetWidth()), GetNormalizedDimensionSize(frame_rect.origin.GetY(), frame_rect.size.GetHeight(), render_attachment_size.GetHeight())) }; diff --git a/Modules/Platform/App/Sources/Methane/Platform/AppBase.cpp b/Modules/Platform/App/Sources/Methane/Platform/AppBase.cpp index e4d5ea4c7..1cf1a4de2 100644 --- a/Modules/Platform/App/Sources/Methane/Platform/AppBase.cpp +++ b/Modules/Platform/App/Sources/Methane/Platform/AppBase.cpp @@ -242,7 +242,11 @@ bool AppBase::Resize(const Data::FrameSize& frame_size, bool is_minimized) m_frame_size = frame_size; } - return m_initialized && is_resizing; + // Resizing is stopped after an error the same way as rendering is stopped in UpdateAndRender(): + // Init() sets m_initialized before the derived application has finished its own initialization, + // so an Init() interrupted by an exception leaves partially constructed application state, which + // the platform apps would otherwise keep resizing until the error alert terminates the process. + return m_initialized && is_resizing && !HasError(); } void AppBase::Alert(const Message& msg, bool deferred) diff --git a/Modules/Platform/Utils/Sources/Methane/Platform/Utils.cpp b/Modules/Platform/Utils/Sources/Methane/Platform/Utils.cpp index 2ee263735..0948ee1ea 100644 --- a/Modules/Platform/Utils/Sources/Methane/Platform/Utils.cpp +++ b/Modules/Platform/Utils/Sources/Methane/Platform/Utils.cpp @@ -31,7 +31,7 @@ namespace Methane::Platform static std::atomic& GetPrintToConsoleEnabledFlag() noexcept { - static std::atomic s_print_to_console_enabled(false); + static std::atomic s_print_to_console_enabled(false); return s_print_to_console_enabled; } diff --git a/Modules/UserInterface/App/Include/Methane/UserInterface/AppController.h b/Modules/UserInterface/App/Include/Methane/UserInterface/AppController.h index 037336e9c..173c3cf05 100644 --- a/Modules/UserInterface/App/Include/Methane/UserInterface/AppController.h +++ b/Modules/UserInterface/App/Include/Methane/UserInterface/AppController.h @@ -44,7 +44,7 @@ class AppController { public: using ActionByKeyboardState = pin::Keyboard::ActionControllerBase::ActionByKeyboardState; - inline static const ActionByKeyboardState default_action_by_keyboard_state{ + inline static const ActionByKeyboardState default_action_by_keyboard_state{ // NOSONAR - per-level default key map is intentional, always referenced fully qualified { { pin::Keyboard::Key::F4 }, AppAction::SwitchHeadsUpDisplayMode }, }; diff --git a/Modules/UserInterface/Types/Include/Methane/UserInterface/Container.h b/Modules/UserInterface/Types/Include/Methane/UserInterface/Container.h index aa9880c50..e7a426f9b 100644 --- a/Modules/UserInterface/Types/Include/Methane/UserInterface/Container.h +++ b/Modules/UserInterface/Types/Include/Methane/UserInterface/Container.h @@ -42,7 +42,7 @@ class Container , public Data::Emitter { public: - Container(Context& ui_context, const UnitRect& ui_rect = {}, const Ptrs& children = {}); + explicit Container(Context& ui_context, const UnitRect& ui_rect = {}, const Ptrs& children = {}); const Ptrs& GetChildren() const noexcept { return m_children; } diff --git a/Modules/UserInterface/Types/Include/Methane/UserInterface/Item.h b/Modules/UserInterface/Types/Include/Methane/UserInterface/Item.h index 2c34a16a4..926c48c22 100644 --- a/Modules/UserInterface/Types/Include/Methane/UserInterface/Item.h +++ b/Modules/UserInterface/Types/Include/Methane/UserInterface/Item.h @@ -45,7 +45,7 @@ class Item , public std::enable_shared_from_this { public: - Item(Context& ui_context, const UnitRect& ui_rect = {}); + explicit Item(Context& ui_context, const UnitRect& ui_rect = {}); Ptr GetPtr() { return shared_from_this(); } const Context& GetUIContext() const noexcept { return m_ui_context; } diff --git a/README.md b/README.md index e6fe76b4e..776a28513 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,6 @@ [![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=methane-powered-kit-windows&metric=alert_status)](https://sonarcloud.io/dashboard?id=methane-powered-kit-windows) [![CodeCov](https://codecov.io/gh/MethanePowered/MethaneKit/branch/master/graph/badge.svg?token=VBY74VXWX5)](https://codecov.io/gh/MethanePowered/MethaneKit) [![Test Space Metric](https://methanepowered.testspace.com/spaces/190760/badge?token=196ea878d56f2b034a8d29a9b4ba6a94c968643c)](https://methanepowered.testspace.com/spaces/190760?utm_campaign=badge&utm_medium=referral&utm_source=test "Test Cases") -[![Gitpod Ready-to-Code](https://img.shields.io/badge/Gitpod-ready--to--code-blue?logo=gitpod)](https://gitpod.io/#https://github.com/MethanePowered/MethaneKit) **Easy to use modern 3D graphics rendering abstraction API and cross-platform application framework:** - **Builds on top of modern native 3D graphics APIs**: DirectX 12 on Windows, Vulkan on Linux, Metal on macOS, iOS & tvOS. @@ -17,8 +16,6 @@ Download [release builds](https://github.com/MethanePowered/MethaneKit/releases) Check the latest build status, tests, code coverage, and analysis results or get build artifacts from [GitHub Actions](https://github.com/MethanePowered/MethaneKit/actions) CI and [Sonar Cloud](https://sonarcloud.io/organizations/methane-powered). See the [Build Instructions](/Build/README.md) topic for manual build instructions and start learning the [Methane Graphics RHI](/Modules/Graphics/RHI) API with [Hello Triangle](/Apps/01-HelloTriangle) and other tutorials' documentation. -[![Open in Gitpod](https://gitpod.io/button/open-in-gitpod.svg)](https://gitpod.io/#https://github.com/MethanePowered/MethaneKit) - | Platform | Graphics API | Master Build | Develop Build | |-------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | **Windows x64** | DirectX 12 | [![Windows x64 Master Build](https://img.shields.io/endpoint?url=https://gist.githubusercontent.com/egorodet/96d788046ccd52b45b3354a99f8569c3/raw/MethaneKit_Win64_DX_Release_master.json)](https://github.com/MethanePowered/MethaneKit/actions/workflows/ci-build.yml?query=branch%3Amaster) | [![Windows x64 Develop Build](https://img.shields.io/endpoint?url=https://gist.githubusercontent.com/egorodet/96d788046ccd52b45b3354a99f8569c3/raw/MethaneKit_Win64_DX_Release_develop.json)](https://github.com/MethanePowered/MethaneKit/actions/workflows/ci-build.yml?query=branch%3Adevelop) | @@ -150,7 +147,7 @@ For detailed features description and development plans please refer to [Modules ### Development Environments - Jet Brains CLion (pre-configured with [.idea](/.idea)) -- Microsoft VS Code and [GitPod](https://gitpod.io/#https://github.com/MethanePowered/MethaneKit) (pre-configured with [CMakePresets.json](/CMakePresets.json) and [.vscode/settings.json](/.vscode/settings.json)) +- Microsoft VS Code (pre-configured with [CMakePresets.json](/CMakePresets.json) and [.vscode/settings.json](/.vscode/settings.json)) - Microsoft Visual Studio 2022 - Solutions and projects build (generate with [Build/Windows/Build.bat](/Build/Windows/Build.bat)) - CMake native build support (pre-configured with [CMakePresets.json](/CMakePresets.json)) diff --git a/Tests/Graphics/RHI/RenderPassTest.cpp b/Tests/Graphics/RHI/RenderPassTest.cpp index fc0de1b06..8a23aec62 100644 --- a/Tests/Graphics/RHI/RenderPassTest.cpp +++ b/Tests/Graphics/RHI/RenderPassTest.cpp @@ -28,6 +28,8 @@ Unit-tests of the RHI RenderPass #include #include #include +#include +#include #include #include @@ -132,6 +134,25 @@ TEST_CASE("RHI Render Pass Functions", "[rhi][render][pass]") CHECK(render_pass.GetSettings() == new_render_pass_resources.settings); } + SECTION("Update Settings Invalidates Attachment Textures Cache") + { + const auto& base_render_pass = dynamic_cast(render_pass.GetInterface()); + + // Populate lazily-initialized attachment texture caches with the original attachments + const Base::Texture* orig_color_texture_ptr = &base_render_pass.GetColorAttachmentTextures().front().get(); + const Base::Texture* orig_depth_texture_ptr = base_render_pass.GetDepthAttachmentTexture(); + CHECK(orig_color_texture_ptr == render_pass_resources.frame_buffer_texture.GetInterfacePtr().get()); + CHECK(orig_depth_texture_ptr == render_pass_resources.depth_stencil_texture.GetInterfacePtr().get()); + + const Test::RenderPassResources new_render_pass_resources = Test::GetRenderPassResources(render_pattern); + CHECK(render_pass.Update(new_render_pass_resources.settings)); + + CHECK(&base_render_pass.GetColorAttachmentTextures().front().get() == + new_render_pass_resources.frame_buffer_texture.GetInterfacePtr().get()); + CHECK(base_render_pass.GetDepthAttachmentTexture() == + new_render_pass_resources.depth_stencil_texture.GetInterfacePtr().get()); + } + SECTION("Release Attachment Textures") { const Rhi::RenderPassSettings& render_pass_settings = render_pass.GetSettings(); @@ -140,3 +161,50 @@ TEST_CASE("RHI Render Pass Functions", "[rhi][render][pass]") CHECK(render_pass_settings.attachments.empty()); } } + +TEST_CASE("RHI Render Pass with Stencil Attachment Functions", "[rhi][render][pass]") +{ + const Rhi::RenderPattern render_pattern = render_context.CreateRenderPattern(Test::GetRenderPatternSettings(true)); + const Test::RenderPassResources render_pass_resources = Test::GetRenderPassResources(render_pattern); + const Rhi::RenderPass render_pass = render_pattern.CreateRenderPass(render_pass_resources.settings); + const auto& base_render_pass = dynamic_cast(render_pass.GetInterface()); + + SECTION("Get Stencil Attachment Texture") + { + REQUIRE(render_pass_resources.stencil_texture.IsInitialized()); + CHECK(base_render_pass.GetStencilAttachmentTexture() == + render_pass_resources.stencil_texture.GetInterfacePtr().get()); + } + + SECTION("Update Settings Invalidates Stencil Attachment Texture Cache") + { + // Populate the lazily-initialized stencil attachment texture cache with the original attachment + REQUIRE(base_render_pass.GetStencilAttachmentTexture() == + render_pass_resources.stencil_texture.GetInterfacePtr().get()); + + const Test::RenderPassResources new_render_pass_resources = Test::GetRenderPassResources(render_pattern); + REQUIRE(new_render_pass_resources.stencil_texture.GetInterfacePtr() != + render_pass_resources.stencil_texture.GetInterfacePtr()); + CHECK(render_pass.Update(new_render_pass_resources.settings)); + + CHECK(base_render_pass.GetStencilAttachmentTexture() == + new_render_pass_resources.stencil_texture.GetInterfacePtr().get()); + } + + SECTION("Update Settings Invalidates Non-Frame-Buffer Attachment Textures Cache") + { + // Frame buffer color attachment is excluded from the non-frame-buffer textures, so only depth & stencil remain + const Ptrs& orig_textures = base_render_pass.GetNonFrameBufferAttachmentTextures(); + REQUIRE(orig_textures.size() == 2U); + CHECK(orig_textures[0].get() == render_pass_resources.depth_stencil_texture.GetInterfacePtr().get()); + CHECK(orig_textures[1].get() == render_pass_resources.stencil_texture.GetInterfacePtr().get()); + + const Test::RenderPassResources new_render_pass_resources = Test::GetRenderPassResources(render_pattern); + CHECK(render_pass.Update(new_render_pass_resources.settings)); + + const Ptrs& new_textures = base_render_pass.GetNonFrameBufferAttachmentTextures(); + REQUIRE(new_textures.size() == 2U); + CHECK(new_textures[0].get() == new_render_pass_resources.depth_stencil_texture.GetInterfacePtr().get()); + CHECK(new_textures[1].get() == new_render_pass_resources.stencil_texture.GetInterfacePtr().get()); + } +} diff --git a/Tests/Graphics/RHI/RhiSettings.hpp b/Tests/Graphics/RHI/RhiSettings.hpp index d3457ac47..36267573e 100644 --- a/Tests/Graphics/RHI/RhiSettings.hpp +++ b/Tests/Graphics/RHI/RhiSettings.hpp @@ -49,7 +49,7 @@ inline Rhi::RenderContextSettings GetRenderContextSettings() }; } -inline Rhi::RenderPatternSettings GetRenderPatternSettings() +inline Rhi::RenderPatternSettings GetRenderPatternSettings(bool with_stencil_attachment = false) { return Rhi::RenderPatternSettings { @@ -67,7 +67,14 @@ inline Rhi::RenderPatternSettings GetRenderPatternSettings() Rhi::RenderPassAttachment::StoreAction::Store, 0.F }, - .stencil_attachment = std::nullopt, + .stencil_attachment = with_stencil_attachment + ? Opt(Rhi::RenderPassStencilAttachment{ + 2U, PixelFormat::Depth32Float, 1U, + Rhi::RenderPassAttachment::LoadAction::Clear, + Rhi::RenderPassAttachment::StoreAction::Store, + 0U + }) + : std::nullopt, .shader_access = Rhi::RenderPassAccessMask{ Rhi::RenderPassAccess::ShaderResources }, .is_final_pass = true }; @@ -77,6 +84,7 @@ struct RenderPassResources { Rhi::Texture frame_buffer_texture; Rhi::Texture depth_stencil_texture; + Rhi::Texture stencil_texture; Rhi::RenderPassSettings settings; }; @@ -97,6 +105,20 @@ inline RenderPassResources GetRenderPassResources(const Rhi::RenderPattern& rend resources.settings.frame_size = frame_size; resources.settings.attachments.push_back(resources.frame_buffer_texture.GetTextureView()); resources.settings.attachments.push_back(resources.depth_stencil_texture.GetTextureView()); + + // Separate stencil attachment texture is added only when the render pattern declares a stencil attachment, + // so that its index in the render pass attachments matches RenderPassStencilAttachment::attachment_index. + if (const Opt& stencil_attachment = render_pattern.GetSettings().stencil_attachment; + stencil_attachment) + { + resources.stencil_texture = render_pattern.GetRenderContext().CreateTexture( + Rhi::TextureSettings::ForDepthStencil( + Dimensions(frame_size), + stencil_attachment->format, + DepthStencilValues(0.f, 0.f), + Rhi::ResourceUsageMask({ Rhi::ResourceUsage::RenderTarget }))); + resources.settings.attachments.push_back(resources.stencil_texture.GetTextureView()); + } return resources; } diff --git a/pull_request_body.md b/pull_request_body.md deleted file mode 100644 index 4a3dd73b0..000000000 --- a/pull_request_body.md +++ /dev/null @@ -1,94 +0,0 @@ -### Summary - -In **Methane Kit v0.8.2**, the following improvements have been introduced: - -- **Compiler Compatibility**: Build compatibility has been fixed for Visual Studio 2026 (MSVC v143+), latest Clang/AppleClang on macOS Sequoia/Xcode 26, and GCC. New CMake presets for VS2026 have been added alongside existing VS2022 presets. VS2026 requires CMake 4.2 or later; other presets require CMake 3.24+. -- **Vulkan RHI Stability and Validation Fixes**: Multiple Vulkan validation errors have been resolved, including semaphore lifecycle issues (`VUID-vkAcquireNextImageKHR-semaphore-01779`), descriptor set copy by-binding fix, empty pipeline stage masks (`VUID-vkCmdPipelineBarrier`), HLSL reflection extensions handling (`VUID-VkShaderModuleCreateInfo-pCode-08742`), and primitive restart behavior on MoltenVK. The per-device Vulkan dispatcher initialization bug on multi-GPU systems has been fixed. Image acquisition now retries on timeout and handles surface-lost conditions. (close #158) -- **Command Execution Stability**: A race condition in `CommandQueueTracking` that could cause command list resets to fail (`WaitUntilCompleted` / `CompleteExecution` FIFO ordering) has been fixed by refactoring `ProcessExecutingCommandListSet` into an atomic pop-and-wait operation. Frame-less command list sets no longer interfere with frame-zero completion state. -- **MacOS / Metal Build Fixes**: Metal Toolchain download is now automated in CI for Xcode 26+. Objective-C ARC handling in Vulkan RHI for Apple platforms has been fixed. MacOS app delegate now suppresses secure restorable state warnings and disables unsupported window state restoration. - -### Graphics libraries - -- Fixed `RenderCommandList::UpdateDrawingState` logic inversion that incorrectly skipped primitive type change tracking. -- `CommandListSet::GetCombinedName()` now returns `std::string` by value (member made `mutable`) to prevent data races when accessed concurrently with name-change callbacks. Added a mutable mutex to synchronize lazy semaphore creation and access. -- `CommandQueueTracking`: Replaced `GetNextExecutingCommandListSet()` with `PopNextExecutingCommandListSet()` that atomically pops and waits, preventing a race between `WaitUntilCompleted`, `CompleteExecution`, and the background execution-waiting thread; added `ProcessExecutingCommandListSet` template helper and `IsExecutingOnFrameIndex` for correct FIFO-ordered frame completion. Frame wait state is now only reset for frame-indexed command list sets, not frame-less ones. -- Added new `Methane::StbImage.h` wrapper header that correctly suppresses compiler warnings and conditionally disables SIMD for GCC debug builds, replacing ad-hoc `stb_image.h` inclusions. The STB dependency is now installed and exported for external consumers. - -### Vulkan RHI - -- **Semaphore lifecycle fix**: Removed the per-frame `vk::Fence` from `RenderContext::FrameSync`; image acquisition now waits on the render queue `WaitUntilCompleted` instead of a CPU-side fence, correctly satisfying `VUID-vkAcquireNextImageKHR-semaphore-01779`. Added `g_image_acquire_timeout_ns` (1 s, 64-bit `uint64_t`) with retry logic for timeout and surface-lost conditions. -- **Image acquisition robustness**: `AcquireNextImage()` now wraps `acquireNextImageKHR` in a retry loop, explicitly handling non-fatal results (`eTimeout`, `eNotReady`) by retrying, and `eErrorSurfaceLostKHR` by destroying the lost surface, recreating it, resetting the swapchain, and retrying. -- **Execution-completed semaphore**: Made `m_vk_unique_execution_completed_semaphore` lazily created and `mutable`; the semaphore is now only created and signalled for render command list sets that target a specific frame buffer, eliminating `VUID-vkQueueSubmit-pSignalSemaphores-00067` errors caused by permanently-signalled semaphores on non-frame command lists. -- **Device dispatcher**: `VULKAN_HPP_DEFAULT_DISPATCHER.init(vk::Device)` is no longer called in `Device` constructor (fixes multi-GPU dispatch to the wrong ICD); it is now called once per `RenderContext` in `Initialize()`. Instance-level initialization is preserved per `Device.cpp` guidelines. -- **HLSL reflection extensions**: Added `VK_GOOGLE_HLSL_FUNCTIONALITY_1` and `VK_GOOGLE_USER_TYPE` device extensions; when not supported, SPIRV byte code is stripped of reflection instructions via `RemoveHlslReflectionFromSpirv()` before shader module creation, fixing `VUID-VkShaderModuleCreateInfo-pCode-08742`. Bounds validation is now always-on to prevent out-of-bounds access on malformed input. -- **Descriptor set copy**: Fixed `ProgramBindings` mutable descriptor copy to iterate per-binding rather than using a single flat count, preventing out-of-bounds descriptor access with non-contiguous bindings. -- **Pipeline barrier stage masks**: Empty `srcStageMask`/`dstStageMask` are now replaced with `eTopOfPipe`/`eBottomOfPipe` respectively, fixing `VUID-vkCmdPipelineBarrier-srcStageMask-03937` / `dstStageMask-03937`. -- **Primitive restart (MoltenVK)**: `RenderState` now detects `VK_KHR_portability_subset` and automatically enables primitive restart for strip/dynamic topologies on devices that cannot disable it, silencing MoltenVK "Metal does not support disabling primitive restart" warnings. -- **Surface lifetime management**: On `eErrorSurfaceLostKHR`, the old `vk::UniqueSurfaceKHR` is now moved to a local variable before creating the replacement, ensuring it is destroyed after swapchain cleanup rather than leaked via `release()`. -- **Removed MoltenVK workarounds** for `VkSubmitInfo` + `VkTimelineSemaphoreSubmitInfo` which are no longer necessary with the current Vulkan SDK. -- Removed stale `DebugUtilsMessengerCallback` message-ID suppression filters that are no longer needed; callback signature updated to use `vk::` types directly. -- Fixed `RenderPass::CreateNativeFrameBuffer` to validate attachment texture dimensions against frame size. -- Disabled Objective-C ARC for `RenderContext.mm` and `PlatformExt.mm` (workaround for `vulkan.hpp` constant-expression CAMetalLayer issue with ARC). -- Added explicit `[m_metal_view release]` in the Apple `RenderContext` destructor for non-ARC builds. - -### Tests - -- Added unit tests to cover `RenderCommandList` drawing with multiple primitive types in a single command list (`Draw` and `DrawIndexed`), covering the `PrimitiveType` change tracking bug. - -### Build - -- Added VS2026 (`Visual Studio 18 2026`) CMake configure and build presets for Win64/Win32 × DX/VK × Default/Profile/Scan variants. CMake 4.2+ is required for VS2026 presets. -- `Build.bat` default generator changed to VS2026; `--vs2022` flag added for backward compatibility. -- `Build/Unix/CI/InstallMacOsPrerequisites.sh` script added to download Metal Toolchain on Xcode 26+ using a BSD-compatible version comparison function. -- Fixed ARM64 architecture detection in `CMake/MethaneModules.cmake` to match all common variants (`ARM64`, `arm64`, `AARCH64`, `aarch64`) with a case-insensitive regex. -- Added `CLAUDE.md` with project guidance for Claude Code and updated CMake version requirements. - -### Continuous Integration - -- All CI workflows updated to use VS2026 presets on Windows. -- All GitHub Actions updated: `actions/checkout@v7`, `actions/cache@v6`, `actions/upload-artifact@v7`, `github/codeql-action/*@v4`. -- Sonar Scanner migrated from the legacy `RunSonarScanner.sh` shell script to `SonarSource/sonarqube-scan-action@v8.2.0` with separate steps for push and pull-request events. -- Added macOS prerequisites installation step in CI build, CodeQL, and Sonar scan workflows. -- Disabled CodeQL scan on macOS due to Metal Toolchain availability issues. -- Added security hardening: `persist-credentials: false` on checkout steps, `contents: read` permission grants, and proper GitHub expression syntax for Sonar project version. - -### External libraries - -- [DirectXShaderCompilerBinary](https://github.com/MethanePowered/DirectXShaderCompilerBinary) updated to `v1.9.2602` (pinned by immutable commit SHA) -- [VulkanHeaders](https://github.com/MethanePowered/VulkanHeaders) updated to `v1.4.357.0` -- [SPIRVCross](https://github.com/MethanePowered/SPIRVCrossBinary) updated to `v1.4.350.1` -- [Tracy](https://github.com/MethanePowered/Tracy) updated to `v0.13.1` -- [DirectXTex](https://github.com/MethanePowered/DirectXTex) updated to `v2.1.1` (2026-05) -- [DirectXHeaders](https://github.com/MethanePowered/DirectXHeaders) updated to `v1.619.5` -- [FMT](https://github.com/fmtlib/fmt) updated to `v12.2.0` -- [FTXUI](https://github.com/MethanePowered/FTXUI) updated to `v7.0.3` -- [HLSL++](https://github.com/MethanePowered/HLSLpp) updated to `v3.9` -- [TaskFlow](https://github.com/MethanePowered/TaskFlow) updated to `v4.1.0` -- [Catch2](https://github.com/MethanePowered/Catch2) updated to `v3.15.3` -- [CLI11](https://github.com/MethanePowered/CLI11) updated to `v2.7.2` -- [MagicEnum](https://github.com/MethanePowered/MagicEnum) updated to `v0.9.8` -- [IttApi](https://github.com/MethanePowered/IttApi) updated to `v3.28.2` -- [CPM.cmake](https://github.com/cpm-cmake/CPM.cmake) updated to `v0.43.1` -- [CMakeModules](https://github.com/MethanePowered/CMakeModules) updated (latest `methane` branch) -- [iOS-Toolchain.cmake](https://github.com/leetal/ios-cmake) updated to `v4.6.0` with correct standard configuration flag variable propagation for `try_compile` - - -## Summary by CodeRabbit - -* **New Features** - * Added Visual Studio 2026 build presets, while retaining Visual Studio 2022 support. - * Expanded Apple platform toolchain support, including visionOS and combined simulator/device builds. - * Added shared image-loading support, improved ARM64 detection, and a cross-platform application test runner. - -* **Bug Fixes** - * Improved Vulkan synchronization, shader compatibility, descriptor updates, and primitive restart handling. - * Improved command-list processing and application shutdown reliability. - * Fixed macOS window restoration compatibility. - * Enhanced image acquisition robustness with retry logic for timeout and surface-lost conditions. - * Fixed race conditions in semaphore access and frame completion tracking. - * Corrected surface lifetime management to prevent leaks on device loss. - -* **Documentation** - * Updated build instructions and Metal toolchain setup guidance. - * Documented CMake version requirements for VS2026 presets. -