From 1ccdad9512504a27efae1bf5b726e7484c70e848 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 03:09:14 +0000 Subject: [PATCH] docking: Floating::with_id for layouts built in code; debug builds warn about rebuilt layouts Floating::new, single and stack draw a fresh id on each call, so a layout built anew each frame opened a new OS window each frame. with_id gives such a layout the same windows every frame. Debug builds log a warning once when three frames running replace every native window with new ones holding the same panels. --- Cargo.lock | 1 + crates/ui/Cargo.toml | 1 + crates/ui/src/docking/model.rs | 19 +++++++++-- crates/ui/src/docking/native_tests.rs | 47 ++++++++++++++++++++++++++ crates/ui/src/docking/render/native.rs | 39 +++++++++++++++++++++ docs/ui-docking.md | 11 ++++-- 6 files changed, 112 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e9ae6cc1a..bb3435fd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -894,6 +894,7 @@ dependencies = [ "egui_extras", "egui_kittest", "image", + "log", "serde", "serde_json", ] diff --git a/crates/ui/Cargo.toml b/crates/ui/Cargo.toml index 554ed27fe..6b1f81457 100644 --- a/crates/ui/Cargo.toml +++ b/crates/ui/Cargo.toml @@ -12,6 +12,7 @@ workspace = true [dependencies] egui = { workspace = true } +log = "0.4" serde = { version = "1", features = ["derive"], optional = true } [features] diff --git a/crates/ui/src/docking/model.rs b/crates/ui/src/docking/model.rs index d539dabe7..543e2bb62 100644 --- a/crates/ui/src/docking/model.rs +++ b/crates/ui/src/docking/model.rs @@ -68,8 +68,8 @@ pub struct StackEntry

