Fix after AI review

Author: YohYamasaki

editor/src/messages/input_mapper/input_mappings.rs

@@ -186,7 +186,7 @@ pub fn input_mappings(zoom_with_scroll: bool) -> Mapping {
 		// MeshGradientToolMessage
 		entry!(DoubleClick(MouseButton::Left); action_dispatch=MeshGradientToolMessage::DoubleClick),
 		entry!(KeyDown(MouseLeft); action_dispatch=MeshGradientToolMessage::PointerDown),
-		entry!(PointerMove; refresh_keys=[Shift, Control], action_dispatch=MeshGradientToolMessage::PointerMove { constrain_axis: Shift, lock_angle: Control }),
+		entry!(PointerMove; refresh_keys=[Shift], action_dispatch=MeshGradientToolMessage::PointerMove { constrain_axis: Shift }),
 		entry!(KeyUp(MouseLeft); action_dispatch=MeshGradientToolMessage::PointerUp),
 		entry!(KeyDown(Delete); action_dispatch=MeshGradientToolMessage::DeleteEdge),
 		entry!(KeyDown(Backspace); action_dispatch=MeshGradientToolMessage::DeleteEdge),

editor/src/messages/tool/common_functionality/graph_modification_utils.rs

@@ -520,13 +520,7 @@ pub fn gradient_orientation_rightward(transform: glam::DAffine2) -> bool {
 
 /// Try to find a "Mesh Gradient Value" node that is connected to a "Fill" node, or to a layer directly.
 pub fn get_upstream_mesh_gradient_value_node_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
-	let target_input = gradient_chain_target_input(layer, network_interface);
-	let walk_from = network_interface.upstream_output_connector(&target_input, &[])?.node_id()?;
-
-	network_interface
-		.upstream_flow_back_from_nodes(vec![walk_from], &[], FlowType::HorizontalFlow)
-		.take_while(|node_id| !network_interface.is_layer(node_id, &[]))
-		.find(|node_id| network_interface.reference(node_id, &[]).as_ref() == Some(&DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::mesh_gradient_value::IDENTIFIER)))
+	get_upstream_paint_value_node_id(layer, network_interface, graphene_std::math_nodes::mesh_gradient_value::IDENTIFIER)
 }
 
 /// Get the current fill of a layer from the closest "Fill" node.

editor/src/messages/tool/tool_messages/mesh_gradient_tool.rs

@@ -26,12 +26,25 @@ pub struct MeshGradientTool {
 	options: MeshGradientOptions,
 }
 
-#[derive(Default)]
 pub struct MeshGradientOptions {
 	space: GradientSpace,
 	interpolation: GradientInterpolation,
 }
 
+impl Default for MeshGradientOptions {
+	fn default() -> Self {
+		let MeshGradientSurface {
+			gradient_space,
+			gradient_interpolation,
+			..
+		} = MeshGradientSurface::default();
+		Self {
+			space: gradient_space,
+			interpolation: gradient_interpolation,
+		}
+	}
+}
+
 #[impl_message(Message, ToolMessage, MeshGradient)]
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[derive(PartialEq, Clone, Debug, serde::Serialize, serde::Deserialize)]
@@ -40,15 +53,13 @@ pub enum MeshGradientToolMessage {
 	Abort,
 	Overlays { context: OverlayContext },
 	SelectionChanged,
-	WorkingColorChanged,
 
 	// Tool-specific messages
 	DeleteEdge,
 	DoubleClick,
-	InsertStop,
 	PointerDown,
-	PointerMove { constrain_axis: Key, lock_angle: Key },
-	PointerOutsideViewport { constrain_axis: Key, lock_angle: Key },
+	PointerMove { constrain_axis: Key },
+	PointerOutsideViewport { constrain_axis: Key },
 	PointerUp,
 	StartTransactionForColorStop,
 	CommitTransactionForColorStop,
@@ -86,9 +97,10 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionMessageContext<'a>> for Mesh
 					MeshGradientOptionsUpdate::Interpolation(interpolation) => self.options.interpolation = interpolation,
 				}
 
