From 13ec65cf08ba05c1a6516afc8bdf495e4bebcb71 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 24 Apr 2023 17:04:10 +0200 Subject: [PATCH] Fix drag-drop and simplification code --- crates/egui_extras/Cargo.toml | 6 +- crates/egui_extras/src/dock/mod.rs | 171 ++++++++++++++++++++++++----- crates/egui_extras/src/lib.rs | 16 +-- examples/dock/Cargo.toml | 2 +- examples/dock/src/main.rs | 7 +- 5 files changed, 153 insertions(+), 49 deletions(-) diff --git a/crates/egui_extras/Cargo.toml b/crates/egui_extras/Cargo.toml index ecaaf0dee..152c722be 100644 --- a/crates/egui_extras/Cargo.toml +++ b/crates/egui_extras/Cargo.toml @@ -32,9 +32,6 @@ datepicker = ["chrono"] ## Support loading svg images. svg = ["resvg", "tiny-skia", "usvg"] -## Log warnings using [`log`](https://docs.rs/log) crate. -log = ["dep:log", "egui/log"] - [dependencies] egui = { version = "0.21.0", path = "../egui", default-features = false } @@ -42,6 +39,7 @@ egui = { version = "0.21.0", path = "../egui", default-features = false } # For dock: # required to make rand work on wasm, see https://github.com/rust-random/rand#wasm-support getrandom = { version = "0.2", features = ["js"] } +log = { version = "0.4", features = ["std"] } rand = { version = "0.8.5", features = ["getrandom", "small_rng"] } serde = { version = "1", features = ["derive"] } @@ -62,8 +60,6 @@ document-features = { version = "0.2", optional = true } ## ``` image = { version = "0.24", optional = true, default-features = false } -# feature "log" -log = { version = "0.4", optional = true, features = ["std"] } # svg feature resvg = { version = "0.28", optional = true, default-features = false } diff --git a/crates/egui_extras/src/dock/mod.rs b/crates/egui_extras/src/dock/mod.rs index 6f1281013..297fde7bc 100644 --- a/crates/egui_extras/src/dock/mod.rs +++ b/crates/egui_extras/src/dock/mod.rs @@ -9,9 +9,7 @@ use egui::{ // Types required for state /// An identifier for a node in the dock tree, be it a branch or a leaf. -#[derive( - Clone, Copy, Debug, Default, Hash, PartialEq, Eq, serde::Serialize, serde::Deserialize, -)] +#[derive(Clone, Copy, Default, Hash, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct NodeId(u128); impl NodeId { @@ -28,6 +26,12 @@ impl NodeId { } } +impl std::fmt::Debug for NodeId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:08X}", self.0 as u32) + } +} + /// The top level type. Contains all peristent state, including layouts and sizes. #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct Dock { @@ -169,6 +173,24 @@ pub enum UiResponse { DragStarted, } +pub struct SimplificationOptions { + pub prune_empty_tabs: bool, + pub prune_single_child_tabs: bool, + pub prune_empty_layouts: bool, + pub prune_single_child_layouts: bool, +} + +impl Default for SimplificationOptions { + fn default() -> Self { + Self { + prune_empty_tabs: true, + prune_single_child_tabs: true, + prune_empty_layouts: true, + prune_single_child_layouts: true, + } + } +} + /// Trait defining how the [`Dock`] and its leaf should be shown. pub trait Behavior { /// Show this leaf node in the given [`egui::Ui`]. @@ -221,6 +243,10 @@ pub trait Behavior { fn gap_width(&self, _style: &Style) -> f32 { 1.0 } + + fn simplification_options(&self) -> SimplificationOptions { + SimplificationOptions::default() + } } // ---------------------------------------------------------------------------- @@ -267,6 +293,26 @@ impl Nodes { self.insert_node(NodeLayout::Horizontal(horizontal).into()) } + fn parent(&self, it: NodeId, needle_child: NodeId) -> Option { + match &self.nodes.get(&it)?.layout { + NodeLayout::Leaf(_) => None, + NodeLayout::Tabs(Tabs { children, .. }) + | NodeLayout::Horizontal(Horizontal { children, .. }) + | NodeLayout::Vertical(Vertical { children, .. }) => { + for &child in children { + if child == needle_child { + return Some(it); + } + if let Some(parent) = self.parent(child, needle_child) { + return Some(parent); + } + } + None + } + } + } + + /// Performs no simplifcations! fn remove_node_id_from_parent(&mut self, it: NodeId, remove: NodeId) -> GcAction { if it == remove { return GcAction::Remove; @@ -278,29 +324,12 @@ impl Nodes { children.retain(|&child| { self.remove_node_id_from_parent(child, remove) == GcAction::Keep }); - - // Simplify: - if children.is_empty() { - return GcAction::Remove; - } } NodeLayout::Horizontal(Horizontal { children, .. }) | NodeLayout::Vertical(Vertical { children, .. }) => { children.retain(|&child| { self.remove_node_id_from_parent(child, remove) == GcAction::Keep }); - - // Simplify: - if children.is_empty() { - return GcAction::Remove; - } - if children.len() == 1 { - // Remove `node` link parent directly to the only remaining child: - if let Some(child) = self.nodes.remove(&children[0]) { - self.nodes.insert(it, child); - return GcAction::Keep; - } - } } } self.nodes.insert(it, node); @@ -314,8 +343,7 @@ impl Nodes { index, } = insertion_point; let Some(mut node) = self.nodes.remove(&parent_id) else { - #[cfg(feature = "log")] - log::warn!("failed to insert"); + log::warn!("Failed to insert: could not find parent {parent_id:?}"); return; }; match layout_type { @@ -378,6 +406,7 @@ impl Dock { } pub fn ui(&mut self, behavior: &mut dyn Behavior, ui: &mut Ui) { + self.simplify(&behavior.simplification_options()); self.nodes.gc_root(behavior, self.root); self.nodes.layout_node( @@ -430,13 +459,33 @@ impl Dock { if ui.input(|i| i.pointer.any_released()) { ui.memory_mut(|mem| mem.stop_dragging()); if let Some(insertion_point) = drop_context.best_insertion { - self.remove_node_id_from_parent(dragged_node_id); - self.nodes.insert(insertion_point, dragged_node_id); + self.move_node(dragged_node_id, insertion_point); } } } } + fn simplify(&mut self, options: &SimplificationOptions) { + match self.nodes.simplify(options, self.root) { + SimplifyAction::Remove => { + log::warn!("Tried to simplify root node!"); // TODO: handle this + } + SimplifyAction::Keep => {} + SimplifyAction::Replace(new_root) => { + self.root = new_root; + } + } + } + + fn move_node(&mut self, moved_node_id: NodeId, insertion_point: InsertionPoint) { + log::debug!( + "Moving {moved_node_id:?} into {:?}", + insertion_point.parent_id + ); + self.remove_node_id_from_parent(moved_node_id); + self.nodes.insert(insertion_point, moved_node_id); + } + /// Find the currently dragged node, if any. fn dragged_id(&self, ctx: &egui::Context) -> Option { if !is_possible_drag(ctx) { @@ -464,6 +513,7 @@ impl Dock { None } + /// Performs no simplifcations! fn remove_node_id_from_parent(&mut self, dragged_node_id: NodeId) { self.nodes .remove_node_id_from_parent(self.root, dragged_node_id); @@ -492,9 +542,14 @@ impl Nodes { let mut visited = HashSet::default(); self.gc_node_id(behavior, &mut visited, root_id); - #[cfg(feature = "log")] if visited.len() < self.nodes.len() { - log::warn!("GC collection {} nodes", self.nodes.len() - visited.len()); + log::warn!( + "GC collecting nodes: {:?}", + self.nodes + .keys() + .filter(|id| !visited.contains(id)) + .collect::>() + ); } self.nodes.retain(|node_id, _| visited.contains(node_id)); @@ -508,7 +563,6 @@ impl Nodes { ) -> GcAction { let Some(mut node) = self.nodes.remove(&node_id) else { return GcAction::Remove; }; if !visited.insert(node_id) { - #[cfg(feature = "log")] log::warn!("Cycle or duplication detected"); return GcAction::Remove; } @@ -837,3 +891,66 @@ impl Nodes { } } } + +// ---------------------------------------------------------------------------- +// Simplification + +#[must_use] +enum SimplifyAction { + Remove, + Keep, + Replace(NodeId), +} + +impl Nodes { + fn simplify(&mut self, options: &SimplificationOptions, it: NodeId) -> SimplifyAction { + let Some(mut node) = self.nodes.remove(&it) else { return SimplifyAction::Remove; }; + + match &mut node.layout { + NodeLayout::Leaf(_) => {} + NodeLayout::Tabs(Tabs { children, .. }) => { + children.retain_mut(|child| match self.simplify(options, *child) { + SimplifyAction::Remove => false, + SimplifyAction::Keep => true, + SimplifyAction::Replace(new) => { + *child = new; + true + } + }); + + if options.prune_empty_tabs && children.is_empty() { + log::debug!("Simplify: removing empty tabs"); + return SimplifyAction::Remove; + } + if options.prune_single_child_tabs && children.len() == 1 { + log::debug!("Simplify: removing single-child tabs"); + return SimplifyAction::Replace(children[0]); + } + } + + NodeLayout::Horizontal(Horizontal { children, .. }) + | NodeLayout::Vertical(Vertical { children, .. }) => { + children.retain_mut(|child| match self.simplify(options, *child) { + SimplifyAction::Remove => false, + SimplifyAction::Keep => true, + SimplifyAction::Replace(new) => { + *child = new; + true + } + }); + + if options.prune_empty_layouts && children.is_empty() { + log::debug!("Simplify: removing empty layout"); + return SimplifyAction::Remove; + } + if options.prune_single_child_layouts && children.len() == 1 { + log::debug!("Simplify: removing single-child layout"); + return SimplifyAction::Replace(children[0]); + } + } + } + + self.nodes.insert(it, node); + SimplifyAction::Keep + } +} diff --git a/crates/egui_extras/src/lib.rs b/crates/egui_extras/src/lib.rs index ece4ce5b8..0c205f0bc 100644 --- a/crates/egui_extras/src/lib.rs +++ b/crates/egui_extras/src/lib.rs @@ -29,27 +29,13 @@ pub use crate::sizing::Size; pub use crate::strip::*; pub use crate::table::*; -/// Log an error with either `log` or `eprintln` -macro_rules! log_err { - ($fmt: literal, $($arg: tt)*) => {{ - #[cfg(feature = "log")] - log::error!($fmt, $($arg)*); - - #[cfg(not(feature = "log"))] - eprintln!( - concat!("egui_extras: ", $fmt), $($arg)* - ); - }}; -} -pub(crate) use log_err; - /// Panic in debug builds, log otherwise. macro_rules! log_or_panic { ($fmt: literal, $($arg: tt)*) => {{ if cfg!(debug_assertions) { panic!($fmt, $($arg)*); } else { - $crate::log_err!($fmt, $($arg)*); + log::error!($fmt, $($arg)*); } }}; } diff --git a/examples/dock/Cargo.toml b/examples/dock/Cargo.toml index ab8ea4968..ea2d0c7d9 100644 --- a/examples/dock/Cargo.toml +++ b/examples/dock/Cargo.toml @@ -12,5 +12,5 @@ publish = false eframe = { path = "../../crates/eframe", features = [ "__screenshot", # __screenshot is so we can dump a screenshot using EFRAME_SCREENSHOT_TO ] } -egui_extras = { path = "../../crates/egui_extras", features = ["log"] } +egui_extras = { path = "../../crates/egui_extras" } env_logger = "0.10" diff --git a/examples/dock/src/main.rs b/examples/dock/src/main.rs index 375c9f71a..1168c20e2 100644 --- a/examples/dock/src/main.rs +++ b/examples/dock/src/main.rs @@ -109,7 +109,12 @@ fn tree_ui( // return; // } - egui::CollapsingHeader::new(behavior.tab_text_for_node(nodes, node_id)) + let text = format!( + "{} - {node_id:?}", + behavior.tab_text_for_node(nodes, node_id).text() + ); + + egui::CollapsingHeader::new(text) .id_source((node_id, "tree")) .default_open(true) .show(ui, |ui| match node {