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

input.conf: bind left click to cycle pause #15405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guidocella
Copy link
Contributor

a6683ea and 639ef09 made it possible to bind MBTN_LEFT without conflicting with dragging, so since cycling pause on left click is common in video players, bind it.

Copy link

github-actions bot commented Nov 29, 2024

Download the artifacts for this pull request:

Windows
macOS

@na-na-hi
Copy link
Contributor

This is not a good default when border/titlebar is disabled, or becomes invisible at fullscreen, because it cycles pause when you just want to click to raise/focus it. MPC-HC has the same problem in minimal layouts.

I tried this after I implemented dragging deadzone but was annoyed by this behavior that I only kept this if the window manager has focus follow mouse enabled.

@kasper93
Copy link
Contributor

kasper93 commented Nov 29, 2024

This is not a good default when border/titlebar is disabled, or becomes invisible at fullscreen, because it cycles pause when you just want to click to raise/focus it. MPC-HC has the same problem in minimal layouts.

I tried this after I implemented dragging deadzone but was annoyed by this behavior that I only kept this if the window manager has focus follow mouse enabled.

I agree with this. This is probably the main reason I don't have it bound to LMB either. However, you can focus with any other mouse button as well, so depending on your configuration, it can be avoided. Another issue arises with scripts that bind actions to the left mouse button, in case you to miss the clickable zone.

I think pause is one of the main commands, so it makes sense to bind it to the primary action button on the mouse. However, it's probably not something everyone prefers.

One thing I would like is to have the context menu / select.lua functionality bound to RMB, which is currently occupied by pause.

It would be useful for touch interfaces to pause on tap, though. For touch, it would also make sense to implement touch zones to make some commands more accessible, like pause/play, next/previous, volume up/down, and so on. But that's a separate topic.

EDIT:

For touch the focus problem also exists, so maybe it should trigger only when mpv is focused.

a6683ea and 639ef09 made it possible to bind MBTN_LEFT without
conflicting with dragging, so since cycling pause on left click is
common in video players, and MBTN_RIGHT can be freed to implemnet a menu
later, bind it.
@verygoodlee
Copy link
Contributor

Another problem is that double left-click cycle fullscreen triggers cycle pause twice, which can only be solved by third-party scripts at the moment.
https://github.com/natural-harmonia-gropius/input-event?tab=readme-ov-file#click-to-pause-double-click-to-fullscreen

@paulguy
Copy link

paulguy commented Nov 29, 2024

I like right click toggling pause and left click set in input.conf to set pause no so it's just a way to always unpause and otherwise do nothing. I use a window manager hotkey to grab the borderless window.

@Akemi
Copy link
Member

Akemi commented Nov 29, 2024

personally i am not in favour of binding left click to toggling pause.

though if the goal is to free right click in the long run, like for a 'context menu' (imo right click should probably be a context menu, since that's widely expected anywhere), maybe middle click would be a good alternative.

though changing the default for right clicking will most likely be a painful discussion. personally i am also a bit conflicted about it.

@llyyr
Copy link
Contributor

llyyr commented Nov 30, 2024

The major reason against this is that users will accidentally pause the video when they just want to switch focus or bring the mpv window to the front. I'm considering both to be different things, when I say switch focus I mean that any keyboard input is sent to mpv and when I say "bring the window to the front" I mean that mpv is at the top z-index in the window manager and displayed on top of everything (which can imply focus but not necessarily).

The only way to make this binding not annoying is if mpv could detect user intention somehow, whether they want to bring mpv to front or if they want to pause. This works if we pretend focus-follows-mouse doesn't exist, and "focused" and "non-occluded" window mean the same thing. But that's not the reality on nearly all platforms mpv is used in. In general, I don't think this kind of detection can be done across every platform.

The major reason for this binding is to free up right click for a context menu, but we're nowhere close to having any kind of context menu ready so this change is one to consider for the future and there's no real value in having this done right now.

I don't think middle click works well enough for laptops, but this hypothetical context menu could have the pause key as the first item in the context menu so users can pause by just right clicking twice maybe. Either way, it's hard to see a way forward when there's no context menu implementation even being worked on, or actually we don't even know if people want to merge a context menu in mpv itself vs as a C plugins or something (similar to what was done for win32). This should be revisited in the future when we have those things sorted out.

@guidocella
Copy link
Contributor Author

I think kasper's idea of context menu was to just use select.lua which is trivial and I already did a prototype in few minutes (screenshot at Samillion/ModernZ#256).

It is also trivial to pause only when focused is true if desired.

@llyyr
Copy link
Contributor

llyyr commented Dec 1, 2024

use select.lua

This can't spawn the menu directly under the cursor, can it? I think that should be immediately disqualifying for any context menu implementation.

It is also trivial to pause only when focused is true if desired.

Correct, but we don't only care about if mpv is focused or not. Imagine a focus follows cursor compositor where mpv is partially occluded by another window and the user wants to bring mpv back to top. The moment the cursor enters mpv window, it's already "focused" but it isn't at the top of the window stack. On X11 and probably other platforms, you can check if mpv is partially occluded or not but on Wayland this isn't possible. Wayland lets you track keyboard focus and pointer focus, but you can't detect occlusion unless it's fully occluded.

@guidocella
Copy link
Contributor Author

We can use the native context menu on Windows and fallback to select on other platforms. Also console could be positioned in a different point, it already changes with OSD margins.

@kasper93
Copy link
Contributor

kasper93 commented Dec 1, 2024

This can't spawn the menu directly under the cursor, can it? I think that should be immediately disqualifying for any context menu implementation.

It's quite easy to implement, even with ASS. Position entries under cursor, do opaque background, make font smaller, highlight element under cursor. ASS is not perfect, but it's is doable, should we need it.

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

Successfully merging this pull request may close these issues.

8 participants