From 35518133bd2c35ccad87470d5a5a35868e5f5e67 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sat, 10 May 2025 23:22:14 +0300 Subject: [PATCH 01/17] Bump MethaneKit version to v0.8.2 --- .github/workflows/ci-build.yml | 2 +- .github/workflows/ci-sonar-scan.yml | 2 +- Build/Unix/Build.sh | 2 +- Build/Windows/Build.bat | 2 +- CMakeLists.txt | 2 +- Modules/Common/Primitives/Include/Methane/Version.h | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index bc487490d..0a7231c3f 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -34,7 +34,7 @@ on: env: METHANE_VERSION_MAJOR: 0 METHANE_VERSION_MINOR: 8 - METHANE_VERSION_PATCH: 1 + METHANE_VERSION_PATCH: 2 METHANE_VERSION_BUILD: ${{ github.run_number }} TRACY_RELEASE_VERSION: "0.11.2.1" VULKAN_SDK_VERSION: "1.3.290.0" diff --git a/.github/workflows/ci-sonar-scan.yml b/.github/workflows/ci-sonar-scan.yml index a1f72d722..e6e2927f6 100644 --- a/.github/workflows/ci-sonar-scan.yml +++ b/.github/workflows/ci-sonar-scan.yml @@ -59,7 +59,7 @@ on: env: METHANE_VERSION_MAJOR: 0 METHANE_VERSION_MINOR: 8 - METHANE_VERSION_PATCH: 1 + METHANE_VERSION_PATCH: 2 METHANE_VERSION_BUILD: ${{ github.run_number }} jobs: diff --git a/Build/Unix/Build.sh b/Build/Unix/Build.sh index 6e2c9a40d..096dc0ac9 100755 --- a/Build/Unix/Build.sh +++ b/Build/Unix/Build.sh @@ -10,7 +10,7 @@ BUILD_VERSION_MAJOR=0 BUILD_VERSION_MINOR=8 -BUILD_VERSION_PATCH=1 +BUILD_VERSION_PATCH=2 BUILD_VERSION=$BUILD_VERSION_MAJOR.$BUILD_VERSION_MINOR.$BUILD_VERSION_PATCH SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd ) diff --git a/Build/Windows/Build.bat b/Build/Windows/Build.bat index cf2216d11..642636c82 100644 --- a/Build/Windows/Build.bat +++ b/Build/Windows/Build.bat @@ -15,7 +15,7 @@ SETLOCAL ENABLEDELAYEDEXPANSION SET BUILD_VERSION_MAJOR=0 SET BUILD_VERSION_MINOR=8 -SET BUILD_VERSION_PATCH=1 +SET BUILD_VERSION_PATCH=2 SET BUILD_VERSION=%BUILD_VERSION_MAJOR%.%BUILD_VERSION_MINOR%.%BUILD_VERSION_PATCH% SET OUTPUT_DIR=%~dp0..\Output diff --git a/CMakeLists.txt b/CMakeLists.txt index 4822001de..ce631c956 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.24.0) # Methane version, build & product info set(METHANE_VERSION_MAJOR 0 CACHE STRING "Methane major version") set(METHANE_VERSION_MINOR 8 CACHE STRING "Methane minor version") -set(METHANE_VERSION_PATCH 1 CACHE STRING "Methane patch version") +set(METHANE_VERSION_PATCH 2 CACHE STRING "Methane patch version") set(METHANE_VERSION_BUILD 0 CACHE STRING "Methane build version") # Define CMake languages list diff --git a/Modules/Common/Primitives/Include/Methane/Version.h b/Modules/Common/Primitives/Include/Methane/Version.h index b0f019aac..27cdb1b89 100644 --- a/Modules/Common/Primitives/Include/Methane/Version.h +++ b/Modules/Common/Primitives/Include/Methane/Version.h @@ -36,11 +36,11 @@ Methane version macro definitions #endif #ifndef METHANE_VERSION_MINOR -#define METHANE_VERSION_MINOR 0 +#define METHANE_VERSION_MINOR 8 #endif #ifndef METHANE_VERSION_PATCH -#define METHANE_VERSION_PATCH 0 +#define METHANE_VERSION_PATCH 2 #endif #ifndef METHANE_VERSION_BUILD From 1480ea850c105f394e133c39d5722f82d7b3e20f Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetsky Date: Sun, 18 May 2025 22:42:01 +0300 Subject: [PATCH 02/17] Update CMakeModules version to "methane" branch --- Externals/CMakeModules.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Externals/CMakeModules.cmake b/Externals/CMakeModules.cmake index 1d4f7a377..2894e58de 100644 --- a/Externals/CMakeModules.cmake +++ b/Externals/CMakeModules.cmake @@ -1,7 +1,7 @@ CPMAddPackage( NAME CMakeModules GITHUB_REPOSITORY MethanePowered/CMakeModules - GIT_TAG 09b04c9d113cd3a5ed49711999de68896e352c60 # last commit from 'methane' branch + GIT_TAG methane DOWNLOAD_ONLY YES ) From f4fb61c12459b16edb63633c4ea8e281baf2a422 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sun, 23 Aug 2026 13:00:40 +0300 Subject: [PATCH 03/17] Fix SonarQube static analysis issues on MacOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule n Change S1709 8 Added explicit to single-arg-callable constructors S7172 4 optional → .has_value() in ITexture.cpp, Resource.cpp:118 S8379 4 Real bug — RenderPass.cpp invalidated its lazy attachment caches without m_mutex, while every getter that fills them takes it S1121 2 EnumMask.hpp:100 — assignment out of the return expression S6012 2 CTAD in Context.cpp:249, Utils.cpp:34 S1117 1 Renamed shadowing param in ISampler.h:54 S8467 1 condition_variable_any → condition_variable in CommandKit.cpp:234 (plain local std::mutex) --- Apps/04-ShadowCube/ShadowCubeApp.cpp | 2 +- .../Primitives/Include/Methane/Exceptions.hpp | 2 +- .../Include/Methane/Data/TimeAnimation.hpp | 2 +- .../Data/Events/Include/Methane/Data/Emitter.hpp | 3 +++ .../Events/Include/Methane/Data/Receiver.hpp | 3 +++ .../Data/Types/Include/Methane/Data/EnumMask.hpp | 4 ++-- .../Methane/Graphics/AppContextController.h | 2 +- .../Include/Methane/Graphics/Base/CommandList.h | 2 ++ .../Sources/Methane/Graphics/Base/CommandKit.cpp | 6 +++--- .../Sources/Methane/Graphics/Base/Context.cpp | 2 +- .../Sources/Methane/Graphics/Base/RenderPass.cpp | 16 ++++++++++++---- .../Sources/Methane/Graphics/Base/Resource.cpp | 2 +- .../Methane/Graphics/RHI/CommandListSet.h | 2 +- .../Include/Methane/Graphics/RHI/ISampler.h | 5 +++-- .../Include/Methane/Graphics/RHI/ITexture.h | 8 ++++---- .../Sources/Methane/Graphics/RHI/ITexture.cpp | 6 +++--- .../Utils/Sources/Methane/Platform/Utils.cpp | 2 +- .../Include/Methane/UserInterface/Container.h | 2 +- .../Types/Include/Methane/UserInterface/Item.h | 2 +- 19 files changed, 45 insertions(+), 28 deletions(-) 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/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..37a8de683 100644 --- a/Modules/Data/Events/Include/Methane/Data/Emitter.hpp +++ b/Modules/Data/Events/Include/Methane/Data/Emitter.hpp @@ -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..0f751fc1a 100644 --- a/Modules/Data/Events/Include/Methane/Data/Receiver.hpp +++ b/Modules/Data/Events/Include/Methane/Data/Receiver.hpp @@ -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/RHI/Base/Include/Methane/Graphics/Base/CommandList.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h index 555dc6439..43be7a029 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h @@ -140,6 +140,8 @@ 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); + // 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; TRACY_GPU_SCOPE_TYPE m_tracy_gpu_scope; 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..fe431c13c 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp @@ -229,9 +229,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&) { 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..34ef2e996 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp @@ -246,7 +246,7 @@ 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 }; for (const auto& [cmd_queue_ptr, cmd_kit_ptr] : m_default_command_kit_ptr_by_queue) { 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..592632bcf 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp @@ -49,9 +49,13 @@ bool RenderPass::Update(const Rhi::RenderPassSettings& settings) m_settings = settings; - m_non_frame_buffer_attachment_textures.clear(); - m_color_attachment_textures.clear(); - m_depth_attachment_texture_ptr = nullptr; + { + // Invalidate lazily initialized attachment texture caches under the same lock which guards their initialization + std::scoped_lock lock_guard(m_mutex); + m_non_frame_buffer_attachment_textures.clear(); + m_color_attachment_textures.clear(); + m_depth_attachment_texture_ptr = nullptr; + } InitAttachmentStates(); return true; @@ -60,7 +64,11 @@ bool RenderPass::Update(const Rhi::RenderPassSettings& settings) void RenderPass::ReleaseAttachmentTextures() { META_FUNCTION_TASK(); - m_non_frame_buffer_attachment_textures.clear(); + { + // Invalidate lazily initialized attachment texture cache under the same lock which guards its initialization + 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/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/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/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/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; } From 8ef65261830515f3d6b27e7f66d03a1ce80c8206 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sun, 23 Aug 2026 14:23:55 +0300 Subject: [PATCH 04/17] Fix GPU timestamp calibration error in Vulkan::QueryPool on Apple with MoltenVK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Time domain selection — replaced the hardcoded constant with a platform preference list, resolved at runtime against what the device actually reports. macOS/iOS now get CLOCK_MONOTONIC_RAW, which MoltenVK does support. I verified against the installed MoltenVK with a standalone probe: it advertises DEVICE + CLOCK_MONOTONIC_RAW, the CPU timestamps match clock_gettime(CLOCK_MONOTONIC_RAW) to within 42 ns (so GetQpcToNSecMultiplier() == 1 is correct), timestampPeriod is 1.0 ns, and calibration deviation is 0–958 ns — well inside the retry loop's bounds. Tracy's calibration path only consumes the nanosecond delta, so the profiling data is genuinely correct, not just non-crashing. 2. Query pool sizing — the pool asked for a fixed 32768 queries while its data buffer only holds frame_buffers_count × 1000 timestamps. That over-allocation exceeded Metal's 32 KB MTLCounterSampleBuffer limit, so MoltenVK printed an error per queue and silently fell back to emulated timestamps, defeating the point of a Profile build. Sizing the pool to the buffer's real capacity fixes it; the effective query limit is unchanged, since the data buffer was always the binding constraint. --- .../Methane/Graphics/Vulkan/QueryPool.h | 1 + .../Methane/Graphics/Vulkan/QueryPool.cpp | 66 ++++++++++++++----- 2 files changed, 52 insertions(+), 15 deletions(-) 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; From b7143c95f4771397be588e4ee3c6eef087ca04f5 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sun, 23 Aug 2026 14:25:40 +0300 Subject: [PATCH 05/17] Fix stencil texture invalidation on RenderPass reset --- .../Methane/Graphics/Base/RenderPass.cpp | 1 + Tests/Graphics/RHI/RenderPassTest.cpp | 68 +++++++++++++++++++ Tests/Graphics/RHI/RhiSettings.hpp | 26 ++++++- 3 files changed, 93 insertions(+), 2 deletions(-) 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 592632bcf..01902d5bb 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/RenderPass.cpp @@ -55,6 +55,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(); 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; } From 0895d45f309db05ce6f6c9fc2b5bf8b9a20aec75 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sun, 23 Aug 2026 14:57:02 +0300 Subject: [PATCH 06/17] Fix Typography app crash on resizing --- Apps/05-Typography/TypographyApp.cpp | 6 ++++++ Modules/Platform/App/Sources/Methane/Platform/AppBase.cpp | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) 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/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) From ff5c1f1260327e88763233f381d2d6b4b879a85e Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sun, 23 Aug 2026 21:47:39 +0300 Subject: [PATCH 07/17] Fix data-race seg-fault for native Metal resources in Metal::ProgramArgumentBinding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MethaneParallelRenderingRootConstants aborted on an uncaught META_CHECK_EQUAL(native_buffers.size(), offsets.size()) at ProgramBindings.mm:457, thrown on a taskflow worker, rethrown by Executor::run(...).get() on the main thread, and escaping -[NSApplication run]. It was a data race on Metal::ProgramArgumentBinding's native resource vectors. SetMetalResourcesForViews cleared and refilled five vectors in place with no lock, while readers in ProgramBindings — which serialize only against each other via the ArgumentsBuffer::DataLock — read them from other threads. 07-ParallelRendering hits this hard: it builds 1727 program bindings on a taskflow (ParallelRenderingApp.cpp:235) and, in the RootConstants variant, rewrites every cube's root constant each frame on another (:413). Each root-constant buffer reallocation broadcasts OnRootConstantBufferChanged (RootConstantBuffer.cpp:236) so one thread rewrites every binding while the others are reading them. Scope correction The check only catches the buffers/offsets pair, but all five vectors plus m_mtl_resource_usage were rewritten in place and read concurrently. The others fail silently and worse — a reader can dereference a std::vector's storage while the writer reallocates it. So I snapshotted the whole set rather than just the pair the check happened to guard. That's a slightly wider change than the literal ask; the narrower fix would have left real UB behind. The fix An immutable NativeResourceViews snapshot published in one step. SetMetalResourcesForViews builds it entirely in a local shared_ptr, then swaps it in; readers take a strong reference and keep it alive for as long as they use it. The mutex is held only for the pointer copy, never while building or using the snapshot — so the parallel update loop isn't serialized. Removing the five old accessors forced every call site to be updated, so no silently-racy read remains. I also added an explicit copy constructor: CreateCopy() relied on the implicit one, which the mutex member would have deleted — and the copy is itself a cross-thread reader, so it takes the source snapshot under the source's lock. --- .../Graphics/Metal/ProgramArgumentBinding.hh | 34 ++++++----- .../Graphics/Metal/ProgramArgumentBinding.mm | 57 +++++++++++++------ .../Methane/Graphics/Metal/ProgramBindings.mm | 29 ++++++---- 3 files changed, 78 insertions(+), 42 deletions(-) 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..a20345b6b 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.m_mtl_resource_views) { } @@ -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) From 09a0743d461cbe186b0fcb28f4f211aa02d1bee4 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Sun, 23 Aug 2026 20:25:42 +0000 Subject: [PATCH 08/17] Fix double-pop of the execution tracking queue in Base::CommandQueueTracking, introduced by commit 1184cff1 ("Fix command list set execution state tracking") MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The background execution-waiting thread does: 1. PopNextExecutingCommandListSet() // locks queue, pops front, waits on fence, Complete() 2. CompleteCommandListSetExecution() // locks queue again, pops front if it == the set just completed The set is already gone from the queue after the first call, so the second pop was meant to be a no-op. But between the two calls the queue mutex is released, and the main thread — which owns the same CommandListSet object for that frame buffer index — can come all the way around the frame ring and re-execute the very same object, pushing it back to the front. CompleteCommandListSetExecution then matched it by address and popped that new execution out of tracking. From there the failure is deterministic: the set is Executing but untracked, so WaitUntilCompleted(frame_index) finds nothing via IsExecutingOnFrameIndex() and returns without waiting, and the next Reset() on its command lists throws InvalidArgumentException — CommandList.cpp:98. Heavy CPU load widens the window: the background thread gets descheduled right after PopNextExecutingCommandListSet() returns, while the main thread runs a full 3-frame ring in well under a millisecond. --- .../Graphics/Base/CommandQueueTracking.h | 1 + .../Graphics/Base/CommandQueueTracking.cpp | 39 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) 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/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp index 8c9d3baed..9fa4a7e00 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp @@ -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() From fe53f558645bf636ab029a5613eb2dfbea33e9e0 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Mon, 24 Aug 2026 05:07:28 +0000 Subject: [PATCH 09/17] Fix wrapping-out sizes of viewports and scissor-rects when Window size is small. Both offset and size are now a true intersection of [offset, offset + dimension_size) with [0, render_attachment_size), computed in int64_t and floored at zero. A block partly outside is clipped; a block fully outside gets a zero-size scissor. --- .../Types/Sources/Methane/Graphics/Types.cpp | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) 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())) }; From 9f1482e0e72a9da48fe6b2d03af7f0ee9b15c16d Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Mon, 24 Aug 2026 18:51:24 +0300 Subject: [PATCH 10/17] Silence wont-fix static analysis issues from SonarQube with in-code comments --- .../Data/Events/Include/Methane/Data/Emitter.hpp | 12 ++++++------ .../Data/Events/Include/Methane/Data/Receiver.hpp | 14 +++++++------- .../App/Include/Methane/Graphics/AppController.h | 2 +- .../Include/Methane/Graphics/Base/CommandList.h | 2 +- .../Methane/Graphics/Base/ProgramBindings.h | 4 ++-- .../Methane/Graphics/Base/RootConstantBuffer.h | 2 +- .../Base/Include/Methane/Graphics/Base/Shader.h | 2 +- .../Methane/Graphics/Base/CommandQueueTracking.cpp | 8 ++++---- .../Include/Methane/Graphics/DirectX/Context.hpp | 2 +- .../Include/Methane/Graphics/DirectX/Device.h | 2 +- .../Methane/Graphics/DirectX/ErrorHandling.h | 2 +- .../Impl/Include/Methane/Graphics/RHI/BufferSet.h | 2 +- .../Graphics/RHI/ParallelRenderCommandList.h | 2 +- .../Include/Methane/UserInterface/AppController.h | 2 +- 14 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Modules/Data/Events/Include/Methane/Data/Emitter.hpp b/Modules/Data/Events/Include/Methane/Data/Emitter.hpp index 37a8de683..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) { diff --git a/Modules/Data/Events/Include/Methane/Data/Receiver.hpp b/Modules/Data/Events/Include/Methane/Data/Receiver.hpp index 0f751fc1a..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; 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/CommandList.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h index 43be7a029..11cd799e3 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h @@ -142,7 +142,7 @@ class CommandList // NOSONAR - custom destructor is used for logging, class has TracyLockable(std::mutex, m_state_change_mutex); // 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; + 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/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/CommandQueueTracking.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp index 9fa4a7e00..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 ) ); } 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/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/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 }, }; From 362c4deddeae95579d1a93d9e7bd3214ad51d26b Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Mon, 24 Aug 2026 19:13:02 +0300 Subject: [PATCH 11/17] Fix data races in Base::Context and Base::CommandKit in parallel rendering scenarios --- .../Methane/Graphics/Base/CommandKit.h | 13 ++++- .../Include/Methane/Graphics/Base/Context.h | 17 +++++- .../Methane/Graphics/Base/CommandKit.cpp | 32 ++++++++-- .../Sources/Methane/Graphics/Base/Context.cpp | 58 +++++++++++++++---- 4 files changed, 99 insertions(+), 21 deletions(-) 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..fd0e6b5ec 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,18 @@ 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. + 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/Context.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h index 330d6c4c8..c01cd8808 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 @@ -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(std::memory_order_acquire); } Ptr GetBaseDevicePtr() const noexcept { return m_device_ptr; } Device& GetBaseDevice(); const Device& GetBaseDevice() const; @@ -119,10 +122,18 @@ 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; + + // Atomic because 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 }; + mutable bool m_is_completing_initialization = false; // NOSONAR - only written by CompleteInitialization() on the render thread }; } // 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 fe431c13c..7589b9706 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())); @@ -108,6 +110,13 @@ bool CommandKit::SetName(std::string_view name) } Rhi::ICommandQueue& CommandKit::GetQueue() const +{ + META_FUNCTION_TASK(); + std::scoped_lock lock_guard(m_mutex); + return GetQueueUnlocked(); +} + +Rhi::ICommandQueue& CommandKit::GetQueueUnlocked() const { META_FUNCTION_TASK(); if (m_cmd_queue_ptr) @@ -121,6 +130,7 @@ Rhi::ICommandQueue& CommandKit::GetQueue() const 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,11 +138,19 @@ 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); @@ -147,9 +165,9 @@ Rhi::ICommandList& CommandKit::GetList(Rhi::CommandListId cmd_list_id = 0U) cons 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 +205,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 +216,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 +226,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 +237,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; } 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 34ef2e996..6a2dc17f5 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp @@ -72,7 +72,14 @@ 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(std::memory_order_acquire); + while (prev_action < action && + !m_requested_action.compare_exchange_weak(prev_action, action, + std::memory_order_acq_rel, + std::memory_order_acquire)) + { } } void Context::CompleteInitialization() @@ -87,7 +94,7 @@ void Context::CompleteInitialization() UploadResourcesAndNotify(); GetDescriptorManager().CompleteInitialization(); - m_requested_action = DeferredAction::None; + m_requested_action.store(DeferredAction::None, std::memory_order_release); m_is_completing_initialization = false; } @@ -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; } @@ -248,7 +271,15 @@ void Context::ExecuteSyncCommandLists(const Rhi::ICommandKit& upload_cmd_kit) co constexpr auto cmd_list_id = static_cast(cmd_list_purpose); const std::vector cmd_list_ids = { cmd_list_id }; - for (const auto& [cmd_queue_ptr, cmd_kit_ptr] : m_default_command_kit_ptr_by_queue) + // 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] : 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(std::memory_order_acquire); + switch(requested_action) { case DeferredAction::None: break; @@ -342,9 +374,13 @@ 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, + std::memory_order_acq_rel, + std::memory_order_relaxed); } void Context::SetDevice(Device& device) From 6881268501a1eaa93d3bb6dca0b273da0c51eb7f Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Tue, 25 Aug 2026 17:41:52 +0000 Subject: [PATCH 12/17] Update Tracy to v0.14.1 (hotfix release) --- .github/workflows/ci-build.yml | 2 +- Externals/README.md | 2 +- Externals/Tracy.cmake | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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/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}" From 65f02c8ccdb8cc598b105075059356588b0cafda Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Tue, 25 Aug 2026 18:22:58 +0000 Subject: [PATCH 13/17] Fix and silence static analysis issues from SonarQube on Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cpp:S8417 — non-default memory ordering (8 issues) in Context.h and Context.cpp Dropped the explicit memory_order arguments on m_requested_action, falling back to default seq_cst. This is free here: on x86-64 and ARM64 a seq_cst load compiles identically to an acquire load, and lock cmpxchg is already a full barrier, so the hot path through RequestDeferredAction() is unchanged. Only the store() in CompleteInitialization() gains a fence on x86, and that runs once per initialization. cpp:S8379 — unprotected mutable field (4 issues) on m_is_completing_initialization Changed mutable bool → mutable std::atomic_bool. This one was a genuine race, not a false positive: the previous // NOSONAR claimed it was "only written by CompleteInitialization() on the render thread", but IsCompletingInitialization() is public IContext API and is read from Text.cpp:642 inside a font-atlas-update callback. Making it atomic fixes the rule and the underlying issue. cpp:S108 — empty compound statement (1 issue) Filled the CAS retry loop body in RequestDeferredAction() with a comment explaining that compare_exchange_weak() has already reloaded prev_action. --- .../Methane/Graphics/Base/CommandKit.h | 3 +++ .../Include/Methane/Graphics/Base/Context.h | 21 ++++++++++----- .../Methane/Graphics/Base/CommandKit.cpp | 20 +++++++------- .../Sources/Methane/Graphics/Base/Context.cpp | 26 +++++++++---------- 4 files changed, 41 insertions(+), 29 deletions(-) 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 fd0e6b5ec..fcb3c5f65 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h @@ -75,6 +75,9 @@ class CommandKit final // 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; 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 c01cd8808..11c8a1b7f 100644 --- a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h +++ b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/Context.h @@ -78,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; @@ -94,7 +94,7 @@ class Context // IObject interface bool SetName(std::string_view name) override; - DeferredAction GetRequestedAction() const noexcept { return m_requested_action.load(std::memory_order_acquire); } + DeferredAction GetRequestedAction() const noexcept { return m_requested_action.load(); } Ptr GetBaseDevicePtr() const noexcept { return m_device_ptr; } Device& GetBaseDevice(); const Device& GetBaseDevice() const; @@ -129,11 +129,20 @@ class Context mutable CommandKitPtrByType m_default_command_kit_ptrs; mutable CommandKitByQueue m_default_command_kit_ptr_by_queue; - // Atomic because RequestDeferredAction() escalates the pending action from any thread: - // IBuffer/ITexture::SetData() and IProgramArgumentBinding::SetRootConstant() are called - // concurrently from parallel-rendering worker threads. + // 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 }; - mutable bool m_is_completing_initialization = false; // NOSONAR - only written by CompleteInitialization() on the render thread + + // 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/Sources/Methane/Graphics/Base/CommandKit.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp index 7589b9706..e0b0c8274 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandKit.cpp @@ -119,12 +119,13 @@ Rhi::ICommandQueue& CommandKit::GetQueue() const Rhi::ICommandQueue& CommandKit::GetQueueUnlocked() const { META_FUNCTION_TASK(); - if (m_cmd_queue_ptr) - return *m_cmd_queue_ptr; + // 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 @@ -155,10 +156,11 @@ Rhi::ICommandList& CommandKit::GetListUnlocked(Rhi::CommandListId cmd_list_id) c 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; @@ -269,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/Context.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp index 6a2dc17f5..af6c844c2 100644 --- a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp +++ b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/Context.cpp @@ -74,28 +74,28 @@ void Context::RequestDeferredAction(DeferredAction action) const noexcept META_FUNCTION_TASK(); // 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(std::memory_order_acquire); - while (prev_action < action && - !m_requested_action.compare_exchange_weak(prev_action, action, - std::memory_order_acq_rel, - std::memory_order_acquire)) - { } + 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.store(DeferredAction::None, std::memory_order_release); - m_is_completing_initialization = false; + m_requested_action.store(DeferredAction::None); + m_is_completing_initialization.store(false); } void Context::WaitForGpu(WaitFor wait_for) @@ -359,7 +359,7 @@ bool Context::UploadResourcesAndNotify() void Context::PerformRequestedAction() { META_FUNCTION_TASK(); - DeferredAction requested_action = m_requested_action.load(std::memory_order_acquire); + DeferredAction requested_action = m_requested_action.load(); switch(requested_action) { case DeferredAction::None: @@ -378,9 +378,7 @@ void Context::PerformRequestedAction() } // 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, - std::memory_order_acq_rel, - std::memory_order_relaxed); + m_requested_action.compare_exchange_strong(requested_action, DeferredAction::None); } void Context::SetDevice(Device& device) From b7716d2274c1d13a4276ceed3fd176739d87019a Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Tue, 25 Aug 2026 19:43:47 +0300 Subject: [PATCH 14/17] Fix Profile build warning on Windows with VS2026 --- .../Instrumentation/Include/Methane/TracyGpu.hpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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); From 3d61e16e11f36a7304bdf2a1b3a7e85d3826a9ef Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Wed, 26 Aug 2026 23:16:47 +0300 Subject: [PATCH 15/17] Fix CodeRabbit review issues --- .../Methane/Graphics/Base/RenderPass.cpp | 21 +++++++------------ .../Graphics/Metal/ProgramArgumentBinding.mm | 2 +- 2 files changed, 9 insertions(+), 14 deletions(-) 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 01902d5bb..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,19 +44,17 @@ 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; m_settings = settings; - { - // Invalidate lazily initialized attachment texture caches under the same lock which guards their initialization - std::scoped_lock lock_guard(m_mutex); - m_non_frame_buffer_attachment_textures.clear(); - m_color_attachment_textures.clear(); - m_depth_attachment_texture_ptr = nullptr; - m_stencil_attachment_texture_ptr = nullptr; - } + 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; @@ -65,11 +63,8 @@ bool RenderPass::Update(const Rhi::RenderPassSettings& settings) void RenderPass::ReleaseAttachmentTextures() { META_FUNCTION_TASK(); - { - // Invalidate lazily initialized attachment texture cache under the same lock which guards its initialization - std::scoped_lock lock_guard(m_mutex); - m_non_frame_buffer_attachment_textures.clear(); - } + std::scoped_lock lock_guard(m_mutex); + m_non_frame_buffer_attachment_textures.clear(); m_settings.attachments.clear(); } 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 a20345b6b..f7696c2c2 100644 --- a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramArgumentBinding.mm +++ b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/ProgramArgumentBinding.mm @@ -91,7 +91,7 @@ static MTLRenderStages ConvertShaderTypeToMetalRenderStages(Rhi::ShaderType shad : Base::ProgramArgumentBinding(other) , m_settings_mt(other.m_settings_mt) , m_mtl_render_stages(other.m_mtl_render_stages) - , m_mtl_resource_views(other.m_mtl_resource_views) + , m_mtl_resource_views(other.GetNativeResourceViews()) { } From f2ff1791c703365ea5ce50510861a2e4f26737fe Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Wed, 26 Aug 2026 23:21:36 +0300 Subject: [PATCH 16/17] Remove pull request body --- pull_request_body.md | 94 -------------------------------------------- 1 file changed, 94 deletions(-) delete mode 100644 pull_request_body.md 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. - From 57e0487c094cf5df16bd4ba03d859bc7cd3f4be7 Mon Sep 17 00:00:00 2001 From: Evgeny Gorodetskiy Date: Wed, 26 Aug 2026 23:22:10 +0300 Subject: [PATCH 17/17] Remove GitPod badges, references and config files --- .gitpod.dockerfile | 16 ---------------- .gitpod.yml | 23 ----------------------- README.md | 5 +---- 3 files changed, 1 insertion(+), 43 deletions(-) delete mode 100644 .gitpod.dockerfile delete mode 100644 .gitpod.yml 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/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))