{ #[derive(Clone, Debug)] pub struct Floating

{ /// Names the window across panel moves, and its OS window with feature `native-windows`. - /// Unique within a layout: [`Self::new`] and loading draw fresh ids, and merging windows - /// keeps the receiving window's. + /// Unique within a layout: [`Self::new`] and loading draw fresh ids, [`Self::with_id`] + /// sets one, and merging windows keeps the receiving window's. pub id: u64, /// `Split`, `Tabs` and `Stack` nodes; a `Dock` is invalid here. pub root: Node

, @@ -122,20 +122,33 @@ pub(super) fn fresh_window_id() -> u64 { } impl

Floating

{ + /// A window holding `root`. Each call makes a new window, with a fresh [`Self::id`]: build + /// a layout once and keep it, or give its windows ids with [`Self::with_id`]. A layout + /// built anew each frame without them opens new OS windows each frame. pub fn new(root: Node

, rect: [f32; 4]) -> Self { Self { id: fresh_window_id(), root, rect, native: None, monitor: None, monitor_size: None, minimized: false } } + /// A window with one group. Each call makes a new window, as [`Self::new`] does. pub fn single(panels: Vec

, active: usize, rect: [f32; 4]) -> Self { Self::new(Node::Tabs { panels, active }, rect) } + /// The window with `id`, for a layout the app builds in code, such as a default layout: + /// built with the same id, it is the same window, and the same OS window, on every frame. + /// Ids must be unique within a layout. + pub fn with_id(mut self, id: u64) -> Self { + self.id = id; + self + } + pub fn panels(&self) -> impl Iterator { tree_panels(&self.root).into_iter() } /// Groups stacked top to bottom, as a v1 layout stored them: each group over a vertical - /// split of the ones below it, sized by their heights. `None` without groups. + /// split of the ones below it, sized by their heights. `None` without groups. Each call + /// makes a new window, as [`Self::new`] does. pub fn stack(groups: Vec>, rect: [f32; 4]) -> Option { super::dock_model::stack_tree(groups).map(|root| Self::new(root, rect)) } diff --git a/crates/ui/src/docking/native_tests.rs b/crates/ui/src/docking/native_tests.rs index e383e1f12..24975f0d7 100644 --- a/crates/ui/src/docking/native_tests.rs +++ b/crates/ui/src/docking/native_tests.rs @@ -1084,3 +1084,50 @@ fn a_manual_resize_keeps_the_opposite_edges_and_the_minimum_size() { assert_eq!(resized(start, D::SouthWest, vec2(500.0, -500.0), min), [240.0, 50.0, 160.0, 100.0]); assert_eq!(resized(start, D::NorthEast, vec2(-500.0, 500.0), min), [100.0, 150.0, 160.0, 100.0]); } + +/// [`layout`] built anew, its window with id 7 when `with_id`. +fn rebuilt_layout(with_id: bool) -> Layout { + let mut layout = layout(true); + if with_id { + layout.floating = layout.floating.into_iter().map(|w| w.with_id(7)).collect(); + } + layout +} + +#[test] +fn a_layout_built_each_frame_with_ids_keeps_its_os_window() { + let mut d = driver(rebuilt_layout(true)); + let viewport = d.viewport(7); + for _ in 0..4 { + d.layout = rebuilt_layout(true); + d.step(vec![]); + } + assert_eq!(d.host().windows.keys().copied().collect::>(), [viewport], "one OS window, the same each frame"); +} + +#[cfg(debug_assertions)] +#[test] +fn a_layout_built_each_frame_without_ids_is_warned_about() { + let warned = |d: &DockDriver| d.ctx.data(|m| m.get_temp::(d.id.with("native-rebuilt"))).unwrap_or(false); + let mut d = driver(rebuilt_layout(false)); + for _ in 0..4 { + d.layout = rebuilt_layout(false); + d.step(vec![]); + } + assert!(warned(&d)); + assert!(d.host().windows.len() > 1, "a new OS window each frame"); + + // A layout kept across frames, and one rebuilt with ids, are not. + let mut d = driver(layout(true)); + d.layout = rebuilt_layout(false); + for _ in 0..4 { + d.step(vec![]); + } + assert!(!warned(&d), "loaded once"); + let mut d = driver(rebuilt_layout(true)); + for _ in 0..4 { + d.layout = rebuilt_layout(true); + d.step(vec![]); + } + assert!(!warned(&d)); +} diff --git a/crates/ui/src/docking/render/native.rs b/crates/ui/src/docking/render/native.rs index 70ce6a0e6..529d1c7e6 100644 --- a/crates/ui/src/docking/render/native.rs +++ b/crates/ui/src/docking/render/native.rs @@ -160,6 +160,8 @@ impl Renderer<'_, P> { natives.push((window, rect, viewport_id(self.id, window.id, n))); } if native { + #[cfg(debug_assertions)] + self.warn_rebuilt(ctx, &natives); self.escape_elsewhere(ctx, None); // A window closed or docked during its own bar drag or resize ends it. let shown = |window: u64| natives.iter().any(|(w, ..)| w.id == window); @@ -198,6 +200,26 @@ impl Renderer<'_, P> { in_window } + /// Debug builds: warn once when for [`REBUILT_FRAMES`] frames running, every native window + /// is new, with the panels of the last frame's. The app builds its layout anew each frame + /// without [`Floating::with_id`], so each frame opens new OS windows. A layout loaded + /// again does it once. + #[cfg(debug_assertions)] + fn warn_rebuilt(&self, ctx: &Context, natives: &[(&Floating

, [f32; 4], ViewportId)]) { + let (key, warned) = (self.id.with("native-previous"), self.id.with("native-rebuilt")); + let current: Vec<(u64, Vec

)> = natives.iter().map(|(w, ..)| (w.id, w.panels().cloned().collect())).collect(); + let (runs, previous) = ctx.data_mut(|d| d.remove_temp::<(usize, Vec<(u64, Vec

)>)>(key)).unwrap_or_default(); + let runs = if rebuilt(&previous, ¤t) { runs + 1 } else { 0 }; + if runs >= REBUILT_FRAMES && !ctx.data(|d| d.get_temp::(warned).unwrap_or(false)) { + log::warn!( + "craft-ui docking: the native floating windows get new ids every frame, so each frame opens new OS windows. \ + Keep the layout across frames, or give its windows ids with Floating::with_id." + ); + ctx.data_mut(|d| d.insert_temp(warned, true)); + } + ctx.data_mut(|d| d.insert_temp(key, (runs, current))); + } + /// The main window and the shown native windows on the desktop, front first: the window /// focused most recently first. A window whose position is unknown is left out. fn map_desktop(&mut self, ctx: &Context, natives: &[(&Floating

