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 global position for InputEventMouse in viewport::push_input #88473

Merged

Conversation

Sauermann
Copy link
Contributor

Global position doesn't get adjusted within InputEventMouse::xformed_by().

resolve #88460
regression from #69598

@Sauermann Sauermann added this to the 4.3 milestone Feb 18, 2024
@Sauermann Sauermann requested a review from a team as a code owner February 18, 2024 01:57
Global position doesn't get adjusted within `InputEventMouse::xformed_by()`.
@Sauermann Sauermann force-pushed the fix-mouse-event-global-position branch from a573c44 to 8de3991 Compare February 18, 2024 02:26
@akien-mga akien-mga requested review from kleonc and a team February 18, 2024 07:43
Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

Tested a little, seems to work fine now. 👍

The change makes sense to me but I'm not that familiar with the input code though. 🙃


@Sauermann BTW pretty much the only place where Viewport::_make_input_local is used is:

godot/scene/main/viewport.cpp

Lines 3340 to 3345 in 5f05e2b

Ref<InputEvent> ev;
if (!p_local_coords) {
ev = _make_input_local(p_event);
} else {
ev = p_event;
}

So I wonder if InputEventMouse.global_position would be correct/meaningful in case p_local_coords is true? Not sure what is expected / how it's supposed to work, just mentioning in case you overlooked / haven't considered this.
But even it there's something to change it's likely for another PR / this one is good as is.

@Sauermann
Copy link
Contributor Author

Thanks for the comment. p_local_coords == true means, that the coordinates don't need to be adjusted, because the event is already in the coordinate system of the viewport. I believe, that this is indeed the correct place for the change.

@akien-mga akien-mga merged commit 0c4d8d7 into godotengine:master Feb 18, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Sauermann Sauermann deleted the fix-mouse-event-global-position branch February 19, 2024 19:01
@kleonc
Copy link
Member

kleonc commented Feb 20, 2024

Should be fine for cherry-picking?

@Sauermann
Copy link
Contributor Author

Should be fine for cherry-picking?

Yes, it should be fine.

@kleonc kleonc added cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Feb 20, 2024
@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.
Cherry-picked for 4.1.4.

@akien-mga akien-mga removed cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Mar 9, 2024
MikeSchulze added a commit to MikeSchulze/gdUnit4 that referenced this pull request Apr 20, 2024
# Why
Godot v4.2.2 and V4.1.4 are out

# What
- update the workflows and added support for Godot v4.2.2 and V4.1.4
- fix workflow errors using invalid matrix parameters
- fix failing test related to the Godot global_position fix
godotengine/godot#88473
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.

InputEventMouseButton global_position becomes broken when window is resized
3 participants