diff --git a/adapters/src/rendering/femtovg/popup_window.rs b/adapters/src/rendering/femtovg/popup_window.rs index 7fa0d2d..182a716 100644 --- a/adapters/src/rendering/femtovg/popup_window.rs +++ b/adapters/src/rendering/femtovg/popup_window.rs @@ -1,5 +1,5 @@ use crate::errors::{RenderingError, Result}; -use crate::wayland::surfaces::popup_manager::PopupManager; +use crate::wayland::surfaces::popup_manager::{OnCloseCallback, PopupId}; use core::ops::Deref; use log::info; use slint::{ @@ -7,7 +7,7 @@ use slint::{ platform::{Renderer, WindowAdapter, WindowEvent, femtovg_renderer::FemtoVGRenderer}, }; use slint_interpreter::ComponentInstance; -use std::cell::{Cell, OnceCell, RefCell}; +use std::cell::{Cell, OnceCell}; use std::rc::{Rc, Weak}; use super::main_window::RenderState; @@ -19,13 +19,12 @@ pub struct PopupWindow { render_state: Cell, size: Cell, scale_factor: Cell, - popup_manager: RefCell>, - popup_key: Cell>, + popup_id: Cell>, + on_close: OnceCell, configured: Cell, component_instance: OnceCell, } -#[allow(dead_code)] impl PopupWindow { #[must_use] pub fn new(renderer: FemtoVGRenderer) -> Rc { @@ -37,17 +36,23 @@ impl PopupWindow { render_state: Cell::new(RenderState::Clean), size: Cell::new(PhysicalSize::default()), scale_factor: Cell::new(1.), - popup_manager: RefCell::new(Weak::new()), - popup_key: Cell::new(None), + popup_id: Cell::new(None), + on_close: OnceCell::new(), configured: Cell::new(false), component_instance: OnceCell::new(), } }) } - pub fn set_popup_manager(&self, popup_manager: Weak, key: usize) { - *self.popup_manager.borrow_mut() = popup_manager; - self.popup_key.set(Some(key)); + #[must_use] + pub fn new_with_callback(renderer: FemtoVGRenderer, on_close: OnCloseCallback) -> Rc { + let window = Self::new(renderer); + window.on_close.set(on_close).ok(); + window + } + + pub fn set_popup_id(&self, id: PopupId) { + self.popup_id.set(Some(id)); } pub fn close_popup(&self) { @@ -57,15 +62,14 @@ impl PopupWindow { info!("Failed to hide popup window: {e}"); } - if let Some(popup_manager) = self.popup_manager.borrow().upgrade() { - if let Some(key) = self.popup_key.get() { - info!("Destroying popup with key {key}"); - popup_manager.destroy_popup(key); + if let Some(id) = self.popup_id.get() { + info!("Destroying popup with id {:?}", id); + if let Some(on_close) = self.on_close.get() { + on_close(id); } } - *self.popup_manager.borrow_mut() = Weak::new(); - self.popup_key.set(None); + self.popup_id.set(None); info!("Popup window cleanup complete"); } @@ -107,7 +111,7 @@ impl PopupWindow { } pub fn popup_key(&self) -> Option { - self.popup_key.get() + self.popup_id.get().map(PopupId::key) } pub fn mark_configured(&self) { diff --git a/adapters/src/wayland/services/popup_service.rs b/adapters/src/wayland/services/popup_service.rs index 829cbb0..82ae1c7 100644 --- a/adapters/src/wayland/services/popup_service.rs +++ b/adapters/src/wayland/services/popup_service.rs @@ -1,5 +1,6 @@ use crate::errors::Result; use crate::rendering::femtovg::popup_window::PopupWindow; +use crate::wayland::surfaces::popup_manager::PopupId; use layer_shika_domain::value_objects::popup_request::{PopupHandle, PopupRequest}; use log::info; use slint::PhysicalSize; @@ -37,7 +38,8 @@ impl PopupService { } pub fn close(&self, handle: PopupHandle) -> Result<()> { - self.manager.destroy_popup(handle.key()); + let id = PopupId::from_key(handle.key()); + self.manager.destroy_popup(id); Ok(()) } diff --git a/adapters/src/wayland/surfaces/popup_manager.rs b/adapters/src/wayland/surfaces/popup_manager.rs index dfdfe61..34102ce 100644 --- a/adapters/src/wayland/surfaces/popup_manager.rs +++ b/adapters/src/wayland/surfaces/popup_manager.rs @@ -5,10 +5,10 @@ use layer_shika_domain::value_objects::popup_config::PopupConfig; use layer_shika_domain::value_objects::popup_positioning_mode::PopupPositioningMode; use layer_shika_domain::value_objects::popup_request::PopupRequest; use log::info; -use slab::Slab; use slint::{platform::femtovg_renderer::FemtoVGRenderer, PhysicalSize, WindowSize}; use smithay_client_toolkit::reexports::protocols_wlr::layer_shell::v1::client::zwlr_layer_surface_v1::ZwlrLayerSurfaceV1; use std::cell::RefCell; +use std::collections::HashMap; use std::rc::Rc; use wayland_client::{ backend::ObjectId, @@ -22,6 +22,23 @@ use wayland_protocols::xdg::shell::client::xdg_wm_base::XdgWmBase; use super::popup_surface::PopupSurface; use super::surface_state::WindowState; +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct PopupId(pub(crate) usize); + +impl PopupId { + #[must_use] + pub const fn key(self) -> usize { + self.0 + } + + #[must_use] + pub const fn from_key(key: usize) -> Self { + Self(key) + } +} + +pub type OnCloseCallback = Box; + #[derive(Debug, Clone, Copy)] pub struct CreatePopupParams { pub last_pointer_serial: u32, @@ -83,21 +100,23 @@ struct PendingPopup { } struct PopupManagerState { - popups: Slab, + popups: HashMap, scale_factor: f32, output_size: PhysicalSize, - current_popup_key: Option, + current_popup_id: Option, pending_popup: Option, + id_generator: usize, } impl PopupManagerState { fn new(initial_scale_factor: f32) -> Self { Self { - popups: Slab::new(), + popups: HashMap::new(), scale_factor: initial_scale_factor, output_size: PhysicalSize::new(0, 0), - current_popup_key: None, + current_popup_id: None, pending_popup: None, + id_generator: 0, } } } @@ -152,15 +171,15 @@ impl PopupManager { } pub fn close_current_popup(&self) { - let key = self.state.borrow_mut().current_popup_key.take(); - if let Some(key) = key { - self.destroy_popup(key); + let id = self.state.borrow_mut().current_popup_id.take(); + if let Some(id) = id { + self.destroy_popup(id); } } #[must_use] pub fn current_popup_key(&self) -> Option { - self.state.borrow().current_popup_key + self.state.borrow().current_popup_id.map(PopupId::key) } pub fn create_popup( @@ -235,7 +254,24 @@ impl PopupManager { let renderer = FemtoVGRenderer::new(context) .map_err(|e| LayerShikaError::FemtoVGRendererCreation { source: e })?; - let popup_window = PopupWindow::new(renderer); + let popup_id = { + let mut state = self.state.borrow_mut(); + let id = PopupId(state.id_generator); + state.id_generator += 1; + id + }; + + let on_close: OnCloseCallback = { + let weak_self = Rc::downgrade(self); + Box::new(move |id: PopupId| { + if let Some(manager) = weak_self.upgrade() { + manager.destroy_popup(id); + } + }) + }; + + let popup_window = PopupWindow::new_with_callback(renderer, on_close); + popup_window.set_popup_id(popup_id); popup_window.set_scale_factor(scale_factor); popup_window.set_size(WindowSize::Logical(slint::LogicalSize::new( params.width, @@ -243,23 +279,25 @@ impl PopupManager { ))); let mut state = self.state.borrow_mut(); - let key = state.popups.insert(ActivePopup { - surface: popup_surface, - window: Rc::clone(&popup_window), - request, - last_serial: params.last_pointer_serial, - }); - popup_window.set_popup_manager(Rc::downgrade(self), key); - state.current_popup_key = Some(key); + state.popups.insert( + popup_id, + ActivePopup { + surface: popup_surface, + window: Rc::clone(&popup_window), + request, + last_serial: params.last_pointer_serial, + }, + ); + state.current_popup_id = Some(popup_id); - info!("Popup window created successfully with key {key}"); + info!("Popup window created successfully with id {:?}", popup_id); Ok(popup_window) } pub fn render_popups(&self) -> Result<()> { let state = self.state.borrow(); - for (_key, popup) in &state.popups { + for popup in state.popups.values() { popup.window.render_frame_if_dirty()?; } Ok(()) @@ -271,7 +309,7 @@ impl PopupManager { pub fn mark_all_popups_dirty(&self) { let state = self.state.borrow(); - for (_key, popup) in &state.popups { + for popup in state.popups.values() { popup.window.request_redraw(); } } @@ -281,55 +319,55 @@ impl PopupManager { .borrow() .popups .iter() - .find_map(|(key, popup)| (popup.surface.surface.id() == *surface_id).then_some(key)) + .find_map(|(id, popup)| (popup.surface.surface.id() == *surface_id).then_some(id.key())) } pub fn find_popup_key_by_fractional_scale_id( &self, fractional_scale_id: &ObjectId, ) -> Option { - self.state.borrow().popups.iter().find_map(|(key, popup)| { + self.state.borrow().popups.iter().find_map(|(id, popup)| { popup .surface .fractional_scale .as_ref() .filter(|fs| fs.id() == *fractional_scale_id) - .map(|_| key) + .map(|_| id.key()) }) } pub fn get_popup_window(&self, key: usize) -> Option> { + let id = PopupId(key); self.state .borrow() .popups - .get(key) + .get(&id) .map(|popup| Rc::clone(&popup.window)) } - pub fn destroy_popup(&self, key: usize) { - if let Some(popup) = self.state.borrow_mut().popups.try_remove(key) { - info!("Destroying popup with key {key}"); + pub fn destroy_popup(&self, id: PopupId) { + if let Some(popup) = self.state.borrow_mut().popups.remove(&id) { + info!("Destroying popup with id {:?}", id); popup.surface.destroy(); } } pub fn find_popup_key_by_xdg_popup_id(&self, xdg_popup_id: &ObjectId) -> Option { - self.state - .borrow() - .popups - .iter() - .find_map(|(key, popup)| (popup.surface.xdg_popup.id() == *xdg_popup_id).then_some(key)) + self.state.borrow().popups.iter().find_map(|(id, popup)| { + (popup.surface.xdg_popup.id() == *xdg_popup_id).then_some(id.key()) + }) } pub fn find_popup_key_by_xdg_surface_id(&self, xdg_surface_id: &ObjectId) -> Option { - self.state.borrow().popups.iter().find_map(|(key, popup)| { - (popup.surface.xdg_surface.id() == *xdg_surface_id).then_some(key) + self.state.borrow().popups.iter().find_map(|(id, popup)| { + (popup.surface.xdg_surface.id() == *xdg_surface_id).then_some(id.key()) }) } pub fn update_popup_viewport(&self, key: usize, logical_width: i32, logical_height: i32) { - if let Some(popup) = self.state.borrow().popups.get(key) { + let id = PopupId(key); + if let Some(popup) = self.state.borrow().popups.get(&id) { popup .surface .update_viewport_size(logical_width, logical_height); @@ -337,15 +375,17 @@ impl PopupManager { } pub fn get_popup_info(&self, key: usize) -> Option<(PopupRequest, u32)> { + let id = PopupId(key); self.state .borrow() .popups - .get(key) + .get(&id) .map(|popup| (popup.request.clone(), popup.last_serial)) } pub fn mark_popup_configured(&self, key: usize) { - if let Some(popup) = self.state.borrow().popups.get(key) { + let id = PopupId(key); + if let Some(popup) = self.state.borrow().popups.get(&id) { popup.window.mark_configured(); } } diff --git a/composition/src/system.rs b/composition/src/system.rs index 059a6ae..90c54fa 100644 --- a/composition/src/system.rs +++ b/composition/src/system.rs @@ -11,7 +11,10 @@ use layer_shika_adapters::platform::slint_interpreter::{ use layer_shika_adapters::wayland::{ config::WaylandWindowConfig, shell_adapter::WaylandWindowingSystem, - surfaces::{popup_manager::PopupManager, surface_state::WindowState}, + surfaces::{ + popup_manager::{PopupId, PopupManager}, + surface_state::WindowState, + }, }; use layer_shika_domain::config::WindowConfig; use layer_shika_domain::errors::DomainError; @@ -19,6 +22,7 @@ use layer_shika_domain::value_objects::popup_positioning_mode::PopupPositioningM use layer_shika_domain::value_objects::popup_request::{ PopupAt, PopupHandle, PopupRequest, PopupSize, }; +use std::cell::Cell; use std::cell::{Ref, RefCell}; use std::os::unix::io::AsFd; use std::rc::{Rc, Weak}; @@ -196,7 +200,8 @@ impl RuntimeState<'_> { popup_manager.set_pending_popup(req, initial_dimensions.0, initial_dimensions.1); - let instance = Self::create_popup_instance(&definition, &popup_manager, 0, resize_sender)?; + let (instance, popup_key_cell) = + Self::create_popup_instance(&definition, &popup_manager, resize_sender)?; let popup_key = popup_manager.current_popup_key().ok_or_else(|| { Error::Domain(DomainError::Configuration { @@ -204,6 +209,8 @@ impl RuntimeState<'_> { }) })?; + popup_key_cell.set(popup_key); + if let Some(popup_window) = popup_manager.get_popup_window(popup_key) { popup_window.set_component_instance(instance); } else { @@ -217,7 +224,8 @@ impl RuntimeState<'_> { pub fn close_popup(&mut self, handle: PopupHandle) -> Result<()> { if let Some(popup_manager) = self.window_state.popup_manager() { - popup_manager.destroy_popup(handle.key()); + let id = PopupId::from_key(handle.key()); + popup_manager.destroy_popup(id); } Ok(()) } @@ -247,11 +255,13 @@ impl RuntimeState<'_> { }) .map(Rc::clone)?; - let (request, _serial) = popup_manager.get_popup_info(key).ok_or_else(|| { - Error::Domain(DomainError::Configuration { - message: format!("Popup with key {} not found", key), - }) - })?; + let Some((request, _serial)) = popup_manager.get_popup_info(key) else { + log::debug!( + "Ignoring resize request for non-existent popup with key {}", + key + ); + return Ok(()); + }; let current_size = request.size.dimensions(); let size_changed = @@ -288,9 +298,8 @@ impl RuntimeState<'_> { fn create_popup_instance( definition: &ComponentDefinition, popup_manager: &Rc, - popup_key: usize, resize_sender: Option>, - ) -> Result { + ) -> Result<(ComponentInstance, Rc>)> { let instance = definition.create().map_err(|e| { Error::Domain(DomainError::Configuration { message: format!("Failed to create popup instance: {}", e), @@ -311,7 +320,10 @@ impl RuntimeState<'_> { }) })?; + let popup_key_cell = Rc::new(Cell::new(0)); + let result = if let Some(sender) = resize_sender { + let key_cell = Rc::clone(&popup_key_cell); instance.set_callback("change_popup_size", move |args| { let width: f32 = args .first() @@ -322,7 +334,13 @@ impl RuntimeState<'_> { .and_then(|v| v.clone().try_into().ok()) .unwrap_or(150.0); - log::info!("change_popup_size callback invoked: {}x{}", width, height); + let popup_key = key_cell.get(); + log::info!( + "change_popup_size callback invoked: {}x{} for key {}", + width, + height, + popup_key + ); if sender .send(PopupCommand::Resize { @@ -338,6 +356,7 @@ impl RuntimeState<'_> { }) } else { let popup_manager_for_resize = Rc::downgrade(popup_manager); + let key_cell = Rc::clone(&popup_key_cell); instance.set_callback("change_popup_size", move |args| { let width: f32 = args .first() @@ -348,7 +367,13 @@ impl RuntimeState<'_> { .and_then(|v| v.clone().try_into().ok()) .unwrap_or(150.0); - log::info!("change_popup_size callback invoked: {}x{}", width, height); + let popup_key = key_cell.get(); + log::info!( + "change_popup_size callback invoked: {}x{} for key {}", + width, + height, + popup_key + ); if let Some(popup_window) = popup_manager_for_resize .upgrade() @@ -372,7 +397,7 @@ impl RuntimeState<'_> { }) })?; - Ok(instance) + Ok((instance, popup_key_cell)) } }