From 6e5c0259e982b68fc175ff79b16fdd045396b0de Mon Sep 17 00:00:00 2001 From: drendog Date: Mon, 3 Nov 2025 22:59:34 +0100 Subject: [PATCH] refactor:extract popup builder and dimensions --- .../src/rendering/femtovg/popup_window.rs | 4 + adapters/src/wayland/shell_adapter.rs | 2 +- adapters/src/wayland/surfaces/mod.rs | 1 + .../src/wayland/surfaces/popup_builder.rs | 200 ++++++++++++++++++ .../src/wayland/surfaces/popup_manager.rs | 82 ++++--- composition/src/system.rs | 14 +- domain/src/value_objects/mod.rs | 1 + domain/src/value_objects/popup_dimensions.rs | 42 ++++ 8 files changed, 315 insertions(+), 31 deletions(-) create mode 100644 adapters/src/wayland/surfaces/popup_builder.rs create mode 100644 domain/src/value_objects/popup_dimensions.rs diff --git a/adapters/src/rendering/femtovg/popup_window.rs b/adapters/src/rendering/femtovg/popup_window.rs index 12a8ec5..050076e 100644 --- a/adapters/src/rendering/femtovg/popup_window.rs +++ b/adapters/src/rendering/femtovg/popup_window.rs @@ -95,6 +95,10 @@ impl PopupWindow { pub fn scale_factor(&self) -> f32 { self.scale_factor.get() } + + pub fn popup_key(&self) -> Option { + self.popup_key.get() + } } impl WindowAdapter for PopupWindow { diff --git a/adapters/src/wayland/shell_adapter.rs b/adapters/src/wayland/shell_adapter.rs index 7c5d510..9350183 100644 --- a/adapters/src/wayland/shell_adapter.rs +++ b/adapters/src/wayland/shell_adapter.rs @@ -180,7 +180,7 @@ impl WaylandWindowingSystem { let serial = serial_holder.get(); - let params = if let Some((request, width, height)) = popup_manager_clone.take_pending_popup_request() { + let params = if let Some((request, width, height)) = popup_manager_clone.take_pending_popup() { log::info!( "Using popup request: component='{}', position=({}, {}), size={}x{}, mode={:?}", request.component, diff --git a/adapters/src/wayland/surfaces/mod.rs b/adapters/src/wayland/surfaces/mod.rs index 7c34f74..093362a 100644 --- a/adapters/src/wayland/surfaces/mod.rs +++ b/adapters/src/wayland/surfaces/mod.rs @@ -1,5 +1,6 @@ pub mod dimensions; pub mod layer_surface; +pub mod popup_builder; pub mod popup_manager; pub mod popup_surface; pub mod surface_builder; diff --git a/adapters/src/wayland/surfaces/popup_builder.rs b/adapters/src/wayland/surfaces/popup_builder.rs new file mode 100644 index 0000000..282e7c5 --- /dev/null +++ b/adapters/src/wayland/surfaces/popup_builder.rs @@ -0,0 +1,200 @@ +use crate::errors::{LayerShikaError, Result}; +use crate::rendering::femtovg::popup_window::PopupWindow; +use layer_shika_domain::value_objects::popup_dimensions::PopupDimensions; +use layer_shika_domain::value_objects::popup_request::{PopupHandle, PopupRequest, PopupSize}; +use log::{debug, info}; +use slint::ComponentHandle; +use slint_interpreter::{ComponentDefinition, ComponentInstance, Value}; +use smithay_client_toolkit::reexports::protocols_wlr::layer_shell::v1::client::zwlr_layer_surface_v1::ZwlrLayerSurfaceV1; +use std::rc::Rc; +use wayland_client::QueueHandle; + +use super::popup_manager::{CreatePopupParams, PopupManager}; +use super::surface_state::WindowState; + +pub struct PopupBuilder<'a> { + popup_manager: &'a Rc, + queue_handle: &'a QueueHandle, + parent_layer_surface: &'a ZwlrLayerSurfaceV1, +} + +impl<'a> PopupBuilder<'a> { + #[must_use] + pub const fn new( + popup_manager: &'a Rc, + queue_handle: &'a QueueHandle, + parent_layer_surface: &'a ZwlrLayerSurfaceV1, + ) -> Self { + Self { + popup_manager, + queue_handle, + parent_layer_surface, + } + } + + pub fn build( + &self, + component_def: &ComponentDefinition, + request: &PopupRequest, + serial: u32, + ) -> Result { + info!( + "Building popup for component '{}' at position ({}, {}) with mode {:?}", + request.component, + request.at.position().0, + request.at.position().1, + request.mode + ); + + let dimensions = Self::resolve_dimensions(component_def, &request.size)?; + dimensions + .validate() + .map_err(|e| LayerShikaError::WindowConfiguration { + message: format!("Invalid popup dimensions: {e}"), + })?; + + debug!( + "Resolved popup dimensions: {}x{}", + dimensions.width(), + dimensions.height() + ); + + let params = CreatePopupParams { + last_pointer_serial: serial, + reference_x: request.at.position().0, + reference_y: request.at.position().1, + width: dimensions.width(), + height: dimensions.height(), + positioning_mode: request.mode, + }; + + let popup_window = self.popup_manager.create_popup( + self.queue_handle, + self.parent_layer_surface, + params, + )?; + + let instance = Self::create_component_instance(component_def, &popup_window)?; + + self.setup_popup_callbacks(&instance, &popup_window)?; + + let handle = PopupHandle::new(popup_window.popup_key().ok_or_else(|| { + LayerShikaError::WindowConfiguration { + message: "Popup window has no key assigned".to_string(), + } + })?); + + info!("Popup built successfully with handle {:?}", handle); + + Ok(handle) + } + + fn resolve_dimensions( + component_def: &ComponentDefinition, + size: &PopupSize, + ) -> Result { + match size { + PopupSize::Fixed { w, h } => { + debug!("Using fixed popup size: {}x{}", w, h); + Ok(PopupDimensions::new(*w, *h)) + } + PopupSize::Content => { + debug!("Measuring popup dimensions from component content"); + Self::measure_component_dimensions(component_def) + } + } + } + + fn measure_component_dimensions( + component_def: &ComponentDefinition, + ) -> Result { + debug!("Creating temporary component instance to measure dimensions"); + + let temp_instance = + component_def + .create() + .map_err(|e| LayerShikaError::WindowConfiguration { + message: format!( + "Failed to create temporary instance for dimension measurement: {}", + e + ), + })?; + + temp_instance + .show() + .map_err(|e| LayerShikaError::WindowConfiguration { + message: format!("Failed to show temporary instance: {}", e), + })?; + + let width = Self::read_property(&temp_instance, "popup-width", 120.0); + let height = Self::read_property(&temp_instance, "popup-height", 120.0); + + debug!( + "Measured dimensions from component properties: {}x{}", + width, height + ); + + Ok(PopupDimensions::new(width, height)) + } + + fn read_property(instance: &ComponentInstance, name: &str, default: f32) -> f32 { + instance + .get_property(name) + .ok() + .and_then(|v| v.try_into().ok()) + .unwrap_or_else(|| { + debug!( + "Property '{}' not found or invalid, using default: {}", + name, default + ); + default + }) + } + + fn create_component_instance( + component_def: &ComponentDefinition, + _popup_window: &Rc, + ) -> Result { + debug!("Creating popup component instance"); + + let instance = + component_def + .create() + .map_err(|e| LayerShikaError::WindowConfiguration { + message: format!("Failed to create popup instance: {}", e), + })?; + + instance + .show() + .map_err(|e| LayerShikaError::WindowConfiguration { + message: format!("Failed to show popup instance: {}", e), + })?; + + debug!("Popup component instance created and shown successfully"); + + Ok(instance) + } + + fn setup_popup_callbacks( + &self, + instance: &ComponentInstance, + _popup_window: &Rc, + ) -> Result<()> { + debug!("Setting up popup callbacks"); + + let popup_manager = Rc::clone(self.popup_manager); + instance + .set_callback("closed", move |_| { + info!("Popup 'closed' callback triggered"); + popup_manager.close_current_popup(); + Value::Void + }) + .map_err(|e| LayerShikaError::WindowConfiguration { + message: format!("Failed to set popup 'closed' callback: {}", e), + })?; + + debug!("Popup callbacks configured successfully"); + + Ok(()) + } +} diff --git a/adapters/src/wayland/surfaces/popup_manager.rs b/adapters/src/wayland/surfaces/popup_manager.rs index bcc32b2..dbd8854 100644 --- a/adapters/src/wayland/surfaces/popup_manager.rs +++ b/adapters/src/wayland/surfaces/popup_manager.rs @@ -68,54 +68,84 @@ struct ActivePopup { window: Rc, } +struct PopupState { + scale_factor: f32, + output_size: PhysicalSize, +} + +struct PendingPopup { + request: PopupRequest, + width: f32, + height: f32, +} + pub struct PopupManager { context: PopupContext, popups: RefCell>, - current_scale_factor: RefCell, - current_output_size: RefCell, - pending_popup_request: RefCell>, - last_popup_key: RefCell>, + state: RefCell, + current_popup_key: RefCell>, + pending_popup: RefCell>, } impl PopupManager { #[must_use] - pub const fn new(context: PopupContext, initial_scale_factor: f32) -> Self { + pub fn new(context: PopupContext, initial_scale_factor: f32) -> Self { Self { context, popups: RefCell::new(Slab::new()), - current_scale_factor: RefCell::new(initial_scale_factor), - current_output_size: RefCell::new(PhysicalSize::new(0, 0)), - pending_popup_request: RefCell::new(None), - last_popup_key: RefCell::new(None), + state: RefCell::new(PopupState { + scale_factor: initial_scale_factor, + output_size: PhysicalSize::new(0, 0), + }), + current_popup_key: RefCell::new(None), + pending_popup: RefCell::new(None), } } - pub fn set_pending_popup_request(&self, request: PopupRequest, width: f32, height: f32) { - *self.pending_popup_request.borrow_mut() = Some((request, width, height)); + pub fn set_pending_popup(&self, request: PopupRequest, width: f32, height: f32) { + *self.pending_popup.borrow_mut() = Some(PendingPopup { + request, + width, + height, + }); } #[must_use] - pub fn take_pending_popup_request(&self) -> Option<(PopupRequest, f32, f32)> { - self.pending_popup_request.borrow_mut().take() + pub fn take_pending_popup(&self) -> Option<(PopupRequest, f32, f32)> { + self.pending_popup + .borrow_mut() + .take() + .map(|p| (p.request, p.width, p.height)) + } + + #[must_use] + pub fn scale_factor(&self) -> f32 { + self.state.borrow().scale_factor + } + + #[must_use] + pub fn output_size(&self) -> PhysicalSize { + self.state.borrow().output_size + } + + pub fn update_scale_factor(&self, scale_factor: f32) { + self.state.borrow_mut().scale_factor = scale_factor; + } + + pub fn update_output_size(&self, output_size: PhysicalSize) { + self.state.borrow_mut().output_size = output_size; } pub fn close_current_popup(&self) { - let key = self.last_popup_key.borrow_mut().take(); + let key = self.current_popup_key.borrow_mut().take(); if let Some(key) = key { self.destroy_popup(key); } } - pub fn update_scale_factor(&self, scale_factor: f32) { - *self.current_scale_factor.borrow_mut() = scale_factor; - } - - pub fn update_output_size(&self, output_size: PhysicalSize) { - *self.current_output_size.borrow_mut() = output_size; - } - - pub fn output_size(&self) -> PhysicalSize { - *self.current_output_size.borrow() + #[must_use] + pub fn current_popup_key(&self) -> Option { + *self.current_popup_key.borrow() } pub fn create_popup( @@ -130,7 +160,7 @@ impl PopupManager { } })?; - let scale_factor = *self.current_scale_factor.borrow(); + let scale_factor = self.scale_factor(); info!( "Creating popup window with scale factor {scale_factor}, reference=({}, {}), size=({} x {}), mode={:?}", params.reference_x, @@ -192,7 +222,7 @@ impl PopupManager { window: Rc::clone(&popup_window), }); popup_window.set_popup_manager(Rc::downgrade(self), key); - *self.last_popup_key.borrow_mut() = Some(key); + *self.current_popup_key.borrow_mut() = Some(key); info!("Popup window created successfully with key {key}"); diff --git a/composition/src/system.rs b/composition/src/system.rs index b304799..fb12020 100644 --- a/composition/src/system.rs +++ b/composition/src/system.rs @@ -132,7 +132,7 @@ impl RuntimeState<'_> { self.window_state.compilation_result() } - pub fn show_popup(&mut self, req: PopupRequest) -> Result<()> { + pub fn show_popup(&mut self, req: PopupRequest) -> Result { let compilation_result = self.compilation_result().ok_or_else(|| { Error::Domain(DomainError::Configuration { message: "No compilation result available for popup creation".to_string(), @@ -172,7 +172,7 @@ impl RuntimeState<'_> { .map(Rc::clone)?; log::debug!( - "Setting pending popup request for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}", + "Creating popup for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}", req.component, width, height, @@ -181,11 +181,17 @@ impl RuntimeState<'_> { req.mode ); - popup_manager.set_pending_popup_request(req, width, height); + popup_manager.set_pending_popup(req.clone(), width, height); Self::create_popup_instance(&definition, &popup_manager)?; - Ok(()) + Ok(PopupHandle::new( + popup_manager.current_popup_key().ok_or_else(|| { + Error::Domain(DomainError::Configuration { + message: "No popup key available after creation".to_string(), + }) + })?, + )) } pub fn close_popup(&mut self, handle: PopupHandle) -> Result<()> { diff --git a/domain/src/value_objects/mod.rs b/domain/src/value_objects/mod.rs index 28ffb54..2e5b4f0 100644 --- a/domain/src/value_objects/mod.rs +++ b/domain/src/value_objects/mod.rs @@ -4,5 +4,6 @@ pub mod keyboard_interactivity; pub mod layer; pub mod margins; pub mod popup_config; +pub mod popup_dimensions; pub mod popup_positioning_mode; pub mod popup_request; diff --git a/domain/src/value_objects/popup_dimensions.rs b/domain/src/value_objects/popup_dimensions.rs new file mode 100644 index 0000000..20d5341 --- /dev/null +++ b/domain/src/value_objects/popup_dimensions.rs @@ -0,0 +1,42 @@ +use crate::errors::{DomainError, Result}; + +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct PopupDimensions { + width: f32, + height: f32, +} + +impl PopupDimensions { + #[must_use] + pub const fn new(width: f32, height: f32) -> Self { + Self { width, height } + } + + #[must_use] + pub const fn width(&self) -> f32 { + self.width + } + + #[must_use] + pub const fn height(&self) -> f32 { + self.height + } + + pub fn validate(&self) -> Result<()> { + if self.width <= 0.0 || self.height <= 0.0 { + return Err(DomainError::Configuration { + message: format!( + "Invalid popup dimensions: width={}, height={}. Both must be positive.", + self.width, self.height + ), + }); + } + Ok(()) + } +} + +impl Default for PopupDimensions { + fn default() -> Self { + Self::new(120.0, 120.0) + } +}