diff --git a/src/draw/shape/transform.rs b/src/draw/shape/transform.rs index 0b6a5a31..a54a3b53 100644 --- a/src/draw/shape/transform.rs +++ b/src/draw/shape/transform.rs @@ -5,6 +5,43 @@ use crate::util; use super::Shape; impl Shape { + /// Limits movement so every translated integer coordinate stays representable. + pub(crate) fn constrain_translation(&self, dx: i32, dy: i32) -> (i32, i32) { + let constrain = |position: i32, delta: i32| { + let position = i64::from(position); + let clamped = i64::from(delta).clamp( + i64::from(i32::MIN) - position, + i64::from(i32::MAX) - position, + ); + i32::try_from(clamped) + .expect("anchor-constrained movement lies between zero and the requested i32 delta") + }; + + match self { + Shape::Line { x1, y1, x2, y2, .. } | Shape::Arrow { x1, y1, x2, y2, .. } => ( + constrain(*x2, constrain(*x1, dx)), + constrain(*y2, constrain(*y1, dy)), + ), + Shape::Rect { x, y, .. } + | Shape::BlurRect { x, y, .. } + | Shape::Image { x, y, .. } + | Shape::Text { x, y, .. } + | Shape::StepMarker { x, y, .. } + | Shape::StickyNote { x, y, .. } => (constrain(*x, dx), constrain(*y, dy)), + Shape::Ellipse { cx, cy, .. } | Shape::Spotlight { cx, cy, .. } => { + (constrain(*cx, dx), constrain(*cy, dy)) + } + Shape::Polygon { points, .. } | Shape::EraserStroke { points, .. } => { + points.iter().fold((dx, dy), |(dx, dy), &(x, y)| { + (constrain(x, dx), constrain(y, dy)) + }) + } + Shape::Freehand { .. } + | Shape::FreehandPressure { .. } + | Shape::MarkerStroke { .. } => (dx, dy), + } + } + pub(crate) fn translate(&mut self, dx: i32, dy: i32) { match self { Shape::Freehand { points, .. } => { diff --git a/src/input/state/core/selection_actions/translation/bounds.rs b/src/input/state/core/selection_actions/translation/bounds.rs index 58d4a7fa..2bb7cf08 100644 --- a/src/input/state/core/selection_actions/translation/bounds.rs +++ b/src/input/state/core/selection_actions/translation/bounds.rs @@ -77,19 +77,30 @@ impl InputState { } } - fn clamp_axis_delta(position: i32, size: i32, screen: i32, delta: i32) -> i32 { - if screen <= 0 || size <= 0 { + fn clamp_axis_delta(position: i32, size: i32, start: i32, extent: i32, delta: i32) -> i32 { + if extent <= 0 || size <= 0 { return delta; } - let end = position.saturating_add(size); - let (min_delta, max_delta) = if size <= screen { - (0i32.saturating_sub(position), screen.saturating_sub(end)) + let position = i64::from(position); + let end = position + i64::from(size); + let start = i64::from(start); + let view_end = start + i64::from(extent); + let (min_delta, max_delta) = if size <= extent { + (start - position, view_end - end) } else { - (screen.saturating_sub(end), 0i32.saturating_sub(position)) + (view_end - end, start - position) }; - delta.clamp(min_delta, max_delta) + // A panned or zoomed view may already clip the selection. Stop further + // outward movement without forcing it back into view against the drag. + // Oversized selections can align past the viewport, but their painted + // bounds must stay within the coordinate range used by shape transforms. + let min_delta = min_delta.min(0).max(i64::from(i32::MIN) - position); + let max_delta = max_delta.max(0).min((i64::from(i32::MAX) - end).max(0)); + let clamped = i64::from(delta).clamp(min_delta, max_delta); + i32::try_from(clamped) + .expect("clamped movement lies between zero and the requested i32 delta") } pub(super) fn clamp_selection_translation( @@ -99,21 +110,32 @@ impl InputState { dy: i32, ) -> Option<(i32, i32)> { let bounds = self.movable_selection_bounds(measurer)?; + let visible = self.visible_canvas_rect(); let (screen_width, screen_height) = self.view.screen_size(); - let screen_width = screen_width.min(i32::MAX as u32) as i32; - let screen_height = screen_height.min(i32::MAX as u32) as i32; - let clamped_dx = if dx == 0 { - 0 + // Shape bounds and pointer deltas are canvas coordinates, so clamp + // against the viewport after applying the board pan and zoom. + let clamped_dx = if dx == 0 || screen_width == 0 { + dx } else { - Self::clamp_axis_delta(bounds.x, bounds.width, screen_width, dx) + Self::clamp_axis_delta(bounds.x, bounds.width, visible.x, visible.width, dx) }; - let clamped_dy = if dy == 0 { - 0 + let clamped_dy = if dy == 0 || screen_height == 0 { + dy } else { - Self::clamp_axis_delta(bounds.y, bounds.height, screen_height, dy) + Self::clamp_axis_delta(bounds.y, bounds.height, visible.y, visible.height, dy) }; - Some((clamped_dx, clamped_dy)) + let frame = self.boards.active_frame(); + let translation = self + .selected_shape_ids() + .iter() + .filter_map(|id| frame.shape(*id)) + .filter(|drawn| !drawn.locked) + .fold((clamped_dx, clamped_dy), |(dx, dy), drawn| { + drawn.shape.constrain_translation(dx, dy) + }); + + Some(translation) } } diff --git a/src/input/state/core/selection_actions/translation/mod.rs b/src/input/state/core/selection_actions/translation/mod.rs index 92ae6fb7..50f2ddec 100644 --- a/src/input/state/core/selection_actions/translation/mod.rs +++ b/src/input/state/core/selection_actions/translation/mod.rs @@ -77,17 +77,20 @@ impl InputState { let Some(bounds) = self.movable_selection_bounds(measurer) else { return false; }; - let screen_width = self.view.screen_width().min(i32::MAX as u32) as i32; - if screen_width <= 0 { + if self.view.screen_width() == 0 { return false; } + let visible = self.visible_canvas_rect(); let target_x = if to_start { - 0 + visible.x } else { - screen_width - bounds.width + visible + .x + .saturating_add(visible.width) + .saturating_sub(bounds.width) }; - let dx = target_x - bounds.x; + let dx = target_x.saturating_sub(bounds.x); if dx == 0 { return false; } @@ -102,17 +105,20 @@ impl InputState { let Some(bounds) = self.movable_selection_bounds(measurer) else { return false; }; - let screen_height = self.view.screen_height().min(i32::MAX as u32) as i32; - if screen_height <= 0 { + if self.view.screen_height() == 0 { return false; } + let visible = self.visible_canvas_rect(); let target_y = if to_start { - 0 + visible.y } else { - screen_height - bounds.height + visible + .y + .saturating_add(visible.height) + .saturating_sub(bounds.height) }; - let dy = target_y - bounds.y; + let dy = target_y.saturating_sub(bounds.y); if dy == 0 { return false; } diff --git a/src/input/state/tests/selection/mod.rs b/src/input/state/tests/selection/mod.rs index e6d24697..eee5c339 100644 --- a/src/input/state/tests/selection/mod.rs +++ b/src/input/state/tests/selection/mod.rs @@ -7,4 +7,5 @@ mod deletion; mod double_click; mod duplicate; mod mid_drag_actions; +mod movement; mod reorder; diff --git a/src/input/state/tests/selection/movement.rs b/src/input/state/tests/selection/movement.rs new file mode 100644 index 00000000..08ebbd07 --- /dev/null +++ b/src/input/state/tests/selection/movement.rs @@ -0,0 +1,462 @@ +use super::*; +use crate::draw::TextMeasurer; +use crate::input::BOARD_ID_WHITEBOARD; +use crate::input::state::InputTextResources; +use crate::ui_text::UiTextEngine; + +#[test] +fn dragging_strokes_follows_the_pointer_in_a_panned_or_zoomed_view() { + let measurer = TextMeasurer::default(); + let ui_engine = UiTextEngine::default(); + let resources = InputTextResources { + measurer: &measurer, + ui_engine: &ui_engine, + }; + + for (board_offset, zoom, origin) in [ + ((0, 0), None, (0, 0)), + ((1600, 1200), None, (1600, 1200)), + ((-1600, -1200), None, (-1600, -1200)), + ((0, 0), Some((2.0, (100.0, 50.0))), (100, 50)), + ((1600, 1200), Some((2.0, (100.0, 50.0))), (1700, 1250)), + ] { + let mut state = create_test_input_state(); + state.update_screen_dimensions(800, 600); + state.switch_board(BOARD_ID_WHITEBOARD); + state + .boards + .active_frame_mut() + .set_view_offset(board_offset.0, board_offset.1); + if let Some((scale, offset)) = zoom { + state.set_zoom_status(true, false, scale, offset); + } + state.set_tool_override(Some(Tool::Select)); + let (x, y) = origin; + let original = Shape::Freehand { + points: vec![ + (f64::from(x + 100), f64::from(y + 100)), + (f64::from(x + 120), f64::from(y + 130)), + (f64::from(x + 140), f64::from(y + 160)), + ], + color: state.style.current_color, + thick: 4.0, + }; + let stroke = state.boards.active_frame_mut().add_shape(original.clone()); + + state.on_mouse_press(MouseButton::Left, x + 80, y + 80); + state.on_mouse_motion(x + 160, y + 180); + state.on_mouse_release(MouseButton::Left, x + 160, y + 180); + assert_eq!(state.selected_shape_ids(), &[stroke], "view {origin:?}"); + + state.on_mouse_press(MouseButton::Left, x + 120, y + 130); + state.on_mouse_motion(x + 160, y + 160); + state.on_mouse_motion(x + 170, y + 170); + state.on_mouse_release(MouseButton::Left, x + 170, y + 170); + + let expected = Shape::Freehand { + points: vec![ + (f64::from(x + 150), f64::from(y + 140)), + (f64::from(x + 170), f64::from(y + 170)), + (f64::from(x + 190), f64::from(y + 200)), + ], + color: state.style.current_color, + thick: 4.0, + }; + assert_eq!( + state + .boards + .active_frame() + .shape(stroke) + .expect("stroke survives drag") + .shape, + expected, + "the stroke must follow the pointer in view {origin:?}" + ); + assert_eq!( + state.boards.active_frame().undo_stack_len(), + 1, + "one drag must create one undo entry in view {origin:?}" + ); + + state.handle_action_with_resources(resources, Action::Undo); + assert_eq!( + state + .boards + .active_frame() + .shape(stroke) + .expect("undo retains stroke") + .shape, + original, + "undo must restore the original stroke in view {origin:?}" + ); + state.handle_action_with_resources(resources, Action::Redo); + assert_eq!( + state + .boards + .active_frame() + .shape(stroke) + .expect("redo retains stroke") + .shape, + expected, + "redo must restore the moved stroke in view {origin:?}" + ); + } +} + +#[test] +fn selection_edge_actions_and_nudges_use_the_visible_canvas() { + let measurer = TextMeasurer::default(); + let ui_engine = UiTextEngine::default(); + let resources = InputTextResources { + measurer: &measurer, + ui_engine: &ui_engine, + }; + + for (offset, zoom, viewport) in [ + ((1600, 1200), false, (1600, 1200, 800, 600)), + ((-1600, -1200), false, (-1600, -1200, 800, 600)), + ((1600, 1200), true, (1700, 1250, 400, 300)), + ] { + let mut state = create_test_input_state(); + state.update_screen_dimensions(800, 600); + state.switch_board(BOARD_ID_WHITEBOARD); + state + .boards + .active_frame_mut() + .set_view_offset(offset.0, offset.1); + if zoom { + state.set_zoom_status(true, false, 2.0, (100.0, 50.0)); + } + let (left, top, width, height) = viewport; + let id = state.boards.active_frame_mut().add_shape(Shape::Rect { + x: left + 100, + y: top + 100, + w: 40, + h: 40, + fill: true, + fill_color: None, + color: state.style.current_color, + thick: 4.0, + }); + state.set_selection(vec![id]); + + for (edge, nudge, expected_x, expected_y) in [ + ( + Action::MoveSelectionToStart, + Action::NudgeSelectionLeft, + left, + top + 98, + ), + ( + Action::MoveSelectionToTop, + Action::NudgeSelectionUp, + left, + top, + ), + ( + Action::MoveSelectionToEnd, + Action::NudgeSelectionRight, + left + width - 44, + top, + ), + ( + Action::MoveSelectionToBottom, + Action::NudgeSelectionDown, + left + width - 44, + top + height - 44, + ), + ] { + state.handle_action_with_resources(resources, edge); + state.handle_action_with_resources(resources, nudge); + + let bounds = state + .selection_bounds_with(&measurer) + .expect("selection has bounds"); + assert_eq!( + (bounds.x, bounds.y, bounds.width, bounds.height), + (expected_x, expected_y, 44, 44), + "{edge:?} and {nudge:?} must use viewport {viewport:?}" + ); + } + } +} + +#[test] +fn dragging_a_clipped_selection_does_not_reverse_the_pointer_delta() { + for (offset, zoom, x, width, press_x) in [ + ((0, 0), true, 250, 300, 300), + ((0, 0), true, 50, 1000, 100), + ((1600, 1200), false, 1650, 1000, 1700), + ] { + let mut state = create_test_input_state(); + state.update_screen_dimensions(800, 600); + state.switch_board(BOARD_ID_WHITEBOARD); + state + .boards + .active_frame_mut() + .set_view_offset(offset.0, offset.1); + if zoom { + state.set_zoom_status(true, false, 2.0, (0.0, 0.0)); + } + state.set_tool_override(Some(Tool::Select)); + let y = offset.1 + 100; + let id = state.boards.active_frame_mut().add_shape(Shape::Rect { + x, + y, + w: width, + h: 100, + fill: true, + fill_color: None, + color: state.style.current_color, + thick: 4.0, + }); + state.set_selection(vec![id]); + let before = state.selection_bounds().expect("rectangle has bounds"); + + state.on_mouse_press(MouseButton::Left, press_x, y + 50); + state.on_mouse_motion(press_x + 5, y + 50); + assert_eq!( + state.selection_bounds(), + Some(before), + "an outward drag must stop without snapping a clipped selection" + ); + + state.on_mouse_motion(press_x - 20, y + 50); + state.on_mouse_release(MouseButton::Left, press_x - 20, y + 50); + let after = state.selection_bounds().expect("rectangle survives drag"); + assert_eq!( + (after.x, after.y, after.width, after.height), + (before.x - 20, before.y, before.width, before.height), + "an inward drag must follow the pointer without an initial jump" + ); + } +} + +#[test] +fn nudging_away_from_an_extreme_view_offset_does_not_panic_or_jump() { + let measurer = TextMeasurer::default(); + let ui_engine = UiTextEngine::default(); + let resources = InputTextResources { + measurer: &measurer, + ui_engine: &ui_engine, + }; + let mut state = create_test_input_state(); + state.update_screen_dimensions(800, 600); + state.switch_board(BOARD_ID_WHITEBOARD); + state + .boards + .active_frame_mut() + .set_view_offset(i32::MIN, i32::MIN); + let original = Shape::Rect { + x: 100, + y: 100, + w: 1000, + h: 1000, + fill: true, + fill_color: None, + color: state.style.current_color, + thick: 4.0, + }; + let id = state.boards.active_frame_mut().add_shape(original.clone()); + state.set_selection(vec![id]); + + state.handle_action_with_resources(resources, Action::NudgeSelectionRight); + state.handle_action_with_resources(resources, Action::NudgeSelectionDown); + + assert_eq!( + state + .boards + .active_frame() + .shape(id) + .expect("rectangle remains in frame") + .shape, + original, + "nudging away from the viewport must stop without an overflow or snap" + ); +} + +#[test] +fn translation_on_a_panned_board_preserves_unknown_screen_dimensions() { + let measurer = TextMeasurer::default(); + + for (width, height) in [(0, 0), (0, 600), (800, 0)] { + let mut state = create_test_input_state(); + state.update_screen_dimensions(width, height); + state.switch_board(BOARD_ID_WHITEBOARD); + state.boards.active_frame_mut().set_view_offset(1600, 1200); + let id = state.boards.active_frame_mut().add_shape(Shape::Rect { + x: 1700, + y: 1300, + w: 40, + h: 40, + fill: true, + fill_color: None, + color: state.style.current_color, + thick: 4.0, + }); + state.set_selection(vec![id]); + + assert!( + state.translate_selection_with_undo_with(&measurer, 20, 30), + "translation must remain available with screen size {width}x{height}" + ); + let bounds = state.selection_bounds().expect("rectangle has bounds"); + assert_eq!( + (bounds.x, bounds.y), + (1718, 1328), + "an unknown screen dimension must not become a one-pixel movement limit" + ); + } +} + +#[test] +fn edge_shortcuts_keep_oversized_lines_within_canvas_coordinate_limits() { + let measurer = TextMeasurer::default(); + let ui_engine = UiTextEngine::default(); + let resources = InputTextResources { + measurer: &measurer, + ui_engine: &ui_engine, + }; + + for offset in [i32::MAX, i32::MAX - 800] { + let mut state = create_test_input_state(); + state.update_screen_dimensions(800, 600); + state.switch_board(BOARD_ID_WHITEBOARD); + state + .boards + .active_frame_mut() + .set_view_offset(offset, offset); + let line = state.boards.active_frame_mut().add_shape(Shape::Line { + x1: 0, + y1: 0, + x2: 1000, + y2: 1000, + color: state.style.current_color, + thick: 4.0, + }); + state.set_selection(vec![line]); + + state.handle_action_with_resources(resources, Action::MoveSelectionToStart); + state.handle_action_with_resources(resources, Action::MoveSelectionToTop); + + match state + .boards + .active_frame() + .shape(line) + .expect("line survives edge moves") + .shape + { + Shape::Line { x1, y1, x2, y2, .. } => { + assert_eq!( + (x1, y1), + (i32::MAX - 1002, i32::MAX - 1002), + "edge moves must preserve the line's starting point and length" + ); + assert_eq!( + (x2, y2), + (i32::MAX - 2, i32::MAX - 2), + "edge moves must leave room for the line's stroke padding" + ); + } + ref other => panic!("expected the original line, got {other:?}"), + } + } +} + +#[test] +fn edge_shortcuts_keep_text_and_note_anchors_within_coordinate_limits() { + let measurer = TextMeasurer::default(); + let ui_engine = UiTextEngine::default(); + let resources = InputTextResources { + measurer: &measurer, + ui_engine: &ui_engine, + }; + + for (offset, text, action, anchor, expected, is_note) in [ + ( + i32::MAX, + "'", + Action::MoveSelectionToTop, + (1000, 1000), + (1000, i32::MAX), + false, + ), + ( + i32::MIN, + " x", + Action::MoveSelectionToStart, + (-1000, 1000), + (i32::MIN, 1000), + false, + ), + ( + i32::MIN, + "אבג", + Action::MoveSelectionToStart, + (-1000, 1000), + (i32::MIN, 1000), + true, + ), + ] { + let mut state = create_test_input_state(); + state.update_screen_dimensions(800, 600); + state.switch_board(BOARD_ID_WHITEBOARD); + state + .boards + .active_frame_mut() + .set_view_offset(offset, offset); + let shape = if is_note { + Shape::StickyNote { + x: anchor.0, + y: anchor.1, + text: text.to_string(), + background: state.style.current_color, + size: 32.0, + font_descriptor: state.style.font_descriptor.clone(), + wrap_width: Some(300), + } + } else { + Shape::Text { + x: anchor.0, + y: anchor.1, + text: text.to_string(), + color: state.style.current_color, + size: 32.0, + font_descriptor: state.style.font_descriptor.clone(), + background_enabled: false, + wrap_width: None, + } + }; + let id = state.boards.active_frame_mut().add_shape(shape); + state.set_selection(vec![id]); + let bounds = state.selection_bounds().expect("text has ink bounds"); + match action { + Action::MoveSelectionToStart => assert!( + bounds.x > anchor.0, + "the horizontal fixture must have its anchor left of the painted bounds" + ), + Action::MoveSelectionToTop => assert!( + bounds.y + bounds.height < anchor.1, + "the vertical fixture must have its baseline below the painted bounds" + ), + other => panic!("unexpected edge action {other:?}"), + } + + state.handle_action_with_resources(resources, action); + + match state + .boards + .active_frame() + .shape(id) + .expect("text survives edge move") + .shape + { + Shape::Text { x, y, .. } | Shape::StickyNote { x, y, .. } => assert_eq!( + (x, y), + expected, + "the text anchor must stop at the coordinate boundary" + ), + ref other => panic!("expected the original text shape, got {other:?}"), + } + } +} diff --git a/src/input/state/tests/transform.rs b/src/input/state/tests/transform.rs index 0e729564..be969547 100644 --- a/src/input/state/tests/transform.rs +++ b/src/input/state/tests/transform.rs @@ -374,9 +374,10 @@ fn nudge_selection_clamps_right_and_bottom_edges() { let mut state = create_test_input_state(); state.update_screen_dimensions(100, 100); + // Include the stroke padding inside the view before nudging toward its edge. let shape_id = state.boards.active_frame_mut().add_shape(Shape::Rect { - x: 90, - y: 90, + x: 87, + y: 87, w: 10, h: 10, fill: false,