Skip to content
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.

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

Add Path tool support for G/R/S rotation and scaling with a single selected handle #2180

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

0SlowPoke0
Copy link
Contributor

@0SlowPoke0 0SlowPoke0 commented Jan 6, 2025

This pr implements some parts of #1870 and also some minor improvements

  • When using the path tool, R/S works about the anchor when a single handle is selected.
  • When using the pen tool, Backspace/Delete removes the current segment's handle (equivalent to clicking the current anchor).

Minor improvements as discussed -

  • G/R/S should have no effect (those keys should be ignored) while in the yellow point insertion sliding state - discord
  • The global G/R/S is actually specific to the Select tool and Path tool - discord
  • G/R/S when the selection is empty should, not activate G/R/S(A layer is selected but not any points in it. - discord
  • Aborts from G/R/S when switching tools using hotkeys - discord

@Keavon
Copy link
Member

Keavon commented Jan 6, 2025

!build

Copy link

github-actions bot commented Jan 6, 2025

📦 Build Complete for 780ddf5
https://f36c6976.graphite.pages.dev

@0SlowPoke0 0SlowPoke0 force-pushed the pen_and_path_tools_fix branch from 6c0881b to 812bdec Compare January 9, 2025 12:35
@0SlowPoke0 0SlowPoke0 marked this pull request as ready for review January 10, 2025 13:59
@Keavon Keavon force-pushed the pen_and_path_tools_fix branch from 812bdec to ab55281 Compare January 10, 2025 21:59
@Keavon Keavon changed the title Rotation and Scaling for Single Handle Selection Add Path tool support for G/R/S rotation and scaling with a single selected handle Jan 10, 2025
@Keavon
Copy link
Member

Keavon commented Jan 10, 2025

!build

Copy link

📦 Build Complete for ab55281
https://366fb012.graphite.pages.dev

@Keavon Keavon force-pushed the pen_and_path_tools_fix branch from ab55281 to ba3cc5a Compare January 13, 2025 02:49
@Keavon
Copy link
Member

Keavon commented Jan 13, 2025

!build

Copy link

📦 Build Complete for ba3cc5a
https://6c09c4a5.graphite.pages.dev

Comment on lines 84 to 85
if let Some(point) = selected_points.first() {
match point {
Copy link
Member

Choose a reason for hiding this comment

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

You can combine this if let and match, I believe.

Comment on lines 169 to 148
if let Some(point) = selected_points.first() {
match point {
Copy link
Member

Choose a reason for hiding this comment

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

Here too.

match point {
ManipulatorPointId::Anchor(_) => {
if let Some([handle1, handle2]) = point.get_handle_pair(&vector_data) {
if (handle1.get_handle_length(&vector_data) == 0.0 && handle2.get_handle_length(&vector_data) == 0.0)
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid using the style 0.0 and use the 0. style instead with decimals.

return;
}

if selected_points.len() == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Please find opportunities to avoid so much nesting here, like early returning to flatten this out. I suppose that might mean needing to return selected.original_transforms.clear(); in a number of places to keep parity with the current last line of this block.

return;
}

if selected_points.len() == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Too much nesting here too, please find ways to early return and combine if lets together.

@0SlowPoke0 0SlowPoke0 force-pushed the pen_and_path_tools_fix branch from ba3cc5a to cc033f4 Compare January 14, 2025 11:33
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.

2 participants