, [f32; 4], ViewportId)], visible: bool) { @@ -613,6 +635,23 @@ impl Renderer<'_, P> { } } +/// Frames running whose native windows are all new, with the last frame's panels, before +/// [`Renderer::warn_rebuilt`] warns. +#[cfg(debug_assertions)] +const REBUILT_FRAMES: usize = 3; + +/// Whether every window of `current` is new to `previous`, and they hold the same panels: +/// windows, with their panels, as `(id, panels)`. +#[cfg(debug_assertions)] +fn rebuilt(previous: &[(u64, Vec

)], current: &[(u64, Vec

)]) -> bool { + !current.is_empty() && current.iter().all(|(id, _)| previous.iter().all(|(p, _)| p != id)) && panels(previous) == panels(current) +} + +#[cfg(debug_assertions)] +fn panels(windows: &[(u64, Vec

)]) -> std::collections::HashSet<&Vec

> { + windows.iter().map(|(_, panels)| panels).collect() +} + /// Move and size a native window so its content is the desktop rect `to`. The size goes /// first, so the move puts the top-left corner in place whichever corner the window system /// keeps on a resize (AppKit's frame origin is the bottom left). diff --git a/docs/ui-docking.md b/docs/ui-docking.md index 415410057..be677fb1e 100644 --- a/docs/ui-docking.md +++ b/docs/ui-docking.md @@ -42,7 +42,11 @@ accordions, no docks), a rect, an optional native-window rect with the monitor i `minimized`. Its `id` names the window across panel moves (see [Native windows](#native-windows)). `Floating::single` makes a window with one group, `Floating::new` one with a tree, and `Floating::stack` one with groups stacked top to bottom (a vertical split sized by the groups' -heights, as an app's own stacked layouts migrate); `panels()` and `contains()` read it. Saved layouts keep loading: a window stored as one group (v1) or as a +heights, as an app's own stacked layouts migrate); `panels()` and `contains()` read it. Each of +these calls makes a new window with a fresh `id`. Build a layout once and keep it, or give its +windows ids with `with_id`: a layout built anew each frame, as an unsaved default layout might +be, otherwise opens new OS windows each frame and loses their drags and places. Debug builds log +a warning when native windows are replaced by new ones with the same panels three frames running. Saved layouts keep loading: a window stored as one group (v1) or as a stack of groups loads as that group or as a vertical split of the stacked groups, sized by their heights. @@ -56,7 +60,7 @@ use craft_ui::docking::{Action, Floating, Layout, Node, Placement}; let mut layout = Layout { root: Some(Node::Tabs { panels: vec!["layers", "properties"], active: 0 }), - floating: vec![Floating::single(vec!["brushes"], 0, [40.0, 80.0, 240.0, 300.0])], + floating: vec![Floating::single(vec!["brushes"], 0, [40.0, 80.0, 240.0, 300.0]).with_id(1)], }; layout.apply(Action::Move { panel: "brushes", @@ -327,7 +331,8 @@ that is not floatable, stays in-window. Floating windows get no taskbar or Alt-T (`with_taskbar(false)`). - **Identity.** `Floating::id` names the OS window, so a window keeps it when its panels change, - including when its first panel moves out. `Floating::new` and loading draw fresh ids; a layout + including when its first panel moves out. `Floating::new` and loading draw fresh ids, and + `Floating::with_id` sets one; a layout loads with unique ids; a window that merges into another takes the receiver's id, and a whole window that moves (floated again, or its only group dragged) keeps its own. - **Level.** The windows stay above other windows (`WindowLevel::AlwaysOnTop`), so clicking the