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

Refactor scale handling in editor to facilitate reuse #26643

Merged
merged 27 commits into from
May 23, 2024
Merged

Conversation

OliBomby
Copy link
Contributor

@OliBomby OliBomby commented Jan 20, 2024

Builds upon the convention created in #24341 to refactor scale handling to a SelectionScaleHandler.

I also changed the logic to accept the scale as a multiplier instead of a coordinate offset. I think this makes it easier to understand and makes implementing a future scale popover easier.

Also added shift hotkey for preserving aspect ratio on scale, because it was very simple to implement with some of the code I copied from the rotation scale handler. Feature mentioned in #24161

I apologise for the huge diff. Its hard to split a refactor like this into parts, because the program wont work currectly until every scale operator is refactored.

@bdach bdach self-requested a review May 23, 2024 11:44
bdach added 7 commits May 23, 2024 13:57
Not only is this simpler, but it also is more correct (for explanation
why, try holding both shift keys while dragging, and just releasing one
of them - the previous code would briefly turn aspect ratio off).
Being able to flip doesn't really feel all that good and `master` was
already clamping, so let's just bring that back for now. Flipping can be
reconsidered in a follow-up if it actually can be made to behave well.
It doesn't make sense and it wasn't doing the right thing.
@bdach
Copy link
Collaborator

bdach commented May 23, 2024

I've done a pass on this. Mostly code quality type stuff, but also some actual fixes/behavioural changes included. Of note: 070668c, 3e34b2d, 128029e (explanations in commit messages).

This is looking pretty good in general though. Aspect ratio finally having a chance to work correctly is nice. @OliBomby a second look to ensure I haven't broken anything would be appreciated if you are willing to have one.

@OliBomby
Copy link
Contributor Author

I've checked your changes, I think it looks good.

I was skeptical about the clamping but I agree its weird when the cursor stops moving with the anchor you're holding, so clamping prevents that.

@bdach bdach merged commit 22c4257 into ppy:master May 23, 2024
13 of 17 checks passed
@OliBomby OliBomby deleted the scaling branch May 23, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants