Add Event::MouseLeave for windows #821

teddemunnik · 2020-04-09T15:06:11Z

Adds a new new event MouseLeft, and implemented it for windows.
The purpose for this event is to give a possibility of the OS to notifiy the druid app that it is no longer going to receive mouse events for some time. Either because the mouse has left the window rect, or because another app is blocking the mouse cursor.

xStrom · 2020-04-09T15:11:26Z

Maybe there's a better name than MouseLeft? That name just immediately makes me think of the left mouse button. 🤔

MouseLost could be similar length but less ambiguous. There's also MouseExited which seems a bit more cumbersome but still clear.

teddemunnik · 2020-04-09T15:13:59Z

Good point, didn't think of the confusion with left mouse button.. I started with MouseLeave, but then noticed that MouseMoved is past tense. MouseLost sounds good to me!

raphlinus · 2020-04-09T15:46:56Z

I think we generally follow web terminology (in this case, mouseleave) unless there is compelling reason to do otherwise. Though now I see the inconsistency with "mousemoved" as opposed to "mousemove". Hmm.

xStrom · 2020-04-09T15:52:20Z

Following web terms would be fine, except we don't with e.g. MouseMoved while web has mousemove.

raphlinus · 2020-04-09T15:54:35Z

Following web terms would be fine, except we don't with e.g. MouseMoved while web has mousemove.

Indeed. Now I regret that.

xStrom · 2020-04-09T15:58:28Z

Well it's not too late to change it to MouseMove but then there's the question of Event::WindowConnected. Might as well change that to WindowConnect then and have everything consistent.

teddemunnik · 2020-04-09T16:07:39Z

Alright! MouseMove and MouseLeave sounds perfect to me as well. How about I change this PR to be MouseLeave and we potentially rename MouseMove and WindowConnected in a future change then?

xStrom · 2020-04-09T16:13:56Z

Yes I agree with that naming plan @teddemunnik, split it into two PRs.

druid-shell/src/platform/windows/window.rs

xStrom

Here are some initial thoughts. I got some deeper thoughts around this whole situation, but I'll have to describe that later as I'm out of time right now.

xStrom · 2020-04-10T15:30:09Z

druid-shell/src/platform/windows/window.rs

