From 000c45d07364bc2f5676708961f4bfa30463ec5a Mon Sep 17 00:00:00 2001 From: hypercube <0hypercube@gmail.com> Date: Fri, 8 Aug 2025 17:48:26 +0100 Subject: [PATCH] Fix tests & gradient tool --- .../messages/input_mapper/input_mappings.rs | 2 +- .../document/overlays/utility_types_vello.rs | 2 + .../common_functionality/shapes/line_shape.rs | 30 +++++------ .../tool/tool_messages/gradient_tool.rs | 50 ++++++++++++++++--- editor/src/node_graph_executor.rs | 7 +++ editor/src/test_utils.rs | 49 +++++++++++++----- 6 files changed, 100 insertions(+), 40 deletions(-) diff --git a/editor/src/messages/input_mapper/input_mappings.rs b/editor/src/messages/input_mapper/input_mappings.rs index 1b722301e6..906547021f 100644 --- a/editor/src/messages/input_mapper/input_mappings.rs +++ b/editor/src/messages/input_mapper/input_mappings.rs @@ -165,7 +165,7 @@ pub fn input_mappings() -> Mapping { entry!(KeyDown(MouseLeft); action_dispatch=GradientToolMessage::PointerDown), entry!(PointerMove; refresh_keys=[Shift], action_dispatch=GradientToolMessage::PointerMove { constrain_axis: Shift }), entry!(KeyUp(MouseLeft); action_dispatch=GradientToolMessage::PointerUp), - entry!(DoubleClick(MouseButton::Left); action_dispatch=GradientToolMessage::InsertStop), + entry!(DoubleClick(MouseButton::Left); action_dispatch=GradientToolMessage::InsertStopProxy), entry!(KeyDown(Delete); action_dispatch=GradientToolMessage::DeleteStop), entry!(KeyDown(Backspace); action_dispatch=GradientToolMessage::DeleteStop), entry!(KeyDown(MouseRight); action_dispatch=GradientToolMessage::Abort), diff --git a/editor/src/messages/portfolio/document/overlays/utility_types_vello.rs b/editor/src/messages/portfolio/document/overlays/utility_types_vello.rs index 59c5b2657f..32b33fbcb6 100644 --- a/editor/src/messages/portfolio/document/overlays/utility_types_vello.rs +++ b/editor/src/messages/portfolio/document/overlays/utility_types_vello.rs @@ -200,6 +200,7 @@ impl core::hash::Hash for OverlayContext { } impl OverlayContext { + #[cfg(not(test))] pub(super) fn new(size: DVec2, device_pixel_ratio: f64, visibility_settings: OverlaysVisibilitySettings) -> Self { Self { internal: Arc::new(Mutex::new(OverlayContextInternal::new(size, device_pixel_ratio, visibility_settings))), @@ -421,6 +422,7 @@ impl Default for OverlayContextInternal { } impl OverlayContextInternal { + #[cfg(not(test))] pub(super) fn new(size: DVec2, device_pixel_ratio: f64, visibility_settings: OverlaysVisibilitySettings) -> Self { Self { scene: Scene::new(), diff --git a/editor/src/messages/tool/common_functionality/shapes/line_shape.rs b/editor/src/messages/tool/common_functionality/shapes/line_shape.rs index 316c4b66d4..f53ee84eb7 100644 --- a/editor/src/messages/tool/common_functionality/shapes/line_shape.rs +++ b/editor/src/messages/tool/common_functionality/shapes/line_shape.rs @@ -354,33 +354,27 @@ mod test_line_tool { let artboard_id = editor.get_selected_layer().await.expect("Should have selected the artboard"); + let transform = DAffine2::from_angle(45_f64.to_radians()); editor .handle_message(GraphOperationMessage::TransformChange { layer: artboard_id, - transform: DAffine2::from_angle(45_f64.to_radians()), + transform, transform_in: TransformIn::Local, skip_rerender: false, }) .await; - editor.drag_tool(ToolType::Line, 50., 50., 150., 150., ModifierKeys::empty()).await; + let expected_start = DVec2::new(55., 42.); + let expected_end = DVec2::new(124., 142.); + editor + .drag_tool(ToolType::Line, expected_start.x, expected_start.y, expected_end.x, expected_end.y, ModifierKeys::empty()) + .await; let (start_input, end_input) = get_line_node_inputs(&mut editor).await.expect("Line was not created successfully within transformed artboard"); - // The line should still be diagonal with equal change in x and y - let line_vector = end_input - start_input; - // Verifying the line is approximately 100*sqrt(2) units in length (diagonal of 100x100 square) - let line_length = line_vector.length(); - assert!( - (line_length - 141.42).abs() < 1., // 100 * sqrt(2) ~= 141.42 - "Line length should be approximately 141.42 units. Got: {line_length}" - ); - assert!((line_vector.x - 100.).abs() < 1., "X-component of line vector should be approximately 100. Got: {}", line_vector.x); - assert!( - (line_vector.y.abs() - 100.).abs() < 1., - "Absolute Y-component of line vector should be approximately 100. Got: {}", - line_vector.y.abs() - ); - let angle_degrees = line_vector.angle_to(DVec2::X).to_degrees(); - assert!((angle_degrees - (-45.)).abs() < 1., "Line angle should be close to -45 degrees. Got: {angle_degrees}"); + let document = editor.editor.dispatcher.message_handlers.portfolio_message_handler.active_document().unwrap(); + assert_eq!(document.metadata().document_to_viewport, DAffine2::IDENTITY); + let [start_viewport, end_viewport] = [start_input, end_input].map(|point| transform.transform_point2(point)); + assert!(start_viewport.abs_diff_eq(expected_start, 1e-10), "expected line to start at {expected_start} not {start_viewport}"); + assert!(end_viewport.abs_diff_eq(expected_end, 1e-10), "expected line to end at {expected_end} not {end_viewport}"); } } diff --git a/editor/src/messages/tool/tool_messages/gradient_tool.rs b/editor/src/messages/tool/tool_messages/gradient_tool.rs index 99e4468900..636766feb0 100644 --- a/editor/src/messages/tool/tool_messages/gradient_tool.rs +++ b/editor/src/messages/tool/tool_messages/gradient_tool.rs @@ -1,5 +1,5 @@ use super::tool_prelude::*; -use crate::consts::{LINE_ROTATE_SNAP_ANGLE, MANIPULATOR_GROUP_MARKER_SIZE, SELECTION_THRESHOLD}; +use crate::consts::{DRAG_THRESHOLD, LINE_ROTATE_SNAP_ANGLE, MANIPULATOR_GROUP_MARKER_SIZE, SELECTION_THRESHOLD}; use crate::messages::portfolio::document::overlays::utility_types::OverlayContext; use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier; use crate::messages::tool::common_functionality::auto_panning::AutoPanning; @@ -29,6 +29,7 @@ pub enum GradientToolMessage { // Tool-specific messages DeleteStop, InsertStop, + InsertStopProxy, PointerDown, PointerMove { constrain_axis: Key }, PointerOutsideViewport { constrain_axis: Key }, @@ -84,6 +85,7 @@ impl<'a> MessageHandler> for Grad PointerMove, Abort, InsertStop, + InsertStopProxy, DeleteStop, ); } @@ -117,6 +119,7 @@ enum GradientToolFsmState { /// Computes the transform from gradient space to viewport space (where gradient space is 0..1) fn gradient_space_transform(layer: LayerNodeIdentifier, document: &DocumentMessageHandler) -> DAffine2 { let bounds = document.metadata().nonzero_bounding_box(layer); + println!("Bounds {bounds:?}"); let bound_transform = DAffine2::from_scale_angle_translation(bounds[1] - bounds[0], 0., bounds[0]); let multiplied = document.metadata().transform_to_viewport(layer); @@ -237,6 +240,7 @@ struct GradientToolData { snap_manager: SnapManager, drag_start: DVec2, auto_panning: AutoPanning, + pointer_up_abort: bool, } impl Fsm for GradientToolFsmState { @@ -343,6 +347,7 @@ impl Fsm for GradientToolFsmState { self } (_, GradientToolMessage::InsertStop) => { + println!("GradientToolMessage::InsertStop insert stop --------------------------"); for layer in document.network_interface.selected_nodes().selected_visible_layers(&document.network_interface) { let Some(mut gradient) = get_gradient(layer, &document.network_interface) else { continue }; // TODO: This transform is incorrect. I think this is since it is based on the Footprint which has not been updated yet @@ -351,8 +356,9 @@ impl Fsm for GradientToolFsmState { let (start, end) = (transform.transform_point2(gradient.start), transform.transform_point2(gradient.end)); // Compute the distance from the mouse to the gradient line in viewport space - let direction = (end - start).normalize_or_zero(); - let distance = direction.dot(mouse - start); + let distance = (end - start).normalize_or_zero().perp_dot(mouse - start).abs(); + + println!("> distance {distance} start {start} end {end} mouse {mouse}"); // If click is on the line then insert point if distance < (SELECTION_THRESHOLD * 2.) { @@ -377,6 +383,20 @@ impl Fsm for GradientToolFsmState { self } + // The undo system clears all clear targets only for double click messages after an abort. This hack fixes that. + (_, GradientToolMessage::InsertStopProxy) => { + if tool_data.pointer_up_abort { + let metadata = document.metadata(); + let all_empty = metadata.click_targets.is_empty() && metadata.upstream_footprints.is_empty() && metadata.local_transforms.is_empty(); + assert!(all_empty, "document metada is properly implemented so the InsertStopProxy should be removed {metadata:#?}"); + } + responses.add(DeferMessage::AfterGraphRun { + messages: vec![GradientToolMessage::InsertStop.into()], + }); + responses.add(NodeGraphMessage::RunDocumentGraph); + + self + } (GradientToolFsmState::Ready, GradientToolMessage::PointerDown) => { let mouse = input.mouse.position; tool_data.drag_start = mouse; @@ -494,7 +514,10 @@ impl Fsm for GradientToolFsmState { state } (GradientToolFsmState::Drawing, GradientToolMessage::PointerUp) => { - input.mouse.finish_transaction(tool_data.drag_start, responses); + let drag_too_small = tool_data.drag_start.distance(input.mouse.position) <= DRAG_THRESHOLD; + responses.add(if drag_too_small { DocumentMessage::AbortTransaction } else { DocumentMessage::EndTransaction }); + tool_data.pointer_up_abort = drag_too_small; + tool_data.snap_manager.cleanup(responses); let was_dragging = tool_data.selected_gradient.is_some(); @@ -694,6 +717,17 @@ mod test_gradient { assert!(transform.transform_point2(gradient.end).abs_diff_eq(DVec2::new(24., 4.), 1e-10)); } + #[tokio::test] + async fn double_click_empty_space() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + editor.drag_tool(ToolType::Rectangle, -5., -5., 105., 105., ModifierKeys::empty()).await; + editor.drag_tool(ToolType::Gradient, 0., 0., 100., 0., ModifierKeys::empty()).await; + editor.pointer_up_double_click(DVec2::new(300., 300.)).await; + let (updated_gradient, _) = get_gradient(&mut editor).await; + assert_eq!(updated_gradient.stops.len(), 2, "Expected 2 stops, found {}", updated_gradient.stops.len()); + } + #[tokio::test] async fn double_click_insert_stop() { let mut editor = EditorTestUtils::create(); @@ -709,7 +743,7 @@ mod test_gradient { assert_eq!(initial_gradient.stops.len(), 2, "Expected 2 stops, found {}", initial_gradient.stops.len()); editor.select_tool(ToolType::Gradient).await; - editor.double_click(DVec2::new(50., 0.)).await; + editor.pointer_up_double_click(DVec2::new(50., 0.)).await; // Check that a new stop has been added let (updated_gradient, _) = get_gradient(&mut editor).await; @@ -803,7 +837,7 @@ mod test_gradient { editor.select_tool(ToolType::Gradient).await; // Add a middle stop at 50% - editor.double_click(DVec2::new(50., 0.)).await; + editor.pointer_up_double_click(DVec2::new(50., 0.)).await; let (initial_gradient, _) = get_gradient(&mut editor).await; assert_eq!(initial_gradient.stops.len(), 3, "Expected 3 stops, found {}", initial_gradient.stops.len()); @@ -878,8 +912,8 @@ mod test_gradient { editor.select_tool(ToolType::Gradient).await; // Add two middle stops - editor.double_click(DVec2::new(25., 0.)).await; - editor.double_click(DVec2::new(75., 0.)).await; + editor.pointer_up_double_click(DVec2::new(25., 0.)).await; + editor.pointer_up_double_click(DVec2::new(75., 0.)).await; let (updated_gradient, _) = get_gradient(&mut editor).await; assert_eq!(updated_gradient.stops.len(), 4, "Expected 4 stops, found {}", updated_gradient.stops.len()); diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index 09374edf6b..d8405fcb4f 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -83,6 +83,13 @@ impl NodeGraphExecutor { }; (node_runtime, node_executor) } + + /// It is useful to get the current execution id for tests to check if any more exeuctions have been queued. + #[cfg(test)] + pub(crate) fn current_execution_id(&self) -> u64 { + self.current_execution_id + } + /// Execute the network by flattening it and creating a borrow stack. fn queue_execution(&mut self, render_config: RenderConfig) -> u64 { let execution_id = self.current_execution_id; diff --git a/editor/src/test_utils.rs b/editor/src/test_utils.rs index addadae0c2..1f469aa3e2 100644 --- a/editor/src/test_utils.rs +++ b/editor/src/test_utils.rs @@ -41,7 +41,8 @@ impl EditorTestUtils { async fn run<'a>(editor: &'a mut Editor, runtime: &'a mut NodeRuntime) -> Result { let portfolio = &mut editor.dispatcher.message_handlers.portfolio_message_handler; let exector = &mut portfolio.executor; - let document = portfolio.documents.get_mut(&portfolio.active_document_id.unwrap()).unwrap(); + let document_id = portfolio.active_document_id.unwrap(); + let document = portfolio.documents.get_mut(&document_id).unwrap(); let instrumented = match exector.update_node_graph_instrumented(document) { Ok(instrumented) => instrumented, @@ -49,19 +50,29 @@ impl EditorTestUtils { }; let viewport_resolution = glam::UVec2::ONE; - if let Err(e) = exector.submit_current_node_graph_evaluation(document, DocumentId(0), viewport_resolution, Default::default()) { + if let Err(e) = exector.submit_current_node_graph_evaluation(document, document_id, viewport_resolution, Default::default()) { return Err(format!("submit_current_node_graph_evaluation failed\n\n{e}")); } - runtime.run().await; - let mut messages = VecDeque::new(); - if let Err(e) = editor.poll_node_graph_evaluation(&mut messages) { - return Err(format!("Graph should render\n\n{e}")); - } - let frontend_messages = messages.into_iter().flat_map(|message| editor.handle_message(message)); + // Run until no more executions are queued. + loop { + println!("Running graph"); + let execution_id = editor.dispatcher.message_handlers.portfolio_message_handler.executor.current_execution_id(); + runtime.run().await; - for message in frontend_messages { - message.check_node_graph_error(); + let mut messages = VecDeque::new(); + if let Err(e) = editor.poll_node_graph_evaluation(&mut messages) { + return Err(format!("Graph should render\n\n{e}")); + } + let frontend_messages = messages.into_iter().flat_map(|message| editor.handle_message(message)); + + for message in frontend_messages { + message.check_node_graph_error(); + } + let next_execution_id = editor.dispatcher.message_handlers.portfolio_message_handler.executor.current_execution_id(); + if next_execution_id == execution_id { + break; + } } Ok(instrumented) @@ -258,12 +269,24 @@ impl EditorTestUtils { self.active_document().network_interface.selected_nodes().selected_layers(self.active_document().metadata()).next() } - pub async fn double_click(&mut self, position: DVec2) { + /// Simulate a pointer up then a double click without seperation of a graph render. + /// + /// This seems to be what WASM does to the test. + pub async fn pointer_up_double_click(&mut self, editor_position: DVec2) { + self.left_mousedown(editor_position.x, editor_position.y, ModifierKeys::empty()).await; + // Simulate a mouse up then double click event without a rerender then double click + self.editor.handle_message(InputPreprocessorMessage::PointerUp { + editor_mouse_state: EditorMouseState { + editor_position, + ..Default::default() + }, + modifier_keys: ModifierKeys::empty(), + }); self.handle_message(InputPreprocessorMessage::DoubleClick { editor_mouse_state: EditorMouseState { - editor_position: position, + editor_position, mouse_keys: MouseKeys::LEFT, - scroll_delta: ScrollDelta::default(), + ..Default::default() }, modifier_keys: ModifierKeys::empty(), })