refactor: migrate to callback popup resizing

This commit is contained in:
drendog 2025-11-04 17:34:37 +01:00
parent 0ff8d19fe3
commit bb26ce9e6b
Signed by: dwenya
GPG key ID: 8DD77074645332D0
6 changed files with 116 additions and 66 deletions

View file

@ -113,6 +113,47 @@ impl PopupWindow {
pub fn popup_key(&self) -> Option<usize> { pub fn popup_key(&self) -> Option<usize> {
self.popup_key.get() self.popup_key.get()
} }
pub fn cleanup_component_instance(&self) {
if let Some(instance) = self.component_instance.borrow_mut().take() {
info!("Cleaning up component instance to break reference cycles");
if let Err(e) = instance.hide() {
info!("Failed to hide component instance during cleanup: {e}");
}
drop(instance);
}
}
pub fn request_resize(&self, width: f32, height: f32) {
info!("Requesting popup resize to {}x{}", width, height);
let logical_size = slint::LogicalSize::new(width, height);
self.set_size(WindowSize::Logical(logical_size));
if let Some(popup_manager) = self.popup_manager.borrow().upgrade() {
if let Some(key) = self.popup_key.get() {
#[allow(clippy::cast_possible_truncation)]
#[allow(clippy::cast_possible_wrap)]
let logical_width = width as i32;
#[allow(clippy::cast_possible_truncation)]
#[allow(clippy::cast_possible_wrap)]
let logical_height = height as i32;
info!("Updating popup viewport to match Slint size: {}x{}", logical_width, logical_height);
popup_manager.update_popup_viewport(key, logical_width, logical_height);
}
}
self.request_redraw();
}
pub fn with_component_instance<F, R>(&self, f: F) -> Option<R>
where
F: FnOnce(&ComponentInstance) -> R,
{
self.component_instance
.borrow()
.as_ref()
.map(f)
}
} }
impl WindowAdapter for PopupWindow { impl WindowAdapter for PopupWindow {

View file

@ -18,7 +18,6 @@ use crate::{
use core::result::Result as CoreResult; use core::result::Result as CoreResult;
use layer_shika_domain::errors::DomainError; use layer_shika_domain::errors::DomainError;
use layer_shika_domain::ports::windowing::WindowingSystemPort; use layer_shika_domain::ports::windowing::WindowingSystemPort;
use layer_shika_domain::value_objects::popup_positioning_mode::PopupPositioningMode;
use log::{error, info}; use log::{error, info};
use slint::{ use slint::{
LogicalPosition, PhysicalSize, PlatformError, WindowPosition, LogicalPosition, PhysicalSize, PlatformError, WindowPosition,
@ -200,21 +199,10 @@ impl WaylandWindowingSystem {
positioning_mode: request.mode, positioning_mode: request.mode,
} }
} else { } else {
let output_size = popup_manager_clone.output_size(); log::warn!("Popup creator called without pending popup request - aborting");
#[allow(clippy::cast_precision_loss)] return Err(PlatformError::Other(
let default_width = output_size.width as f32; "No popup request available - cannot create popup without parameters".to_string()
#[allow(clippy::cast_precision_loss)] ));
let default_height = output_size.height as f32;
log::warn!("No popup request provided, using output size ({default_width}x{default_height}) as defaults");
CreatePopupParams {
last_pointer_serial: serial,
reference_x: 0.0,
reference_y: 0.0,
width: default_width,
height: default_height,
positioning_mode: PopupPositioningMode::TopLeft,
}
}; };
let popup_window = popup_manager_clone let popup_window = popup_manager_clone

View file

@ -68,6 +68,13 @@ struct ActivePopup {
window: Rc<PopupWindow>, window: Rc<PopupWindow>,
} }
impl Drop for ActivePopup {
fn drop(&mut self) {
info!("ActivePopup being dropped - cleaning up resources");
self.window.cleanup_component_instance();
}
}
struct PopupState { struct PopupState {
scale_factor: f32, scale_factor: f32,
output_size: PhysicalSize, output_size: PhysicalSize,
@ -277,6 +284,9 @@ impl PopupManager {
pub fn destroy_popup(&self, key: usize) { pub fn destroy_popup(&self, key: usize) {
if let Some(popup) = self.popups.borrow_mut().try_remove(key) { if let Some(popup) = self.popups.borrow_mut().try_remove(key) {
info!("Destroying popup with key {key}"); info!("Destroying popup with key {key}");
popup.window.cleanup_component_instance();
popup.surface.destroy(); popup.surface.destroy();
} }
} }
@ -287,4 +297,10 @@ impl PopupManager {
.iter() .iter()
.find_map(|(key, popup)| (popup.surface.xdg_popup.id() == *xdg_popup_id).then_some(key)) .find_map(|(key, popup)| (popup.surface.xdg_popup.id() == *xdg_popup_id).then_some(key))
} }
pub fn update_popup_viewport(&self, key: usize, logical_width: i32, logical_height: i32) {
if let Some(popup) = self.popups.borrow().get(key) {
popup.surface.update_viewport_size(logical_width, logical_height);
}
}
} }

View file

@ -139,6 +139,16 @@ impl PopupSurface {
self.xdg_popup.grab(seat, serial); self.xdg_popup.grab(seat, serial);
} }
pub fn update_viewport_size(&self, logical_width: i32, logical_height: i32) {
if let Some(ref vp) = self.viewport {
info!(
"Updating popup viewport destination to logical size: {}x{}",
logical_width, logical_height
);
vp.set_destination(logical_width, logical_height);
}
}
pub fn destroy(&self) { pub fn destroy(&self) {
info!("Destroying popup surface"); info!("Destroying popup surface");
self.xdg_popup.destroy(); self.xdg_popup.destroy();

View file

@ -157,14 +157,6 @@ impl RuntimeState<'_> {
self.close_current_popup()?; self.close_current_popup()?;
let (width, height) = match req.size {
PopupSize::Fixed { w, h } => {
log::debug!("Using fixed popup size: {}x{}", w, h);
(w, h)
}
PopupSize::Content => self.measure_popup_dimensions(&definition)?,
};
let popup_manager = self let popup_manager = self
.window_state .window_state
.popup_manager() .popup_manager()
@ -176,19 +168,30 @@ impl RuntimeState<'_> {
}) })
.map(Rc::clone)?; .map(Rc::clone)?;
let initial_dimensions = match req.size {
PopupSize::Fixed { w, h } => {
log::debug!("Using fixed popup size: {}x{}", w, h);
(w, h)
}
PopupSize::Content => {
log::debug!("Using content-based sizing - will measure after instance creation");
(600.0, 300.0)
}
};
log::debug!( log::debug!(
"Creating popup for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}", "Creating popup for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}",
req.component, req.component,
width, initial_dimensions.0,
height, initial_dimensions.1,
req.at.position().0, req.at.position().0,
req.at.position().1, req.at.position().1,
req.mode req.mode
); );
popup_manager.set_pending_popup(req, width, height); popup_manager.set_pending_popup(req, initial_dimensions.0, initial_dimensions.1);
let instance = Self::create_popup_instance(&definition, &popup_manager)?; let instance = Self::create_popup_instance(&definition, &popup_manager, 0)?;
let popup_key = popup_manager.current_popup_key().ok_or_else(|| { let popup_key = popup_manager.current_popup_key().ok_or_else(|| {
Error::Domain(DomainError::Configuration { Error::Domain(DomainError::Configuration {
@ -221,45 +224,10 @@ impl RuntimeState<'_> {
Ok(()) Ok(())
} }
fn measure_popup_dimensions(&mut self, definition: &ComponentDefinition) -> Result<(f32, f32)> {
log::debug!(
"Creating temporary popup instance to read dimensions from component properties"
);
let temp_instance = definition.create().map_err(|e| {
Error::Domain(DomainError::Configuration {
message: format!("Failed to create temporary popup instance: {}", e),
})
})?;
let width: f32 = temp_instance
.get_property("popup-width")
.ok()
.and_then(|v| v.try_into().ok())
.unwrap_or(120.0);
let height: f32 = temp_instance
.get_property("popup-height")
.ok()
.and_then(|v| v.try_into().ok())
.unwrap_or(120.0);
log::debug!(
"Read popup dimensions from component properties: {}x{} (popup-width, popup-height)",
width,
height
);
drop(temp_instance);
self.close_current_popup()?;
log::debug!("Destroyed temporary popup instance");
Ok((width, height))
}
fn create_popup_instance( fn create_popup_instance(
definition: &ComponentDefinition, definition: &ComponentDefinition,
popup_manager: &Rc<PopupManager>, popup_manager: &Rc<PopupManager>,
popup_key: usize,
) -> Result<ComponentInstance> { ) -> Result<ComponentInstance> {
let instance = definition.create().map_err(|e| { let instance = definition.create().map_err(|e| {
Error::Domain(DomainError::Configuration { Error::Domain(DomainError::Configuration {
@ -281,6 +249,33 @@ impl RuntimeState<'_> {
}) })
})?; })?;
let popup_manager_for_resize = Rc::downgrade(popup_manager);
let result = instance.set_callback("change_popup_size", move |args| {
let width: f32 = args
.first()
.and_then(|v| v.clone().try_into().ok())
.unwrap_or(200.0);
let height: f32 = args
.get(1)
.and_then(|v| v.clone().try_into().ok())
.unwrap_or(150.0);
log::info!("change_popup_size callback invoked: {}x{}", width, height);
if let Some(popup_mgr) = popup_manager_for_resize.upgrade() {
if let Some(popup_window) = popup_mgr.get_popup_window(popup_key) {
popup_window.request_resize(width, height);
}
}
Value::Void
});
if let Err(e) = result {
log::warn!("Failed to set change_popup_size callback: {}", e);
} else {
log::info!("change_popup_size callback registered successfully");
}
instance.show().map_err(|e| { instance.show().map_err(|e| {
Error::Domain(DomainError::Configuration { Error::Domain(DomainError::Configuration {
message: format!("Failed to show popup instance: {}", e), message: format!("Failed to show popup instance: {}", e),

View file

@ -45,7 +45,7 @@ impl PopupRequest {
} }
} }
#[derive(Debug, Clone)] #[derive(Debug, Clone, Copy)]
pub enum PopupAt { pub enum PopupAt {
Absolute { x: f32, y: f32 }, Absolute { x: f32, y: f32 },
Cursor, Cursor,
@ -77,7 +77,7 @@ impl PopupAt {
} }
} }
#[derive(Debug, Clone)] #[derive(Debug, Clone, Copy)]
pub enum PopupSize { pub enum PopupSize {
Fixed { w: f32, h: f32 }, Fixed { w: f32, h: f32 },
Content, Content,