-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Refactor mouse_entered
and mouse_exited
signals
#67791
Refactor mouse_entered
and mouse_exited
signals
#67791
Conversation
I disagree with adding new signals that differ from the existing ones as a single mouse cursor can only be in one location at a time so it cannot be both inside and outside of a region at the same time which is what is implied by having multiple signals. No other UI toolkit that I know has two separate entered/exited signals that have different rules. Instead I think the existing signals are incorrectly implemented and need to be modified to correctly detect when the cursor enters and exits a region under all circumstances. |
@lmurray I also would prefer to replace the old signals. However i have not yet tested the places, where the old signals are used within the code-base. So I provided new signals in this initial version of this PR, so that they can be tested and compared. |
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.
Adding a new signals/notifications is probably unnecessary, it should replace existing viewport signals/methods NOTIFICATION_VP_MOUSE_*
(for native windows there's NOTIFICATION_WM_MOUSE_*
).
1ac437e
to
d4bd57c
Compare
@bruvzg thanks for your insight. I left My next step will be investigating, if replacing the |
d4bd57c
to
49e00e2
Compare
49e00e2
to
1277eee
Compare
With the latest update, I have implemented a replacement for the
|
Since the Windows and linuxbsd Display server behave quite differently, it makes sense to merge this PR only after the differences are solved. The PR #67903 makes this happen. |
1277eee
to
a8c5bd4
Compare
a8c5bd4
to
99dcf52
Compare
99dcf52
to
161224e
Compare
161224e
to
1ed7b0a
Compare
1ed7b0a
to
4526a14
Compare
mouse_entered
and mouse_exited
signals
I just updated to v4.2.dev3.official [013e8e3] from v4.1.1 and I'm having a problem with what I suspect is this in particular.
Now they do not work at all. Can I get a quick glance at this and maybe someone say why it needs to be this way when it did work. I'd like to know if this might be a bug, and I need to provide more info or documentation or if I need to rewrite how I'm handling capture on this. I'd really rather not as it was more a pain to poll mouse events before I just settled on using mouse enter and exit as the main part of measuring capture. |
@1xch from your short description, it is difficult to guess, what the problem might be. I just noticed, that you use a |
I think a clearer statement is that G_Window & G_Pane in the above are controls that previously responded with mouse_entered & mouse_exited signals and now they do not. Both nodes still receive InputEvents, my core issue being the signals and not knowing why they aren't triggering when they were and should to my understanding. edit: OK, so the signals work in this minimal replication, but in an unexpected manner from previous expectations. Going into G_Pane from G_Window triggers exit from G_Window and enter of G_Pane, and vice versa where previous there was an overlapping. This ruins my workflow and I do not like it. That being said my project still does not have mouse enter/exit signals as above at all and I cannot figure out why, so I probably need to I don't know yet. |
@1xch Thanks for the MRP. I have tested it in v4.0.stable.official [92bee43], v4.1.1.stable.official [bd6af8e] and in v4.2.dev.custom_build [fa3428f] on Linux. Your MRP behaved identical in my tests: when moving the mouse from I have tested the behavior in 3.5.1 and also there the parent Control receives a mouse-exited signal, when the mouse moves to a child-Control. I would also kindly ask you to make future MRP less complicated: You use multiple |
Has there been a discussion about this somewhere? I see the work started back in 2022. I personally don't understand why the Props to @Sauermann for all the work and now adding PRs to notify users in the docs, but this seems to me like it's just letting users know that events will not behave correctly. I think more discussion should definitely happen in #82530 , which sums up the situation quite well. |
Yeah I gotta say, this broke my game too, I was relying on mouse pass to pass the mouse signal down while still being able to use the mouse entered and mouse exited signal. I have a control node that is further down the node stack that uses the mouse signal for a tooltip that will no longer work. Why make these changes? Is there any plans to add mouse pass work? |
@emirljuca That issue should already be resolved in master as of #84547. |
That is great to hear, testing on newest build seems to show this to be the case, thanks! |
fix variant of #20881 where Controls are in different Viewports
fix #66293
fix no longer #57349 (that is already fixed for the case of a single Viewport)
prerequisite for #67531
prerequisite for #77781
addresses #48634 after #77933
related to #54565 (maybe supersedes it)
done: don't merge before #67903 (fix display-server differences between Windows and linuxbsd)
The current implementation for signals
mouse_entered
andmouse_exited
have shortcomings that relate to focused windows and pressed mouse buttons. For example aControl
can be hovered by mouse, even if it is occluded by an embeddedWindow
.This patch changes the behavior, so that
Control
andViewports
send their mouse-enter/exit-notifications based solely on mouse position, visible area and input restrictions and not on which window has focus or which mouse buttons are pressed. This implicitly also changes when themouse_entered
andmouse_exited
signals are sent.Rewriting a core functionality like
mouse_entered
andmouse_exited
should be tested thoroughly. While I tested it in lots of situations and had a look at the locations where it is used in the code base, it can be, that I have missed something.So far, tests on the following platforms were successful:
Other platforms:
mouse_entered
andmouse_exited
makes sense in this context.The concept for this implementation is:
MRP: BugVisualMouseNotifications.zip (updated 2022-10-26)
This functionality can not be implemented as a part of
Viewport::_gui_input_event
, because of its interplay with Windows and becauseViewport::_gui_input_event
is based on input and not on visibility.BugVisualMouseNotification.mp4
Updated 2022-11-13: Fix location of
sw->_update_mouse_over
call.Updated 2022-11-15: Fix merge conflict with #68019
Updated 2022-11-19: Fix location of
sw->_update_mouse_over
call.Updated 2023-01-06: Fix merge conflict with #69972
Updated 2023-01-27: Fix merge conflict with #66692
Updated 2023-02-18: Fix merge conflict with #72764
Updated 2023-06-07: Fix merge conflict with #77933
Updated 2023-06-29: Made necessary adjustments for Gui in 3D Demo
Updated 2023-07-07: Fix merge conflict with #78078
Updated 2023-07-27: Fix merge conflict with #79248