Skip to content

Change flipping to use the joint bounding box of the selection #323

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

Merged
merged 5 commits into from
Jul 31, 2021

Conversation

henryksloan
Copy link
Contributor

@henryksloan henryksloan commented Jul 31, 2021

Closes #316

  • Replaced the FlipLayer message with a new FlipSelectedLayers message that takes a FlipAxis enum
  • Changed the Select ToolMessages to use this new message
  • Implemented the FlipSelectedLayers message
    • Collects the max and min X or Y coordinates of each layer's bounding box
    • Aggregates the max's and min's to get the extrema of the bounding box in the given axis
    • Takes the average of these extrema as the flipping axis
    • Scales each layer by -1. along the axis of flipping, and translates it to the same distance on the other side of the axis

Caveats

  • The flipping is still in canvas space, similarly to alignment. This is because the current bounding box implementation (I believe) exclusively works in canvas space.
  • The code related to joint bounding boxes is quite repetitive and verbose between the flipping and alignment procedures
    • Add a bounding box around selected layers in the Select tool #282 is supposed to add a merging function between two bounding boxes, but it is not yet ready to review
    • I could add the function for use in these methods, and it could be reused when work on that PR continues
    • Either way, that should probably be saved for another PR

This change is Reviewable

@henryksloan henryksloan requested review from Keavon and TrueDoctor July 31, 2021 18:55
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 31, 2021

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0364b18
Status: ✅  Deploy successful!
Preview URL: https://5757404d.graphite-master.pages.dev

View logs

Keavon
Keavon previously approved these changes Jul 31, 2021
@Keavon
Copy link
Member

Keavon commented Jul 31, 2021

Before:
image

After horizontal flip:
image

Flipping it back restores it to the original design.

After vertical flip:
image

@Keavon
Copy link
Member

Keavon commented Jul 31, 2021

The above bugs arise from their rotated coordinate system when they are created. We'll have to fix that later.

@Keavon Keavon merged commit 09695f7 into master Jul 31, 2021
@Keavon Keavon deleted the bounding-box-flip branch July 31, 2021 22:18
Keavon pushed a commit that referenced this pull request Jun 16, 2022
* Change flipping to use the joint bounding box

* Fix minor untested changes

* Replace unwrap with question mark
Keavon pushed a commit that referenced this pull request Jul 30, 2023
* Change flipping to use the joint bounding box

* Fix minor untested changes

* Replace unwrap with question mark
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.

Improve Flip actions to work based on selection bounding box
2 participants