From de7c78accd7b6cfaf751d13ad0efab538b9bbb31 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Mon, 9 Feb 2026 17:07:03 -0800 Subject: [PATCH 1/2] layout/tiling: Make `cleanup_drag` push tree only if something changes Since `copy_clone()` preserved IDs, `traverse_pre_order_ids()` can be called on the old tree, without collecting into a `Vec`. Then we can also `copy_clone()` only if there's actually a change, and also only call `push_tree()` in that case. (Once the `LazyCell::get()` stabilization is released, we could use that here, but `Option::get_or_insert_with()` may be more readable anyway.) With this, `cleanup_drag()` should be pretty low-cost, so we shouldn't have to worry about whether or not it's redundant. --- src/shell/layout/tiling/mod.rs | 38 ++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/shell/layout/tiling/mod.rs b/src/shell/layout/tiling/mod.rs index ffeeb52a..85efb2ce 100644 --- a/src/shell/layout/tiling/mod.rs +++ b/src/shell/layout/tiling/mod.rs @@ -2637,28 +2637,36 @@ impl TilingLayout { } pub fn cleanup_drag(&mut self) { - let gaps = self.gaps(); + let old_tree = &self.queue.trees.back().unwrap().0; + let mut new_tree = None; - let mut tree = self.queue.trees.back().unwrap().0.copy_clone(); - - if let Some(root) = tree.root_node_id() { - for id in tree - .traverse_pre_order_ids(root) - .unwrap() - .collect::>() - .into_iter() - { - match tree.get_mut(&id).map(|node| node.data_mut()) { - Ok(Data::Placeholder { .. }) => TilingLayout::unmap_internal(&mut tree, &id), + if let Some(root) = old_tree.root_node_id() { + for id in old_tree.traverse_pre_order_ids(root).unwrap() { + match old_tree.get(&id).map(|node| node.data()) { + Ok(Data::Placeholder { .. }) => { + // Copy a tree on write + let new_tree = new_tree.get_or_insert_with(|| old_tree.copy_clone()); + TilingLayout::unmap_internal(new_tree, &id) + } Ok(Data::Group { pill_indicator, .. }) if pill_indicator.is_some() => { - pill_indicator.take(); + let new_tree = new_tree.get_or_insert_with(|| old_tree.copy_clone()); + match new_tree.get_mut(&id).unwrap().data_mut() { + Data::Group { pill_indicator, .. } => { + *pill_indicator = None; + } + _ => unreachable!(), + } } _ => {} } } - let blocker = TilingLayout::update_positions(&self.output, &mut tree, gaps); - self.queue.push_tree(tree, ANIMATION_DURATION, blocker); + // If anything was changed, push updated tree + if let Some(mut new_tree) = new_tree { + let blocker = + TilingLayout::update_positions(&self.output, &mut new_tree, self.gaps()); + self.queue.push_tree(new_tree, ANIMATION_DURATION, blocker); + } } } From 7e48191253ae04ba95f2d7907cc04b3ae81edce0 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Mon, 9 Feb 2026 17:13:07 -0800 Subject: [PATCH 2/2] grabs/moving: Call `cleanup_drag()` unconditionally Previously, drag placeholder would be removed in the call to `tiling_layer.drop_window()` when dropping onto a tiling layer, but would not be removed when dropping to a floating layer. Which would leave a placeholder taking up space, and cause a panic on a future drag operation. Instead, call `cleanup_drag()` regardless, after `drop_window()`, to do any cleanup that is still needed. This moves the call that was previously added in 67d0a825. --- src/shell/grabs/moving.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/shell/grabs/moving.rs b/src/shell/grabs/moving.rs index f59f93d6..8b01d086 100644 --- a/src/shell/grabs/moving.rs +++ b/src/shell/grabs/moving.rs @@ -906,20 +906,22 @@ impl Drop for MoveGrab { } } } else { - let mut shell = state.common.shell.write(); - shell - .workspaces - .active_mut(&cursor_output) - .unwrap() - .tiling_layer - .cleanup_drag(); - shell.set_overview_mode(None, state.common.event_loop_handle.clone()); None } } else { None }; + let mut shell = state.common.shell.write(); + shell + .workspaces + .active_mut(&cursor_output) + .unwrap() + .tiling_layer + .cleanup_drag(); + shell.set_overview_mode(None, state.common.event_loop_handle.clone()); + drop(shell); + { let cursor_state = seat.user_data().get::().unwrap(); cursor_state.lock().unwrap().unset_shape();