diff --git a/modules/surface/CMakeLists.txt b/modules/surface/CMakeLists.txt index 30e9f5e..8f14740 100644 --- a/modules/surface/CMakeLists.txt +++ b/modules/surface/CMakeLists.txt @@ -13,4 +13,4 @@ target_link_libraries(surface PUBLIC ) add_test_module(surface system.test.cpp) -target_link_libraries(surface_tests PRIVATE glfw) +add_fuzz_module(surface system.fuzz.cpp) diff --git a/modules/surface/private/linux/system.cpp b/modules/surface/private/linux/system.cpp index 542bb9e..5096299 100644 --- a/modules/surface/private/linux/system.cpp +++ b/modules/surface/private/linux/system.cpp @@ -5,6 +5,11 @@ namespace lt::surface { +void glfw_error_callbac(int32_t code, const char *description) +{ + log_err("GLFW ERROR: {} -> {}", code, description); +} + void handle_event(GLFWwindow *window, const SurfaceComponent::Event &event) { auto &callbacks = *static_cast *>( @@ -92,9 +97,15 @@ void bind_glfw_events(GLFWwindow *handle) }); } +void init_glfw() {}; + System::System(Ref registry): m_registry(std::move(registry)) { + glfwSetErrorCallback(&glfw_error_callbac); + ensure(glfwInit(), "Failed to initialize 'glfw'"); + ensure(m_registry, "Failed to initialize surface system: null registry"); + ensure( m_registry->view().size() == 0, "Failed to initialize surface system: registry has surface component(s)" @@ -129,7 +140,6 @@ System::~System() m_registry->view().each([&](const entt::entity entity, SurfaceComponent &) { - std::cout << "REMOVED SURFACE COMPONENT ON DESTRUCTION" << std::endl; m_registry->get_entt_registry().remove(entity); }); @@ -138,28 +148,33 @@ System::~System() void System::on_surface_construct(entt::registry ®istry, entt::entity entity) { - ensure(glfwInit(), "Failed to initialize 'glfw'"); + try + { + auto &surface = registry.get(entity); + ensure_component_sanity(surface); - glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 4); - glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 5); - glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE); - // glfwWindowHint(GLFW_VISIBLE, GLFW_FALSE); + glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 4); + glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 5); + glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE); - auto &surface = registry.get(entity); - auto [width, height] = surface.get_size(); + surface.m_glfw_handle = glfwCreateWindow( + static_cast(surface.get_resolution().x), + static_cast(surface.get_resolution().y), + surface.get_title().begin(), + nullptr, + nullptr + ); + ensure(surface.m_glfw_handle, "Failed to create 'GLFWwindow'"); - surface.m_glfw_handle = glfwCreateWindow( - static_cast(width), - static_cast(height), - surface.get_title().begin(), - nullptr, - nullptr - ); - ensure(surface.m_glfw_handle, "Failed to create 'GLFWwindow'"); - - glfwSetWindowUserPointer(surface.m_glfw_handle, &surface.m_event_callbacks); - surface.m_native_handle = glfwGetX11Window(surface.m_glfw_handle); - bind_glfw_events(surface.m_glfw_handle); + glfwSetWindowUserPointer(surface.m_glfw_handle, &surface.m_event_callbacks); + surface.m_native_handle = glfwGetX11Window(surface.m_glfw_handle); + bind_glfw_events(surface.m_glfw_handle); + } + catch (...) + { + registry.remove(entity); + throw; + } } void System::on_surface_update(entt::registry ®istry, entt::entity entity) @@ -171,7 +186,11 @@ void System::on_surface_update(entt::registry ®istry, entt::entity entity) void System::on_surface_destroy(entt::registry ®istry, entt::entity entity) { auto &surface = registry.get(entity); - glfwDestroyWindow(surface.m_glfw_handle); + + if (surface.m_glfw_handle) + { + glfwDestroyWindow(surface.m_glfw_handle); + } } void System::set_title(ecs::Entity entity, std::string_view new_title) @@ -179,11 +198,15 @@ void System::set_title(ecs::Entity entity, std::string_view new_title) auto &surface = entity.get_component(); surface.m_title = new_title; - glfwSetWindowTitle(surface.m_glfw_handle, surface.m_title.begin()); + glfwSetWindowTitle(surface.m_glfw_handle, surface.m_title.c_str()); } auto System::tick() -> bool { + m_registry->view().each([](SurfaceComponent &surface) { + glfwSwapBuffers(surface.m_glfw_handle); + }); + glfwPollEvents(); return false; } @@ -191,7 +214,7 @@ auto System::tick() -> bool void System::set_size(ecs::Entity surface_entity, const math::uvec2 &new_size) { auto &surface = surface_entity.get_component(); - surface.m_size = new_size; + surface.m_resolution = new_size; glfwSetWindowSize( surface.m_glfw_handle, @@ -232,6 +255,36 @@ void System::add_event_listener( surface.m_event_callbacks.emplace_back(std::move(callback)); } +void System::ensure_component_sanity(const SurfaceComponent &component) +{ + auto [width, height] = component.get_resolution(); + + ensure(width != 0u, "Received bad values for surface component: width({}) == 0", width); + + ensure(height != 0u, "Received bad values for surface component: height({}) == 0", height); + + ensure( + width < SurfaceComponent::max_dimension, + "Received bad values for surface component: width({}) > max_dimension({})", + width, + SurfaceComponent::max_dimension + ); + + ensure( + height < SurfaceComponent::max_dimension, + "Received bad values for surface component: height({}) > max_dimension({})", + height, + SurfaceComponent::max_dimension + ); + + ensure( + component.get_title().size() < SurfaceComponent::max_title_length, + "Received bad values for surface component: title.size({}) > max_title_length({})", + component.get_title().size(), + SurfaceComponent::max_title_length + ); +} + } // namespace lt::surface namespace lt { diff --git a/modules/surface/private/system.fuzz.cpp b/modules/surface/private/system.fuzz.cpp new file mode 100644 index 0000000..2581f90 --- /dev/null +++ b/modules/surface/private/system.fuzz.cpp @@ -0,0 +1,103 @@ +#include +#include +#include +#include + +namespace lt::surface { + +enum class Action : uint8_t +{ + create_entity, + + create_surface_component, + + destroy_surface_component, + + tick, +}; + +void create_surface_component(test::FuzzDataProvider &provider, ecs::Registry ®istry) +{ + const auto length = std::min(provider.consume().value_or(16), 255u); + const auto title = provider.consume_string(length).value_or(""); + + const auto resolution = math::uvec2 { + provider.consume().value_or({ 32 }), + provider.consume().value_or({ 64 }), + }; + const auto visible = provider.consume().value_or(false); + const auto vsync = provider.consume().value_or(false); + + try + { + registry.create_entity("").add_component( + surface::SurfaceComponent::CreateInfo { + .title = std::move(title), + .resolution = resolution, + .vsync = vsync, + .visible = visible, + } + ); + } + catch (const std::exception &exp) + { + std::ignore = exp; + } +} + +void remove_surface_component(ecs::Registry ®istry) +{ + const auto view = registry.get_entt_registry().view(); + + if (!view->empty()) + { + registry.get_entt_registry().remove(*view.begin()); + } +} + +void check_invariants() +{ +} + +test::FuzzHarness harness = [](const uint8_t *data, size_t size) { + auto provider = test::FuzzDataProvider { data, size }; + + auto registry = create_ref(); + auto system = surface::System { registry }; + + while (auto action = provider.consume()) + { + switch (static_cast(action.value())) + { + case Action::create_entity: + { + const auto length = std::min(provider.consume().value_or(16), 255u); + const auto tag = provider.consume_string(length).value_or(""); + registry->create_entity(tag); + + break; + } + case Action::create_surface_component: + { + create_surface_component(provider, *registry); + break; + } + case Action::destroy_surface_component: + { + remove_surface_component(*registry); + break; + } + case Action::tick: + { + system.tick(); + break; + } + } + + check_invariants(); + } + + return 0; +}; + +} // namespace lt::surface diff --git a/modules/surface/private/system.test.cpp b/modules/surface/private/system.test.cpp index 31cdcbd..8531f9f 100644 --- a/modules/surface/private/system.test.cpp +++ b/modules/surface/private/system.test.cpp @@ -26,22 +26,24 @@ public: return m_registry; } - auto add_surface_component() -> SurfaceComponent & + auto add_surface_component( + SurfaceComponent::CreateInfo info = SurfaceComponent::CreateInfo { + .title = title, + .resolution = { width, height }, + .vsync = vsync, + .visible = visible, + } + ) -> SurfaceComponent & { auto entity = m_registry->create_entity(""); - return entity.add_component(SurfaceComponent::CreateInfo { - .title = title, - .size = { width, height }, - .vsync = vsync, - .visible = visible, - }); + return entity.add_component(info); } void check_values(const SurfaceComponent &component) { expect_ne(std::get(component.get_native_handle()), 0); - expect_eq(component.get_size().x, width); - expect_eq(component.get_size().y, height); + 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); @@ -57,9 +59,11 @@ Suite raii = [] { ignore = System { fixture.registry() }; }; - Case { "many won't throw" } = [] { + Case { "many won't freeze/throw" } = [] { auto fixture = Fixture {}; - for (auto idx : std::views::iota(0, 100'001)) + + /* range is small since glfw init/terminate is slow. */ + for (auto idx : std::views::iota(0, 100)) { ignore = System { fixture.registry() }; } @@ -78,6 +82,17 @@ Suite raii = [] { auto system = System { fixture.registry() }; expect_eq(fixture.registry()->view()->size(), 0); }; + + Case { "post destruct has correct state" } = [] { + auto fixture = Fixture {}; + auto system = create_scope(fixture.registry()); + + fixture.add_surface_component(); + expect_eq(fixture.registry()->view()->size(), 1); + + system.reset(); + expect_eq(fixture.registry()->view()->size(), 0); + }; }; Suite system_events = [] { @@ -109,6 +124,41 @@ Suite registry_events = [] { 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.add_surface_component({ .resolution = { 0, height } }); }); + + expect_throw([&] { + fixture.add_surface_component( + { .title = "", .resolution = { SurfaceComponent::max_dimension + 1, height } } + ); + }); + + expect_throw([&] { + fixture.add_surface_component( + { .title = "", .resolution = { width, SurfaceComponent::max_dimension + 1 } } + ); + }); + + 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 } }); + }); + }; + + Case { "unhappy on_construct removes component" } = [] { + auto fixture = Fixture {}; + auto system = System { fixture.registry() }; + + expect_throw([&] { fixture.add_surface_component({ .resolution = { width, 0 } }); }); + expect_eq(fixture.registry()->view().size(), 0); + }; + Case { "on_destrroy cleans up component" } = [] { auto fixture = Fixture {}; auto system = create_scope(fixture.registry()); @@ -137,7 +187,7 @@ Suite tick = [] { }; Case { "ticking on chaotic registry won't throw" } = [] { - } + }; }; Suite property_setters = [] { @@ -146,6 +196,3 @@ Suite property_setters = [] { Suite listeners = [] { }; - -Suite fuzzy = [] { -}; diff --git a/modules/surface/public/components.hpp b/modules/surface/public/components.hpp index 798edab..ad1ac38 100644 --- a/modules/surface/public/components.hpp +++ b/modules/surface/public/components.hpp @@ -47,11 +47,15 @@ public: using NativeHandle = std::variant; + static constexpr auto max_dimension = 4096; + + static constexpr auto max_title_length = 256; + struct CreateInfo { std::string_view title; - math::uvec2 size; + math::uvec2 resolution; bool vsync; @@ -60,20 +64,20 @@ public: SurfaceComponent(const CreateInfo &info) : m_title(info.title) - , m_size(info.size) + , m_resolution(info.resolution) , m_vsync(info.vsync) , m_visible(info.visible) { } - [[nodiscard]] auto get_title() const -> const std::string_view & + [[nodiscard]] auto get_title() const -> std::string_view { return m_title; } - [[nodiscard]] auto get_size() const -> const math::uvec2 & + [[nodiscard]] auto get_resolution() const -> const math::uvec2 & { - return m_size; + return m_resolution; } [[nodiscard]] auto is_vsync() const -> bool @@ -97,9 +101,9 @@ private: return m_glfw_handle; } - std::string_view m_title; + std::string m_title; - math::uvec2 m_size; + math::uvec2 m_resolution; bool m_vsync; diff --git a/modules/surface/public/system.hpp b/modules/surface/public/system.hpp index b7709ab..06f7dcf 100644 --- a/modules/surface/public/system.hpp +++ b/modules/surface/public/system.hpp @@ -32,7 +32,7 @@ public: auto tick() -> bool override; - void set_title(ecs::Entity surface_entity, std::string_view new_title); + static void set_title(ecs::Entity surface_entity, std::string_view new_title); void set_size(ecs::Entity surface_entity, const math::uvec2 &new_size); @@ -49,6 +49,8 @@ private: void on_surface_destroy(entt::registry ®istry, entt::entity entity); + void ensure_component_sanity(const SurfaceComponent &component); + Ref m_registry; };