Methane Kit v0.8.2.1 hot-fixes

.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'
 

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()
         });
 

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;
 

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

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}"

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<uint8_t>(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);

Modules/Common/Primitives/Include/Methane/Exceptions.hpp

@@ -206,7 +206,7 @@ class UnexpectedArgumentException : public ArgumentExceptionBase<std::invalid_ar
 class NotImplementedException : public std::logic_error
 {
 public:
-    NotImplementedException(const std::source_location& source_location, const std::string& description = "")
+    explicit NotImplementedException(const std::source_location& source_location, const std::string& description = "")
         : std::logic_error(fmt::format("Function '{}' in file '{}({}:{})' is not implemented{}.",
                                        source_location.function_name(), source_location.file_name(),
                                        source_location.line(), source_location.column(),

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

Modules/Data/Animation/Include/Methane/Data/TimeAnimation.hpp

@@ -38,7 +38,7 @@ class TimeAnimation
     : public Animation
 {
 public:
-    TimeAnimation(const FunctorType& update_function, double duration_sec = std::numeric_limits<double>::max())
+    explicit TimeAnimation(const FunctorType& update_function, double duration_sec = std::numeric_limits<double>::max())
         : Animation(duration_sec)
         , m_update_function(update_function)
     { }

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<EventType>& 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<EventType>& receiver) noexcept final
     {
         META_FUNCTION_TASK();
-        std::lock_guard lock(m_connected_receivers_mutex);
+        std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration
 
         const auto connected_receiver_it = FindConnectedReceiver(receiver);
         if (connected_receiver_it == m_connected_receivers.end())
@@ -142,7 +142,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap
     void Emit(FuncType&& func_ptr, ArgTypes&&... args)
     {
         META_FUNCTION_TASK();
-        std::lock_guard lock(m_connected_receivers_mutex);
+        std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration
 
         // Additional receivers may be non-empty before emitting connected receiver calls
         // only in case when current emit is called during another emitted callback for the same emitter
@@ -216,7 +216,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap
     inline void CleanupConnectedReceivers() noexcept
     {
         // Erase receivers disconnected during emit cycle from the connected receivers
-        std::lock_guard lock(m_connected_receivers_mutex);
+        std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration
         for(auto connected_receiver_it  = m_connected_receivers.begin(); connected_receiver_it != m_connected_receivers.end();)
         {
             if (connected_receiver_it->first)
@@ -228,7 +228,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap
 
     inline void ConnectReceivers() noexcept
     {
-        std::lock_guard lock(m_connected_receivers_mutex);
+        std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration
         for(const ReceiverAndPriority& receiver_and_priority : m_connected_receivers)
         {
             if (receiver_and_priority.first)
@@ -239,7 +239,7 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap
     inline auto DisconnectReceivers() noexcept
     {
         // Move connected receivers so that OnDisconnected callbacks are not processed (m_connected_receivers would be empty)
-        std::lock_guard lock(m_connected_receivers_mutex);
+        std::lock_guard lock(m_connected_receivers_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_receivers_mutex declaration
         const auto connected_receivers = std::move(m_connected_receivers);
         for(const ReceiverAndPriority& receiver_and_priority : connected_receivers)
         {
@@ -254,6 +254,9 @@ class Emitter // NOSONAR - custom destructor is required, rule of zero is not ap
     bool                                m_is_emitting = false;
     std::vector<ReceiverAndPriority>    m_connected_receivers;
     std::vector<ReceiverAndPriority>    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;

Modules/Data/Events/Include/Methane/Data/Receiver.hpp

@@ -44,22 +44,22 @@ 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();
     }
 
     Receiver(Receiver&& other) noexcept
         : 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<EventType>& 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<EventType>& emitter) noexcept
     {
         META_FUNCTION_TASK();
-        std::lock_guard lock(m_connected_emitter_refs_mutex);
+        std::lock_guard lock(m_connected_emitter_refs_mutex); // NOSONAR - recursive mutex is required, see comment at m_connected_emitter_refs_mutex declaration
         const auto connected_emitter_ref_it = FindConnectedEmitter(emitter);
         if (connected_emitter_ref_it == m_connected_emitter_refs.end())
             return;
@@ -148,6 +148,9 @@ class Receiver : public EventType // NOSONAR - custom destructor is required, ru
     }
 
     Refs<IEmitter<EventType>> 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);
 };
 

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)); }

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;

Modules/Graphics/App/Include/Methane/Graphics/AppController.h

@@ -45,7 +45,7 @@ class AppController
 {
 public:
     using ActionByKeyboardState = pin::Keyboard::ActionControllerBase<AppAction>::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  }
     };
 

Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandKit.h

@@ -26,8 +26,10 @@ Methane command kit implementation.
 #include "Object.h"
 
 #include <Methane/Graphics/RHI/ICommandKit.h>
+#include <Methane/Instrumentation.h>
 
 #include <map>
+#include <mutex>
 
 namespace Methane::Graphics::Base
 {
@@ -62,11 +64,21 @@ class CommandKit final
     using CommandListIndexById = std::map<Rhi::CommandListId, uint32_t>;
     using CommandListSetById   = std::map<CommandListSetId, Ptr<Rhi::ICommandListSet>>;
 
-    CommandListIndex GetCommandListIndexById(Rhi::CommandListId cmd_list_id) const noexcept;
-    CommandListSetId GetCommandListSetId(Rhi::CommandListIdSpan cmd_list_ids, Opt<Data::Index> 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<Data::Index> frame_index_opt) const;
+    Rhi::ICommandQueue& GetQueueUnlocked() const;
+    Rhi::ICommandList&  GetListUnlocked(Rhi::CommandListId cmd_list_id) const;
 
     const Rhi::IContext&            m_context;
     Rhi::CommandListType            m_cmd_list_type;
+
+    // Command queue, lists, fences and list sets are lazily created by the const getters above,
+    // which are reachable from resource upload paths running on parallel-rendering worker threads.
+    // All accesses to the mutable members below are made with m_mutex locked, either by the public
+    // getter itself or by its caller when the access is made from a *Unlocked() helper. Static
+    // analysis can not follow the latter, so those accesses are marked with NOSONAR in CommandKit.cpp.
+    mutable TracyLockable(std::mutex, m_mutex);
     mutable Ptr<Rhi::ICommandQueue> m_cmd_queue_ptr;
     mutable Ptrs<Rhi::ICommandList> m_cmd_list_ptrs;
     mutable CommandListIndexById    m_cmd_list_index_by_id;

Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandList.h

@@ -140,7 +140,9 @@ class CommandList // NOSONAR - custom destructor is used for logging, class has
 
     mutable TracyLockable(std::recursive_mutex, m_state_mutex);
     TracyLockable(std::mutex,   m_state_change_mutex);
-    std::condition_variable_any m_state_change_condition_var;
+    // condition_variable_any is required because m_state_change_mutex is tracy::Lockable<std::mutex>
+    // in profiling builds, which std::condition_variable can not wait on.
+    std::condition_variable_any m_state_change_condition_var; // NOSONAR - condition_variable_any is required, see comment above
     TRACY_GPU_SCOPE_TYPE        m_tracy_gpu_scope;
 
 #ifdef METHANE_GPU_INSTRUMENTATION_ENABLED