Skip to content

Make Path Editable#3735

Draft
jsjgdh wants to merge 4 commits intoGraphiteEditor:masterfrom
jsjgdh:workflow
Draft

Make Path Editable#3735
jsjgdh wants to merge 4 commits intoGraphiteEditor:masterfrom
jsjgdh:workflow

Conversation

@jsjgdh
Copy link
Contributor

@jsjgdh jsjgdh commented Feb 8, 2026

@Keavon Keavon force-pushed the master branch 4 times, most recently from 3697b7d to 90533e6 Compare March 11, 2026 12:15
@jsjgdh
Copy link
Contributor Author

jsjgdh commented Mar 13, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request extends the 'Make Path Editable' functionality to support Table<Graphic> layers. The changes in utility_functions.rs are correct, but the related logic in path_tool.rs for identifying vector layers was not updated consistently, which could lead to incorrect behavior. I've pointed out this inconsistency and also suggested a refactoring to improve code clarity and performance.

Comment on lines +1590 to +1596
let is_vector_layer = if let Some(node) = node_id {
let output_connector = OutputConnector::node(node, 0);
let output_type = document.network_interface.output_type(&output_connector, &[]);
output_type.compiled_nested_type() == Some(&concrete!(Table<Vector>))
} else {
false
};
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The logic to identify a vector layer should also include Table<Graphic> to be consistent with the changes in make_path_editable_is_allowed. Currently, it only checks for Table<Vector>, which could lead to the path tool incorrectly trying to drill down into Graphic layers instead of treating them as editable paths.

Suggested change
let is_vector_layer = if let Some(node) = node_id {
let output_connector = OutputConnector::node(node, 0);
let output_type = document.network_interface.output_type(&output_connector, &[]);
output_type.compiled_nested_type() == Some(&concrete!(Table<Vector>))
} else {
false
};
let is_vector_layer = if let Some(node) = node_id {
let output_connector = OutputConnector::node(node, 0);
let output_type = document.network_interface.output_type(&output_connector, &[]);
let ty = output_type.compiled_nested_type();
ty == Some(&concrete!(Table<Vector>)) || ty == Some(&concrete!(Table<Graphic>))
} else {
false
};

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant