From c2d5b7ed12db44933f9960c74d6bad41a2a490c7 Mon Sep 17 00:00:00 2001 From: drendog Date: Thu, 27 Nov 2025 10:40:09 +0100 Subject: [PATCH] refactor: popup dropping path --- Cargo.lock | 7 ++-- Cargo.toml | 1 - crates/adapters/Cargo.toml | 1 - crates/adapters/src/rendering/egl/context.rs | 2 +- .../src/rendering/femtovg/popup_window.rs | 38 +++++++++++++++---- .../src/wayland/surfaces/app_state.rs | 9 ----- .../src/wayland/surfaces/popup_manager.rs | 2 +- 7 files changed, 35 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a2ca4a8..700a5b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1866,7 +1866,6 @@ dependencies = [ "layer-shika-domain", "log", "raw-window-handle", - "slab", "slint", "slint-interpreter", "smithay-client-toolkit 0.20.0", @@ -3329,12 +3328,12 @@ dependencies = [ [[package]] name = "smithay-clipboard" -version = "0.7.2" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc8216eec463674a0e90f29e0ae41a4db573ec5b56b1c6c1c71615d249b6d846" +checksum = "71704c03f739f7745053bde45fa203a46c58d25bc5c4efba1d9a60e9dba81226" dependencies = [ "libc", - "smithay-client-toolkit 0.19.2", + "smithay-client-toolkit 0.20.0", "wayland-backend", ] diff --git a/Cargo.toml b/Cargo.toml index 216098b..da134d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,6 @@ glutin = { version = "0.32.3", default-features = false, features = [ spin_on = "0.1" log = "0.4.28" raw-window-handle = "0.6.2" -slab = "0.4" slint = { version = "1.14.1", default-features = false, features = [ "compat-1-2", "renderer-femtovg", diff --git a/crates/adapters/Cargo.toml b/crates/adapters/Cargo.toml index 04032c7..826d7ce 100644 --- a/crates/adapters/Cargo.toml +++ b/crates/adapters/Cargo.toml @@ -18,7 +18,6 @@ glutin.workspace = true layer-shika-domain.workspace = true log.workspace = true raw-window-handle.workspace = true -slab.workspace = true slint.workspace = true slint-interpreter.workspace = true smithay-client-toolkit.workspace = true diff --git a/crates/adapters/src/rendering/egl/context.rs b/crates/adapters/src/rendering/egl/context.rs index 67868ff..948fd80 100644 --- a/crates/adapters/src/rendering/egl/context.rs +++ b/crates/adapters/src/rendering/egl/context.rs @@ -131,7 +131,7 @@ impl Drop for EGLContext { if let Err(e) = self.context.make_not_current_in_place() { log::error!("Failed to make EGL context not current during cleanup: {e}"); } else { - log::debug!("Successfully made EGL context not current during cleanup"); + log::info!("Successfully made EGL context not current during cleanup"); } } } diff --git a/crates/adapters/src/rendering/femtovg/popup_window.rs b/crates/adapters/src/rendering/femtovg/popup_window.rs index a4720b8..9cdb337 100644 --- a/crates/adapters/src/rendering/femtovg/popup_window.rs +++ b/crates/adapters/src/rendering/femtovg/popup_window.rs @@ -9,7 +9,7 @@ use slint::{ platform::{Renderer, WindowAdapter, WindowEvent, femtovg_renderer::FemtoVGRenderer}, }; use slint_interpreter::ComponentInstance; -use std::cell::{Cell, OnceCell}; +use std::cell::{Cell, OnceCell, RefCell}; use std::rc::{Rc, Weak}; pub struct PopupWindow { @@ -21,7 +21,7 @@ pub struct PopupWindow { popup_handle: Cell>, on_close: OnceCell, configured: Cell, - component_instance: OnceCell, + component_instance: RefCell>, } impl PopupWindow { @@ -38,7 +38,7 @@ impl PopupWindow { popup_handle: Cell::new(None), on_close: OnceCell::new(), configured: Cell::new(false), - component_instance: OnceCell::new(), + component_instance: RefCell::new(None), } }) } @@ -54,13 +54,26 @@ impl PopupWindow { self.popup_handle.set(Some(handle)); } - pub fn close_popup(&self) { - info!("Closing popup window - cleaning up resources"); + pub(crate) fn cleanup_resources(&self) { + info!("Cleaning up popup window resources to break reference cycles"); if let Err(e) = self.window.hide() { info!("Failed to hide popup window: {e}"); } + if let Some(component) = self.component_instance.borrow_mut().take() { + info!("Dropping ComponentInstance to break reference cycle"); + drop(component); + } + + info!("Popup window resource cleanup complete"); + } + + pub fn close_popup(&self) { + info!("Closing popup window - cleaning up resources"); + + self.cleanup_resources(); + if let Some(handle) = self.popup_handle.get() { info!("Destroying popup with handle {:?}", handle); if let Some(on_close) = self.on_close.get() { @@ -88,9 +101,11 @@ impl PopupWindow { pub fn set_component_instance(&self, instance: ComponentInstance) { info!("Setting component instance for popup window"); - if self.component_instance.set(instance).is_err() { - info!("Component instance already set for popup window"); + let mut comp = self.component_instance.borrow_mut(); + if comp.is_some() { + info!("Component instance already set for popup window - replacing"); } + *comp = Some(instance); } pub fn request_resize(&self, width: f32, height: f32) { @@ -177,6 +192,13 @@ impl Deref for PopupWindow { impl Drop for PopupWindow { fn drop(&mut self) { - info!("PopupWindow being dropped - resources will be released"); + info!("PopupWindow being dropped - cleaning up resources"); + + if let Some(component) = self.component_instance.borrow_mut().take() { + info!("Dropping any remaining ComponentInstance in PopupWindow::drop"); + drop(component); + } + + info!("PopupWindow drop complete"); } } diff --git a/crates/adapters/src/wayland/surfaces/app_state.rs b/crates/adapters/src/wayland/surfaces/app_state.rs index 59cc096..674b280 100644 --- a/crates/adapters/src/wayland/surfaces/app_state.rs +++ b/crates/adapters/src/wayland/surfaces/app_state.rs @@ -144,15 +144,6 @@ impl AppState { self.output_mapping.get(output_id) } - pub fn register_popup_surface( - &mut self, - popup_surface_id: ObjectId, - output_handle: OutputHandle, - ) { - self.surface_to_output - .insert(popup_surface_id, output_handle); - } - pub fn set_active_output_handle(&mut self, handle: Option) { self.output_registry.set_active(handle); } diff --git a/crates/adapters/src/wayland/surfaces/popup_manager.rs b/crates/adapters/src/wayland/surfaces/popup_manager.rs index 7d43ad4..d3d4e78 100644 --- a/crates/adapters/src/wayland/surfaces/popup_manager.rs +++ b/crates/adapters/src/wayland/surfaces/popup_manager.rs @@ -425,7 +425,7 @@ impl PopupManager { fn destroy_popup(&self, id: PopupId) { if let Some(popup) = self.state.borrow_mut().popups.remove(&id) { info!("Destroying popup with id {:?}", id); - + popup.window.cleanup_resources(); popup.surface.destroy(); } }