Skip to content

New node: 'Attach Markers'#4172

Open
jsjgdh wants to merge 1 commit into
GraphiteEditor:masterfrom
jsjgdh:svg-marker-node
Open

New node: 'Attach Markers'#4172
jsjgdh wants to merge 1 commit into
GraphiteEditor:masterfrom
jsjgdh:svg-marker-node

Conversation

@jsjgdh

@jsjgdh jsjgdh commented May 25, 2026

Copy link
Copy Markdown
Contributor

WIP

@jsjgdh

jsjgdh commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@cubic-dev

@cubic-dev-ai

cubic-dev-ai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev

@jsjgdh I have started the AI code review. It will take a few minutes to complete.

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

Copy link
Copy Markdown
Contributor

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 introduces an "Attach Markers" node to place marker artwork at the start, middle, and end vertices of a path following SVG marker placement semantics. It adds helper functions and structures to compute tangents and transform markers. The reviewer suggested using the idiomatic .to_radians() method instead of manual conversion for the angle offset.

Comment thread node-graph/nodes/vector/src/vector_nodes.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@jsjgdh jsjgdh force-pushed the svg-marker-node branch 2 times, most recently from 725ba9e to f307f18 Compare May 25, 2026 21:35
@jsjgdh jsjgdh force-pushed the svg-marker-node branch 2 times, most recently from e15b572 to 70c6796 Compare June 9, 2026 17:20
@jsjgdh jsjgdh marked this pull request as ready for review June 9, 2026 17:29

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 1 file

Confidence score: 3/5

  • There is a concrete functional risk in node-graph/nodes/vector/src/vector_nodes.rs: flattening vertices across subpaths drops intermediate subpath boundaries, which can misclassify start/end vertices and produce incorrect marker behavior.
  • Given the high severity/confidence on boundary handling (8/10, 10/10), this is more than a housekeeping concern and introduces real regression risk if merged as-is.
  • The PR-title formatting issue in node-graph/nodes/vector/src/vector_nodes.rs context is process-only (5/10) and easy to fix, but it does not materially change runtime behavior.
  • Pay close attention to node-graph/nodes/vector/src/vector_nodes.rs - subpath boundary handling needs validation so intermediate subpaths keep correct start/end classification.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread node-graph/nodes/vector/src/vector_nodes.rs
Comment thread node-graph/nodes/vector/src/vector_nodes.rs
@jsjgdh jsjgdh changed the title New Node: "Attach Markers" New node: 'Attach Markers' Jun 9, 2026
@jsjgdh jsjgdh force-pushed the svg-marker-node branch from 70c6796 to f8e340b Compare June 9, 2026 18:30
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