From eb9e358d83b4a27254f22a23cb6a8fb4ce8a0dce Mon Sep 17 00:00:00 2001 From: light7734 Date: Wed, 8 Oct 2025 08:22:03 +0330 Subject: [PATCH] refactor(renderer): turn debug messenger from component to normal object --- .../renderer/private/backend/vk/messenger.cpp | 57 +++------- .../renderer/private/backend/vk/messenger.hpp | 23 ++-- .../renderer/private/backend/vk/raii/raii.hpp | 33 ++++++ .../renderer/private/frontend/messenger.cpp | 13 ++- modules/renderer/private/system.cpp | 46 +------- modules/renderer/private/system.test.cpp | 10 +- modules/renderer/private/test/utils.hpp | 51 ++++++--- .../renderer/public/components/messenger.hpp | 100 ------------------ .../renderer/public/frontend/messenger.hpp | 49 ++++++++- modules/renderer/public/system.hpp | 9 +- 10 files changed, 159 insertions(+), 232 deletions(-) create mode 100644 modules/renderer/private/backend/vk/raii/raii.hpp diff --git a/modules/renderer/private/backend/vk/messenger.cpp b/modules/renderer/private/backend/vk/messenger.cpp index d28f335..5b96915 100644 --- a/modules/renderer/private/backend/vk/messenger.cpp +++ b/modules/renderer/private/backend/vk/messenger.cpp @@ -2,46 +2,20 @@ namespace lt::renderer::vk { -Messenger::Messenger(IInstance *instance, ecs::Entity entity) +Messenger::Messenger(IInstance *instance, CreateInfo info) : m_instance(static_cast(instance)) - - // Move this to heap for pointer-stability of .pUserData - , m_entity(memory::create_scope(std::move(entity))) - + , m_user_data(std::move(info.user_data)) + , m_debug_messenger( + m_instance, + VkDebugUtilsMessengerCreateInfoEXT { + .sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT, + .messageSeverity = to_native_severity(info.severities), + .messageType = to_native_type(info.types), + .pfnUserCallback = &native_callback, + .pUserData = this, + } + ) { - const auto &component = m_entity->get(); - - ensure( - component.get_severities() != MessageSeverity::none, - "Failed to create vk::Messenger: severities == none" - ); - - ensure( - component.get_types() != MessageType::none, - "Failed to create vk::Messenger: types == none" - ); - - ensure(component.get_callback(), "Failed to create vk::Messenger: null callback"); - - m_debug_messenger = m_instance->create_messenger( - VkDebugUtilsMessengerCreateInfoEXT { - .sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT, - .messageSeverity = to_native_severity(component.get_severities()), - .messageType = to_native_type(component.get_types()), - .pfnUserCallback = &native_callback, - .pUserData = m_entity.get(), - } - ); -} - -Messenger::~Messenger() -{ - if (!m_instance) - { - return; - } - - m_instance->destroy_messenger(m_debug_messenger); } /*static*/ auto Messenger::native_callback( @@ -55,15 +29,14 @@ Messenger::~Messenger() { ensure(vulkan_user_data, "Null vulkan_user_data received in messenger callback"); - auto *messenger = std::bit_cast(vulkan_user_data); - auto &component = messenger->get(); - component.get_callback()( + auto *messenger = std::bit_cast(vulkan_user_data); + messenger->m_user_callback( from_native_severity(severity), from_native_type(type), { .message = callback_data->pMessage, }, - component.get_user_data() + messenger->m_user_data ); } catch (const std::exception &exp) diff --git a/modules/renderer/private/backend/vk/messenger.hpp b/modules/renderer/private/backend/vk/messenger.hpp index a395c40..ce7b662 100644 --- a/modules/renderer/private/backend/vk/messenger.hpp +++ b/modules/renderer/private/backend/vk/messenger.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -12,17 +13,7 @@ namespace lt::renderer::vk { class Messenger: public IMessenger { public: - Messenger(IInstance *instance, ecs::Entity entity); - - ~Messenger() override; - - Messenger(Messenger &&) = default; - - Messenger(const Messenger &) = delete; - - auto operator=(Messenger &&) -> Messenger & = default; - - auto operator=(const Messenger &) const -> Messenger & = delete; + Messenger(IInstance *instance, CreateInfo info); private: static auto native_callback( @@ -42,15 +33,17 @@ private: [[nodiscard]] static auto from_native_type(VkDebugUtilsMessageTypeFlagsEXT type) -> MessageType; - memory::NullOnMove m_instance {}; + class Instance *m_instance {}; - memory::Scope m_entity; - - VkDebugUtilsMessengerEXT m_debug_messenger = VK_NULL_HANDLE; + raii::DebugMessenger m_debug_messenger; MessageSeverity m_severities {}; MessageType m_types {}; + + Callback_T m_user_callback; + + std::any m_user_data; }; } // namespace lt::renderer::vk diff --git a/modules/renderer/private/backend/vk/raii/raii.hpp b/modules/renderer/private/backend/vk/raii/raii.hpp new file mode 100644 index 0000000..d7ce873 --- /dev/null +++ b/modules/renderer/private/backend/vk/raii/raii.hpp @@ -0,0 +1,33 @@ +#include +#include +#include + +namespace lt::renderer::vk::raii { + +// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) +class DebugMessenger +{ +public: + DebugMessenger(Instance *instance, VkDebugUtilsMessengerCreateInfoEXT info) + : m_instance(instance) + , m_object(m_instance->create_messenger(info)) + { + } + + ~DebugMessenger() + { + if (!m_instance) + { + return; + } + + m_instance->destroy_messenger(m_object); + } + +private: + memory::NullOnMove m_instance {}; + + VkDebugUtilsMessengerEXT m_object; +}; + +} // namespace lt::renderer::vk::raii diff --git a/modules/renderer/private/frontend/messenger.cpp b/modules/renderer/private/frontend/messenger.cpp index 5e91aeb..9870c4d 100644 --- a/modules/renderer/private/frontend/messenger.cpp +++ b/modules/renderer/private/frontend/messenger.cpp @@ -7,12 +7,21 @@ namespace lt::renderer { [[nodiscard]] /* static */ auto IMessenger::create( Api target_api, IInstance *instance, - ecs::Entity entity + CreateInfo info ) -> memory::Scope { + ensure( + info.severities != MessageSeverity::none, + "Failed to create vk::Messenger: severities == none" + ); + + ensure(info.types != MessageType::none, "Failed to create vk::Messenger: types == none"); + + ensure(info.callback, "Failed to create vk::Messenger: null callback"); + switch (target_api) { - case Api::vulkan: return memory::create_scope(instance, std::move(entity)); + case Api::vulkan: return memory::create_scope(instance, std::move(info)); case Api::none: case Api::metal: case Api::direct_x: throw std::runtime_error { "Invalid API" }; diff --git a/modules/renderer/private/system.cpp b/modules/renderer/private/system.cpp index 7db7d2e..36a0367 100644 --- a/modules/renderer/private/system.cpp +++ b/modules/renderer/private/system.cpp @@ -33,21 +33,7 @@ System::System(CreateInfo info) frames_in_flight_upper_limit ); - if (info.messenger_info.has_value()) - { - ensure( - create_messenger_component(m_registry->create_entity(), info.messenger_info.value()), - "Failed to initialize renderer::System: failed to create messenger component" - ); - } - else - { - log_wrn( - "Creating renderer::System without a default messenger component, this is not " - "recommended" - ); - } - + m_messenger = IMessenger::create(m_api, m_instance, info.debug_callback_info); m_surface = ISurface::create(m_api, m_instance, m_surface_entity); m_gpu = IGpu::create(m_api, m_instance); m_device = IDevice::create(m_api, m_gpu.get(), m_surface.get()); @@ -62,16 +48,6 @@ System::System(CreateInfo info) System::~System() { - auto entities_to_remove = std::vector {}; - for (auto &[entity, surface] : m_registry->view()) - { - entities_to_remove.emplace_back(entity); - } - - for (auto entity : entities_to_remove) - { - m_registry->remove(entity); - } } void System::on_register() @@ -108,24 +84,4 @@ void System::tick(app::TickInfo tick) m_frame_idx = (m_frame_idx + 1) % m_max_frames_in_flight; } -[[nodiscard]] auto System::create_messenger_component( - ecs::EntityId entity, - MessengerComponent::CreateInfo info -) -> bool -try -{ - auto &component = m_registry->add(entity, std::move(info)); - component.m_implementation = IMessenger::create(m_api, m_instance, { m_registry, entity }); - // component.m_user_data = info.user_data; - return true; -} -catch (const std::exception &exp) -{ - log_err("Failed to create renderer::MessengerComponent:"); - log_err("\twhat: {}", exp.what()); - m_registry->remove(entity); - return false; -} - - } // namespace lt::renderer diff --git a/modules/renderer/private/system.test.cpp b/modules/renderer/private/system.test.cpp index 3b437cb..5f1ee2a 100644 --- a/modules/renderer/private/system.test.cpp +++ b/modules/renderer/private/system.test.cpp @@ -11,10 +11,8 @@ using namespace lt; using std::ignore; using test::Case; using test::expect_throw; -using test::expect_true; using test::Suite; -using lt::renderer::MessageSeverity; using renderer::System; struct SurfaceContext @@ -30,15 +28,15 @@ struct RendererContext }; -Suite raii = "raii"_suite = [] { +Suite raii = "system_raii"_suite = [] { Case { "happy path won't throw" } = [] { ignore = Fixture_RendererSystem {}; }; Case { "happy path has no errors" } = [] { auto fixture = Fixture_RendererSystem {}; - expect_false(fixture.has_any_messages_of(MessageSeverity::error)); - expect_false(fixture.has_any_messages_of(MessageSeverity::warning)); + expect_false(fixture.has_any_messages_of(renderer::IMessenger::MessageSeverity::error)); + expect_false(fixture.has_any_messages_of(renderer::IMessenger::MessageSeverity::warning)); }; Case { "unhappy path throws" } = [] { @@ -85,7 +83,7 @@ Suite raii = "raii"_suite = [] { }); expect_throw([=] mutable { - info.messenger_info = lt::renderer::MessengerComponent::CreateInfo {}; + info.debug_callback_info = lt::renderer::IMessenger::CreateInfo {}; ignore = System { info }; }); diff --git a/modules/renderer/private/test/utils.hpp b/modules/renderer/private/test/utils.hpp index e6ceb7a..8149068 100644 --- a/modules/renderer/private/test/utils.hpp +++ b/modules/renderer/private/test/utils.hpp @@ -30,6 +30,16 @@ constexpr auto frames_in_flight = uint32_t { 3u }; } // namespace constants + +inline void noop_messenger_callback( + lt::renderer::IMessenger::MessageSeverity severity, + lt::renderer::IMessenger::MessageType type, + const lt::renderer::IMessenger::MessageData &data, + std::any &user_data +) +{ +} + class Fixture_SurfaceSystem { public: @@ -53,6 +63,12 @@ public: }, .registry = registry(), .surface_entity = surface_entity(), + .debug_callback_info = { + .severities = lt::renderer::IMessenger::MessageSeverity::all, + .types= lt::renderer::IMessenger::MessageType::all, + .callback = noop_messenger_callback, + .user_data = {}, + } } ; } @@ -154,33 +170,40 @@ public: [[nodiscard]] auto has_any_messages() const -> bool { - return m_has_any_messages; + return m_user_data->m_has_any_messages; } - [[nodiscard]] auto has_any_messages_of(lt::renderer::MessageSeverity severity) const -> uint32_t + [[nodiscard]] auto has_any_messages_of( + lt::renderer::IMessenger ::MessageSeverity severity + ) const -> uint32_t { - return m_severity_counter.contains(severity); + return m_user_data->m_severity_counter.contains(severity); } private: static void messenger_callback( - lt::renderer::MessageSeverity severity, - lt::renderer::MessageType type, - lt::renderer::MessageData data, + lt::renderer::IMessenger::MessageSeverity severity, + lt::renderer::IMessenger::MessageType type, + const lt::renderer::IMessenger::MessageData &data, std::any &user_data ) { - std::ignore = type; std::ignore = data; + std::ignore = type; - auto *fixture = std::any_cast(user_data); + auto *fixture = std::any_cast(user_data); fixture->m_has_any_messages = true; ++fixture->m_severity_counter[severity]; } - std::unordered_map m_severity_counter; + struct UserData + { + std::unordered_map m_severity_counter; - bool m_has_any_messages {}; + bool m_has_any_messages {}; + }; + + lt::memory::Scope m_user_data = lt::memory::create_scope(); lt::renderer::System m_system = lt::renderer::System::CreateInfo { .config = { @@ -189,11 +212,11 @@ private: }, .registry = registry(), .surface_entity = surface_entity(), - .messenger_info = lt::renderer::MessengerComponent::CreateInfo { - .severities = lt::renderer::MessageSeverity::all, - .types = lt::renderer::MessageType::all, + .debug_callback_info = lt::renderer::IMessenger ::CreateInfo { + .severities = lt::renderer::IMessenger ::MessageSeverity::all, + .types = lt::renderer::IMessenger ::MessageType::all, .callback = &messenger_callback, - .user_data = this, + .user_data = m_user_data.get(), } }; }; diff --git a/modules/renderer/public/components/messenger.hpp b/modules/renderer/public/components/messenger.hpp index 34ea09e..e69de29 100644 --- a/modules/renderer/public/components/messenger.hpp +++ b/modules/renderer/public/components/messenger.hpp @@ -1,100 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -namespace lt::renderer { - -enum class MessageSeverity : uint8_t -{ - none = 0u, - - verbose = bitwise::bit(0u), - info = bitwise::bit(1u), - warning = bitwise::bit(2u), - error = bitwise::bit(3u), - - all = verbose | info | warning | error, -}; - -enum class MessageType : uint8_t -{ - none = 0u, - general = bitwise::bit(0u), - validation = bitwise::bit(1u), - performance = bitwise::bit(2u), - - all = general | validation | performance, -}; - -struct MessageData -{ - std::string message; -}; - -using Callback_T = std::function; - -class MessengerComponent -{ -public: - friend class System; - - struct CreateInfo - { - MessageSeverity severities; - - MessageType types; - - Callback_T callback; - - std::any user_data; - }; - - [[nodiscard]] auto get_severities() const -> MessageSeverity - { - return m_severities; - } - - [[nodiscard]] auto get_types() const -> MessageType - { - return m_types; - } - - [[nodiscard]] auto get_callback() const -> const Callback_T & - { - return m_callback; - } - - [[nodiscard]] auto get_user_data() -> std::any & - { - return m_user_data; - } - -private: - MessengerComponent(CreateInfo info) - : m_severities(info.severities) - , m_types(info.types) - , m_callback(std::move(info.callback)) - , m_user_data(std::move(info.user_data)) - { - } - - MessageSeverity m_severities; - - MessageType m_types; - - Callback_T m_callback; - - std::any m_user_data; - - memory::Scope m_implementation; -}; - -} // namespace lt::renderer diff --git a/modules/renderer/public/frontend/messenger.hpp b/modules/renderer/public/frontend/messenger.hpp index 88cc70c..9605124 100644 --- a/modules/renderer/public/frontend/messenger.hpp +++ b/modules/renderer/public/frontend/messenger.hpp @@ -1,15 +1,62 @@ #pragma once +#include #include #include #include namespace lt::renderer { + class IMessenger { public: - [[nodiscard]] static auto create(Api target_api, class IInstance *instance, ecs::Entity entity) + enum class MessageSeverity : uint8_t + { + none = 0u, + + verbose = bitwise::bit(0u), + info = bitwise::bit(1u), + warning = bitwise::bit(2u), + error = bitwise::bit(3u), + + all = verbose | info | warning | error, + }; + + enum class MessageType : uint8_t + { + none = 0u, + general = bitwise::bit(0u), + validation = bitwise::bit(1u), + performance = bitwise::bit(2u), + + all = general | validation | performance, + }; + + struct MessageData + { + std::string message; + }; + + using Callback_T = std::function; + + struct CreateInfo + { + MessageSeverity severities; + + MessageType types; + + Callback_T callback; + + std::any user_data; + }; + + [[nodiscard]] static auto create(Api target_api, class IInstance *instance, CreateInfo info) -> memory::Scope; IMessenger() = default; diff --git a/modules/renderer/public/system.hpp b/modules/renderer/public/system.hpp index 9ad20c0..59453ab 100644 --- a/modules/renderer/public/system.hpp +++ b/modules/renderer/public/system.hpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include namespace lt::renderer { @@ -34,7 +34,7 @@ public: ecs::Entity surface_entity; - std::optional messenger_info; + IMessenger::CreateInfo debug_callback_info; }; System(CreateInfo info); @@ -80,11 +80,6 @@ public: return m_renderer.get(); } - [[nodiscard]] auto create_messenger_component( - ecs::EntityId entity, - MessengerComponent::CreateInfo info - ) -> bool; - [[nodiscard]] auto get_last_tick_result() const -> const app::TickResult & override { return m_last_tick_result;