From cc5ca1ce29f2391376242010de6fd9326213e901 Mon Sep 17 00:00:00 2001 From: Ashley Wulber Date: Thu, 3 Sep 2026 12:16:45 -0400 Subject: [PATCH] fix: handling of grab chains and deletion of pending popups --- .../wayland/event_loop/popup.rs | 31 ++++ .../wayland/event_loop/state.rs | 142 ++++++++++++------ 2 files changed, 128 insertions(+), 45 deletions(-) diff --git a/winit/src/platform_specific/wayland/event_loop/popup.rs b/winit/src/platform_specific/wayland/event_loop/popup.rs index 09cfe96c..109e9af6 100644 --- a/winit/src/platform_specific/wayland/event_loop/popup.rs +++ b/winit/src/platform_specific/wayland/event_loop/popup.rs @@ -36,6 +36,23 @@ impl PopupManager { pos.map(|pos| (&mut self.chains[pos.0], pos.1)) } + pub(crate) fn chain_for_popup( + &self, + id: &WlSurface, + ) -> Option<(&Vec, usize)> { + let pos = self.chains.iter().enumerate().find_map(move |(pos, c)| { + if let Some(pop_pos) = + c.into_iter().position(|p| p.popup.wl_surface() == id) + { + Some((pos, pop_pos)) + } else { + None + } + }); + + pos.map(|pos| (&self.chains[pos.0], pos.1)) + } + pub(crate) fn popup(&self, id: &WlSurface) -> Option<&SctkPopup> { self.chains.iter().find_map(move |c| { c.into_iter().find(|p| p.popup.wl_surface() == id) @@ -114,6 +131,20 @@ impl PopupManager { return ret; } + + pub(crate) fn remove_ignore_children( + &mut self, + popup: &WlSurface, + ) -> Option> { + // must perform cleanup so that the popups are dropped in the correct order + let ret = if let Some((chain, pos)) = self.chain_for_popup_mut(popup) { + Some(chain.drain(pos..).rev()) + } else { + None + }; + + return ret; + } } #[derive(Debug)] diff --git a/winit/src/platform_specific/wayland/event_loop/state.rs b/winit/src/platform_specific/wayland/event_loop/state.rs index 12bb75d0..642da54c 100644 --- a/winit/src/platform_specific/wayland/event_loop/state.rs +++ b/winit/src/platform_specific/wayland/event_loop/state.rs @@ -1318,10 +1318,28 @@ impl SctkState { return Ok(()); } else if !self.destroyed.is_empty() || self.popmgr.popup_id(settings.id).is_some() || self.popmgr.active_grab().is_some() { - let active_grab = self.popmgr.active_grab(); - if let Some(grab) = self.popmgr.root_grab().zip(active_grab).and_then(|(root, active)| (active.data.id != settings.parent).then_some(root.data.id)) { - let _ = self.handle_action(Action::Popup(platform_specific::wayland::popup::Action::Destroy { id: grab })); + // must clean up popup chains so that there is just one grab chain... + let to_destroy = if settings.grab { + if let Some((grab_chain, p_i)) = self.popmgr.active_grab().and_then(|g| self.popmgr.chain_for_popup(g.popup.wl_surface())) { + + if let Some(pos) = grab_chain.iter().position(|p| p.data.id == settings.parent) { + // destroy children of parent + grab_chain.get(pos + 1).map(|g| g.data.id) + } + else { + // destroy whole chain... + grab_chain.get(0).map(|g| g.data.id) + } + } else { + None + } + } else { + None + }; + if let Some(to_destroy) = to_destroy { + _ = self.destroy_popup(to_destroy, true); } + if self.popmgr.popup_id(settings.id).is_some() { let _ = self.handle_action(Action::Popup(platform_specific::wayland::popup::Action::Destroy { id: settings.id })); } @@ -1367,8 +1385,11 @@ impl SctkState { let wl_surface = surface.wl_surface().clone(); send_event(&self.events_sender, &self.proxy, SctkEvent::PopupEvent { - variant: crate::platform_specific::wayland::sctk_event::PopupEventVariant::Created(self.queue_handle.clone(), surface, id, common, self.connection.display()), - toplevel_id, parent_id, id: wl_surface }); + variant: crate::platform_specific::wayland::sctk_event::PopupEventVariant::Created( + self.queue_handle.clone(), surface, id, common, self.connection.display() + ), + toplevel_id, parent_id, id: wl_surface } + ); } Err(err) => { log::error!("Failed to create popup. {err:?}"); @@ -1379,46 +1400,8 @@ impl SctkState { // XXX popup destruction must be done carefully // first destroy the uppermost popup, then work down to the requested popup platform_specific::wayland::popup::Action::Destroy { id } => { - let Some(to_destroy) = self.popmgr.popup_id(id) else { - log::warn!("Destroyed popup does not exist."); - return Ok(()); - }; - let s = to_destroy.popup.wl_surface().clone(); - - let Some(to_destroy) = self.popmgr.remove(&s) else { - return Ok(()); - }; - - for popup in to_destroy { - if let Some(id) = self.id_map.remove(&popup.popup.wl_surface().id()) { - _ = self.destroyed.insert(id); - } - - if let Some(blurred) = self.blur_surfaces.remove(&id) { - blurred.destroy(); - } - _ = self.corner_radii.remove(&id); - - - let (removed, remaining): (Vec<_>, Vec<_>) = self - .subsurfaces - .drain(..) - .partition(|s| { - s.instance.parent == *popup.popup.wl_surface() - }); - - self.subsurfaces = remaining; - for s in removed - { - crate::subsurface_widget::remove_iced_subsurface( - &s.instance.wl_surface, - ); - send_event(&self.events_sender, &self.proxy, - SctkEvent::SubsurfaceEvent( crate::sctk_event::SubsurfaceEventVariant::Destroyed(s.instance) ) - ); - } - _ = send_event(&self.events_sender, &self.proxy, - SctkEvent::PopupEvent { variant: crate::sctk_event::PopupEventVariant::Done, toplevel_id: popup.data.toplevel.clone(), parent_id: popup.data.parent.wl_surface().clone(), id: popup.popup.wl_surface().clone() }); + if let Some(value) = self.destroy_popup(id, false) { + return value; } }, platform_specific::wayland::popup::Action::Size { id, width, height } => { @@ -1809,6 +1792,75 @@ impl SctkState { Ok(()) } + fn destroy_popup( + &mut self, + id: core::window::Id, + ignore_children: bool, + ) -> Option> { + let Some(to_destroy) = self.popmgr.popup_id(id) else { + if self.pending_popup.take().is_none_or(|p_id| p_id.0.id != id) { + log::warn!( + "Destroyed popup does not exist. {id:?}, existing: {:?}", + self.popmgr.popups().map(|p| p.data.id).collect::>() + ); + } + return Some(Ok(())); + }; + let s = to_destroy.popup.wl_surface().clone(); + let d: Option> = if ignore_children { + self.popmgr.remove(&s).map(|v| v.collect()) + } else { + self.popmgr.remove_ignore_children(&s).map(|v| v.collect()) + }; + let Some(to_destroy) = d else { + return Some(Ok(())); + }; + + for popup in to_destroy { + if let Some(id) = self.id_map.remove(&popup.popup.wl_surface().id()) + { + _ = self.destroyed.insert(id); + } + + if let Some(blurred) = self.blur_surfaces.remove(&id) { + blurred.destroy(); + } + _ = self.corner_radii.remove(&id); + + let (removed, remaining): (Vec<_>, Vec<_>) = self + .subsurfaces + .drain(..) + .partition(|s| s.instance.parent == *popup.popup.wl_surface()); + + self.subsurfaces = remaining; + for s in removed { + crate::subsurface_widget::remove_iced_subsurface( + &s.instance.wl_surface, + ); + send_event( + &self.events_sender, + &self.proxy, + SctkEvent::SubsurfaceEvent( + crate::sctk_event::SubsurfaceEventVariant::Destroyed( + s.instance, + ), + ), + ); + } + _ = send_event( + &self.events_sender, + &self.proxy, + SctkEvent::PopupEvent { + variant: crate::sctk_event::PopupEventVariant::Done, + toplevel_id: popup.data.toplevel.clone(), + parent_id: popup.data.parent.wl_surface().clone(), + id: popup.popup.wl_surface().clone(), + }, + ); + } + None + } + fn apply_blur( &mut self, id: core::window::Id,