Graphite icon indicating copy to clipboard operation
Graphite copied to clipboard

Spacing type radio button for repeat and circular repeat node

Open Ripper53 opened this issue 8 months ago • 5 comments

Closes #2551

  1. Added Spacing radio parameter to Repeat node, containing Span, Envelope, Pitch, and Gap options for spacing. See issue for how they must be spaced.
  2. Added Angle Pitch float parameter and Spacing radio parameter for Circular Repeat node. The Span option uses the calculations which already existed, the Pitch option rotates the next instance around the radius of the imaginary circle according to the Angle Pitch parameter and places the shape there.
  3. Added a unit macro attribute. Given string parameter is displayed as a unit type for the parameter.
  4. Added a step macro attribute. Given integer parameter is used as the step value for the parameter (ex. the arrow buttons on a number input increment/decrement by step amount).
  5. Added a display_decimal_places macro attribute. Given positive integer parameter is used to know how many decimal places to display for the number input if they are not 0.

Usage for macros:

#[unit(" px")]
#[step(2.)]
#[display_decimal_places(1)]
num: f64

Ripper53 avatar May 26 '25 16:05 Ripper53

!build

Keavon avatar Jun 09 '25 01:06 Keavon

📦 Build Complete for bb5b448af05188cf4641c52be569fad4b2b9c9c0
https://21f936f1.graphite.pages.dev

github-actions[bot] avatar Jun 09 '25 01:06 github-actions[bot]

Finally got to code review, sorry it's taken a while. I was busy doing a sizable refactor to how the vector nodes handle their repeat transforms. This affected the two nodes you've worked on here, so I just synced up your branch and resolved the merge conflicts. However, I commented out the Circular Repeat changes which require a bit more thought to get working, and I hope you can take it from here after the following feedback which pertains to my observations before I synced with master using the build link, as you left the code last, in the comment above.

Repeat node:

  • It appears that you have altered the behavior for now the Angle parameter works. Instead of twisting the whole repeated array of shapes, it now goes linearly but spins each shape. This is not desirable, the old behavior is.
  • Please order the radio buttons as: Envelope, Span, Pitch, Gap.
  • "Gap" mode causes an offset in the Y axis even if I set "Direction" to purely an X value, with Y at 0. I believe we discussed this topic in Discord, unless I'm confusing it with a different mode.

Circular Repeat node:

  • Let's fix this up after the commented out parts before I can spend the time to write feedback on it yet.

Other notes:

  • Please break out the code dealing with the new parameter widget attributes and submit those in a separate PR. I'd like to get those merged independently and sooner. It is easier to review the two bits separately.
  • Please take a moment to leave a comment in this PR's linked issue, #2674, so that I can assign that issue to you for tracking purposes. I don't have permission to assign it until you've commented in the thread.

Thank you, and it's nice to see this coming along!

Keavon avatar Jun 09 '25 02:06 Keavon

Macros split into their own PR: https://github.com/GraphiteEditor/Graphite/pull/2706

Ripper53 avatar Jun 11 '25 02:06 Ripper53

I have force pushed this branch to move it to a new base. Ensure you have the branch checked out. Then run git stash on any local changes (and ask me before following these steps if you have any un-pushed commits on your local machine). Then fetch this branch and git reset --hard origin/spacing-type-radio-button. And finally git stash pop your stashed local changes, if you stashed them.

Keavon avatar Jun 20 '25 08:06 Keavon