Skip to content

New Node: "Attach Markers"#4172

Draft
jsjgdh wants to merge 2 commits into
GraphiteEditor:masterfrom
jsjgdh:svg-marker-node
Draft

New Node: "Attach Markers"#4172
jsjgdh wants to merge 2 commits into
GraphiteEditor:masterfrom
jsjgdh:svg-marker-node

Conversation

@jsjgdh
Copy link
Copy Markdown
Contributor

@jsjgdh jsjgdh commented May 25, 2026

WIP

@jsjgdh
Copy link
Copy Markdown
Contributor Author

jsjgdh commented May 25, 2026

@cubic-dev

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai Bot commented May 25, 2026

@cubic-dev

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

Copy link
Copy Markdown
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 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.

0.
};

DAffine2::from_scale_angle_translation(DVec2::splat(self.scale), angle + self.angle_offset / 360. * TAU, vertex.position)
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.

medium

Using the standard library's .to_radians() method is more idiomatic, readable, and less error-prone than manually dividing by 360. and multiplying by TAU.

Suggested change
DAffine2::from_scale_angle_translation(DVec2::splat(self.scale), angle + self.angle_offset / 360. * TAU, vertex.position)
DAffine2::from_scale_angle_translation(DVec2::splat(self.scale), angle + self.angle_offset.to_radians(), vertex.position)

Copy link
Copy Markdown
Contributor

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

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 from 8d5f322 to b6afafa Compare May 25, 2026 21:22
@jsjgdh jsjgdh force-pushed the svg-marker-node branch from 725ba9e to f307f18 Compare May 25, 2026 21:35
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