+                    if !s.has_mouse_focus && is_point_in_client_rect(hwnd, x, y) {
+                        s.has_mouse_focus = true;
+                        let mut desc = TRACKMOUSEEVENT {
+                            cbSize: std::mem::size_of::<TRACKMOUSEEVENT>() as DWORD,


Let's use just mem::size_of instead of std::mem::size_of.

xStrom · 2020-04-10T15:36:23Z

druid-shell/src/platform/windows/window.rs

+                            if TrackMouseEvent(&mut desc) == FALSE {
+                                warn!(
+                                    "failed to TrackMouseEvent: {}",
+                                    Error::Hr(HRESULT_FROM_WIN32(GetLastError()))
+                                );


This is an interesting failure possibility. I think it might be better to set s.has_mouse_focus to false so that we'll try again on the next move event. Otherwise we'll forever be without this tracking.

I'm curious in what circumstances this call would fail, besides being passed wrong arguments. But good call with not setting has_mouse_focus.

xStrom · 2020-04-10T15:49:06Z

druid/src/event.rs

+    /// Called when the mouse has left the application area.
+    ///
+    /// The `MouseLeave` event is propagated to the active widget, if
+    /// there is one, otherwise to hot widgets (see `HotChanged`).
+    MouseLeave,


~~the application area~~ the window

Also the event will be propagated to both active and hot widgets. Multiple can be active, and hot widgets will receive this regardless of whether there are active.

It's true, I copied the comment and behavior from MouseMove, but I think they don't match there either?

Probably remnants of an era where druid was single window or something.

The event notifies the app that the cursor is no longer hovering the application window.

When the MouseLeft is propagated, druid automatically clears is_hot.

xStrom · 2020-04-11T22:11:13Z

As per the discussion in zulip let's make MouseLeave internal. Pending #826 this doesn't actually mean anything in terms of code. However let's at least add a note to the Event::MouseLeave documentation stating that it is internal and LifeCycle::HotChanged should be used instead.

xStrom

Okay so #826 is moving forward a bit quicker than anticipated. As such I'll just merge this as is and deal with the event internalizing as part of #826.

teddemunnik changed the title ~~Add Event::MouseLeft for windows~~ Add Event::MouseLost for windows Apr 9, 2020

xStrom reviewed Apr 9, 2020

View reviewed changes

druid-shell/src/platform/windows/window.rs Outdated Show resolved Hide resolved

teddemunnik force-pushed the windows_mouse_leave branch from 6a84c74 to dadd264 Compare April 9, 2020 17:36

teddemunnik changed the title ~~Add Event::MouseLost for windows~~ Add Event::MouseLeave for windows Apr 9, 2020

teddemunnik mentioned this pull request Apr 10, 2020

Rename Event::MouseMoved to Event::MouseMove #825

Merged

xStrom reviewed Apr 10, 2020

View reviewed changes

cmyr added the S-needs-review waits for review label Apr 10, 2020

teddemunnik added 7 commits April 10, 2020 21:38

Add windows specific mouse_leave event on druid shell

c816333

The event notifies the app that the cursor is no longer hovering the application window.

Add MouseLeft event to druid

908c4f4

When the MouseLeft is propagated, druid automatically clears is_hot.

Handle error for TrackMouseEvent

4261139

Rename MouseLeft -> MouseLeave

1e9e0ac

Fix comment in WM_MOUSEMOVE

7eeb2fc

Some smaller changes from code review

20b8604

Only set has_mouse_focus when TrackMouseEvent succeeds.

a92e988

teddemunnik force-pushed the windows_mouse_leave branch from 95026b6 to a92e988 Compare April 10, 2020 19:41

xStrom mentioned this pull request Apr 11, 2020

Mouse behavior status report #828

Open

xStrom added S-waiting-on-author waits for changes from the submitter and removed S-needs-review waits for review labels Apr 11, 2020

xStrom approved these changes Apr 13, 2020

View reviewed changes

xStrom merged commit f3a755f into linebender:master Apr 13, 2020

teddemunnik deleted the windows_mouse_leave branch May 9, 2020 21:56

xStrom removed the S-waiting-on-author waits for changes from the submitter label May 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Event::MouseLeave for windows #821

Add Event::MouseLeave for windows #821

teddemunnik commented Apr 9, 2020

xStrom commented Apr 9, 2020

teddemunnik commented Apr 9, 2020

raphlinus commented Apr 9, 2020 •

edited

Loading

xStrom commented Apr 9, 2020

raphlinus commented Apr 9, 2020

xStrom commented Apr 9, 2020

teddemunnik commented Apr 9, 2020

xStrom commented Apr 9, 2020

xStrom left a comment

xStrom Apr 10, 2020

xStrom Apr 10, 2020

teddemunnik Apr 10, 2020

xStrom Apr 10, 2020

teddemunnik Apr 10, 2020

xStrom Apr 10, 2020

xStrom commented Apr 11, 2020

xStrom left a comment

Add Event::MouseLeave for windows #821

Add Event::MouseLeave for windows #821

Conversation

teddemunnik commented Apr 9, 2020

xStrom commented Apr 9, 2020

teddemunnik commented Apr 9, 2020

raphlinus commented Apr 9, 2020 • edited Loading

xStrom commented Apr 9, 2020

raphlinus commented Apr 9, 2020

xStrom commented Apr 9, 2020

teddemunnik commented Apr 9, 2020

xStrom commented Apr 9, 2020

xStrom left a comment

Choose a reason for hiding this comment

xStrom Apr 10, 2020

Choose a reason for hiding this comment

xStrom Apr 10, 2020

Choose a reason for hiding this comment

teddemunnik Apr 10, 2020

Choose a reason for hiding this comment

xStrom Apr 10, 2020

Choose a reason for hiding this comment

teddemunnik Apr 10, 2020

Choose a reason for hiding this comment

xStrom Apr 10, 2020

Choose a reason for hiding this comment

xStrom commented Apr 11, 2020

xStrom left a comment

Choose a reason for hiding this comment

raphlinus commented Apr 9, 2020 •

edited

Loading