-
Notifications
You must be signed in to change notification settings - Fork 326
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 hover events. #7177
Fix mouse hover events. #7177
Conversation
let left = currently_hovered.iter().filter(|t| !new_hovered.contains(t)); | ||
for d in left { | ||
let out_event = event.clone().unchecked_convert_to::<mouse::Out>(); | ||
let leave_event = event.clone().unchecked_convert_to::<mouse::Leave>(); | ||
d.emit_event(out_event); | ||
d.emit_event_without_bubbling(leave_event); | ||
}); | ||
self.pointer_target_registry.with_mouse_target(new_target, |t, d| { | ||
t.mouse_over.emit(()); | ||
} | ||
let entered = new_hovered.iter().filter(|t| !currently_hovered.contains(t)); |
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.
The O(N x M)
scaling of these Vec
set operations worries me a little. The display object hierarchy is already deep, and it could reach a depth where it suddenly becomes a performance problem.
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 just assumed that even if object hierarchy is 50 or so levels deep, iterating over 50 elements may be still better than allocating additional HashMaps
.
But actually, we can easily avoid allocation while improving complexity by just sorting the vecs. I'll push changes soon.
Co-authored-by: Kaz Wesley <[email protected]>
@@ -138,6 +138,8 @@ pub struct Mouse { | |||
/// sampled and the most recent sample is stored here. Please note, that this sample may be | |||
/// from a few frames back, as it is not guaranteed when we will receive the data from GPU. | |||
pub pointer_target_encoded: Uniform<Vector4<u32>>, | |||
/// The display objects which believe they are currently hovered by mouse. |
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.
Is there a reason that we ever believe one is hovered while if it is not hovered? If so we should mention the caveats here. Or just say that this are the currently hovered items.
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.
The caveat is that their information may be outdated (we read what shape is under the mouse with some offset, no greater than several frames). I'll extend the comment here.
QA passed 🍏 |
Pull Request Description
Fixes #6850 - at least scenario described in the issue's description. Probably many other issues with unclickable/unhoverable elements will go.
hovering-fixed-2023-06-30_16.25.09.mp4
Plainly, the solution described in this comment was implemented.
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.