From 3a06d51ee4349f8fd624e453da8af04d1d8083e5 Mon Sep 17 00:00:00 2001 From: light7734 Date: Wed, 8 Oct 2025 06:27:03 +0330 Subject: [PATCH] fix(surface): some errors & some problems with tests --- modules/surface/CMakeLists.txt | 1 + modules/surface/private/linux/system.cpp | 9 ++-- modules/surface/private/system.test.cpp | 60 ++++++++++++------------ modules/surface/public/system.hpp | 3 +- 4 files changed, 35 insertions(+), 38 deletions(-) diff --git a/modules/surface/CMakeLists.txt b/modules/surface/CMakeLists.txt index 3458ef8..3bf9473 100644 --- a/modules/surface/CMakeLists.txt +++ b/modules/surface/CMakeLists.txt @@ -12,6 +12,7 @@ target_link_libraries(surface PUBLIC app math memory + tbb PRIVATE logger lt_debug diff --git a/modules/surface/private/linux/system.cpp b/modules/surface/private/linux/system.cpp index cf41093..e1b44b2 100644 --- a/modules/surface/private/linux/system.cpp +++ b/modules/surface/private/linux/system.cpp @@ -100,8 +100,7 @@ void System::on_unregister() { } -auto System::create_component(ecs::EntityId entity, SurfaceComponent::CreateInfo info) - -> std::optional +void System::create_surface_component(ecs::EntityId entity, SurfaceComponent::CreateInfo info) try { auto &component = m_registry->add(entity, info); @@ -183,8 +182,6 @@ try { XUnmapWindow(display, main_window); } - - return &component; } catch (const std::exception &exp) { @@ -192,7 +189,6 @@ catch (const std::exception &exp) log_err("\tentity: {}", entity); log_err("\twhat: {}", exp.what()); m_registry->remove(entity); - return {}; } void System::on_surface_destruct(ecs::Registry ®istry, ecs::EntityId entity) @@ -394,7 +390,7 @@ void System::modify_resolution(SurfaceComponent &surface, const ModifyResolution void System::modify_position(SurfaceComponent &surface, const ModifyPositionRequest &request) { - surface.m_position = request.position; + // surface.m_position = request.position; auto &[display, window, _] = surface.m_native_data; const auto &[x, y] = request.position; @@ -428,6 +424,7 @@ void System::modify_position(SurfaceComponent &surface, const ModifyPositionRequ // So we just put the event back into the queue and move on. XPutBackEvent(display, &event); XSync(display, False); + XFlush(display); } void System::modify_visiblity(SurfaceComponent &surface, const ModifyVisibilityRequest &request) diff --git a/modules/surface/private/system.test.cpp b/modules/surface/private/system.test.cpp index 47d95e7..fe9adb4 100644 --- a/modules/surface/private/system.test.cpp +++ b/modules/surface/private/system.test.cpp @@ -16,6 +16,7 @@ using test::expect_eq; using test::expect_ne; using test::expect_not_nullptr; using test::expect_throw; +using test::expect_true; using test::Suite; [[nodiscard]] auto tick_info() -> app::TickInfo @@ -48,35 +49,39 @@ public: return m_registry; } - auto add_surface_component( + auto create_component( SurfaceComponent::CreateInfo info = SurfaceComponent::CreateInfo { .title = title, .resolution = { width, height }, .vsync = vsync, .visible = visible, } - ) -> SurfaceComponent & + ) -> std::optional { auto entity = m_registry->create_entity(); - return m_registry->add(entity, info); + m_system.create_surface_component(entity, info); + + return &m_registry->get(entity); } - void check_values(const SurfaceComponent &component) + void check_values(SurfaceComponent *component) { #ifdef LIGHT_PLATFORM_LINUX - expect_not_nullptr(component.get_native_data().display); - expect_ne(component.get_native_data().window, 0); + expect_not_nullptr(component->get_native_data().display); + expect_ne(component->get_native_data().window, 0); #endif - expect_eq(component.get_resolution().x, width); - expect_eq(component.get_resolution().y, height); - expect_eq(component.get_title(), title); - expect_eq(component.is_vsync(), vsync); - expect_eq(component.is_visible(), visible); + expect_eq(component->get_resolution().x, width); + expect_eq(component->get_resolution().y, height); + expect_eq(component->get_title(), title); + expect_eq(component->is_vsync(), vsync); + expect_eq(component->is_visible(), visible); } private: memory::Ref m_registry = memory::create_ref(); + + System m_system { m_registry }; }; @@ -96,10 +101,6 @@ Suite raii = "raii"_suite = [] { Case { "unhappy path throws" } = [] { expect_throw([] { ignore = System { {} }; }); - - auto fixture = Fixture {}; - fixture.add_surface_component(); - expect_throw([&] { ignore = System { fixture.registry() }; }); }; Case { "post construct has correct state" } = [] { @@ -112,7 +113,7 @@ Suite raii = "raii"_suite = [] { auto fixture = Fixture {}; auto system = memory::create_scope(fixture.registry()); - fixture.add_surface_component(); + fixture.create_component(); expect_eq(fixture.registry()->view().get_size(), 1); system.reset(); @@ -142,29 +143,28 @@ Suite system_events = "system_events"_suite = [] { Suite registry_events = "registry_events"_suite = [] { Case { "on_construct initializes component" } = [] { auto fixture = Fixture {}; - auto system = System { fixture.registry() }; - const auto &component = fixture.add_surface_component(); + const auto &component = fixture.create_component(); expect_eq(fixture.registry()->view().get_size(), 1); - fixture.check_values(component); + fixture.check_values(*component); }; Case { "unhappy on_construct throws" } = [] { auto fixture = Fixture {}; auto system = System { fixture.registry() }; - expect_throw([&] { fixture.add_surface_component({ .resolution = { width, 0 } }); }); + expect_throw([&] { fixture.create_component({ .resolution = { width, 0 } }); }); - expect_throw([&] { fixture.add_surface_component({ .resolution = { 0, height } }); }); + expect_throw([&] { fixture.create_component({ .resolution = { 0, height } }); }); expect_throw([&] { - fixture.add_surface_component( + fixture.create_component( { .title = "", .resolution = { SurfaceComponent::max_dimension + 1, height } } ); }); expect_throw([&] { - fixture.add_surface_component( + fixture.create_component( { .title = "", .resolution = { width, SurfaceComponent::max_dimension + 1 } } ); }); @@ -172,7 +172,7 @@ Suite registry_events = "registry_events"_suite = [] { auto big_str = std::string {}; big_str.resize(SurfaceComponent::max_title_length + 1); expect_throw([&] { - fixture.add_surface_component({ .title = big_str, .resolution = { width, height } }); + fixture.create_component({ .title = big_str, .resolution = { width, height } }); }); }; @@ -180,7 +180,7 @@ Suite registry_events = "registry_events"_suite = [] { auto fixture = Fixture {}; auto system = System { fixture.registry() }; - expect_throw([&] { fixture.add_surface_component({ .resolution = { width, 0 } }); }); + expect_throw([&] { fixture.create_component({ .resolution = { width, 0 } }); }); expect_eq(fixture.registry()->view().get_size(), 0); }; @@ -188,9 +188,9 @@ Suite registry_events = "registry_events"_suite = [] { auto fixture = Fixture {}; auto system = memory::create_scope(fixture.registry()); - const auto &component = fixture.add_surface_component(); + const auto &component = fixture.create_component(); expect_eq(fixture.registry()->view().get_size(), 1); - fixture.check_values(component); + fixture.check_values(*component); system.reset(); expect_eq(fixture.registry()->view().get_size(), 0); @@ -207,7 +207,7 @@ Suite tick = "tick"_suite = [] { auto fixture = Fixture {}; auto system = System { fixture.registry() }; - fixture.add_surface_component(); + fixture.create_component(); system.tick(tick_info()); }; }; @@ -216,7 +216,7 @@ Suite tick_handles_events = "tick_handles_events"_suite = [] { Case { "ticking clears previous tick's events" } = [] { auto fixture = Fixture {}; auto system = System { fixture.registry() }; - auto &surface = fixture.add_surface_component(); + auto &surface = **fixture.create_component(); // flush window-creation events system.tick(tick_info()); @@ -237,7 +237,7 @@ Suite tick_handles_requests = "tick_handles_requests"_suite = [] { Case { "ticking clears requests" } = [] { auto fixture = Fixture {}; auto system = System { fixture.registry() }; - auto &surface = fixture.add_surface_component(); + auto &surface = **fixture.create_component(); constexpr auto title = "ABC"; constexpr auto position = math::ivec2 { 50, 50 }; diff --git a/modules/surface/public/system.hpp b/modules/surface/public/system.hpp index e3ce324..24e36e9 100644 --- a/modules/surface/public/system.hpp +++ b/modules/surface/public/system.hpp @@ -27,8 +27,7 @@ public: void on_unregister() override; - auto create_component(ecs::EntityId entity, SurfaceComponent::CreateInfo info) - -> std::optional; + void create_surface_component(ecs::EntityId entity, SurfaceComponent::CreateInfo info); void tick(app::TickInfo tick) override;