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

x11_common: handle window dragging in ButtonPress event #13457

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

na-na-hi
Copy link
Contributor

Begin the _NET_WM_MOVERESIZE window dragging in ButtonPress event to match the behavior of win32 and wayland, simplify logic by dropping the win_drag_button1_down hack required by the old method, and fix a race condition described in commit 19f101d, which happens when moving the mouse and releasing the button at the same time.

The race condition can be easily triggered when using a touch screen (tested with libinput driver), where a tap is translated to MotionNotify, ButtonPress, MotionNotify, and ButtonRelease in sequence, with the last 2 events having the same timestamp. This has caused some window managers to not stop dragging after the ButtonRelease, resulting in window being stuck in dragging state after a single tap.

Begin the _NET_WM_MOVERESIZE window dragging in ButtonPress event
to match the behavior of win32 and wayland, simplify logic by dropping
the win_drag_button1_down hack required by the old method, and fix a race
condition described in commit 19f101d,
which happens when moving the mouse and releasing the button at the same time.

The race condition can be easily triggered when using a touch screen
(tested with libinput driver), where a tap is translated to MotionNotify,
ButtonPress, MotionNotify, and ButtonRelease in sequence, with the last 2
events having the same timestamp. This has caused some window managers to
not stop dragging after the ButtonRelease, resulting in window being stuck
in dragging state after a single tap.
@sfan5 sfan5 self-requested a review February 14, 2024 20:15
Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

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

Works for me

@sfan5 sfan5 merged commit 531704e into mpv-player:master Feb 21, 2024
10 of 11 checks passed
@sfan5
Copy link
Member

sfan5 commented Feb 26, 2024

This breaks double-clicking to fullscreen for me on xfwm4.
The windows resizes, becomes borderless but isn't moved to 0,0.

possibly(?) related bug: if you double-click to fullscreen and hold the second click the fullscreening will trigger but you are still able to drag the mpv window around.
this works before and after this PR so might not be related at all.

edit: more info:
before this PR the drag cursor only appears if I click and move the mpv window.
after this PR the drag cursor appears as soon as I click even without moving. I suppose the problem here could be that the double click counts as a drag.

@na-na-hi
Copy link
Contributor Author

na-na-hi commented Feb 26, 2024

This seems to depend on the particular behavior of different window managers. One wm I used deferred the PropertyNotify of the fullscreen state to after the dragging is done so it worked. But another wm did not and entered fullscreen immediately after the second mouse down although it was moved to 0,0, but the fullscreen window moved when I moved the mouse afterwards while keep holding it down.

I suppose the problem here could be that the double click counts as a drag.

The root cause of all these window dragging problems is that the vo and the input system don't have enough information to work with them. The vo has no clue whether a click is a double click or not because it's determined by the input system. I already had to deal with this problem on win32 with a hack to freeze the window in place if the dragging is already happening in fullscreen state.

On the other hand, single click has the problem of conflicting with mouse key bindings like #7563. Now the input system doesn't know if window dragging has started because the vo doesn't convey it.

I think the only proper solution to this mess is to centralize vo dragging so that the system that handles it has sufficient information to determine the correct behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants