fix: options.click_threshold

[christoph-heinrich]

It's a stripped down version of https://gist.github.com/christoph-heinrich/a3e7fdf1ed24e8625f5f9934ebe1313a

Clicks are only registered when the window can't be dragged around. I don't know if that is how you want it to behave, but with window dragging also being left click, it's ambiguous how it should behave.

Also dragging when the window is currently not dragable is also filtered out with a threshold.

[hooke007]

Nope...click_threshold = 5000 shares the same behavior with 1000 or 2000

3a759d2.mp4

pr.mp4

[christoph-heinrich]

When I set it to 1000 and press the left mouse button for over a second, the command does not get executed. But if I set it to 5000 and hold the mouse button for a bit over a second (like before) it does get executed.

I fail to see the problem.

[hooke007]

The previous : click → execute the command 1000ms later
Now : click → execute the command at once
You logic sounds reasonable, but it behaves differently than before. Maybe I got it wrong.

[po5]

Why doesn't this respect --input-doubleclick-time?

[natural-harmonia-gropius]

These very odd patterns lead "double click" to double "double click" for me.

up,down,up
up,down,up,down,up
down,up,down,up,down
up,down,up,down,up,down

[tomasklaen]

This is way more code than I'm willing to dedicate to this feature. It also doesn't solve the main issue the old one had: it doesn't filter out down->drag->up as not a click.

I've searched the docs, tried some stuff too, and also asked on IRC, and afaik there's no way to detect window dragging, so this seems unsolvable.

We should then just simplify/improve the old implementation. Here's what I came up with:

-- Click detection
if options.click_threshold > 0 then
	-- Executes custom command for clicks shorter than `options.click_threshold`
	-- while filtering out double clicks.
	local click_time = options.click_threshold / 1000
	local doubleclick_time = mp.get_property_native('input-doubleclick-time') / 1000
	local last_down, last_up = 0, 0
	local click_timer = mp.add_timeout(math.max(click_time, doubleclick_time), function()
		if last_up > last_down and last_up - last_down < click_time then mp.command(options.click_command) end
	end)
	click_timer:kill()
	mp.set_key_bindings({{'mbtn_left',
		function() last_up = mp.get_time() end,
		function()
			last_down = mp.get_time()
			if click_timer:is_enabled() then click_timer:kill() else click_timer:resume() end
		end,
	},}, 'mouse_movement', 'force')
	mp.enable_key_bindings('mouse_movement', 'allow-vo-dragging+allow-hide-cursor')
end

If you don't see anything wrong with it just paste it there and lets be done with it. (I prefer set_key_bindings as it overwrites input.conf, which should happen if you enable click_command in uosc imo)

[tomasklaen]

Oh and I guess add a click_threshold note in uosc.conf specifying that command execution always waits for input-doubleclick-time.

[natural-harmonia-gropius]

filter out down->drag->up as not a click.

What about add an invisible element on the back and make it does not cover the top bar.
Like dragging the timeline doesn't make the window move.

It makes things complicated and doesn't solve the problem, sorry

[christoph-heinrich]

Why doesn't this respect --input-doubleclick-time?

Now it does.

This is way more code than I'm willing to dedicate to this feature. It also doesn't solve the main issue the old one had: it doesn't filter out down->drag->up as not a click.

It sure does for me. Maybe you didn't drag enough? Currently it filters out drags of 30px or more, but I can reduce the distance if that's too much for you.

I've searched the docs, tried some stuff too, and also asked on IRC, and afaik there's no way to detect window dragging, so this seems unsolvable.

Look at the code, there is a window_drag variable. It is true when the window is being dragged.

I prefer set_key_bindings as it overwrites input.conf, which should happen if you enable click_command in uosc imo

Alright, it's forced now.

These very odd patterns lead "double click" to double "double click" for me.

up,down,up
up,down,up,down,up
down,up,down,up,down
up,down,up,down,up,down

How do you even start with a button up?
down,up,down,up,down doesn't produce two double clicks anymore.

[natural-harmonia-gropius]

How do you even start with a button up?

After a long_click

[tomasklaen]

Hm? It doesn't work for me at all (win 10). The mbtn_left fires only on button down or up. It doesn't fire when I start dragging the window. And from what I can see the window_drag is true if up fired less than 20ms after down? Even if dragging would fire mbtn_left events, this is not correct. What if I'd do down->wait_30ms->drag->up? Dragging wouldn't get detected.

[christoph-heinrich]

Well I can't test it on windows, and I had expected that those events will behave the same on every platform.
Here on linux using kwin on wayland it get the following behavior from mbtn_left. The number is the time since the last event.

fullscreen.mp4

windowed.mp4

