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

Fix ViewPanner panning-mouse-warp #100444

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Dec 15, 2024

Currently the mouse cursor jumps in unexpected ways, when a ViewPanner is used in SubViewports or embedded Windows.

This is caused by providing wrong coordinate systems to Input::warp_mouse_motion.

This PR replaces the use of Input::warp_mouse_motion with Viewport::wrap_mouse_in_rect and makes sure, that the correct coordinate systems are used.

This change makes it necessary, that all classes, that currently use ViewPanner, need to provide the correct Viewport to ViewPanner (necessary changes are included in this PR).

The obligatory before and after:

BugViewPannerMouseWarp.mp4
FixViewPannerMouseWarp.mp4

One long-term goal of this PR is to move Input::warp_mouse_motion from Input to Viewport. It doesn't directly require any functionality of the Input class and so it makes sense to move it to a class that is not in core.

  • updated 2024-12-16: rename replacement-function to Viewport::wrap_mouse_in_rect

Copy link
Member

@Geometror Geometror left a comment

Choose a reason for hiding this comment

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

Tested locally on Windows 11 after I realized that mouse warping doesn't work on Wayland currenlty (although unrelated to this PR).
Works great! Implementation looks fine too, although I would call the new method Viewport::wrap_mouse_in_rect instead of Viewport::warp_mouse_into_rect
(wrap not warp, no typo here ;) Since wrapping more accurately/commonly describes the concept of something going out of bounds on one side and appearing on the other; so wrapping the mouse pointer in[side] the rect is done via warping it to specific position at the right time. Besides that, "wrap...into" implies that it just moves the mouse into a rectangle).

@Sauermann Sauermann force-pushed the fix-view-panner-mouse-warp branch from 4664c64 to 0568cbf Compare December 16, 2024 20:54
@Sauermann
Copy link
Contributor Author

Makes sense. I have updated the function name.

@Geometror Geometror requested a review from KoBeWi December 18, 2024 21:23
@KoBeWi
Copy link
Member

KoBeWi commented Dec 19, 2024

Does set_viewport() need to be called more than once or the viewport could be part of setup() maybe?

@Sauermann
Copy link
Contributor Author

Sauermann commented Dec 19, 2024

Does set_viewport() need to be called more than once or the viewport could be part of setup() maybe?

setup is not called from GraphEdit, so I have created a new function, that doesn't rely on setup.

When moving the node to a different location in the scene tree, it's possible, that setting a different viewport is required. So I believe, that ENTER_TREE is the right place to include it.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Overall looks fine.

Currently the mouse cursor jumps in unexpected ways, when a `ViewPanner`
is used in SubViewports or embedded Windows.

This is caused by providing wrong coordinate systems to
Input::warp_mouse_motion.

This PR replaces the use of `Input::warp_mouse_motion` with
`Viewport::wrap_mouse_in_rect` and makes sure, that the correct
coordinate systems are used.

This change makes it necessary, that all classes, that currently
use ViewPanner, need to provide the correct Viewport to ViewPanner.
@Sauermann Sauermann force-pushed the fix-view-panner-mouse-warp branch from 0568cbf to 4887172 Compare December 19, 2024 23:30
@Repiteo Repiteo merged commit cbfc34d into godotengine:master Dec 20, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 20, 2024

Thanks!

@Sauermann Sauermann deleted the fix-view-panner-mouse-warp branch December 20, 2024 02:14
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.

ViewPanner panning-mouse-warp doesn't work in SubViewports or embedded Windows
4 participants