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

Handle warped mouse motion as floating point #88343

Merged
merged 1 commit into from
May 6, 2024

Conversation

Riteo
Copy link
Contributor

@Riteo Riteo commented Feb 14, 2024

Fixes #88246.

Fixes certain issues where sub-pixel motions would get discarded while the mouse is captured, such as when free look is enabled in the editor (at least when turned on while holding right click).

Very slightly compat breaking, as actual public APIs are changed, although with "compatible" types (Point2i->Point2).

Fixes certain issues where sub-pixel motions would get discarded while
the mouse is captured, such as when free look is enabled in the editor
(at least when turned on while holding right click).

Very slightly compat breaking, as actual public APIs are changed,
although with "compatible" types (Point2i->Point2).
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.

The rest of the mouse position/movement functions use Vector2, so this is more consistent and should not break anything.

But it's strange that sub-pixel mouse movement is possible, is it something related to hiDPI?

@Riteo
Copy link
Contributor Author

Riteo commented Feb 19, 2024

@bruvzg

But it's strange that sub-pixel mouse movement is possible, is it something related to hiDPI?

Also, but more importantly it's related to relative motion of high definition mice. Positions are accumulated by the compositor so whatever, but relative motions are sent raw so we have no way of actually detecting them as they all get truncated.

Copy link
Contributor

@Sauermann Sauermann left a comment

Choose a reason for hiding this comment

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

I don't see, how this PR is compatibility breaking, because neither Input::warp_mouse_motion nor Node3DEditorViewport::_get_warped_mouse_motion is exposed to scripting.
Note, that the function Input::warp_mouse is exposed to scripting.

Changes make sense to me, but I haven't tested them though.

const Point2i rel_sign(p_motion->get_relative().x >= 0.0f ? 1 : -1, p_motion->get_relative().y >= 0.0 ? 1 : -1);
const Size2i warp_margin = p_rect.size * 0.5f;
const Point2i rel_warped(
const Point2 rel_sign(p_motion->get_relative().x >= 0.0f ? 1 : -1, p_motion->get_relative().y >= 0.0 ? 1 : -1);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering, why Vector2::sign is not used (rel_sign = p_motion->get_relative().sign()). Perhaps because 0.0f is not wanted as a output value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mh. I'm a bit unsure on why we care for the 0 case so much, as we aren't moving :/

I'm struggling a bit to understand the code... If the position is zero, rel_sign is 1, so we fmod half p_rect basically, but then we subtract it with the same thing.

Mh. I think that this might be material for a separate PR, as this might need some more investigation.

@Riteo
Copy link
Contributor Author

Riteo commented May 4, 2024

I don't see, how this PR is compatibility breaking

Mh we're changing some types but nothing scripting-related, right.

Note, that the function Input::warp_mouse is exposed to scripting.

Note that this PR does not change its type. It's related to a different issue reported later.

Update: I can do that here if you're ok.

@Sauermann
Copy link
Contributor

Note that this PR does not change its type. It's related to a different issue reported later.

Update: I can do that here if you're ok.

I would say, let's keep that for a separate PR.

@akien-mga akien-mga merged commit 1069d7b into godotengine:master May 6, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@elvisish
Copy link

elvisish commented May 15, 2024

I'm using this in 3.5 to ensure mouselook is consistent between window sizes (thanks @kleonc) :

func xform_the_mouse(event):
	var root: Viewport = get_tree().root
	var input_pre_xform := Transform2D().translated(-OS.window_position).scaled(root.size / OS.get_window_size())
	var transform_to_apply: Transform2D = input_pre_xform.affine_inverse() * root.get_final_transform()
	var event_to_save_the_data_from: InputEvent = event.xformed_by(transform_to_apply)
	return event_to_save_the_data_from.relative

It feels a little like sub-pixel precision is lost when using this instead of the event.relative directly, would this be caused by this? And is it only being fixed on the 4.x branch?

@Riteo
Copy link
Contributor Author

Riteo commented May 16, 2024

@elvisish I don't think so. The issue here was related to the editor truncating the data for its panning logic, while here you're working with the "source" straight from the OS, which presumably is correct. Perhaps the truncating actions is being done by something in the chain of transform operations?

BTW, it is possible that this exact issue might still exist (broken sub-pixel editor panning), but I have never really worked a lot with 3.x code.

@Calinou
Copy link
Member

Calinou commented May 16, 2024

I'm using this in 3.5 to ensure mouselook is consistent between window sizes (thanks @kleonc) :

In 4.3 and later, event.screen_relative should be used instead of event.relative to get this behavior automatically: godotengine/godot-demo-projects#1055

@elvisish
Copy link

I'm using this in 3.5 to ensure mouselook is consistent between window sizes (thanks @kleonc) :

In 4.3 and later, event.screen_relative should be used instead of event.relative to get this behavior automatically: godotengine/godot-demo-projects#1055

Thanks, that's really good to know. I'm still finishing up a project with 3.x so not sure if this will be backported to 3.6 or not.

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.

Mouse Input under Wayland misses slow movement
6 participants