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

Bug: click areas out of menu would triggers MBTN_LEFT #23

Closed
dyphire opened this issue Jan 4, 2024 · 10 comments
Closed

Bug: click areas out of menu would triggers MBTN_LEFT #23

dyphire opened this issue Jan 4, 2024 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@dyphire
Copy link
Contributor

dyphire commented Jan 4, 2024

ref #19 (comment), #19 (comment)

@tsl0922
Copy link
Owner

tsl0922 commented Jan 9, 2024

I can't find a regular way to resolve it well.

Although I can set a flag before menu open, but it's pretty tricky.

  • The event that trigger menu close may be mouse click(inside window or outside the window) or a key (ESC), or other?
  • TrackPopupMenuEx will not return until menu close (should be running it's own event loop), there's no good way to distinguish the event source.

@tsl0922 tsl0922 added the help wanted Extra attention is needed label Jan 9, 2024
@butterw
Copy link
Contributor

butterw commented Jan 9, 2024

As a workaround you can include an empty command in the menu, clicking it closes the menu.
Keyboard is the usual way to dismiss the menu with Esc or Alt.

@dyphire
Copy link
Contributor Author

dyphire commented Jan 9, 2024

I can't find a regular way to resolve it well.

Although I can set a flag before menu open, but it's pretty tricky.

  • The event that trigger menu close may be mouse click(inside window or outside the window) or a key (), or other?ESC
  • TrackPopupMenuEx will not return until menu close (should be running it's own event loop), there's no good way to distinguish the event source.

The real problem here is that it blocks the mpv keybinding when the menu is opened, and triggers the event of closing the menu when the left mouse button is clicked; After the menu is closed, it will continue to forward mouse events to trigger the mpv predetermined keybinding function. If we can prevent forwarding left mouse button events when the menu is closed, it will fix this issue.

@butterw
Copy link
Contributor

butterw commented Jan 9, 2024

I've checked on native windows video players, and the way they handle it is: when menu is open: block player left-click command and bind it to dismiss menu.
I use left-click to toggle play/pause.

@Youxikong
Copy link
Contributor

How about replacing MBTN event's command when menu open and reversing it when menu closing?

@tsl0922
Copy link
Owner

tsl0922 commented Jan 14, 2024

It's possible, but a little tricky, because you may get multiple bindings to MBTN_LEFT:

image

@tsl0922
Copy link
Owner

tsl0922 commented Jan 15, 2024

Well, it's working now (EDIT: not always working, because script message is async).

--- a/lua/dyn_menu.lua
+++ b/lua/dyn_menu.lua
@@ -477,6 +477,14 @@ mp.register_script_message('update', function(keyword, json)
     menu_items_dirty = true
 end)
 
+mp.register_script_message('menu-open', function()
+    mp.add_forced_key_binding('MBTN_LEFT', 'click_ignore')
+end)
+
+mp.register_script_message('menu-close', function()
+    mp.remove_key_binding('click_ignore')
+end)
+
 -- commit menu items when idle, this reduces the update frequency
 mp.register_idle(function()
     if menu_items_dirty then
diff --git a/src/menu.c b/src/menu.c
index 8cc553d..665d5df 100644
--- a/src/menu.c
+++ b/src/menu.c
@@ -266,8 +266,10 @@ void show_menu(plugin_ctx *ctx, POINT *pt) {
     if (!PtInRect(&rc, *pt)) return;

     ClientToScreen(ctx->hwnd, pt);
+    mpv_command(ctx->mpv, (const char *[]){"script-message", "menu-open", NULL});
     TrackPopupMenuEx(ctx->hmenu, TPM_LEFTALIGN | TPM_LEFTBUTTON, pt->x, pt->y,
                      ctx->hwnd, NULL);
+    mpv_command(ctx->mpv, (const char *[]){"script-message", "menu-close", NULL});
 }

@natural-harmonia-gropius

because script message is async

I think the reason is that the event is deferred. Works fine to me with additional timeout.

https://github.com/natural-harmonia-gropius/mpv-menu-plugin/commit/7b2a04f87d298ceed3c5a67552566d265471d514

@dyphire
Copy link
Contributor Author

dyphire commented Jan 31, 2024

because script message is async

I think the reason is that the event is deferred. Works fine to me with additional timeout.

natural-harmonia-gropius@7b2a04f

I can confirm that it works.

@tsl0922
Copy link
Owner

tsl0922 commented Feb 4, 2024

I've added the menu-open and menu-close message to the C plugin, you may want to patch it yourself.

@tsl0922 tsl0922 closed this as completed Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants