-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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 mouse position with screen transform #77923
Fix mouse position with screen transform #77923
Conversation
scene/main/viewport.cpp
Outdated
@@ -1351,7 +1351,12 @@ Ref<InputEvent> Viewport::_make_input_local(const Ref<InputEvent> &ev) { | |||
|
|||
Vector2 Viewport::get_mouse_position() const { | |||
ERR_READ_THREAD_GUARD_V(Vector2()); | |||
if (DisplayServer::get_singleton()->has_feature(DisplayServer::FEATURE_MOUSE)) { | |||
if (!is_inside_tree() || (Object::cast_to<SubViewport>(this) && !Object::cast_to<SubViewportContainer>(get_parent()))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this condition can be rewritten based on some already existing is-attached-to-screen functionality or at least should be put in a new virtual bool is_attached_to_screen()
, or the like.
Maybe not. It's that checking the type of this
seems a bit odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not aware of any existing functionality, that covers this.
is_attached_to_screen
could be something like:
bool SubViewport::is_attached_to_screen() const {
return Object::cast_to<SubViewportContainer>(get_parent()) && get_parent()->get_viewport() && get_parent()->get_viewport()->is_attached_to_screen();
}
bool Window::is_attached_to_screen() const {
if (get_embedder()) {
return get_embedder()->is_attached_to_screen();
}
return is_inside_tree();
}
Alternatively we could introduce a virtual function:
bool Viewport::is_window() const {
return true;
}
bool SubViewport::is_window() const {
return false;
}
And replace Object::cast_to<SubViewport>(this)
by !_is_window()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the PR and introduced a is_directly_attached_to_screen
method.
When a Viewport is not directly attached to the screen, the function `Viewport::get_mouse_position` can't rely on `get_screen_transform`, because that function is ambiguous in these situations. In these cases it is necessary to use the mouse position from the most recent mouse IputEvent.
90db2cf
to
d1fa284
Compare
Thanks! |
When a Viewport is not in the scene tree or not a child of a
SubViewportContainer
, the functionViewport::get_mouse_position
can't rely onget_screen_transform
, because that function is ambiguous in these situations.In these cases it is necessary to use the mouse position from the most recent mouse InputEvent.
resolve #74868