From 9b7818e2807ff29c7da63c68b5f036fed2150f6f Mon Sep 17 00:00:00 2001 From: drendog Date: Mon, 19 Aug 2024 02:25:26 +0200 Subject: [PATCH] refactor: modularize ev loop and improve error propagation --- src/rendering/egl_context.rs | 32 ++++++---- src/rendering/femtovg_window.rs | 10 +-- src/windowing/mod.rs | 101 ++++++++++++++++++++---------- src/windowing/state/dispatches.rs | 16 ++++- src/windowing/state/mod.rs | 36 +++++++---- 5 files changed, 131 insertions(+), 64 deletions(-) diff --git a/src/rendering/egl_context.rs b/src/rendering/egl_context.rs index 6e1f77a..9675ad2 100644 --- a/src/rendering/egl_context.rs +++ b/src/rendering/egl_context.rs @@ -74,7 +74,7 @@ impl EGLContextBuilder { .ok_or_else(|| anyhow!("Surface ID is required"))?; let size = self.size.ok_or_else(|| anyhow!("Size is required"))?; - let display_handle = create_wayland_display_handle(&display_id); + let display_handle = create_wayland_display_handle(&display_id)?; let glutin_display = unsafe { Display::new(display_handle) }?; let config_template = self.config_template.unwrap_or_default(); @@ -85,7 +85,7 @@ impl EGLContextBuilder { let context = create_context(&glutin_display, &config, context_attributes)?; - let surface_handle = create_surface_handle(&surface_id); + let surface_handle = create_surface_handle(&surface_id)?; let surface = create_surface(&glutin_display, &config, surface_handle, size)?; let context = context @@ -109,11 +109,11 @@ impl EGLContext { } } -fn create_wayland_display_handle(display_id: &ObjectId) -> RawDisplayHandle { +fn create_wayland_display_handle(display_id: &ObjectId) -> Result { let display = NonNull::new(display_id.as_ptr().cast::()) - .expect("NonNull pointer creation failed"); + .ok_or_else(|| anyhow!("Failed to create NonNull pointer for display"))?; let handle = WaylandDisplayHandle::new(display); - RawDisplayHandle::Wayland(handle) + Ok(RawDisplayHandle::Wayland(handle)) } fn select_config( @@ -134,11 +134,11 @@ fn create_context( .map_err(|e| anyhow!("Failed to create context: {}", e)) } -fn create_surface_handle(surface_id: &ObjectId) -> RawWindowHandle { +fn create_surface_handle(surface_id: &ObjectId) -> Result { let surface = NonNull::new(surface_id.as_ptr().cast::()) - .expect("NonNull pointer creation failed"); + .ok_or_else(|| anyhow!("Failed to create NonNull pointer for surface"))?; let handle = WaylandWindowHandle::new(surface); - RawWindowHandle::Wayland(handle) + Ok(RawWindowHandle::Wayland(handle)) } fn create_surface( @@ -147,11 +147,17 @@ fn create_surface( surface_handle: RawWindowHandle, size: PhysicalSize, ) -> Result> { - let attrs = SurfaceAttributesBuilder::::new().build( - surface_handle, - NonZeroU32::new(size.width).unwrap(), - NonZeroU32::new(size.height).unwrap(), - ); + let Some(width) = NonZeroU32::new(size.width) else { + return Err(anyhow!("Width cannot be zero")); + }; + + let Some(height) = NonZeroU32::new(size.height) else { + return Err(anyhow!("Height cannot be zero")); + }; + + let attrs = + SurfaceAttributesBuilder::::new().build(surface_handle, width, height); + unsafe { glutin_display.create_window_surface(config, &attrs) } .map_err(|e| anyhow!("Failed to create window surface: {}", e)) } diff --git a/src/rendering/femtovg_window.rs b/src/rendering/femtovg_window.rs index 24c8734..5200a18 100644 --- a/src/rendering/femtovg_window.rs +++ b/src/rendering/femtovg_window.rs @@ -1,3 +1,4 @@ +use anyhow::{anyhow, Result}; use log::info; use slint::{ platform::{femtovg_renderer::FemtoVGRenderer, Renderer, WindowAdapter, WindowEvent}, @@ -33,15 +34,16 @@ impl FemtoVGWindow { }) } - pub fn render_frame_if_dirty(&self) { + pub fn render_frame_if_dirty(&self) -> Result<()> { if matches!( self.render_state.replace(RenderState::Clean), RenderState::Dirty ) { - if let Err(e) = self.renderer.render() { - log::error!("Error rendering frame: {}", e); - } + self.renderer + .render() + .map_err(|e| anyhow!("Error rendering frame: {}", e))?; } + Ok(()) } pub fn set_scale_factor(&self, scale_factor: f32) { diff --git a/src/windowing/mod.rs b/src/windowing/mod.rs index e6e0034..0443a72 100644 --- a/src/windowing/mod.rs +++ b/src/windowing/mod.rs @@ -1,14 +1,12 @@ use self::state::WindowState; use crate::{ bind_globals, - rendering::{ - egl_context::EGLContext, femtovg_window::FemtoVGWindow, slint_platform::CustomSlintPlatform, - }, + rendering::{egl_context::EGLContext, femtovg_window::FemtoVGWindow}, }; use anyhow::{Context, Result}; use config::WindowConfig; -use log::{debug, info}; -use slint::{platform::femtovg_renderer::FemtoVGRenderer, ComponentHandle, LogicalPosition}; +use log::{debug, error, info}; +use slint::{platform::femtovg_renderer::FemtoVGRenderer, LogicalPosition}; use slint_interpreter::ComponentInstance; use smithay_client_toolkit::reexports::{ calloop::{self, EventLoop, Interest, LoopHandle, Mode, PostAction}, @@ -49,7 +47,7 @@ impl WindowingSystem { let (compositor, output, layer_shell, seat) = Self::bind_globals(&global_list, &event_queue.handle())?; - let mut state = WindowState::new(config); + let mut state = WindowState::new(config)?; Self::setup_surface( &compositor, @@ -164,7 +162,9 @@ impl WindowingSystem { fn create_renderer(state: &WindowState, display: &WlDisplay) -> Result { let size = state.size(); - let surface = state.surface().unwrap(); + let surface = state + .surface() + .ok_or_else(|| anyhow::anyhow!("Failed to get surface"))?; debug!("Creating EGL context with size: {:?}", size); let context = EGLContext::builder() @@ -205,48 +205,83 @@ impl WindowingSystem { pub fn run(&mut self) -> Result<()> { info!("Starting WindowingSystem main loop"); - if let Some(window) = &self.state.window() { - window.render_frame_if_dirty(); - } - self.state.show_component()?; - self.setup_wayland_event_source(); + + self.initialize_component()?; + self.setup_wayland_event_source()?; + + let connection = Rc::clone(&self.connection); + let event_queue = &mut self.event_queue; self.event_loop - .run(None, &mut self.state, |shared_data| { - let _ = self.connection.flush(); - if let Some(guard) = self.event_queue.prepare_read() { - let _ = guard.read(); + .run(None, &mut self.state, move |shared_data| { + if let Err(e) = + Self::process_events(&Rc::clone(&connection), event_queue, shared_data) + { + error!("Error processing events: {}", e); } - - let _ = self.event_queue.dispatch_pending(shared_data); - - slint::platform::update_timers_and_animations(); - shared_data - .window() - .as_ref() - .unwrap() - .render_frame_if_dirty(); }) .map_err(|e| anyhow::anyhow!("Failed to run event loop: {}", e)) } - pub fn setup_wayland_event_source(&self) { + fn initialize_component(&mut self) -> Result<()> { + if let Some(window) = &self.state.window() { + window.render_frame_if_dirty()?; + } else { + return Err(anyhow::anyhow!("Window not initialized")); + } + + self.state.show_component()?; + Ok(()) + } + + fn setup_wayland_event_source(&self) -> Result<()> { debug!("Setting up Wayland event source"); let connection = Rc::clone(&self.connection); - let _ = self.event_loop.handle().insert_source( - calloop::generic::Generic::new(connection, Interest::READ, Mode::Level), - move |_, _connection, _shared_data| Ok(PostAction::Continue), - ); + self.event_loop + .handle() + .insert_source( + calloop::generic::Generic::new(connection, Interest::READ, Mode::Level), + move |_, _connection, _shared_data| Ok(PostAction::Continue), + ) + .map_err(|e| anyhow::anyhow!("Failed to set up Wayland event source: {}", e))?; + + Ok(()) } - pub fn component_instance(&self) -> &ComponentInstance { + fn process_events( + connection: &Rc, + event_queue: &mut EventQueue, + shared_data: &mut WindowState, + ) -> Result<()> { + connection.flush()?; + + if let Some(guard) = event_queue.prepare_read() { + guard + .read() + .map_err(|e| anyhow::anyhow!("Failed to read events: {}", e))?; + } + + event_queue.dispatch_pending(shared_data)?; + + slint::platform::update_timers_and_animations(); + + if let Some(window) = shared_data.window() { + window.render_frame_if_dirty()?; + } else { + return Err(anyhow::anyhow!("Window not initialized")); + } + + Ok(()) + } + + pub const fn component_instance(&self) -> Option<&ComponentInstance> { self.state.component_instance() } - pub fn window(&self) -> Rc { - Rc::clone(self.state().window().as_ref().unwrap()) + pub fn window(&self) -> Option> { + self.state().window().as_ref().map(Rc::clone) } pub const fn state(&self) -> &WindowState { diff --git a/src/windowing/state/dispatches.rs b/src/windowing/state/dispatches.rs index e50b532..301fcaf 100644 --- a/src/windowing/state/dispatches.rs +++ b/src/windowing/state/dispatches.rs @@ -1,5 +1,5 @@ use crate::impl_empty_dispatch; -use log::info; +use log::{info, warn}; use slint::{ platform::{PointerEventButton, WindowEvent}, PhysicalSize, @@ -42,10 +42,20 @@ impl Dispatch for WindowState { info!("Layer surface configured with size: {}x{}", width, height); layer_surface.ack_configure(serial); if width > 0 && height > 0 { - state.update_size(state.output_size().width, state.height()); + state + .update_size(state.output_size().width, state.height()) + .unwrap_or(warn!( + "Failed to update window size with width: {} and height: {}", + width, height + )); } else { let current_size = state.output_size(); - state.update_size(current_size.width, current_size.height); + state + .update_size(current_size.width, current_size.height) + .unwrap_or(warn!( + "Failed to update window size with width: {} and height: {}", + width, height + )); } } zwlr_layer_surface_v1::Event::Closed => { diff --git a/src/windowing/state/mod.rs b/src/windowing/state/mod.rs index 8cac5ff..493a313 100644 --- a/src/windowing/state/mod.rs +++ b/src/windowing/state/mod.rs @@ -1,4 +1,4 @@ -use std::{rc::Rc}; +use std::rc::Rc; use log::info; use slint::{LogicalPosition, PhysicalSize, ComponentHandle}; use slint_interpreter::{ComponentDefinition, ComponentInstance}; @@ -26,8 +26,13 @@ pub struct WindowState { } impl WindowState { - pub fn new(config: &mut WindowConfig) -> Self { - Self { + pub fn new(config: &mut WindowConfig) -> Result { + let component_definition = config + .component_definition + .take() + .ok_or_else(|| anyhow::anyhow!("Component definition is required"))?; + + Ok(Self { surface: None, layer_surface: None, size: PhysicalSize::default(), @@ -38,9 +43,9 @@ impl WindowState { scale_factor: config.scale_factor, height: config.height, exclusive_zone: config.exclusive_zone, - component_definition: config.component_definition.take().unwrap(), + component_definition, component_instance: None, - } + }) } pub fn show_component(&mut self) -> Result<()> { @@ -52,11 +57,16 @@ impl WindowState { self.component_instance = Some(self.component_definition.create()?); - self.component_instance.as_ref().unwrap().show()?; + if let Some(component_instance) = &self.component_instance { + component_instance.show()?; + } else { + return Err(anyhow::anyhow!("Component instance not initialized")); + } + Ok(()) } - pub fn update_size(&mut self, width: u32, height: u32) { + pub fn update_size(&mut self, width: u32, height: u32) -> Result<()> { let new_size = PhysicalSize::new(width, height); if let Some(window) = &self.window() { info!("Updating window size to {}x{}", width, height); @@ -70,10 +80,14 @@ impl WindowState { layer_surface.set_exclusive_zone(self.exclusive_zone); } - if let Some(s) = self.surface.as_ref() { - s.commit(); + match self.surface.as_ref() { + Some(surface) => surface.commit(), + None => { + return Err(anyhow::anyhow!("Surface not initialized")); + } } self.size = new_size; + Ok(()) } pub fn set_current_pointer_position(&mut self, physical_x: f64, physical_y: f64) { @@ -130,7 +144,7 @@ impl WindowState { self.pointer = Some(pointer); } - pub fn component_instance(&self) -> &ComponentInstance { - self.component_instance.as_ref().unwrap() + pub const fn component_instance(&self) -> Option<&ComponentInstance> { + self.component_instance.as_ref() } }