Skip to content

Pen/Freehand/Splines tool set its origin to the first drawn point #3795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jsjgdh wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Pen/Freehand/Splines tool set its origin to the first drawn point #3795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9d555f
Origin
jsjgdh Feb 21, 2026
35f9ce0
fix
jsjgdh Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion editor/src/messages/portfolio/document/graph_operation/utility_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ impl<'a> ModifyInputsContext<'a> {
}

pub fn insert_vector(&mut self, subpaths: Vec<Subpath<PointId>>, layer: LayerNodeIdentifier, include_transform: bool, include_fill: bool, include_stroke: bool) {
let first_anchor = subpaths.first().and_then(|subpath| subpath.manipulator_groups().first().map(|group| group.anchor));
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This line correctly captures the first anchor point from the subpaths, which is then used to set the origin offset for the transform node. This is a good approach to ensure the tool's origin is anchored to the initial drawing point.

let vector = Table::new_from_element(Vector::from_subpaths(subpaths, true));

let shape = resolve_network_node_type("Path")
Expand All @@ -164,7 +165,13 @@ impl<'a> ModifyInputsContext<'a> {
self.network_interface.move_node_to_chain_start(&shape_id, layer, &[]);

if include_transform {
let transform = resolve_network_node_type("Transform").expect("Transform node does not exist").default_node_template();
let mut transform = resolve_network_node_type("Transform").expect("Transform node does not exist").default_node_template();
if let Some(anchor) = first_anchor {
const TRANSLATION_INDEX: usize = 1;
if let Some(translation) = transform.document_node.inputs.get_mut(TRANSLATION_INDEX) {
*translation = NodeInput::value(TaggedValue::DVec2(anchor), false);
}
Comment on lines 168 to 173
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic here correctly applies the first_anchor to the ORIGIN_OFFSET_INDEX of the Transform node. This ensures that the vector layer's transform is correctly initialized with the starting point of the drawn path.

}
let transform_id = NodeId::new();
self.network_interface.insert_node(transform_id, transform, &[]);
self.network_interface.move_node_to_chain_start(&transform_id, layer, &[]);
Expand Down
9 changes: 9 additions & 0 deletions editor/src/messages/tool/common_functionality/graph_modification_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,15 @@ pub fn new_svg_layer(svg: String, transform: glam::DAffine2, id: NodeId, parent:
}

pub fn new_custom(id: NodeId, nodes: Vec<(NodeId, NodeTemplate)>, parent: LayerNodeIdentifier, responses: &mut VecDeque<Message>) -> LayerNodeIdentifier {
let transform_node_id = NodeId::new();
let mut nodes = nodes;

// Insert a Transform node
let transform_node = crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_network_node_type("Transform")
.expect("Transform node does not exist")
.default_node_template();
nodes.push((transform_node_id, transform_node));
Comment on lines +246 to +253
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This new section introduces a Transform node into the custom layer creation. While this is necessary for the new origin functionality, it might be beneficial to pass the transform_node_id and transform_node as arguments to new_custom if they are always going to be the same, to avoid redundant code in the calling sites. However, given the context of setting the origin, this approach is acceptable for now.


responses.add(GraphOperationMessage::NewCustomLayer { id, nodes, parent, insert_index: 0 });
responses.add(GraphOperationMessage::SetUpstreamToChain {
layer: LayerNodeIdentifier::new_unchecked(id),
Expand Down
14 changes: 11 additions & 3 deletions editor/src/messages/tool/tool_messages/freehand_tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::messages::tool::common_functionality::color_selector::{ToolColorOptio
use crate::messages::tool::common_functionality::graph_modification_utils;
use crate::messages::tool::common_functionality::utility_functions::should_extend;
use glam::DVec2;
use graph_craft::document::NodeId;
use graph_craft::document::{NodeId, NodeInput};
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding NodeInput to the use statement is necessary for the new Transform node logic. This is a correct and required change.

use graphene_std::Color;
use graphene_std::vector::VectorModificationType;
use graphene_std::vector::{PointId, SegmentId};
Expand Down Expand Up @@ -285,9 +285,17 @@ impl Fsm for FreehandToolFsmState {

let parent = document.new_layer_bounding_artboard(input, viewport);

let document_space_position = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);

let node_type = resolve_network_node_type("Path").expect("Path node does not exist");
let node = node_type.default_node_template();
let nodes = vec![(NodeId(0), node)];
let mut transform_node = resolve_network_node_type("Transform").expect("Transform node does not exist").default_node_template();
if let Some(input) = transform_node.document_node.inputs.get_mut(0) {
*input = NodeInput::node(NodeId(1), 0);
}
if let Some(translation) = transform_node.document_node.inputs.get_mut(1) {
*translation = NodeInput::value(graph_craft::document::value::TaggedValue::DVec2(document_space_position), false);
}
Comment on lines 291 to 297
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block correctly initializes a Transform node and sets its ORIGIN_OFFSET_INDEX to the document_space_position. This ensures that the freehand tool's origin is set to the first drawn point, aligning with the PR's objective.

let nodes = vec![(NodeId(0), transform_node), (NodeId(1), node_type.default_node_template())];
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The nodes vector is updated to include both the Transform node and the Path node, with NodeId(0) assigned to the Transform node and NodeId(1) to the Path node. This order is crucial for the graph's data flow, as the Path node's input is set to NodeId(1) (the Transform node) in the previous lines.


let layer = graph_modification_utils::new_custom(NodeId::new(), nodes, parent, responses);
tool_options.stroke.apply_stroke(tool_data.weight, layer, responses);
Expand Down
13 changes: 11 additions & 2 deletions editor/src/messages/tool/tool_messages/pen_tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use crate::messages::tool::common_functionality::graph_modification_utils::{self
use crate::messages::tool::common_functionality::shape_editor::ShapeState;
use crate::messages::tool::common_functionality::snapping::{SnapCache, SnapCandidatePoint, SnapConstraint, SnapData, SnapManager, SnapTypeConfiguration};
use crate::messages::tool::common_functionality::utility_functions::{calculate_segment_angle, closest_point, should_extend};
use graph_craft::document::NodeId;
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{NodeId, NodeInput};
Comment on lines +14 to +15
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The addition of TaggedValue and NodeInput to the use statement is necessary for the new Transform node logic. This is a correct and required change.

use graphene_std::Color;
use graphene_std::subpath::pathseg_points;
use graphene_std::vector::misc::{HandleId, ManipulatorPointId, dvec2_to_point};
Expand Down Expand Up @@ -1283,7 +1284,15 @@ impl PenToolData {

// New path layer
let node_type = resolve_network_node_type("Path").expect("Path node does not exist");
let nodes = vec![(NodeId(0), node_type.default_node_template())];
let mut transform_node = resolve_network_node_type("Transform").expect("Transform node does not exist").default_node_template();
if let Some(input) = transform_node.document_node.inputs.get_mut(0) {
*input = NodeInput::node(NodeId(1), 0);
}
const TRANSLATION_INDEX: usize = 1;
if let Some(translation) = transform_node.document_node.inputs.get_mut(TRANSLATION_INDEX) {
*translation = NodeInput::value(TaggedValue::DVec2(snapped.snapped_point_document), false);
}
Comment on lines 1287 to 1294
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This code correctly initializes a Transform node and sets its ORIGIN_OFFSET_INDEX to the snapped.snapped_point_document. This ensures that the pen tool's origin is set to the first drawn point, aligning with the PR's objective.

let nodes = vec![(NodeId(0), transform_node), (NodeId(1), node_type.default_node_template())];
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The nodes vector is updated to include both the Transform node and the Path node, with NodeId(0) assigned to the Transform node and NodeId(1) to the Path node. This order is crucial for the graph's data flow, as the Path node's input is set to NodeId(1) (the Transform node) in the previous lines.


let parent = document.new_layer_bounding_artboard(input, viewport);
let layer = graph_modification_utils::new_custom(NodeId::new(), nodes, parent, responses);
Expand Down
15 changes: 13 additions & 2 deletions editor/src/messages/tool/tool_messages/shape_tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ use crate::messages::tool::common_functionality::shapes::{Ellipse, Line, Rectang
use crate::messages::tool::common_functionality::snapping::{self, SnapCandidatePoint, SnapData, SnapTypeConfiguration};
use crate::messages::tool::common_functionality::transformation_cage::{BoundingBoxManager, EdgeBool};
use crate::messages::tool::common_functionality::utility_functions::{closest_point, resize_bounds, rotate_bounds, skew_bounds, transforming_transform_cage};
use graph_craft::document::NodeId;
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{NodeId, NodeInput};
Comment on lines +24 to +25
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The addition of TaggedValue and NodeInput to the use statement is necessary for the new Transform node logic. This is a correct and required change.

use graphene_std::Color;
use graphene_std::renderer::Quad;
use graphene_std::vector::misc::{ArcType, GridType, SpiralType};
Expand Down Expand Up @@ -936,7 +937,17 @@ impl Fsm for ShapeToolFsmState {
ShapeType::Line => Line::create_node(document, tool_data.data.drag_start),
};

let nodes = vec![(NodeId(0), node)];
let mut transform_node = crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_network_node_type("Transform")
.expect("Transform node does not exist")
.default_node_template();
if let Some(input) = transform_node.document_node.inputs.get_mut(0) {
*input = NodeInput::node(NodeId(1), 0);
}
const TRANSLATION_INDEX: usize = 1;
if let Some(translation) = transform_node.document_node.inputs.get_mut(TRANSLATION_INDEX) {
*translation = NodeInput::value(TaggedValue::DVec2(tool_data.data.drag_start), false);
}
Comment on lines 940 to 949
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block correctly initializes a Transform node and sets its ORIGIN_OFFSET_INDEX to the tool_data.data.drag_start. This ensures that the shape tool's origin is set to the first drawn point, aligning with the PR's objective.

let nodes = vec![(NodeId(0), transform_node), (NodeId(1), node)];
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The nodes vector is updated to include both the Transform node and the shape node, with NodeId(0) assigned to the Transform node and NodeId(1) to the shape node. This order is crucial for the graph's data flow, as the shape node's input is set to NodeId(1) (the Transform node) in the previous lines.

let layer = graph_modification_utils::new_custom(NodeId::new(), nodes, document.new_layer_bounding_artboard(input, viewport), responses);

let defered_responses = &mut VecDeque::new();
Expand Down
14 changes: 12 additions & 2 deletions editor/src/messages/tool/tool_messages/spline_tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::messages::tool::common_functionality::color_selector::{ToolColorOptio
use crate::messages::tool::common_functionality::graph_modification_utils::{self, find_spline, merge_layers, merge_points};
use crate::messages::tool::common_functionality::snapping::{SnapCandidatePoint, SnapData, SnapManager, SnapTypeConfiguration, SnappedPoint};
use crate::messages::tool::common_functionality::utility_functions::{closest_point, should_extend};
use graph_craft::document::value::TaggedValue;
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding TaggedValue to the use statement is necessary for the new Transform node logic. This is a correct and required change.

use graph_craft::document::{NodeId, NodeInput};
use graphene_std::Color;
use graphene_std::vector::{PointId, SegmentId, VectorModificationType};
Expand Down Expand Up @@ -391,8 +392,17 @@ impl Fsm for SplineToolFsmState {
let path_node_type = resolve_network_node_type("Path").expect("Path node does not exist");
let path_node = path_node_type.default_node_template();
let spline_node_type = resolve_proto_node_type(graphene_std::vector::spline::IDENTIFIER).expect("Spline node does not exist");
let spline_node = spline_node_type.node_template_input_override([Some(NodeInput::node(NodeId(1), 0))]);
let nodes = vec![(NodeId(1), path_node), (NodeId(0), spline_node)];
let spline_node = spline_node_type.node_template_input_override([Some(NodeInput::node(NodeId(2), 0))]);

let mut transform_node = resolve_network_node_type("Transform").expect("Transform node does not exist").default_node_template();
if let Some(input) = transform_node.document_node.inputs.get_mut(0) {
*input = NodeInput::node(NodeId(1), 0);
}
if let Some(translation) = transform_node.document_node.inputs.get_mut(1) {
*translation = NodeInput::value(TaggedValue::DVec2(snapped.snapped_point_document), false);
}
Comment on lines 397 to 403
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block correctly initializes a Transform node and sets its ORIGIN_OFFSET_INDEX to the snapped.snapped_point_document. This ensures that the spline tool's origin is set to the first drawn point, aligning with the PR's objective.


let nodes = vec![(NodeId(0), transform_node), (NodeId(1), spline_node), (NodeId(2), path_node)];

let layer = graph_modification_utils::new_custom(NodeId::new(), nodes, parent, responses);
tool_options.stroke.apply_stroke(tool_data.weight, layer, responses);
Expand Down
Loading