As you can see, in windowed mode I get the up event very shortly after the down event because of the window dragging, but in fullscreen I get the up event when the mouse button actually gets released.

If you don't see anything wrong with it

I can't get it to not register single clicks in windowed mode. Do we even want to register single clicks in windowed mode? Imo only registering single clicks when the window can't be dragged (e.g. fullscreen) is fine. Otherwise it will lead to unintentional clicks when dragging the window.
Dragging doesn't get ignored even in fullscreen, where down > move > up can be detected with the coordinates from mouse-pos.

[natural-harmonia-gropius]

move can be detected with the coordinates from mouse-pos.

for me (win11) in windowed mode, It stopped when dragging.

mp.observe_property('mouse-pos', 'native', function(_, mouse)
    print(mouse.hover, mouse.x, mouse.y)
end)

[christoph-heinrich]

After a long_click

I see. Does it work fine now after my last change? For me it behaves as expected.

for me (win11) in windowed mode, It stopped when dragging.

Yes in windowed mode, but not in fullscreen. That's why detecting window dragging is important.
It does give one event at the end of window dragging though.

[christoph-heinrich]

I've made the gist into it's own repository now https://github.com/christoph-heinrich/mpv-pointer-event
If we can manage to get very robust detection on all platforms and good configurability, then there is really no need for that functionality in uosc anymore. The volume/seeking/pause/menu open implementation is commented out for easier testing.

[christoph-heinrich]

Just tested it on a touch device (also kwin wayland) and input handling has to change a bit to be able to handle touch...

[tomasklaen]

That's why detecting window dragging is important.

The issue is that there is no way to do so afaik. Neither mouse-pos nor mbtn_left events fire when window starts to be, is, or ends being dragged on windows. Everything is silent.

I can't get it to not register single clicks in windowed mode...

Hm? Why would you expect click_command to work only in fullscreen?

Otherwise it will lead to unintentional clicks when dragging the window.

Yes, that's the limitation of the current environment. User has to drag for at least click_threshold of time to not get a click, and it's upon them to configure it to a time they are comfortable with.

[christoph-heinrich]

That's why detecting window dragging is important.

The issue is that there is no way to do so afaik.

Time based. If down and up event are too close together, it's because window dragging has started. We can reduce the 20ms if you think they are too long, but I don't think it's possible to actually click so fast with your mouse to achieve a less then 20ms duration between those two events. Also hover goes to false right after, but I think time based is enough.

I can't get it to not register single clicks in windowed mode...

Hm? Why would you expect click_command to work only in fullscreen?

Because of window dragging 😉. Also with "can't get it to not register single clicks" I mean that no matter how long I press the button, it still executes the command in windowed mode.

[tomasklaen]

Time based. If down and up event are too close together

I'll repeat, there are no additional events triggered by dragging on windows.

actions:  left mouse button down  ->  drag X pixels  ->  left mouse button up
 events:  mbtn_left_down          ->     NOTHING     ->  mbtn_left_up

There's a single mbtn_left_down and a single mbtn_left_up fired for down->up, AND down->drag->up. There's no other mbtn_* or mouse-pos events in between. That 20ms check never triggers, as there is nothing to check.

no matter how long I press the button, it still executes the command in windowed mode

Are you sure you pasted the snippet properly? It works fine for me in both fullscreen or windowed, and I don't see any reason why it shouldn't. There is no special handling for the two.

[christoph-heinrich]

Are you sure you pasted the snippet properly? It works fine for me in both fullscreen or windowed, and I don't see any reason why it shouldn't. There is no special handling for the two.

It's because I get an up event shortly after the down event in windowed mode. last_up is then very close to last_down, which means in the timer last_up - last_down < click_time is true and it's registered as a click shorter then click_time even though my mouse button is still down in reality.

Events being fired differently on different platforms seems like a problem on the mpv side, maybe we should open an issue over there?

[tomasklaen]

It's because I get an up event shortly after the down event in windowed mode

Oh, yeah that needs to be filtered out then. But the point stands: we can't detect drags.

So I guess just take my snippet, and add the 20ms check to up handler to filter out the noise.

The main reason why I want this as part of uosc is to allow people to drag and pause video with primary mouse button, so that they can open menu with the secondary, as that is the convention everywhere.

[christoph-heinrich]

Adding it to the timer required fewer changes and has the same effect.

Btw touch input support in uosc is pretty bad and I have no idea how to fix it... the problem is that the down event happens before the move event, and since we're only binding the mouse button when it's over an element, that gets missed and it looks like the mouse moved...

Edit: Switching to mouse-pos made touch input worse. I've mentioned the problems in an issue over on mpv, let's see if there is a solution at some point.