From a5e48f1b9f440edb8d751975e9029051563eece2 Mon Sep 17 00:00:00 2001 From: drendog Date: Tue, 4 Nov 2025 23:56:25 +0100 Subject: [PATCH] fix: popup flicker due render before configuration event --- .../src/rendering/femtovg/popup_window.rs | 67 ++++++------------- .../event_handling/event_dispatcher.rs | 11 +++ .../src/wayland/surfaces/popup_manager.rs | 19 ++++-- .../src/wayland/surfaces/popup_surface.rs | 3 + 4 files changed, 50 insertions(+), 50 deletions(-) diff --git a/adapters/src/rendering/femtovg/popup_window.rs b/adapters/src/rendering/femtovg/popup_window.rs index 0f0ddf2..1ae8161 100644 --- a/adapters/src/rendering/femtovg/popup_window.rs +++ b/adapters/src/rendering/femtovg/popup_window.rs @@ -3,7 +3,7 @@ use crate::wayland::surfaces::popup_manager::PopupManager; use core::ops::Deref; use log::info; use slint::{ - ComponentHandle, PhysicalSize, Window, WindowSize, + PhysicalSize, Window, WindowSize, platform::{Renderer, WindowAdapter, WindowEvent, femtovg_renderer::FemtoVGRenderer}, }; use slint_interpreter::ComponentInstance; @@ -21,6 +21,7 @@ pub struct PopupWindow { scale_factor: Cell, popup_manager: RefCell>, popup_key: Cell>, + configured: Cell, component_instance: RefCell>, } @@ -38,15 +39,12 @@ impl PopupWindow { scale_factor: Cell::new(1.), popup_manager: RefCell::new(Weak::new()), popup_key: Cell::new(None), + configured: Cell::new(false), component_instance: RefCell::new(None), } }) } - pub fn set_component_instance(&self, instance: ComponentInstance) { - *self.component_instance.borrow_mut() = Some(instance); - } - pub fn set_popup_manager(&self, popup_manager: Weak, key: usize) { *self.popup_manager.borrow_mut() = popup_manager; self.popup_key.set(Some(key)); @@ -55,13 +53,6 @@ impl PopupWindow { pub fn close_popup(&self) { info!("Closing popup window - cleaning up resources"); - if let Some(instance) = self.component_instance.borrow_mut().take() { - info!("Hiding ComponentInstance to release strong reference from show()"); - if let Err(e) = instance.hide() { - info!("Failed to hide component instance: {e}"); - } - } - if let Err(e) = self.window.hide() { info!("Failed to hide popup window: {e}"); } @@ -80,6 +71,11 @@ impl PopupWindow { } pub fn render_frame_if_dirty(&self) -> Result<()> { + if !self.configured.get() { + info!("Popup not yet configured, skipping render"); + return Ok(()); + } + if matches!( self.render_state.replace(RenderState::Clean), RenderState::Dirty @@ -114,46 +110,25 @@ impl PopupWindow { 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 mark_configured(&self) { + info!("Popup window marked as configured"); + self.configured.set(true); + } + + pub fn is_configured(&self) -> bool { + self.configured.get() + } + + pub fn set_component_instance(&self, instance: ComponentInstance) { + info!("Setting component instance for popup window"); + *self.component_instance.borrow_mut() = Some(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.set_size(WindowSize::Logical(slint::LogicalSize::new(width, height))); self.request_redraw(); } - - pub fn with_component_instance(&self, f: F) -> Option - where - F: FnOnce(&ComponentInstance) -> R, - { - self.component_instance - .borrow() - .as_ref() - .map(f) - } } impl WindowAdapter for PopupWindow { diff --git a/adapters/src/wayland/event_handling/event_dispatcher.rs b/adapters/src/wayland/event_handling/event_dispatcher.rs index 2332ddd..da70646 100644 --- a/adapters/src/wayland/event_handling/event_dispatcher.rs +++ b/adapters/src/wayland/event_handling/event_dispatcher.rs @@ -268,6 +268,17 @@ impl Dispatch for WindowState { height, } => { info!("XdgPopup Configure: position=({x}, {y}), size=({width}x{height})"); + + if let Some(popup_manager) = &state.popup_manager() { + let popup_id = xdg_popup.id(); + if let Some(key) = popup_manager.find_popup_key_by_xdg_popup_id(&popup_id) { + info!( + "Marking popup with key {key} as configured after XdgPopup::Configure" + ); + popup_manager.mark_popup_configured(key); + popup_manager.mark_all_popups_dirty(); + } + } } xdg_popup::Event::PopupDone => { info!("XdgPopup dismissed by compositor"); diff --git a/adapters/src/wayland/surfaces/popup_manager.rs b/adapters/src/wayland/surfaces/popup_manager.rs index d5b931f..c767221 100644 --- a/adapters/src/wayland/surfaces/popup_manager.rs +++ b/adapters/src/wayland/surfaces/popup_manager.rs @@ -73,7 +73,6 @@ struct ActivePopup { impl Drop for ActivePopup { fn drop(&mut self) { info!("ActivePopup being dropped - cleaning up resources"); - self.window.cleanup_component_instance(); } } @@ -290,8 +289,6 @@ impl PopupManager { if let Some(popup) = self.popups.borrow_mut().try_remove(key) { info!("Destroying popup with key {key}"); - popup.window.cleanup_component_instance(); - popup.surface.destroy(); } } @@ -303,9 +300,17 @@ impl PopupManager { .find_map(|(key, popup)| (popup.surface.xdg_popup.id() == *xdg_popup_id).then_some(key)) } + pub fn find_popup_key_by_xdg_surface_id(&self, xdg_surface_id: &ObjectId) -> Option { + self.popups.borrow().iter().find_map(|(key, popup)| { + (popup.surface.xdg_surface.id() == *xdg_surface_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); + popup + .surface + .update_viewport_size(logical_width, logical_height); } } @@ -315,4 +320,10 @@ impl PopupManager { .get(key) .map(|popup| (popup.request.clone(), popup.last_serial)) } + + pub fn mark_popup_configured(&self, key: usize) { + if let Some(popup) = self.popups.borrow().get(key) { + popup.window.mark_configured(); + } + } } diff --git a/adapters/src/wayland/surfaces/popup_surface.rs b/adapters/src/wayland/surfaces/popup_surface.rs index f02e80c..b5e4a12 100644 --- a/adapters/src/wayland/surfaces/popup_surface.rs +++ b/adapters/src/wayland/surfaces/popup_surface.rs @@ -137,6 +137,9 @@ impl PopupSurface { pub fn grab(&self, seat: &WlSeat, serial: u32) { info!("Grabbing popup with serial {serial}"); self.xdg_popup.grab(seat, serial); + + info!("Committing popup surface to trigger configure event"); + self.surface.commit(); } pub fn update_viewport_size(&self, logical_width: i32, logical_height: i32) {