refactor(renderer): turn debug messenger from component to normal object

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 *>(instance))
-
+    , m_user_data(std::move(info.user_data))
-    // Move this to heap for pointer-stability of .pUserData
+    , m_debug_messenger(
-    , m_entity(memory::create_scope<ecs::Entity>(std::move(entity)))
+          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<MessengerComponent>();
-
-	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<ecs::Entity *>(vulkan_user_data);
+		auto *messenger = std::bit_cast<vk::Messenger *>(vulkan_user_data);
-		auto &component = messenger->get<MessengerComponent>();
+		messenger->m_user_callback(
-		component.get_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)

modules/renderer/private/backend/vk/messenger.hpp

@@ -3,6 +3,7 @@
 #include <any>
 #include <memory/pointer_types/null_on_move.hpp>
 #include <renderer/backend/vk/context/instance.hpp>
+#include <renderer/backend/vk/raii/raii.hpp>
 #include <renderer/backend/vk/vulkan.hpp>
 #include <renderer/components/messenger.hpp>
 #include <renderer/frontend/messenger.hpp>
@@ -12,17 +13,7 @@ namespace lt::renderer::vk {
 class Messenger: public IMessenger
 {
 public:
-	Messenger(IInstance *instance, ecs::Entity entity);
+	Messenger(IInstance *instance, CreateInfo info);
-
-	~Messenger() override;
-
-	Messenger(Messenger &&) = default;
-
-	Messenger(const Messenger &) = delete;
-
-	auto operator=(Messenger &&) -> Messenger & = default;
-
-	auto operator=(const Messenger &) const -> Messenger & = delete;
 
 private:
 	static auto native_callback(
@@ -42,15 +33,17 @@ private:
 
 	[[nodiscard]] static auto from_native_type(VkDebugUtilsMessageTypeFlagsEXT type) -> MessageType;
 
-	memory::NullOnMove<class Instance *> m_instance {};
+	class Instance *m_instance {};
 
-	memory::Scope<ecs::Entity> m_entity;
+	raii::DebugMessenger m_debug_messenger;
-
-	VkDebugUtilsMessengerEXT m_debug_messenger = VK_NULL_HANDLE;
 
 	MessageSeverity m_severities {};
 
 	MessageType m_types {};
+
+	Callback_T m_user_callback;
+
+	std::any m_user_data;
 };
 
 } // namespace lt::renderer::vk

modules/renderer/private/backend/vk/raii/raii.hpp

@@ -0,0 +1,33 @@
+#include <memory/pointer_types/null_on_move.hpp>
+#include <renderer/backend/vk/context/instance.hpp>
+#include <renderer/backend/vk/vulkan.hpp>
+
+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<Instance *> m_instance {};
+
+	VkDebugUtilsMessengerEXT m_object;
+};
+
+} // namespace lt::renderer::vk::raii

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<IMessenger>
 {
+	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<vk::Messenger>(instance, std::move(entity));
+	case Api::vulkan: return memory::create_scope<vk::Messenger>(instance, std::move(info));
 	case Api::none:
 	case Api::metal:
 	case Api::direct_x: throw std::runtime_error { "Invalid API" };

modules/renderer/private/system.cpp

@@ -33,21 +33,7 @@ System::System(CreateInfo info)
 	    frames_in_flight_upper_limit
 	);
 
-	if (info.messenger_info.has_value())
+	m_messenger = IMessenger::create(m_api, m_instance, info.debug_callback_info);
-	{
-		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_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<ecs::EntityId> {};
-	for (auto &[entity, surface] : m_registry->view<MessengerComponent>())
-	{
-		entities_to_remove.emplace_back(entity);
-	}
-
-	for (auto entity : entities_to_remove)
-	{
-		m_registry->remove<MessengerComponent>(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<MessengerComponent>(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<MessengerComponent>(entity);
-	return false;
-}
-
-
 } // namespace lt::renderer

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(renderer::IMessenger::MessageSeverity::error));
-		expect_false(fixture.has_any_messages_of(MessageSeverity::warning));
+		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 };
 		});
 

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::IMessenger::MessageSeverity severity,
-	    lt::renderer::MessageType type,
+	    lt::renderer::IMessenger::MessageType type,
-	    lt::renderer::MessageData data,
+	    const lt::renderer::IMessenger::MessageData &data,
 	    std::any &user_data
 	)
 	{
-		std::ignore = type;
 		std::ignore = data;
+		std::ignore = type;
 
-		auto *fixture = std::any_cast<Fixture_RendererSystem *>(user_data);
+		auto *fixture = std::any_cast<UserData *>(user_data);
 		fixture->m_has_any_messages = true;
 		++fixture->m_severity_counter[severity];
 	}
 
-	std::unordered_map<lt::renderer::MessageSeverity, uint32_t> m_severity_counter;
+	struct UserData
+	{
+		std::unordered_map<lt::renderer::IMessenger::MessageSeverity, uint32_t> m_severity_counter;
 
-	bool m_has_any_messages {};
+		bool m_has_any_messages {};
+	};
+
+	lt::memory::Scope<UserData> m_user_data = lt::memory::create_scope<UserData>();
 
 	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 {
+        .debug_callback_info = lt::renderer::IMessenger ::CreateInfo {
-		        .severities = lt::renderer::MessageSeverity::all,
+		        .severities = lt::renderer::IMessenger ::MessageSeverity::all,
-		        .types = lt::renderer::MessageType::all,
+		        .types = lt::renderer::IMessenger ::MessageType::all,
 		        .callback = &messenger_callback,
-		        .user_data = this,
+		        .user_data = m_user_data.get(),
         }
 	};
 };

modules/renderer/public/components/messenger.hpp

@@ -1,100 +0,0 @@
-#pragma once
-
-#include <any>
-#include <bitwise/operations.hpp>
-#include <memory/scope.hpp>
-#include <renderer/frontend/messenger.hpp>
-
-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<void(
-    MessageSeverity message_severity,
-    MessageType message_type,
-    MessageData data,
-    std::any &user_data
-)>;
-
-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<IMessenger> m_implementation;
-};
-
-} // namespace lt::renderer

modules/renderer/public/frontend/messenger.hpp

@@ -1,15 +1,62 @@
 #pragma once
 
+#include <bitwise/operations.hpp>
 #include <ecs/entity.hpp>
 #include <memory/scope.hpp>
 #include <renderer/api.hpp>
 
 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<void(
+	    MessageSeverity message_severity,
+	    MessageType message_type,
+	    MessageData data,
+	    std::any &user_data
+	)>;
+
+	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>;
 
 	IMessenger() = default;

modules/renderer/public/system.hpp

@@ -6,7 +6,7 @@
 #include <memory/reference.hpp>
 #include <memory/scope.hpp>
 #include <renderer/api.hpp>
-#include <renderer/components/messenger.hpp>
+#include <renderer/frontend/messenger.hpp>
 
 namespace lt::renderer {
 
@@ -34,7 +34,7 @@ public:
 
 		ecs::Entity surface_entity;
 
-		std::optional<MessengerComponent::CreateInfo> 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;