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

Trigger zoom from pan gestures when pressing ctrl #80994

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

aitorciki
Copy link
Contributor

@aitorciki aitorciki commented Aug 25, 2023

Fixes #72609

After the gestures refactor in #71685, zooming with ctrl + trackpad panning in macOS stopped working (4.0 beta16).

This PR aims at recovering that behavior, and extending it to other UI elements exposing zoom callbacks (pre-beta16, only CanvasItemEditor had this functionality hardcoded).

Panning up or right zooms in, down or left zooms out.

Zoom speed is tricky, as there is no factor in InputEventPanGesture we can leverage, and I haven’t found a way to consistently calculate one from deltas. I’ve decided to mimic the delta to factor logic in platform/macos/godot_content_view.mm, multiplying the default scroll zoom factor increase ratio by 0.3. Seems to work well for some UI elements (e.g. tilemaps), but is still too fast for others (e.g. animation tracks).

To keep things simple and avoid breaking existing user expectations, I’m not honoring the scroll pans/zooms toggle. I think it makes sense for panning to… pan by default (if this is deemed as potentially confusing to users who expect pan to behave like scroll, it’s an easy fix though).

EDIT: slightly reduced zoom speed, still feels good and natural in the canvas editor, and makes it a bit more precise in animation track editor.

scene/gui/view_panner.cpp Outdated Show resolved Hide resolved
scene/gui/view_panner.h Outdated Show resolved Hide resolved
@aitorciki
Copy link
Contributor Author

Changes applied as per @AThousandShips comments.

Enables zooming using pan + ctrl on macOS trackpads / Magic Mouse.
Windows and Linux don't emit pan gesture events, so shouldn't be
affected. Not tested on Android.
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

It works on macOS (should be tested on Android as well, since pan gesture support is not macOS specific).

But I'm not sure why it's necessary, what's the point of using Ctrl+Pan instead of Zoom gesture?

Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

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

While we're at it I tested this on the WIP Wayland backend, which implements pan/zoom gestures. It works great! Too bad that this speed isn't really controllable but it looks like a bigger, more general, issue.

@bruvzg I actually see some value in this. I can see some people having issues with pinching reliably and honestly I kinda prefer this way of zooming as it's easier on the hand.

Still, considering that this is an already existing behavior I don't think that this is proposal material, so I approve :)

(Note that this hasn't still been tested on Android)

@akien-mga akien-mga modified the milestones: 4.2, 4.3 Nov 13, 2023
@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Nov 13, 2023
@aitorciki
Copy link
Contributor Author

As @Riteo says, the pinch to zoom gesture is less precise than scrolling (specially for smaller steps) and for many folks not very ergonomic.
Also, scroll to zoom in pinch-supporting trackpads was lost in 4.0 beta 16 during a refactor, but had been available until then, this PR aims at recovering that existing functionality rather than adding new one (to @Riteo's point, configurable speed would be a great addition, but the current focus is in recovering the lost functionality).

@YuriSizov YuriSizov requested a review from a team December 4, 2023 15:52
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Tested on Android and it works well!

@akien-mga akien-mga changed the title [macOS] Trigger zoom from pan gestures when pressing ctrl Trigger zoom from pan gestures when pressing ctrl Dec 8, 2023
@YuriSizov YuriSizov merged commit ae4e482 into godotengine:master Dec 8, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

YuriSizov commented Dec 8, 2023

Thanks!

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 25, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

@aitorciki aitorciki deleted the zoom-from-pan-gesture branch June 22, 2024 22:09
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.

[Godot 4 beta 17] Zoom with magic mouse in 2D editor does not work
9 participants