From 7a497d3b4cd72cd80ceb0bef454e458f962e5ea4 Mon Sep 17 00:00:00 2001 From: Lewis Date: Thu, 11 Jun 2026 14:13:39 +0300 Subject: [PATCH] featuretree: extrude nodes w/ edit & rename Lewis: May this revision serve well! --- crates/bone-app/src/hotkeys.rs | 2 +- crates/bone-app/src/main.rs | 439 ++++++++++++++++-- crates/bone-app/src/shell.rs | 254 +++++++++- crates/bone-app/src/sketch_mode.rs | 25 +- .../src/document/feature_tree.rs | 13 +- crates/bone-document/src/document/mod.rs | 151 +++++- crates/bone-document/src/io/folder.rs | 27 +- crates/bone-document/src/lib.rs | 4 +- .../bone-document/tests/folder_roundtrip.rs | 22 +- .../bone-document/tests/folder_snapshots.rs | 21 +- .../folder_snapshots__document_header.snap | 2 +- .../folder_snapshots__extrude_file.snap | 3 +- .../folder_snapshots__sketch_file.snap | 2 +- crates/bone-render/src/camera3.rs | 110 +++++ crates/bone-render/src/lib.rs | 6 +- crates/bone-render/src/navigate.rs | 116 ++++- crates/bone-types/src/lib.rs | 24 +- crates/bone-types/src/schema.rs | 2 +- crates/bone-ui/src/widgets/tree_view.rs | 57 ++- 19 files changed, 1142 insertions(+), 138 deletions(-) diff --git a/crates/bone-app/src/hotkeys.rs b/crates/bone-app/src/hotkeys.rs index c29b526..1755d65 100644 --- a/crates/bone-app/src/hotkeys.rs +++ b/crates/bone-app/src/hotkeys.rs @@ -455,7 +455,7 @@ mod tests { }); assert!( !bound_in_sketch, - "{action:?} must ship unbound; SolidWorks stock has no default for it" + "{action:?} must ship unbound with no stock default" ); }); } diff --git a/crates/bone-app/src/main.rs b/crates/bone-app/src/main.rs index 0f16f49..b542cd4 100644 --- a/crates/bone-app/src/main.rs +++ b/crates/bone-app/src/main.rs @@ -12,13 +12,13 @@ use bone_render::{ Camera2, ChromeInstance, ChromePipeline, ChromeTextPipeline, DragModifiers, EdgeScene, NavGesture, PickQuery, PickedItem, PixelsPerMm, RenderTargets, SdfGlyphInstance, SketchPreview, SketchRenderer, SketchScene, SolidFrameView, SolidRenderer, SolidScene, Style, SurfaceContext, - ViewportExtent, ViewportNavigator, ViewportPoint, ViewportPx, ViewportRegion, - frame_standard_view, zoom_about_pixel, + ViewportExtent, ViewportNavigator, ViewportPoint, ViewportPx, ViewportRegion, frame_current, + frame_standard_view, orbit_pitch, orbit_yaw, pan_pixels, roll_by, zoom_about_pixel, }; use bone_types::{ - Aabb3, AngleTolerance, BudgetCeiling, Camera3, ChordHeightTolerance, DisplayMode, DocumentId, - FeatureId, GeometryGeneration, Length, Point2, SketchId, SketchItemId, StandardView, Vec2, - ZoomFactor, + Aabb3, Angle, AngleTolerance, BudgetCeiling, Camera3, ChordHeightTolerance, DisplayMode, + DocumentId, ExtrudeId, FeatureId, GeometryGeneration, Length, Point2, SketchId, SketchItemId, + StandardView, Vec2, ZoomFactor, }; use bone_ui::a11y::AccessTreeBuilder; use bone_ui::focus::FocusManager; @@ -37,6 +37,7 @@ use bone_ui::theme::{Theme, ThemeMode}; use bone_ui::{MaskAtlas, MaskAtlasParams, Shaper}; use swash::FontRef; use tracing_subscriber::EnvFilter; +use uom::si::angle::degree; use uom::si::length::millimeter; use winit::{ application::ApplicationHandler, @@ -106,6 +107,8 @@ const DEFAULT_LOG_FILTER: &str = "bone_app=info,bone_render=info,bone_document=i const ZOOM_STEP_PER_LINE: f64 = 1.1; const ZOOM_STEP_PER_PIXEL: f64 = 1.0025; const ZOOM_KEY_STEP: f64 = 1.25; +const ORBIT_KEY_STEP_DEG: f64 = 15.0; +const ORBIT_KEY_SNAP_DEG: f64 = 90.0; const ZOOM_MIN: f64 = 0.01; const ZOOM_MAX: f64 = 1.0e5; const INITIAL_ZOOM_PX_PER_MM: f64 = 12.0; @@ -138,6 +141,7 @@ struct RenderState { solid_renderer: SolidRenderer, solid_view: Option, camera3: Option, + framed_extrude: Option, navigator: ViewportNavigator, focus: FocusManager, hit_state: HitState, @@ -552,7 +556,7 @@ fn refresh_active_scene(state: &mut RenderState) { fn active_sketch_id(mode: &Mode, plane_sketches: &BTreeMap) -> Option { match mode { Mode::Sketch { sketch_id, .. } => Some(*sketch_id), - Mode::Extrude(ExtrudeArming::Profile(feature)) => Some(feature.sketch), + Mode::Extrude(ExtrudeArming::Profile { feature, .. }) => Some(feature.sketch), Mode::Extrude(ExtrudeArming::AwaitingSketch) | Mode::Idle => { plane_sketches.get(&Plane::Xy).copied() } @@ -598,11 +602,72 @@ fn arm_extruded_boss_base(state: &mut RenderState) { fn apply_extrude_edit(state: &mut RenderState, edit: Option) { let Some(edit) = edit else { return }; - let Mode::Extrude(ExtrudeArming::Profile(feature)) = &state.mode else { + let Mode::Extrude(ExtrudeArming::Profile { feature, target }) = &state.mode else { return; }; let next = edit.apply(*feature); - state.mode = Mode::Extrude(ExtrudeArming::Profile(next)); + let target = *target; + state.mode = Mode::Extrude(ExtrudeArming::Profile { + feature: next, + target, + }); +} + +fn apply_extrude_activation(state: &mut RenderState, activated: Option) { + if let Some(mode) = extrude_edit_mode(&state.document, &state.mode, activated) { + if let Mode::Extrude(ExtrudeArming::Profile { + target: Some(id), .. + }) = mode + { + state.framed_extrude = Some(id); + } + state.mode = mode; + } +} + +fn extrude_edit_mode( + document: &Document, + current: &Mode, + activated: Option, +) -> Option { + let id = activated?; + if current.is_sketch() { + return None; + } + let feature = document.extrude(id).copied()?; + Some(Mode::Extrude(ExtrudeArming::edit(id, feature))) +} + +fn apply_extrude_confirm(state: &mut RenderState, confirm: Option) { + if let Some(id) = + commit_armed_extrude(&mut state.document, &mut state.undo, &state.mode, confirm) + { + state.framed_extrude = Some(id); + } +} + +fn commit_armed_extrude( + document: &mut Document, + undo: &mut UndoStack, + mode: &Mode, + confirm: Option, +) -> Option { + let Some(shell::ConfirmAction::Accept) = confirm else { + return None; + }; + let Mode::Extrude(ExtrudeArming::Profile { feature, target }) = mode else { + return None; + }; + let snapshot = document.clone(); + let committed = match target { + Some(id) => { + document.insert_extrude(*id, *feature); + *id + } + None => document.commit_extrude(*feature), + }; + undo.record(snapshot); + Some(committed) } struct SolidViewData { @@ -621,14 +686,38 @@ struct ExtrudePreview { const PREVIEW_CHORD_MM: f64 = 0.05; const PREVIEW_ANGLE: AngleTolerance = AngleTolerance::from_radians(0.2); -fn sync_extrude_preview(state: &mut RenderState) { - let Mode::Extrude(ExtrudeArming::Profile(feature)) = &state.mode else { +fn active_solid_feature( + mode: &Mode, + document: &Document, + framed: Option, +) -> Option { + match mode { + Mode::Extrude(ExtrudeArming::Profile { feature, .. }) => Some(*feature), + Mode::Sketch { .. } => None, + Mode::Idle | Mode::Extrude(ExtrudeArming::AwaitingSketch) => framed + .and_then(|id| document.extrude(id).copied()) + .or_else(|| { + document + .feature_tree() + .iter() + .filter_map(|(_, node)| match node { + FeatureNode::Extrude(id) => Some(id), + _ => None, + }) + .last() + .and_then(|id| document.extrude(id).copied()) + }), + } +} + +fn sync_solid_view(state: &mut RenderState) { + let Some(feature) = active_solid_feature(&state.mode, &state.document, state.framed_extrude) + else { state.extrude_preview = None; state.solid_view = None; state.camera3 = None; return; }; - let feature = *feature; let Some(sketch_version) = state.document.sketch(feature.sketch).map(Sketch::version) else { state.extrude_preview = None; state.solid_view = None; @@ -770,15 +859,20 @@ fn viewport_local_point( } fn drag_gesture(modifiers: ModifiersState) -> NavGesture { - let base = if modifiers.shift_key() { - DragModifiers::NONE.with_shift() + let with_ctrl = if modifiers.control_key() || modifiers.super_key() { + DragModifiers::NONE.with_ctrl() } else { DragModifiers::NONE }; + let with_shift = if modifiers.shift_key() { + with_ctrl.with_shift() + } else { + with_ctrl + }; let resolved = if modifiers.alt_key() { - base.with_alt() + with_shift.with_alt() } else { - base + with_shift }; resolved.gesture() } @@ -939,6 +1033,55 @@ fn keyboard_camera(code: KeyCode, input: &InputState, state: &RenderState) -> Op } } +fn zoom_key3( + camera: Camera3, + extent: ViewportExtent, + pixel: ViewportPoint, + factor: f64, +) -> Option { + ZoomFactor::new(factor) + .ok() + .and_then(|f| zoom_about_pixel(camera, extent, pixel, f).ok()) +} + +fn keyboard_camera3(code: KeyCode, input: &InputState, state: &RenderState) -> Option { + let camera = state.camera3?; + let region = solid_viewport_region(state.viewport_rect, state.surface.extent())?; + let extent = region.extent(); + let ctrl = input.modifiers.control_key() || input.modifiers.super_key(); + let shift = input.modifiers.shift_key(); + let alt = input.modifiers.alt_key(); + let cx = f64::from(extent.width().value()) * 0.5; + let cy = f64::from(extent.height().value()) * 0.5; + let center = ViewportPoint::new(cx, cy).ok()?; + let pan_to = |dx: f64, dy: f64| { + ViewportPoint::new(cx + dx, cy + dy) + .ok() + .and_then(|to| pan_pixels(camera, extent, center, to).ok()) + }; + let step = Angle::new::(if shift { + ORBIT_KEY_SNAP_DEG + } else { + ORBIT_KEY_STEP_DEG + }); + match code { + KeyCode::ArrowLeft if ctrl => pan_to(-PAN_STEP_PX, 0.0), + KeyCode::ArrowRight if ctrl => pan_to(PAN_STEP_PX, 0.0), + KeyCode::ArrowUp if ctrl => pan_to(0.0, -PAN_STEP_PX), + KeyCode::ArrowDown if ctrl => pan_to(0.0, PAN_STEP_PX), + KeyCode::ArrowLeft if alt => roll_by(camera, step).ok(), + KeyCode::ArrowRight if alt => roll_by(camera, -step).ok(), + KeyCode::ArrowLeft => orbit_yaw(camera, step).ok(), + KeyCode::ArrowRight => orbit_yaw(camera, -step).ok(), + KeyCode::ArrowUp => orbit_pitch(camera, step).ok(), + KeyCode::ArrowDown => orbit_pitch(camera, -step).ok(), + KeyCode::KeyZ if shift => zoom_key3(camera, extent, center, 1.0 / ZOOM_KEY_STEP), + KeyCode::KeyZ | KeyCode::Equal => zoom_key3(camera, extent, center, ZOOM_KEY_STEP), + KeyCode::Minus => zoom_key3(camera, extent, center, 1.0 / ZOOM_KEY_STEP), + _ => None, + } +} + fn build_hotkey_table() -> HotkeyTable { let Ok(table) = hotkeys::compose_table(&hotkeys::HotkeyOverrides::default()) else { unreachable!("default hotkey bindings are conflict-free"); @@ -988,7 +1131,9 @@ fn resolve_pick( if mode.is_extrude() { return match frame.sketch_activated { Some(id) => match &mode { - Mode::Extrude(ExtrudeArming::Profile(feature)) if feature.sketch == id => mode, + Mode::Extrude(ExtrudeArming::Profile { feature, .. }) if feature.sketch == id => { + mode + } _ => Mode::Extrude(ExtrudeArming::profile(id)), }, None => mode, @@ -1116,6 +1261,7 @@ impl ApplicationHandler for App { solid_renderer, solid_view: None, camera3: None, + framed_extrude: None, navigator: ViewportNavigator::new(), focus: FocusManager::new(), hit_state: HitState::new(), @@ -1231,15 +1377,29 @@ impl App { if let Some(camera) = state.camera3 && let Some(region) = solid_viewport_region(state.viewport_rect, state.surface.extent()) - && let Some(cursor) = self - .input - .cursor_px - .and_then(|p| viewport_local_point(p, region)) - && let Ok(factor) = ZoomFactor::new(zoom_factor(delta)) - && let Ok(next) = - zoom_about_pixel(camera, region.extent(), cursor, factor) { - state.camera3 = Some(next); + match delta { + MouseScrollDelta::PixelDelta(p) => { + if let Ok(next) = state + .navigator + .orbit_pixels(camera, region.extent(), p.x, p.y) + { + state.camera3 = Some(next); + } + } + MouseScrollDelta::LineDelta(..) => { + if let Some(cursor) = self + .input + .cursor_px + .and_then(|p| viewport_local_point(p, region)) + && let Ok(factor) = ZoomFactor::new(zoom_factor(delta)) + && let Ok(next) = + zoom_about_pixel(camera, region.extent(), cursor, factor) + { + state.camera3 = Some(next); + } + } + } } } else { state.camera = @@ -1384,9 +1544,14 @@ impl App { } if let Some(code) = physical_code && !suppress_camera - && let Some(next) = keyboard_camera(code, &self.input, state) { - state.camera = next; + if state.solid_view.is_some() { + if let Some(next) = keyboard_camera3(code, &self.input, state) { + state.camera3 = Some(next); + } + } else if let Some(next) = keyboard_camera(code, &self.input, state) { + state.camera = next; + } } let _ = event_loop; } @@ -1512,6 +1677,8 @@ fn render_frame( } } let escape_requested = hotkey_actions.contains(&sketch_mode::ESCAPE_ACTION); + apply_extrude_edit(state, frame.extrude_edit); + apply_extrude_confirm(state, frame.confirm_action); let prev_active_sketch = active_sketch_id(&state.mode, &state.plane_sketches); state.mode = next_mode( core::mem::take(&mut state.mode), @@ -1520,6 +1687,7 @@ fn render_frame( &state.plane_sketches, ); apply_feature_tool(state, frame.activated_feature_tool); + apply_extrude_activation(state, frame.extrude_activated); if active_sketch_id(&state.mode, &state.plane_sketches) != prev_active_sketch { refresh_active_scene(state); } @@ -1531,8 +1699,7 @@ fn render_frame( _ => frame.dimension_edit, }; apply_dimension_edit(state, dimension_edit); - apply_extrude_edit(state, frame.extrude_edit); - sync_extrude_preview(state); + sync_solid_view(state); let solid_region = solid_viewport_region(state.viewport_rect, extent); sync_solid_camera(state, solid_region); let cursor_layout = input_state.cursor_px.map(physical_to_layout_pos); @@ -1541,6 +1708,7 @@ fn render_frame( apply_settings_change(state, frame.settings_change); apply_relation_action(state, frame.activated_relation); apply_sketch_rename(state, frame.sketch_rename.clone()); + apply_extrude_rename(state, frame.extrude_rename.clone()); let cursor_world = input_state .cursor_px .filter(|c| state.viewport_rect.contains(physical_to_layout_pos(*c))) @@ -2292,6 +2460,8 @@ fn suppress_pointer_activations(frame: shell::ShellFrame) -> shell::ShellFrame { plane_picked: None, sketch_activated: None, sketch_rename: None, + extrude_activated: None, + extrude_rename: None, exit_sketch: false, confirm_action: None, menu_action: None, @@ -3001,7 +3171,15 @@ fn apply_menu_action(state: &mut RenderState, action: Option) refresh_active_scene(state); } Some(shell::MenuAction::ZoomFit) => { - state.camera = zoom_fit(state.camera, &state.scene, state.viewport_rect); + let solid_fit = state.solid_view.as_ref().map(|view| view.aabb).and_then(|aabb| { + let region = solid_viewport_region(state.viewport_rect, state.surface.extent())?; + frame_current(state.camera3?, aabb, region.extent()).ok() + }); + if let Some(next) = solid_fit { + state.camera3 = Some(next); + } else { + state.camera = zoom_fit(state.camera, &state.scene, state.viewport_rect); + } } Some(shell::MenuAction::OpenSettings) => { state.shell.state.settings_dialog_open = true; @@ -3064,6 +3242,7 @@ fn apply_new_document(state: &mut RenderState) { state.scene = scene; state.mode = Mode::Idle; state.selection = Selection::default(); + state.framed_extrude = None; state.current_folder = None; state.pending_overwrite = None; let Some(undo_capacity) = NonZeroUsize::new(UNDO_CAPACITY) else { @@ -3308,6 +3487,7 @@ fn install_loaded_document( state.plane_sketches = plane_sketches; state.mode = Mode::Idle; state.selection = Selection::default(); + state.framed_extrude = None; state.current_folder = folder; state.pending_overwrite = None; let Some(undo_capacity) = NonZeroUsize::new(UNDO_CAPACITY) else { @@ -3368,13 +3548,42 @@ fn apply_sketch_rename(state: &mut RenderState, request: Option) { + let Some(req) = request else { return }; + apply_extrude_rename_into(&mut state.document, &mut state.undo, req); +} + +fn apply_extrude_rename_into( + document: &mut Document, + undo: &mut UndoStack, + request: shell::ExtrudeRenameRequest, +) { + let shell::ExtrudeRenameRequest { id, label } = request; + let trimmed = label.trim(); + let Some(current) = document.extrude_label(id) else { + return; + }; + if trimmed.is_empty() || current == trimmed { + return; + } + let snapshot = document.clone(); + match document.rename_extrude(id, &label) { + Ok(()) => undo.record(snapshot), + Err(e) => tracing::warn!(error = %e, ?id, "extrude rename rejected"), + } +} + fn apply_sketch_rename_into( document: &mut Document, undo: &mut UndoStack, request: shell::SketchRenameRequest, ) { let shell::SketchRenameRequest { id, label } = request; - if document.sketch_label(id).is_some_and(|l| l == label.trim()) { + let trimmed = label.trim(); + let Some(current) = document.sketch_label(id) else { + return; + }; + if trimmed.is_empty() || current == trimmed { return; } let snapshot = document.clone(); @@ -3623,6 +3832,8 @@ mod tests { plane_picked: None, sketch_activated: None, sketch_rename: None, + extrude_activated: None, + extrude_rename: None, exit_sketch: false, confirm_action: None, menu_action: None, @@ -3856,18 +4067,19 @@ mod tests { } #[test] - fn drag_gesture_maps_modifiers_to_solidworks_navigation() { + fn drag_gesture_maps_modifiers_to_orbit_pan_zoom_roll() { use winit::keyboard::ModifiersState; assert_eq!( super::drag_gesture(ModifiersState::empty()), NavGesture::Orbit ); - assert_eq!(super::drag_gesture(ModifiersState::SHIFT), NavGesture::Pan); + assert_eq!(super::drag_gesture(ModifiersState::CONTROL), NavGesture::Pan); + assert_eq!(super::drag_gesture(ModifiersState::SHIFT), NavGesture::Zoom); assert_eq!(super::drag_gesture(ModifiersState::ALT), NavGesture::Roll); assert_eq!( - super::drag_gesture(ModifiersState::SHIFT | ModifiersState::ALT), + super::drag_gesture(ModifiersState::CONTROL | ModifiersState::SHIFT), NavGesture::Pan, - "shift wins so a held shift never rolls", + "ctrl outranks shift so a held ctrl always pans", ); } @@ -4302,6 +4514,165 @@ mod tests { ); } + fn extrude_node_count(document: &Document) -> usize { + document + .feature_tree() + .iter() + .filter(|(_, node)| matches!(node, FeatureNode::Extrude(_))) + .count() + } + + #[test] + fn commit_armed_extrude_on_accept_adds_node_and_records_undo() { + let (mut document, sketch) = doc_with_default_sketch(); + let mut undo = UndoStack::with_capacity(NonZeroUsize::MIN); + let mode = Mode::Extrude(ExtrudeArming::profile(sketch)); + commit_armed_extrude( + &mut document, + &mut undo, + &mode, + Some(shell::ConfirmAction::Accept), + ); + assert_eq!(extrude_node_count(&document), 1); + assert_eq!(undo.past_len(), 1); + } + + #[test] + fn commit_armed_extrude_ignores_cancel_and_non_extrude_mode() { + let (mut document, sketch) = doc_with_default_sketch(); + let mut undo = UndoStack::with_capacity(NonZeroUsize::MIN); + commit_armed_extrude( + &mut document, + &mut undo, + &Mode::Extrude(ExtrudeArming::profile(sketch)), + Some(shell::ConfirmAction::Cancel), + ); + commit_armed_extrude( + &mut document, + &mut undo, + &Mode::Idle, + Some(shell::ConfirmAction::Accept), + ); + assert_eq!(extrude_node_count(&document), 0); + assert_eq!(undo.past_len(), 0); + } + + #[test] + fn commit_armed_extrude_edit_target_updates_in_place_keeping_label() { + let (mut document, sketch) = doc_with_default_sketch(); + let id = document.commit_extrude(sketch_mode::default_extrude_feature(sketch)); + let Ok(()) = document.rename_extrude(id, "Boss") else { + panic!("rename accepts"); + }; + let mut undo = UndoStack::with_capacity(NonZeroUsize::MIN); + let mode = Mode::Extrude(ExtrudeArming::edit( + id, + sketch_mode::default_extrude_feature(sketch), + )); + commit_armed_extrude( + &mut document, + &mut undo, + &mode, + Some(shell::ConfirmAction::Accept), + ); + assert_eq!(extrude_node_count(&document), 1, "editing reuses the node"); + assert_eq!(document.extrude_label(id), Some("Boss")); + } + + #[test] + fn extrude_edit_mode_arms_from_idle_but_not_from_sketch() { + let (mut document, sketch) = doc_with_default_sketch(); + let id = document.commit_extrude(sketch_mode::default_extrude_feature(sketch)); + let from_idle = extrude_edit_mode(&document, &Mode::Idle, Some(id)); + assert!(matches!( + from_idle, + Some(Mode::Extrude(ExtrudeArming::Profile { target: Some(t), .. })) if t == id + )); + assert_eq!( + extrude_edit_mode(&document, &Mode::enter_sketch(sketch), Some(id)), + None, + "double-click is inert while sketching", + ); + assert_eq!( + extrude_edit_mode(&document, &Mode::Idle, Some(ExtrudeId::default())), + None, + "unknown extrude id arms nothing", + ); + } + + #[test] + fn active_solid_feature_tracks_mode_then_falls_back_to_committed() { + let (mut document, sketch) = doc_with_default_sketch(); + let armed = sketch_mode::default_extrude_feature(sketch); + assert_eq!( + active_solid_feature( + &Mode::Extrude(ExtrudeArming::profile(sketch)), + &document, + None, + ), + Some(armed), + "an armed profile previews its own feature", + ); + assert_eq!( + active_solid_feature(&Mode::enter_sketch(sketch), &document, None), + None, + "sketching shows the 2D scene, not a solid", + ); + assert_eq!( + active_solid_feature(&Mode::Idle, &document, None), + None, + "idle with no committed extrude shows no solid", + ); + let _ = document.commit_extrude(armed); + assert_eq!( + active_solid_feature(&Mode::Idle, &document, None), + Some(armed), + "idle falls back to the committed extrude", + ); + } + + #[test] + fn framed_extrude_overrides_last_committed_in_idle() { + let (mut document, sketch) = doc_with_default_sketch(); + let first_feature = sketch_mode::default_extrude_feature(sketch); + let mut second_feature = first_feature; + second_feature.merge_result = bone_document::MergeResult::Separate; + let first = document.commit_extrude(first_feature); + let _second = document.commit_extrude(second_feature); + assert_eq!( + active_solid_feature(&Mode::Idle, &document, None), + Some(second_feature), + "with no framed id, idle frames the last-committed extrude", + ); + assert_eq!( + active_solid_feature(&Mode::Idle, &document, Some(first)), + Some(first_feature), + "a framed id wins over the tree tip, so editing a non-last extrude stays framed", + ); + assert_eq!( + active_solid_feature(&Mode::Idle, &document, Some(ExtrudeId::default())), + Some(second_feature), + "a stale framed id self-heals to the last-committed extrude", + ); + } + + #[test] + fn apply_extrude_rename_into_writes_label_and_records_undo() { + let (mut document, sketch) = doc_with_default_sketch(); + let id = document.commit_extrude(sketch_mode::default_extrude_feature(sketch)); + let mut undo = UndoStack::with_capacity(NonZeroUsize::MIN); + apply_extrude_rename_into( + &mut document, + &mut undo, + shell::ExtrudeRenameRequest { + id, + label: "Boss".to_owned(), + }, + ); + assert_eq!(document.extrude_label(id), Some("Boss")); + assert_eq!(undo.past_len(), 1); + } + #[test] fn sketch_activated_from_idle_enters_that_sketch_without_plane_map() { let sketch_id = SketchId::default(); diff --git a/crates/bone-app/src/shell.rs b/crates/bone-app/src/shell.rs index ef92f01..33baa05 100644 --- a/crates/bone-app/src/shell.rs +++ b/crates/bone-app/src/shell.rs @@ -3,11 +3,12 @@ use std::collections::BTreeMap; use std::sync::Arc; use bone_document::{ - DimensionKind, DimensionValue, Document, ExtrudeEndCondition, ExtrudeFeature, MergeResult, - Sketch, SketchDimension, SketchEntity, SketchRelation, SketchStatusReport, SketchVersion, + DimensionKind, DimensionValue, Document, ExtrudeEndCondition, ExtrudeFeature, FeatureNode, + MergeResult, Sketch, SketchDimension, SketchEntity, SketchRelation, SketchStatusReport, + SketchVersion, }; use bone_types::{ - Angle, Length, Point2, PositiveLength, SketchDimensionId, SketchEntityId, SketchId, + Angle, ExtrudeId, Length, Point2, PositiveLength, SketchDimensionId, SketchEntityId, SketchId, }; use bone_ui::a11y::{AccessNode, Role}; use bone_ui::frame::{FrameCtx, InteractDeclaration}; @@ -430,6 +431,8 @@ pub struct ShellFrame { pub plane_picked: Option, pub sketch_activated: Option, pub sketch_rename: Option, + pub extrude_activated: Option, + pub extrude_rename: Option, pub exit_sketch: bool, pub confirm_action: Option, pub menu_action: Option, @@ -457,6 +460,8 @@ impl ShellFrame { plane_picked: None, sketch_activated: None, sketch_rename: None, + extrude_activated: None, + extrude_rename: None, exit_sketch: false, confirm_action: None, menu_action: None, @@ -674,8 +679,10 @@ impl Shell { self.state.status_panel_open = false; } } + let confirm_visible = + mode.is_sketch() || matches!(mode, Mode::Extrude(ExtrudeArming::Profile { .. })); let confirm = - render_confirm_corner(ctx, viewport_rect, &self.ids, mode.is_sketch(), &mut paints); + render_confirm_corner(ctx, viewport_rect, &self.ids, confirm_visible, &mut paints); let confirm_action = confirm; let exit_sketch = confirm_action.is_some() || menu_action == Some(MenuAction::ExitSketch); let activated_tool = activated_widget.and_then(|id| self.tool_index.get(&id).copied()); @@ -698,6 +705,8 @@ impl Shell { .and_then(|id| self.ids.plane_for(id)); let sketch_activated = feature_tree.sketch_activated; let sketch_rename = feature_tree.sketch_rename; + let extrude_activated = feature_tree.extrude_activated; + let extrude_rename = feature_tree.extrude_rename; let mut dialog_paints: Vec = Vec::new(); let settings_change = render_settings_dialog( ctx, @@ -732,6 +741,8 @@ impl Shell { plane_picked, sketch_activated, sketch_rename, + extrude_activated, + extrude_rename, exit_sketch, confirm_action, menu_action, @@ -1614,16 +1625,60 @@ pub struct SketchRenameRequest { pub label: String, } +#[derive(Clone, Debug, PartialEq)] +pub struct ExtrudeRenameRequest { + pub id: ExtrudeId, + pub label: String, +} + struct FeatureTreeOutcome { double_activated: Option, sketch_activated: Option, sketch_rename: Option, + extrude_activated: Option, + extrude_rename: Option, } fn sketch_widget_id(part_id: WidgetId, sketch_id: SketchId) -> WidgetId { part_id.child_indexed(WidgetKey::new("sketch"), sketch_id.as_u64()) } +fn extrude_widget_id(part_id: WidgetId, extrude_id: ExtrudeId) -> WidgetId { + part_id.child_indexed(WidgetKey::new("extrude"), extrude_id.as_u64()) +} + +fn sketch_tree_rows(document: &Document, part_id: WidgetId) -> Vec<(SketchId, WidgetId, TreeNode)> { + document + .sketches() + .map(|(sketch_id, _)| { + let widget_id = sketch_widget_id(part_id, sketch_id); + let label = document.sketch_label(sketch_id).unwrap_or("").to_owned(); + let node = TreeNode::leaf_owned(widget_id, label).with_glyph(GlyphMark::TreeSketch); + (sketch_id, widget_id, node) + }) + .collect() +} + +fn extrude_tree_rows( + document: &Document, + part_id: WidgetId, +) -> Vec<(ExtrudeId, WidgetId, TreeNode)> { + document + .feature_tree() + .iter() + .filter_map(|(_, node)| match node { + FeatureNode::Extrude(extrude_id) => Some(extrude_id), + _ => None, + }) + .map(|extrude_id| { + let widget_id = extrude_widget_id(part_id, extrude_id); + let label = document.extrude_label(extrude_id).unwrap_or("").to_owned(); + let node = TreeNode::leaf_owned(widget_id, label).with_glyph(GlyphMark::TreeFeature); + (extrude_id, widget_id, node) + }) + .collect() +} + fn render_feature_tree( ctx: &mut FrameCtx<'_>, rect: LayoutRect, @@ -1638,6 +1693,8 @@ fn render_feature_tree( double_activated: None, sketch_activated: None, sketch_rename: None, + extrude_activated: None, + extrude_rename: None, }; } let leaf = |key: &'static str, label: StringKey| { @@ -1648,18 +1705,17 @@ fn render_feature_tree( let placeholder = |key: &'static str, label: StringKey| feature_leaf(key, label).disabled(true); let plane_leaf = |key: &'static str, label: StringKey| leaf(key, label).with_glyph(GlyphMark::TreePlane); - let sketch_rows: Vec<(SketchId, WidgetId, TreeNode)> = document - .sketches() - .map(|(sketch_id, _)| { - let widget_id = sketch_widget_id(part_id, sketch_id); - let label = document.sketch_label(sketch_id).unwrap_or("").to_owned(); - let node = TreeNode::leaf_owned(widget_id, label).with_glyph(GlyphMark::TreeSketch); - (sketch_id, widget_id, node) - }) + let sketch_rows = sketch_tree_rows(document, part_id); + let extrude_rows = extrude_tree_rows(document, part_id); + let renamable: Vec = sketch_rows + .iter() + .map(|(_, w, _)| *w) + .chain(extrude_rows.iter().map(|(_, w, _)| *w)) .collect(); - let renamable: Vec = sketch_rows.iter().map(|(_, w, _)| *w).collect(); let widget_to_sketch: BTreeMap = sketch_rows.iter().map(|(s, w, _)| (*w, *s)).collect(); + let widget_to_extrude: BTreeMap = + extrude_rows.iter().map(|(e, w, _)| (*w, *e)).collect(); let children: Vec = [ placeholder("history", strings::FEATURE_HISTORY), placeholder("sensors", strings::FEATURE_SENSORS), @@ -1673,6 +1729,7 @@ fn render_feature_tree( ] .into_iter() .chain(sketch_rows.into_iter().map(|(_, _, node)| node)) + .chain(extrude_rows.into_iter().map(|(_, _, node)| node)) .collect(); let part = TreeNode::parent_owned(part_id, document.name().to_owned(), children); let roots = [part]; @@ -1685,21 +1742,40 @@ fn render_feature_tree( let sketch_activated = response .double_activated .and_then(|id| widget_to_sketch.get(&id).copied()); + let extrude_activated = response + .double_activated + .and_then(|id| widget_to_extrude.get(&id).copied()); let sketch_rename = response .rename_committed + .as_ref() .and_then(|RenameCommit { id, text }| { widget_to_sketch - .get(&id) + .get(id) .copied() .map(|sketch_id| SketchRenameRequest { id: sketch_id, - label: text, + label: text.clone(), }) }); + let extrude_rename = + response + .rename_committed + .as_ref() + .and_then(|RenameCommit { id, text }| { + widget_to_extrude + .get(id) + .copied() + .map(|extrude_id| ExtrudeRenameRequest { + id: extrude_id, + label: text.clone(), + }) + }); FeatureTreeOutcome { double_activated: response.double_activated, sketch_activated, sketch_rename, + extrude_activated, + extrude_rename, } } @@ -1739,7 +1815,7 @@ fn render_property_pane( if !matches!(resolved, Some((_, SelectionTarget::Dimension(_, _)))) { *editors.dim = None; } - if !matches!(state.mode, Mode::Extrude(ExtrudeArming::Profile(_))) { + if !matches!(state.mode, Mode::Extrude(ExtrudeArming::Profile { .. })) { *editors.extrude = None; } if rect.size.width.value() <= 0.0 || rect.size.height.value() <= 0.0 { @@ -1747,7 +1823,7 @@ fn render_property_pane( } if let Mode::Extrude(arming) = state.mode { return match arming { - ExtrudeArming::Profile(feature) => PropertyPaneOutcome { + ExtrudeArming::Profile { feature, .. } => PropertyPaneOutcome { dimension_edit: None, extrude_edit: render_extrude_rows( ctx, @@ -3468,7 +3544,8 @@ mod tests { } fn profile_feature() -> ExtrudeFeature { - let ExtrudeArming::Profile(feature) = ExtrudeArming::profile(SketchId::default()) else { + let ExtrudeArming::Profile { feature, .. } = ExtrudeArming::profile(SketchId::default()) + else { unreachable!("profile arming holds a feature"); }; feature @@ -4282,6 +4359,19 @@ mod tests { .child_indexed(WidgetKey::new("sketch"), sketch_id.as_u64()) } + fn extrude_widget(ids: &ShellIds, extrude_id: ExtrudeId) -> WidgetId { + ids.feature_part + .child_indexed(WidgetKey::new("extrude"), extrude_id.as_u64()) + } + + fn document_with_extrude() -> (Document, ExtrudeId) { + let sketch = bone_document::Sketch::new(crate::sketch_mode::Plane::Xy.basis()); + let (mut document, sketch_id) = document_with_sketch(sketch); + let extrude_id = + document.commit_extrude(crate::sketch_mode::default_extrude_feature(sketch_id)); + (document, extrude_id) + } + #[test] fn f2_with_focused_sketch_row_starts_rename_in_full_shell() { let sketch = bone_document::Sketch::new(crate::sketch_mode::Plane::Xy.basis()); @@ -4604,6 +4694,134 @@ mod tests { ); } + #[test] + fn double_click_extrude_row_emits_extrude_activated() { + use bone_ui::input::{PointerButton, PointerButtonMask, PointerSample}; + let (document, extrude_id) = document_with_extrude(); + let mut shell = Shell::new(); + let widget = extrude_widget(&shell.ids, extrude_id); + let mut focus = FocusManager::new(); + let mut prev = HitState::new(); + + let mut warm = InputSnapshot::idle(FrameInstant::ZERO); + let (_, hits) = shell_drive( + &mut shell, + &document, + &Mode::Idle, + &Selection::default(), + &mut focus, + &mut prev, + &mut warm, + ); + let Some(row_rect) = hits + .items() + .iter() + .find(|item| item.id == widget) + .map(|item| item.rect) + else { + panic!("extrude row must register a hit item"); + }; + let center = LayoutPos::new( + LayoutPx::new(row_rect.origin.x.value() + row_rect.size.width.value() / 2.0), + LayoutPx::new(row_rect.origin.y.value() + row_rect.size.height.value() / 2.0), + ); + let click = |shell: &mut Shell, focus: &mut FocusManager, prev: &mut HitState| { + let mut press = InputSnapshot::idle(FrameInstant::ZERO); + press.pointer = Some(PointerSample::new(center)); + press.buttons_pressed = PointerButtonMask::just(PointerButton::Primary); + let _ = drive_with_snap( + shell, + &document, + &Mode::Idle, + &Selection::default(), + focus, + prev, + press, + ); + let mut release = InputSnapshot::idle(FrameInstant::ZERO); + release.pointer = Some(PointerSample::new(center)); + release.buttons_released = PointerButtonMask::just(PointerButton::Primary); + drive_with_snap( + shell, + &document, + &Mode::Idle, + &Selection::default(), + focus, + prev, + release, + ) + }; + let _ = click(&mut shell, &mut focus, &mut prev); + let _ = click(&mut shell, &mut focus, &mut prev); + let mut idle = InputSnapshot::idle(FrameInstant::ZERO); + idle.pointer = Some(PointerSample::new(center)); + let (frame, _) = drive_with_snap( + &mut shell, + &document, + &Mode::Idle, + &Selection::default(), + &mut focus, + &mut prev, + idle, + ); + assert_eq!( + frame.extrude_activated, + Some(extrude_id), + "double-click on extrude row must emit extrude_activated for that extrude", + ); + } + + #[test] + fn f2_with_focused_extrude_row_starts_rename_in_full_shell() { + let (document, extrude_id) = document_with_extrude(); + let mut shell = Shell::new(); + let widget = extrude_widget(&shell.ids, extrude_id); + let mut focus = FocusManager::new(); + let mut prev = HitState::new(); + + let mut warm = InputSnapshot::idle(FrameInstant::ZERO); + let (_, _) = shell_drive( + &mut shell, + &document, + &Mode::Idle, + &Selection::default(), + &mut focus, + &mut prev, + &mut warm, + ); + focus.request_focus(widget); + let mut warm2 = InputSnapshot::idle(FrameInstant::ZERO); + let (_, _) = shell_drive( + &mut shell, + &document, + &Mode::Idle, + &Selection::default(), + &mut focus, + &mut prev, + &mut warm2, + ); + + let mut f2 = InputSnapshot::idle(FrameInstant::ZERO); + f2.keys_pressed.push(bone_ui::input::KeyEvent::new( + bone_ui::input::KeyCode::Named(bone_ui::input::NamedKey::F2), + bone_ui::input::ModifierMask::NONE, + )); + let (_, _) = shell_drive( + &mut shell, + &document, + &Mode::Idle, + &Selection::default(), + &mut focus, + &mut prev, + &mut f2, + ); + assert_eq!( + shell.state.feature_tree.renaming, + Some(widget), + "F2 with extrude row focused must enter rename", + ); + } + fn render_with_locale(size: LayoutSize, locale: Locale) -> ShellFrame { let strings = crate::strings::make_strings(locale); let mut shell = Shell::new(); diff --git a/crates/bone-app/src/sketch_mode.rs b/crates/bone-app/src/sketch_mode.rs index 2b44aac..f0b2320 100644 --- a/crates/bone-app/src/sketch_mode.rs +++ b/crates/bone-app/src/sketch_mode.rs @@ -5,8 +5,8 @@ use bone_document::{ SketchDimension, SketchEntity, }; use bone_types::{ - Length, Point2, Point3, PositiveLength, SketchEntityId, SketchId, SketchPlaneBasis, Tolerance, - UnitVec3, + ExtrudeId, Length, Point2, Point3, PositiveLength, SketchEntityId, SketchId, SketchPlaneBasis, + Tolerance, UnitVec3, }; use bone_ui::hotkey::ActionId; use uom::si::length::millimeter; @@ -56,14 +56,28 @@ impl FeatureTool { #[derive(Copy, Clone, Debug, PartialEq)] pub enum ExtrudeArming { - Profile(ExtrudeFeature), + Profile { + feature: ExtrudeFeature, + target: Option, + }, AwaitingSketch, } impl ExtrudeArming { #[must_use] pub fn profile(sketch: SketchId) -> Self { - Self::Profile(default_extrude_feature(sketch)) + Self::Profile { + feature: default_extrude_feature(sketch), + target: None, + } + } + + #[must_use] + pub fn edit(target: ExtrudeId, feature: ExtrudeFeature) -> Self { + Self::Profile { + feature, + target: Some(target), + } } } @@ -745,9 +759,10 @@ mod tests { #[test] fn extrude_arming_profile_carries_the_sketch() { let id = SketchId::default(); - let ExtrudeArming::Profile(feature) = ExtrudeArming::profile(id) else { + let ExtrudeArming::Profile { feature, target } = ExtrudeArming::profile(id) else { panic!("profile arming holds a feature"); }; assert_eq!(feature.sketch, id); + assert_eq!(target, None); } } diff --git a/crates/bone-document/src/document/feature_tree.rs b/crates/bone-document/src/document/feature_tree.rs index f9ca611..721d724 100644 --- a/crates/bone-document/src/document/feature_tree.rs +++ b/crates/bone-document/src/document/feature_tree.rs @@ -2,7 +2,7 @@ use bone_kernel::ExtrudeFeature; use bone_types::{BodyId, ExtrudeId, FeatureId, SketchId}; use serde::{Deserialize, Serialize}; -use super::{key_from_index, key_index}; +use super::{key_from_index, next_key}; #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum PrincipalPlane { @@ -174,16 +174,7 @@ impl FeatureTree { } pub(crate) fn allocate(&self) -> FeatureId { - let highest = self - .entries - .iter() - .map(|e| key_index(e.id)) - .max() - .unwrap_or(0); - let Some(next) = highest.checked_add(1) else { - panic!("FeatureTree exhausted 32-bit feature id space"); - }; - key_from_index(next) + next_key(self.entries.iter().map(|e| e.id)) } } diff --git a/crates/bone-document/src/document/mod.rs b/crates/bone-document/src/document/mod.rs index 0dd91f1..8b2760e 100644 --- a/crates/bone-document/src/document/mod.rs +++ b/crates/bone-document/src/document/mod.rs @@ -132,6 +132,14 @@ pub enum RenameSketchError { EmptyLabel, } +#[derive(Copy, Clone, Debug, PartialEq, Eq, thiserror::Error)] +pub enum RenameExtrudeError { + #[error("extrude {0:?} not found")] + UnknownExtrude(ExtrudeId), + #[error("extrude label must contain non-whitespace characters")] + EmptyLabel, +} + #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct DocumentParameters { @@ -180,6 +188,8 @@ pub struct DocumentHeader { pub sketches: SketchRegistry, #[serde(skip)] pub extrudes: BTreeMap, + #[serde(skip)] + pub extrude_labels: BTreeMap, } impl DocumentHeader { @@ -194,6 +204,7 @@ impl DocumentHeader { feature_tree: FeatureTree::seeded(), sketches: SketchRegistry::new(), extrudes: BTreeMap::new(), + extrude_labels: BTreeMap::new(), } } @@ -225,14 +236,16 @@ impl SketchFile { pub struct ExtrudeFile { pub schema: SchemaHeader, pub feature: ExtrudeFeature, + pub label: String, } impl ExtrudeFile { #[must_use] - pub fn new(feature: ExtrudeFeature) -> Self { + pub fn new(feature: ExtrudeFeature, label: String) -> Self { Self { schema: SchemaHeader::bone_document(), feature, + label, } } } @@ -369,13 +382,49 @@ impl Document { pub fn insert_extrude(&mut self, id: ExtrudeId, feature: ExtrudeFeature) { self.header.feature_tree.push_extrude(id, &feature); self.header.extrudes.insert(id, feature); + self.header + .extrude_labels + .entry(id) + .or_insert_with(|| format!("Extrude{}", key_index(id))); + } + + #[must_use] + pub fn commit_extrude(&mut self, feature: ExtrudeFeature) -> ExtrudeId { + let id = next_key(self.header.extrudes.keys().copied()); + self.insert_extrude(id, feature); + id } pub fn remove_extrude(&mut self, id: ExtrudeId) -> Option { self.header.feature_tree.remove_extrude(id); + self.header.extrude_labels.remove(&id); self.header.extrudes.remove(&id) } + #[must_use] + pub fn extrude(&self, id: ExtrudeId) -> Option<&ExtrudeFeature> { + self.header.extrudes.get(&id) + } + + #[must_use] + pub fn extrude_label(&self, id: ExtrudeId) -> Option<&str> { + self.header.extrude_labels.get(&id).map(String::as_str) + } + + pub fn rename_extrude(&mut self, id: ExtrudeId, label: &str) -> Result<(), RenameExtrudeError> { + let trimmed = label.trim(); + if trimmed.is_empty() { + return Err(RenameExtrudeError::EmptyLabel); + } + let slot = self + .header + .extrude_labels + .get_mut(&id) + .ok_or(RenameExtrudeError::UnknownExtrude(id))?; + trimmed.clone_into(slot); + Ok(()) + } + fn extrudes_of_sketch(&self, sketch: SketchId) -> impl Iterator + '_ { self.header .extrudes @@ -425,7 +474,7 @@ impl Document { build: impl FnOnce(FeatureId) -> Result, ) -> Result<(FeatureId, BodyId), E> { let feature = self.header.feature_tree.allocate(); - let body = self.next_body_id(); + let body = next_key(self.bodies.keys().copied()); let solid = build(feature)?; self.header.feature_tree.push_imported_body(feature, body); self.bodies.insert(body, ImportedSolid::new(solid)); @@ -451,20 +500,6 @@ impl Document { pub fn imported_bodies(&self) -> impl Iterator + '_ { self.bodies.iter().map(|(id, body)| (*id, body.solid())) } - - fn next_body_id(&self) -> BodyId { - let highest = self - .bodies - .keys() - .copied() - .map(key_index) - .max() - .unwrap_or(0); - let Some(next) = highest.checked_add(1) else { - panic!("document exhausted 32-bit body id space"); - }; - key_from_index(next) - } } pub(crate) fn key_index(id: K) -> u32 { @@ -478,6 +513,17 @@ pub(crate) fn key_from_index(idx: u32) -> K { K::from(KeyData::from_ffi((1u64 << 32) | u64::from(idx))) } +pub(crate) fn next_key(existing: impl Iterator) -> K { + let highest = existing.map(key_index).max().unwrap_or(0); + let Some(next) = highest.checked_add(1) else { + panic!( + "exhausted 32-bit key space for {}", + core::any::type_name::() + ); + }; + key_from_index(next) +} + fn id_stem(id: K) -> String { format!("{:016x}", id.data().as_ffi()) } @@ -505,7 +551,7 @@ pub fn body_labels_filename(id: BodyId) -> String { #[cfg(test)] mod tests { use super::feature_tree::sample_blind_extrude; - use super::{Document, RenameSketchError, Sketch, SketchId}; + use super::{Document, RenameExtrudeError, RenameSketchError, Sketch, SketchId}; use bone_types::{DocumentId, ExtrudeId, Point3, SketchPlaneBasis, Tolerance, UnitVec3}; fn xy_basis() -> SketchPlaneBasis { @@ -632,4 +678,75 @@ mod tests { }] ); } + + #[test] + fn commit_extrude_assigns_incrementing_default_labels() { + let (mut document, sketch) = doc_with_sketch(); + let first = document.commit_extrude(sample_blind_extrude(sketch)); + let second = document.commit_extrude(sample_blind_extrude(sketch)); + assert_ne!(first, second); + assert_eq!(document.extrude_label(first), Some("Extrude1")); + assert_eq!(document.extrude_label(second), Some("Extrude2")); + } + + #[test] + fn rename_extrude_updates_and_trims_label() { + let (mut document, sketch) = doc_with_sketch(); + let id = document.commit_extrude(sample_blind_extrude(sketch)); + let Ok(()) = document.rename_extrude(id, " Boss \n") else { + panic!("rename should trim and accept"); + }; + assert_eq!(document.extrude_label(id), Some("Boss")); + } + + #[test] + fn rename_extrude_rejects_empty_and_unknown() { + let (mut document, sketch) = doc_with_sketch(); + let id = document.commit_extrude(sample_blind_extrude(sketch)); + assert_eq!( + document.rename_extrude(id, " "), + Err(RenameExtrudeError::EmptyLabel) + ); + assert_eq!(document.extrude_label(id), Some("Extrude1")); + let stranger = ExtrudeId::default(); + assert_eq!( + document.rename_extrude(stranger, "Boss"), + Err(RenameExtrudeError::UnknownExtrude(stranger)) + ); + } + + #[test] + fn remove_extrude_drops_label() { + let (mut document, sketch) = doc_with_sketch(); + let id = document.commit_extrude(sample_blind_extrude(sketch)); + document.remove_extrude(id); + assert_eq!(document.extrude_label(id), None); + } + + #[test] + fn commit_after_removing_earlier_extrude_keeps_labels_unique() { + let (mut document, sketch) = doc_with_sketch(); + let first = document.commit_extrude(sample_blind_extrude(sketch)); + let second = document.commit_extrude(sample_blind_extrude(sketch)); + document.remove_extrude(first); + let third = document.commit_extrude(sample_blind_extrude(sketch)); + assert_eq!(document.extrude_label(second), Some("Extrude2")); + assert_eq!(document.extrude_label(third), Some("Extrude3")); + assert_ne!( + document.extrude_label(second), + document.extrude_label(third), + "default labels track the id, so remove + re-commit cannot collide", + ); + } + + #[test] + fn reinsert_existing_extrude_preserves_renamed_label() { + let (mut document, sketch) = doc_with_sketch(); + let id = document.commit_extrude(sample_blind_extrude(sketch)); + let Ok(()) = document.rename_extrude(id, "Boss") else { + panic!("rename accepts"); + }; + document.insert_extrude(id, sample_blind_extrude(sketch)); + assert_eq!(document.extrude_label(id), Some("Boss")); + } } diff --git a/crates/bone-document/src/io/folder.rs b/crates/bone-document/src/io/folder.rs index 7aaa3d7..02d1109 100644 --- a/crates/bone-document/src/io/folder.rs +++ b/crates/bone-document/src/io/folder.rs @@ -239,7 +239,8 @@ pub fn save(document: &Document, folder: &DocumentFolder) -> Result<(), FolderEr .filter(|(id, _)| tree_extrudes.contains(*id)) .try_for_each(|(id, feature)| -> Result<(), FolderError> { let path = folder.extrude_path(*id); - let ron = to_ron(&path, &ExtrudeFile::new(*feature))?; + let label = document.extrude_label(*id).unwrap_or_default().to_owned(); + let ron = to_ron(&path, &ExtrudeFile::new(*feature, label))?; write_if_different(&path, &ron) })?; @@ -335,8 +336,9 @@ pub fn load(folder: &DocumentFolder) -> Result { let header_text = read_to_string(&header_path)?; let mut header: DocumentHeader = from_ron(&header_path, &header_text)?; check_schema(&header.schema)?; - let extrudes = read_extrudes(folder, &header)?; + let (extrudes, extrude_labels) = read_extrudes(folder, &header)?; header.extrudes = extrudes; + header.extrude_labels = extrude_labels; validate_header(&header)?; let sketches = @@ -420,13 +422,18 @@ fn missing_body(error: FolderError, id: BodyId) -> FolderError { } } +type ExtrudeData = ( + BTreeMap, + BTreeMap, +); + fn read_extrudes( folder: &DocumentFolder, header: &DocumentHeader, -) -> Result, FolderError> { - tree_extrude_ids(header) - .into_iter() - .try_fold(BTreeMap::new(), |mut acc, id| { +) -> Result { + tree_extrude_ids(header).into_iter().try_fold( + (BTreeMap::new(), BTreeMap::new()), + |(mut features, mut labels), id| { let path = folder.extrude_path(id); let text = read_to_string(&path).map_err(|e| match e.into_kind() { FolderErrorKind::Io { source, .. } if source.kind() == io::ErrorKind::NotFound => { @@ -436,9 +443,11 @@ fn read_extrudes( })?; let file: ExtrudeFile = from_ron(&path, &text)?; check_schema(&file.schema)?; - acc.insert(id, file.feature); - Ok::<_, FolderError>(acc) - }) + features.insert(id, file.feature); + labels.insert(id, file.label); + Ok::<_, FolderError>((features, labels)) + }, + ) } fn validate_header(header: &DocumentHeader) -> Result<(), FolderError> { diff --git a/crates/bone-document/src/lib.rs b/crates/bone-document/src/lib.rs index 981fe8c..63c4c20 100644 --- a/crates/bone-document/src/lib.rs +++ b/crates/bone-document/src/lib.rs @@ -11,8 +11,8 @@ pub use bone_kernel::{ }; pub use document::{ Document, DocumentHeader, DocumentParameters, ExtrudeFile, FeatureEdge, FeatureNode, - FeatureTree, ImportedSolid, PrincipalPlane, RenameSketchError, SketchFile, SketchRegistry, - SketchRegistryEntry, UnitsPreference, extrude_filename, sketch_filename, + FeatureTree, ImportedSolid, PrincipalPlane, RenameExtrudeError, RenameSketchError, SketchFile, + SketchRegistry, SketchRegistryEntry, UnitsPreference, extrude_filename, sketch_filename, }; pub use evaluator::{ EvaluatedExtrude, EvaluatedSketch, ExtrudeError, FeatureCache, evaluate_extrude, diff --git a/crates/bone-document/tests/folder_roundtrip.rs b/crates/bone-document/tests/folder_roundtrip.rs index 9cfb809..ba6ec2d 100644 --- a/crates/bone-document/tests/folder_roundtrip.rs +++ b/crates/bone-document/tests/folder_roundtrip.rs @@ -229,7 +229,7 @@ fn load_refuses_unknown_schema_major() { else { panic!("expected UnsupportedMajor"); }; - assert_eq!(found, SchemaVersion::new(9999, 1)); + assert_eq!(found, SchemaVersion::new(9999, 2)); assert_eq!( supported, SchemaVersion::new( @@ -592,6 +592,26 @@ fn extrude_roundtrips_through_folder() { assert_eq!(loaded.feature_tree().edges().len(), 1); } +#[test] +fn renamed_extrude_label_survives_folder_roundtrip() { + let dir = ok_dir(); + let folder = DocumentFolder::new(dir.path().join("labeled.bone")); + let mut doc = Document::new(document_id(1), "labeled".to_owned()); + doc.insert_sketch(sketch_id(1), "S".to_owned(), rectangle()); + doc.insert_extrude(extrude_id(1), blind_extrude(sketch_id(1))); + let Ok(()) = doc.rename_extrude(extrude_id(1), "Boss") else { + panic!("rename accepts a non-empty label"); + }; + assert_save(&doc, &folder); + + let loaded = assert_load(&folder); + assert_eq!( + loaded.extrude_label(extrude_id(1)), + Some("Boss"), + "the file's stored label is read back, not recomputed from the id", + ); +} + #[test] fn two_extrudes_keep_canonical_edge_order_through_folder() { let dir = ok_dir(); diff --git a/crates/bone-document/tests/folder_snapshots.rs b/crates/bone-document/tests/folder_snapshots.rs index 7ec9025..7888bc9 100644 --- a/crates/bone-document/tests/folder_snapshots.rs +++ b/crates/bone-document/tests/folder_snapshots.rs @@ -203,16 +203,19 @@ fn extrude_file_ron_surface() { let Ok(depth) = PositiveLength::new(mm(10.0)) else { panic!("positive depth"); }; - let file = ExtrudeFile::new(ExtrudeFeature { - sketch: sketch_id(7), - direction: ExtrudeDirection::Normal { - sense: ExtrudeSense::Forward, + let file = ExtrudeFile::new( + ExtrudeFeature { + sketch: sketch_id(7), + direction: ExtrudeDirection::Normal { + sense: ExtrudeSense::Forward, + }, + end_condition: ExtrudeEndCondition::Blind { depth }, + draft: None, + thin_wall: None, + merge_result: MergeResult::Merge, }, - end_condition: ExtrudeEndCondition::Blind { depth }, - draft: None, - thin_wall: None, - merge_result: MergeResult::Merge, - }); + "Extrude1".to_owned(), + ); let ron = assert_ron(&file); insta::assert_snapshot!("extrude_file", ron); } diff --git a/crates/bone-document/tests/snapshots/folder_snapshots__document_header.snap b/crates/bone-document/tests/snapshots/folder_snapshots__document_header.snap index f17551a..d07b466 100644 --- a/crates/bone-document/tests/snapshots/folder_snapshots__document_header.snap +++ b/crates/bone-document/tests/snapshots/folder_snapshots__document_header.snap @@ -10,7 +10,7 @@ DocumentHeader( name: "bone-document", version: SchemaVersion( major: 1, - minor: 1, + minor: 2, ), ), id: SerKey( diff --git a/crates/bone-document/tests/snapshots/folder_snapshots__extrude_file.snap b/crates/bone-document/tests/snapshots/folder_snapshots__extrude_file.snap index 3b8b830..31bd7aa 100644 --- a/crates/bone-document/tests/snapshots/folder_snapshots__extrude_file.snap +++ b/crates/bone-document/tests/snapshots/folder_snapshots__extrude_file.snap @@ -10,7 +10,7 @@ ExtrudeFile( name: "bone-document", version: SchemaVersion( major: 1, - minor: 1, + minor: 2, ), ), feature: ExtrudeFeature( @@ -28,4 +28,5 @@ ExtrudeFile( thin_wall: None, merge_result: Merge, ), + label: "Extrude1", ) diff --git a/crates/bone-document/tests/snapshots/folder_snapshots__sketch_file.snap b/crates/bone-document/tests/snapshots/folder_snapshots__sketch_file.snap index e9e4b47..bf30426 100644 --- a/crates/bone-document/tests/snapshots/folder_snapshots__sketch_file.snap +++ b/crates/bone-document/tests/snapshots/folder_snapshots__sketch_file.snap @@ -10,7 +10,7 @@ SketchFile( name: "bone-document", version: SchemaVersion( major: 1, - minor: 1, + minor: 2, ), ), sketch: Sketch( diff --git a/crates/bone-render/src/camera3.rs b/crates/bone-render/src/camera3.rs index 52b831c..54921e3 100644 --- a/crates/bone-render/src/camera3.rs +++ b/crates/bone-render/src/camera3.rs @@ -150,6 +150,25 @@ pub fn arcball_rotation( )) } +pub fn orbit_yaw(camera: Camera3, angle: Angle) -> Result { + orbit_about_point(camera, camera.target(), AxisAngle::new(camera.up(), angle)) +} + +pub fn orbit_pitch(camera: Camera3, angle: Angle) -> Result { + let right = screen_right(camera)?; + orbit_about_point(camera, camera.target(), AxisAngle::new(right, angle)) +} + +pub fn roll_by(camera: Camera3, angle: Angle) -> Result { + let forward = view_direction(camera)?; + Camera3::new( + camera.eye(), + camera.target(), + camera.up().rotated(AxisAngle::new(forward, angle)), + camera.projection(), + ) +} + pub fn roll_about_view( camera: Camera3, extent: ViewportExtent, @@ -172,6 +191,11 @@ pub fn roll_about_view( ) } +pub fn frame_current(camera: Camera3, aabb: Aabb3, extent: ViewportExtent) -> Result { + let to_eye = (camera.eye() - camera.target()).try_normalize(RAY_TOLERANCE)?; + frame_along(aabb, extent, to_eye, camera.up()) +} + pub fn frame_isometric(aabb: Aabb3, extent: ViewportExtent) -> Result { let direction = UnitVec3::try_from_components(1.0, 1.0, 1.0, RAY_TOLERANCE)?; frame_along(aabb, extent, direction, UnitVec3::z_axis()) @@ -332,6 +356,18 @@ fn view_direction(camera: Camera3) -> Result { (camera.target() - camera.eye()).try_normalize(RAY_TOLERANCE) } +fn screen_right(camera: Camera3) -> Result { + let (fx, fy, fz) = view_direction(camera)?.components(); + let (ux, uy, uz) = camera.up().components(); + let right = NVec3::new(fx, fy, fz).cross(&NVec3::new(ux, uy, uz)); + let norm = right.norm(); + if norm < ARCBALL_MIN_AXIS { + return Err(TypesError::ZeroLengthAxis); + } + let unit = right / norm; + Ok(UnitVec3::new_unchecked(unit.x, unit.y, unit.z)) +} + fn arcball_vector( camera: Camera3, extent: ViewportExtent, @@ -717,6 +753,80 @@ mod tests { ); } + #[test] + fn orbit_yaw_holds_the_target_and_swings_the_eye() { + let Ok(yawed) = orbit_yaw(ortho_camera(), Angle::new::(30.0)) else { + panic!("a yaw rotates the camera"); + }; + assert!( + close(yawed.target(), ortho_camera().target(), 1e-9), + "a yaw pivots on the target" + ); + assert_ne!( + yawed.eye(), + ortho_camera().eye(), + "a yaw swings the eye about the up axis" + ); + } + + #[test] + fn orbit_pitch_holds_the_target_and_swings_the_eye() { + let Ok(pitched) = orbit_pitch(ortho_camera(), Angle::new::(20.0)) else { + panic!("a pitch rotates the camera"); + }; + assert!( + close(pitched.target(), ortho_camera().target(), 1e-9), + "a pitch pivots on the target" + ); + assert_ne!( + pitched.eye(), + ortho_camera().eye(), + "a pitch swings the eye about the screen-right axis" + ); + } + + #[test] + fn roll_by_keeps_eye_and_target_but_reorients_up() { + let Ok(rolled) = roll_by(ortho_camera(), Angle::new::(25.0)) else { + panic!("a roll rotates the camera"); + }; + assert!( + close(rolled.eye(), ortho_camera().eye(), 1e-9) + && close(rolled.target(), ortho_camera().target(), 1e-9), + "a roll keeps the eye and target fixed" + ); + assert_ne!( + rolled.up(), + ortho_camera().up(), + "a roll reorients the up vector" + ); + } + + #[test] + fn frame_current_keeps_the_view_direction_and_centers_the_box() { + let cube = Aabb3::from_corners( + Point3::from_mm(0.0, 0.0, 0.0), + Point3::from_mm(2.0, 2.0, 2.0), + ); + let before = ortho_camera(); + let Ok(framed) = frame_current(before, cube, extent()) else { + panic!("a non-degenerate box frames"); + }; + assert!( + close(focal(framed, center()), cube.center(), 1e-6), + "fit looks at the box center" + ); + let (Ok(before_dir), Ok(after_dir)) = (view_direction(before), view_direction(framed)) + else { + panic!("both cameras have a view direction"); + }; + let dot = before_dir.dot(after_dir); + assert!( + (dot - 1.0).abs() < 1e-6, + "fit reframes without changing the view direction: {dot}" + ); + } + #[test] fn isometric_frames_unit_cube_centered() { let cube = Aabb3::from_corners( diff --git a/crates/bone-render/src/lib.rs b/crates/bone-render/src/lib.rs index 1a319c7..6ad998b 100644 --- a/crates/bone-render/src/lib.rs +++ b/crates/bone-render/src/lib.rs @@ -13,9 +13,9 @@ pub mod tween; pub use camera::{Camera2, GridSpacing, PixelsPerMm, ViewportExtent, ViewportPx, ViewportRegion}; pub use camera3::{ - ViewportPoint, arcball_rotation, clip_from_world, frame_isometric, frame_standard_view, - orbit_about_pixel, orbit_about_point, pan_pixels, roll_about_view, world_from_clip, - world_on_focal_plane, world_ray, zoom_about_pixel, + ViewportPoint, arcball_rotation, clip_from_world, frame_current, frame_isometric, + frame_standard_view, orbit_about_pixel, orbit_about_point, orbit_pitch, orbit_yaw, pan_pixels, + roll_about_view, roll_by, world_from_clip, world_on_focal_plane, world_ray, zoom_about_pixel, }; pub use diff::{PixelDiff, PixelDiffError, PixelDiffReport, PixelDiffThreshold, PixelMismatch}; pub use gpu::{BackendTag, Capabilities, Gpu, OffscreenContext}; diff --git a/crates/bone-render/src/navigate.rs b/crates/bone-render/src/navigate.rs index 1bd5d36..bac9f45 100644 --- a/crates/bone-render/src/navigate.rs +++ b/crates/bone-render/src/navigate.rs @@ -1,25 +1,40 @@ -use bone_types::{AxisAngle, Camera3, OrbitState, Result}; +use bone_types::{AxisAngle, Camera3, OrbitState, Result, ZoomFactor}; use crate::camera::ViewportExtent; use crate::camera3::{ ViewportPoint, arcball_rotation, orbit_about_point, pan_pixels, roll_about_view, + zoom_about_pixel, }; +const ZOOM_DRAG_PER_PIXEL: f64 = 1.0075; + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct DragModifiers { + ctrl: bool, shift: bool, alt: bool, } impl DragModifiers { pub const NONE: Self = Self { + ctrl: false, shift: false, alt: false, }; + #[must_use] + pub const fn with_ctrl(self) -> Self { + Self { + ctrl: true, + shift: self.shift, + alt: self.alt, + } + } + #[must_use] pub const fn with_shift(self) -> Self { Self { + ctrl: self.ctrl, shift: true, alt: self.alt, } @@ -28,6 +43,7 @@ impl DragModifiers { #[must_use] pub const fn with_alt(self) -> Self { Self { + ctrl: self.ctrl, shift: self.shift, alt: true, } @@ -35,8 +51,10 @@ impl DragModifiers { #[must_use] pub const fn gesture(self) -> NavGesture { - if self.shift { + if self.ctrl { NavGesture::Pan + } else if self.shift { + NavGesture::Zoom } else if self.alt { NavGesture::Roll } else { @@ -50,6 +68,7 @@ pub enum NavGesture { Orbit, Pan, Roll, + Zoom, } #[derive(Copy, Clone, Debug, PartialEq)] @@ -104,14 +123,13 @@ impl ViewportNavigator { return Ok(camera); }; let next = match drag.gesture { - NavGesture::Orbit => { - let delta = arcball_rotation(camera, extent, cursor, drag.last)?; - let oriented = orbit_about_point(camera, camera.target(), delta)?; - self.orbit = self.orbit.rotated(delta); - oriented - } + NavGesture::Orbit => self.orbit_step(camera, extent, cursor, drag.last)?, NavGesture::Pan => pan_pixels(camera, extent, drag.last, cursor)?, NavGesture::Roll => roll_about_view(camera, extent, cursor, drag.last)?, + NavGesture::Zoom => { + let factor = ZoomFactor::new(ZOOM_DRAG_PER_PIXEL.powf(drag.last.y() - cursor.y()))?; + zoom_about_pixel(camera, extent, cursor, factor)? + } }; self.drag = Some(Drag { gesture: drag.gesture, @@ -119,6 +137,33 @@ impl ViewportNavigator { }); Ok(next) } + + pub fn orbit_pixels( + &mut self, + camera: Camera3, + extent: ViewportExtent, + dx: f64, + dy: f64, + ) -> Result { + let cx = f64::from(extent.width().value()) * 0.5; + let cy = f64::from(extent.height().value()) * 0.5; + let from = ViewportPoint::new(cx, cy)?; + let to = ViewportPoint::new(cx + dx, cy + dy)?; + self.orbit_step(camera, extent, to, from) + } + + fn orbit_step( + &mut self, + camera: Camera3, + extent: ViewportExtent, + from: ViewportPoint, + to: ViewportPoint, + ) -> Result { + let delta = arcball_rotation(camera, extent, from, to)?; + let oriented = orbit_about_point(camera, camera.target(), delta)?; + self.orbit = self.orbit.rotated(delta); + Ok(oriented) + } } impl Default for ViewportNavigator { @@ -170,19 +215,25 @@ mod tests { #[test] fn modifiers_select_the_gesture() { assert_eq!(DragModifiers::NONE.gesture(), NavGesture::Orbit); - assert_eq!(DragModifiers::NONE.with_shift().gesture(), NavGesture::Pan); + assert_eq!(DragModifiers::NONE.with_ctrl().gesture(), NavGesture::Pan); + assert_eq!(DragModifiers::NONE.with_shift().gesture(), NavGesture::Zoom); assert_eq!(DragModifiers::NONE.with_alt().gesture(), NavGesture::Roll); } #[test] - fn shift_takes_precedence_over_alt() { + fn ctrl_outranks_shift_outranks_alt() { assert_eq!( - DragModifiers::NONE.with_shift().with_alt().gesture(), + DragModifiers::NONE.with_ctrl().with_shift().gesture(), NavGesture::Pan, - "holding both shift and alt resolves to pan, never roll" + "ctrl pans even when shift is also held" + ); + assert_eq!( + DragModifiers::NONE.with_shift().with_alt().gesture(), + NavGesture::Zoom, + "shift zooms even when alt is also held" ); assert_eq!( - DragModifiers::NONE.with_alt().with_shift().gesture(), + DragModifiers::NONE.with_alt().with_ctrl().gesture(), NavGesture::Pan, "the precedence is independent of the order the modifiers were set" ); @@ -272,6 +323,45 @@ mod tests { assert_ne!(rolled.up(), camera().up(), "a roll reorients the up vector"); } + #[test] + fn scroll_orbit_holds_the_target_and_accumulates_rotation() { + let mut nav = ViewportNavigator::new(); + let Ok(orbited) = nav.orbit_pixels(camera(), extent(), 60.0, 0.0) else { + panic!("a scroll delta orbits the camera"); + }; + assert!( + close(orbited.target(), camera().target(), 1e-9), + "a scroll orbit pivots on the target" + ); + assert_ne!( + orbited.eye(), + camera().eye(), + "a horizontal scroll orbit moves the eye" + ); + let rotated = nav.orbit_rotation().angle().get::(); + assert!( + rotated.abs() > 1e-3, + "a scroll orbit accumulates rotation like a drag: {rotated}" + ); + } + + #[test] + fn zoom_drag_down_enlarges_the_orthographic_view() { + use bone_types::ProjectionKind; + let mut nav = ViewportNavigator::new(); + nav.begin_drag(NavGesture::Zoom, vp(128.0, 100.0)); + let Ok(zoomed) = nav.drag_to(vp(128.0, 160.0), camera(), extent()) else { + panic!("a zoom drag transforms the camera"); + }; + let ProjectionKind::Orthographic { half_height } = zoomed.projection().kind() else { + panic!("the seed camera is orthographic"); + }; + assert!( + half_height.get::() > 2.0, + "dragging the zoom gesture downward zooms out, growing the half height" + ); + } + #[test] fn end_drag_clears_the_active_gesture() { let mut nav = ViewportNavigator::new(); diff --git a/crates/bone-types/src/lib.rs b/crates/bone-types/src/lib.rs index 17b1922..8ba49f7 100644 --- a/crates/bone-types/src/lib.rs +++ b/crates/bone-types/src/lib.rs @@ -112,17 +112,23 @@ slotmap::new_key_type! { pub struct BrepLoopId; } -impl SketchId { - /// Encodes this id as an opaque `u64`, stable for the same slotmap slot+version - /// within a single process. Use only for deterministic widget-key derivation; the - /// representation is not portable across builds or persisted artifacts. - #[must_use] - pub fn as_u64(self) -> u64 { - use slotmap::Key; - self.data().as_ffi() - } +macro_rules! impl_as_u64 { + ($($key:ty),+ $(,)?) => { + $(impl $key { + /// Encodes this id as an opaque `u64`, stable for the same slotmap slot+version + /// within a single process. Use only for deterministic widget-key derivation; the + /// representation is not portable across builds or persisted artifacts. + #[must_use] + pub fn as_u64(self) -> u64 { + use slotmap::Key; + self.data().as_ffi() + } + })+ + }; } +impl_as_u64!(SketchId, ExtrudeId); + #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] pub struct Tolerance(f64); diff --git a/crates/bone-types/src/schema.rs b/crates/bone-types/src/schema.rs index 7b275bf..87edc4e 100644 --- a/crates/bone-types/src/schema.rs +++ b/crates/bone-types/src/schema.rs @@ -30,7 +30,7 @@ pub struct SchemaHeader { impl SchemaHeader { pub const BONE_DOCUMENT_NAME: &'static str = "bone-document"; pub const BONE_DOCUMENT_MAJOR: u32 = 1; - pub const BONE_DOCUMENT_MINOR: u32 = 1; + pub const BONE_DOCUMENT_MINOR: u32 = 2; #[must_use] pub fn bone_document() -> Self { diff --git a/crates/bone-ui/src/widgets/tree_view.rs b/crates/bone-ui/src/widgets/tree_view.rs index b6a1bd5..6eda4c8 100644 --- a/crates/bone-ui/src/widgets/tree_view.rs +++ b/crates/bone-ui/src/widgets/tree_view.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use crate::a11y::{AccessNode, Role}; use crate::frame::{FrameCtx, InteractDeclaration}; use crate::hit_test::Sense; -use crate::input::{KeyCode, ModifierMask, NamedKey}; +use crate::input::{KeyCode, ModifierMask, NamedKey, PointerButton}; use crate::layout::{LayoutPos, LayoutPx, LayoutRect, LayoutSize}; use crate::strings::StringKey; use crate::theme::{Color, Step12}; @@ -231,7 +231,13 @@ pub fn show_tree_view(ctx: &mut FrameCtx<'_>, view: TreeView<'_, '_>) -> TreeVie acc }); paint.extend(row_paint); - commit_pending_rename(ctx, &visible, state, renamable); + let pending_row_rect = state.pending_rename.and_then(|pending| { + visible + .iter() + .position(|row| row.id == pending.id) + .map(|idx| row_rect_at(rect, idx, row_height)) + }); + commit_pending_rename(ctx, &visible, state, renamable, pending_row_rect); let rename_committed = if let Some(id) = state.renaming && take_key(ctx.input, &[TakeKey::named(NamedKey::Enter)]).is_some() { @@ -440,6 +446,7 @@ fn commit_pending_rename( visible: &[VisibleRow], state: &mut TreeViewState, renamable: &[WidgetId], + pending_row_rect: Option, ) { let Some(pending) = state.pending_rename else { return; @@ -448,6 +455,14 @@ fn commit_pending_rename( state.pending_rename = None; return; } + let pressed_off_row = ctx.input.buttons_pressed.contains(PointerButton::Primary) + && !pending_row_rect + .zip(ctx.input.pointer.map(|sample| sample.position)) + .is_some_and(|(row, cursor)| row.contains(cursor)); + if pressed_off_row && pending.at != ctx.input.frame { + state.pending_rename = None; + return; + } let Some(row) = visible.iter().find(|r| r.id == pending.id) else { state.pending_rename = None; return; @@ -1323,6 +1338,44 @@ mod tests { assert_eq!(state.rename_buffer.text, "Profile"); } + #[test] + fn primary_press_outside_pending_row_cancels_rename() { + let sketch_id = root_id("sketch"); + let roots = vec![TreeNode::leaf_owned(sketch_id, "Profile".to_owned())]; + let mut state = TreeViewState { + selection: BTreeSet::from([sketch_id]), + ..TreeViewState::default() + }; + let mut focus = FocusManager::new(); + let mut prev = HitState::new(); + let row_pos = LayoutPos::new(LayoutPx::new(80.0), LayoutPx::new(11.0)); + let elsewhere = LayoutPos::new(LayoutPx::new(500.0), LayoutPx::new(500.0)); + let armed = FrameInstant::from_duration(core::time::Duration::from_millis(10)); + let pressed_away = FrameInstant::from_duration(core::time::Duration::from_millis(20)); + let frames = [ + press_at(row_pos, FrameInstant::ZERO), + release_at(row_pos, FrameInstant::ZERO), + idle_at(row_pos, armed), + press_at(elsewhere, pressed_away), + ]; + frames.into_iter().for_each(|mut snap| { + let (_, next) = render_with( + &roots, + &mut state, + &mut focus, + &mut snap, + &prev, + &[sketch_id], + ); + prev = next; + }); + assert_eq!( + state.pending_rename, None, + "pressing outside the pending row cancels the slow-click rename", + ); + assert_eq!(state.renaming, None, "no rename editor opens"); + } + #[test] fn extra_slow_click_on_already_pending_row_does_not_reset_pending_at() { let sketch_id = root_id("sketch"); -- 2.51.2