-				apply_mesh_gradient_options(context, responses, |surface| {
-					surface.gradient_space = self.options.space;
-					surface.gradient_interpolation = self.options.interpolation;
+				// Write back only the setting that actually changed, so a layer whose other setting differs keeps it
+				apply_mesh_gradient_options(context, responses, |surface| match &options {
+					MeshGradientOptionsUpdate::Space(space) => surface.gradient_space = *space,
+					MeshGradientOptionsUpdate::Interpolation(interpolation) => surface.gradient_interpolation = *interpolation,
 				});
 				self.refresh_options(responses);
 			}
@@ -327,64 +339,7 @@ fn resolve_mesh_gradient_source(layer: LayerNodeIdentifier, network_interface: &
 	}
 }
 
-fn approximate_valid_region_bounds(initial_position: DVec2, [min, max]: [DVec2; 2], mut is_valid: impl FnMut(DVec2) -> bool) -> Option<[DVec2; 2]> {
-	const SUBDIVISIONS: usize = 12;
-
-	let mut x_samples = (0..=SUBDIVISIONS).map(|index| min.x + (max.x - min.x) * index as f64 / SUBDIVISIONS as f64).collect::<Vec<_>>();
-	let mut y_samples = (0..=SUBDIVISIONS).map(|index| min.y + (max.y - min.y) * index as f64 / SUBDIVISIONS as f64).collect::<Vec<_>>();
-	x_samples.push(initial_position.x);
-	y_samples.push(initial_position.y);
-	x_samples.sort_by(f64::total_cmp);
-	y_samples.sort_by(f64::total_cmp);
-	x_samples.dedup();
-	y_samples.dedup();
-
-	let columns = x_samples.len();
-	let rows = y_samples.len();
-	let seed_column = x_samples.iter().position(|&x| x == initial_position.x)?;
-	let seed_row = y_samples.iter().position(|&y| y == initial_position.y)?;
-	let seed_index = seed_row * columns + seed_column;
-
-	let valid_samples = y_samples.iter().flat_map(|&y| x_samples.iter().map(move |&x| DVec2::new(x, y))).map(&mut is_valid).collect::<Vec<_>>();
-
-	if !valid_samples[seed_index] {
-		return None;
-	}
-
-	let mut visited = vec![false; rows * columns];
-	let mut queue = VecDeque::from([seed_index]);
-	let mut bounds_min = initial_position;
-	let mut bounds_max = initial_position;
-
-	while let Some(index) = queue.pop_front() {
-		if visited[index] || !valid_samples[index] {
-			continue;
-		}
-		visited[index] = true;
-
-		let row = index / columns;
-		let column = index % columns;
-		let position = DVec2::new(x_samples[column], y_samples[row]);
-		bounds_min = bounds_min.min(position);
-		bounds_max = bounds_max.max(position);
-
-		if row > 0 {
-			queue.push_back(index - columns);
-		}
-		if row + 1 < rows {
-			queue.push_back(index + columns);
-		}
-		if column > 0 {
-			queue.push_back(index - 1);
-		}
-		if column + 1 < columns {
-			queue.push_back(index + 1);
-		}
-	}
-
-	Some([bounds_min, bounds_max])
-}
-
+/// Walks back from `target` toward `valid_region_center` for the furthest position that keeps the mesh free of foldovers.
 fn constrain_to_valid_region(target: DVec2, valid_region_center: DVec2, candidate: impl Fn(DVec2) -> Option<MeshGradient>) -> Option<MeshGradient> {
 	candidate(target).or_else(|| {
 		const BINARY_SEARCH_ITERATIONS: usize = 12;
@@ -435,7 +390,6 @@ impl ToolTransition for MeshGradientTool {
 		EventToMessageMap {
 			tool_abort: Some(MeshGradientToolMessage::Abort.into()),
 			selection_changed: Some(MeshGradientToolMessage::SelectionChanged.into()),
-			working_color_changed: Some(MeshGradientToolMessage::WorkingColorChanged.into()),
 			overlay_provider: Some(|context| MeshGradientToolMessage::Overlays { context }.into()),
 			..Default::default()
 		}
@@ -615,9 +569,12 @@ impl Fsm for MeshGradientToolFsmState {
 
 			(_state @ MeshGradientToolFsmState::Ready { .. }, MeshGradientToolMessage::DeleteEdge) => {
 				let Some(selected_mesh) = tool_data.selected_mesh.as_mut() else { return self };
-				if let MeshGradientTarget::Segment { segment_id, .. } = selected_mesh.target {
-					selected_mesh.surface.mesh.remove_edge(segment_id);
-				};
+				let MeshGradientTarget::Segment { segment_id, .. } = selected_mesh.target else { return self };
+				let mut mesh = selected_mesh.surface.mesh.clone();
+				if mesh.remove_edge(segment_id).is_none() {
+					return self;
+				}
+				selected_mesh.surface.mesh = mesh;
 
 				responses.add(DocumentMessage::StartTransaction);
 				selected_mesh.update_gradient_in_graph(responses);
@@ -721,18 +678,6 @@ impl Fsm for MeshGradientToolFsmState {
 
 								if distance_squared < tolerance_squared {
 									responses.add(DocumentMessage::StartTransaction);
-									let valid_region_center = gradient
-										.geometry()
-										.bounding_box()
-										.and_then(|bounds| {
-											approximate_valid_region_bounds(corner.position, bounds, |position| {
-												let mut candidate = gradient.clone();
-												candidate.set_corner_position(corner.index, position).is_some()
-													&& candidate.patches().all(|patch| patch.is_some_and(|patch| patch.sampled_no_foldover()))
-											})
-										})
-										.map(|[min, max]| min.midpoint(max))
-										.unwrap_or(corner.position);
 
 									tool_data.selected_mesh = Some(SelectedMeshGradient {
 										layer,
@@ -744,7 +689,7 @@ impl Fsm for MeshGradientToolFsmState {
 											corner_index: corner.index,
 											initial_mouse: local_mouse,
 											initial_corner: corner.position,
-											valid_region_center,
+											valid_region_center: corner.position,
 										},
 									});
 
@@ -782,37 +727,27 @@ impl Fsm for MeshGradientToolFsmState {
 										consider_handle(HandleId::end(segment_id), handle_end, bezier.end, None);
 									}
 								}
+							}
 
-								if let Some((handle_id, initial_handle, _)) = closest_handle {
-									responses.add(DocumentMessage::StartTransaction);
-									let valid_region_center = gradient
-										.geometry()
-										.bounding_box()
-										.and_then(|bounds| {
-											approximate_valid_region_bounds(initial_handle, bounds, |position| {
-												let mut candidate = gradient.clone();
-												candidate.set_handle_position(handle_id, position).is_some() && candidate.patches().all(|patch| patch.is_some_and(|patch| patch.sampled_no_foldover()))
-											})
-										})
-										.map(|[min, max]| min.midpoint(max))
-										.unwrap_or(initial_handle);
-
-									tool_data.selected_mesh = Some(SelectedMeshGradient {
-										layer,
-										mesh_index: index,
-										surface: mesh_gradient_surface(meshes, index, gradient),
-										mesh_to_document,
-										source,
-										target: MeshGradientTarget::Handle {
-											handle_id,
-											initial_mouse: local_mouse,
-											initial_handle,
-											valid_region_center,
-										},
-									});
+							// Resolved only after every segment has been offered, so the nearest-wins comparison spans the whole mesh
+							if let Some((handle_id, initial_handle, _)) = closest_handle {
+								responses.add(DocumentMessage::StartTransaction);
+
+								tool_data.selected_mesh = Some(SelectedMeshGradient {
+									layer,
+									mesh_index: index,
+									surface: mesh_gradient_surface(meshes, index, gradient),
+									mesh_to_document,
+									source,
+									target: MeshGradientTarget::Handle {
+										handle_id,
+										initial_mouse: local_mouse,
+										initial_handle,
+										valid_region_center: initial_handle,
+									},
+								});
 
-									return MeshGradientToolFsmState::Dragging;
-								}
+								return MeshGradientToolFsmState::Dragging;
 							}
 
 							for edge in gradient.edges() {
@@ -826,31 +761,11 @@ impl Fsm for MeshGradientToolFsmState {
 
 									let handles = match (points.p1, points.p2) {
 										(Some(p1), Some(p2)) => [p1, p2],
-										(Some(p1), None) | (None, Some(p1)) => [p1, points.p3],
+										(Some(control), None) | (None, Some(control)) => [points.p0 + (control - points.p0) * 2. / 3., points.p3 + (control - points.p3) * 2. / 3.],
 										(None, None) => [points.p0 + (points.p3 - points.p0) / 3., points.p3 + (points.p0 - points.p3) / 3.],
 									};
 
 									responses.add(DocumentMessage::StartTransaction);
-									let valid_region_center = gradient
-										.geometry()
-										.bounding_box()
-										.and_then(|bounds| {
-											approximate_valid_region_bounds(local_mouse, bounds, |position| {
-												let delta = position - local_mouse;
-												let mut candidate = gradient.clone();
-												candidate
-													.set_edge_handles(
-														edge.segment_id,
-														BezierHandles::Cubic {
-															handle_start: handles[0] + delta,
-															handle_end: handles[1] + delta,
-														},
-													)
-													.is_some() && candidate.patches().all(|patch| patch.is_some_and(|patch| patch.sampled_no_foldover()))
-											})
-										})
-										.map(|[min, max]| min.midpoint(max))
-										.unwrap_or(local_mouse);
 
 									tool_data.selected_mesh = Some(SelectedMeshGradient {
 										layer,
@@ -862,7 +777,7 @@ impl Fsm for MeshGradientToolFsmState {
 											segment_id: edge.segment_id,
 											initial_mouse: local_mouse,
 											initial_handles: handles,
-											valid_region_center,
+											valid_region_center: local_mouse,
 										},
 									});
 
@@ -904,7 +819,7 @@ impl Fsm for MeshGradientToolFsmState {
 
 				self
 			}
-			(MeshGradientToolFsmState::Dragging, MeshGradientToolMessage::PointerMove { constrain_axis, lock_angle }) => {
+			(MeshGradientToolFsmState::Dragging, MeshGradientToolMessage::PointerMove { constrain_axis }) => {
 				let MeshGradientToolData {
 					selected_mesh,
 					snap_manager,
@@ -1032,8 +947,8 @@ impl Fsm for MeshGradientToolFsmState {
 
 				// Auto-panning
 				let messages = [
-					MeshGradientToolMessage::PointerOutsideViewport { constrain_axis, lock_angle }.into(),
-					MeshGradientToolMessage::PointerMove { constrain_axis, lock_angle }.into(),
+					MeshGradientToolMessage::PointerOutsideViewport { constrain_axis }.into(),
+					MeshGradientToolMessage::PointerMove { constrain_axis }.into(),
 				];
 				auto_panning.setup_by_mouse_position(input, viewport, &messages, responses);
 
@@ -1074,10 +989,10 @@ impl Fsm for MeshGradientToolFsmState {
 
 				MeshGradientToolFsmState::Dragging
 			}
-			(state, MeshGradientToolMessage::PointerOutsideViewport { constrain_axis, lock_angle }) => {
+			(state, MeshGradientToolMessage::PointerOutsideViewport { constrain_axis }) => {
 				let messages = [
-					MeshGradientToolMessage::PointerOutsideViewport { constrain_axis, lock_angle }.into(),
-					MeshGradientToolMessage::PointerMove { constrain_axis, lock_angle }.into(),
+					MeshGradientToolMessage::PointerOutsideViewport { constrain_axis }.into(),
+					MeshGradientToolMessage::PointerMove { constrain_axis }.into(),
 				];
 				tool_data.auto_panning.stop(&messages, responses);