Fix mouse warping container #2820

Emantor · 2018-10-11T08:12:28Z

For the drm backend the pointer would warp to 0,0 on window if mouse_warping container was set.
Check that the pointer does not warp to 0,0 and move the arrange functions into view_map.
This is actually a decrease in code size, if somebody has a fancier idea than passing fullscreen and decoration as arguments, I'm all for it. The cursor now correctly warps to the middle of the newly created window.

Edit: codestyle has not been fixed thoroughly, will do that when I can remove the [WIP] comment.

RyanDwyer · 2018-10-11T08:20:20Z

Can't the 0,0 check in seat.c be removed now that the focus happens after the size has been determined?

Emantor · 2018-10-11T08:23:46Z

Yes, I can drop that.

Edit: And its gone.

RyanDwyer · 2018-10-11T08:27:21Z

Code looks good apart from style issues. I want to do some testing though, particularly with unusual cases such as views that map into the scratchpad via criteria and views that launch into fullscreen. Will do the testing later.

RyanDwyer · 2018-10-11T12:32:17Z

When the focused view unmaps, the new cursor position is chosen based on the layout that was there before the view unmapped. For example, create layout H[view view], focus the right view and close it. The cursor will move to the center of the left half.

The fix for this will be in view_unmap. Focus is chosen when the unmap signal is emitted. This will need to come after the arrange.

Emantor · 2018-10-11T15:46:22Z

This will be a lot harder to untangle, since the cursor is refocused in container_begin_destroy:

#0  0x0000555c0236271d in seat_set_focus_warp (seat=0x555c0461fc30, node=0x555c04846090, warp=true, notify=true) at ../sway/input/seat.c:628
#1  0x0000555c02362f6b in seat_set_focus (seat=0x555c0461fc30, node=0x555c04846090) at ../sway/input/seat.c:810
#2  0x0000555c0236143d in handle_seat_node_destroy (listener=0x555c04847460, data=0x555c04862930) at ../sway/input/seat.c:184
#3  0x0000555c02380774 in wl_signal_emit (signal=0x555c04862960, data=0x555c04862930) at /usr/include/wayland-server-core.h:468
#4  0x0000555c02380ac7 in container_begin_destroy (con=0x555c04862930) at ../sway/tree/container.c:90
#5  0x0000555c0238746e in view_unmap (view=0x555c04861ce0) at ../sway/tree/view.c:626

And without the destroyed node, the workspace can't be arranged.

RyanDwyer · 2018-10-12T23:21:24Z

I think the best way to do this is to move the focus setting code out of handle_seat_node_destroy and into view_unmap (or rather, make view_unmap call a new function such as choose_focus and put the logic there).

handle_seat_node_destroy would then be almost a one-liner which calls seat_node_destroy, so those two functions can be merged into one.

Back in view_unmap, you'll probably need to determine needs_new_focus before calling container_begin_destroy, then do the rest afterwards.

Emantor · 2018-10-13T19:18:29Z

I implemented your suggestion. From cursory testing it works, I'm not entirely sure focus is still handled correctly. Please test.

RyanDwyer · 2018-10-14T04:00:22Z

Focus needs to be changed for each seat. This is getting complicated, and I'm beginning to think that warping the cursor a second time is the lesser of two evils. Let me know what you think.

This is out of scope for this PR, but I think seat_set_focus_warp needs to refactored in order to fix this properly. Perhaps some of the additional logic like cursor warping could be moved into the focus and workspace commands. Or perhaps the function should be split into two, where there's a high level function that does all the additional logic and a low level function that only manipulates the focus stack. Then functions like handle_seat_node_destroy could call the low level function. Or perhaps a mix of the two. I'll think about this over the next few days.

Emantor · 2018-10-15T06:34:03Z

Warping the cursor a second time may also introduced other bugs since we can't simply warp the cursor if the focus was already changed and the node to warp to is the same node focus is set on (early return in seat_set_focus_warp).

To the overall focus vs cursor problem:
I think it maybe overall beneficial to separate focus from the cursor movement. At the moment cursor movement and focus warp are very entangled inside seat_set_focus_warp, imo its going to be much clearer when reading the code if the cursor is moved explicitly, instead of in the seat_set_focus_warp function. This also makes it possible to configure the cursor move semantics in depth, if that is requested.

My concern is that it may be hard to achieve this regression free without tests. I noticed the swaywm/sway-tests repository, it may be very useful to have tests for focus and cursor behaviour to cache regressions.

RyanDwyer · 2018-10-15T06:57:51Z

What about creating a cursor_warp_to_container function, and calling it from seat_set_focus_warp and at the end of view_unmap?

Emantor · 2018-10-15T12:00:45Z

Yes, I'll do that when I'm back from work this evening.

Emantor · 2018-10-15T14:26:33Z

Pushed a new version which currently warps the cursor again. Eventually I'd like to refactor all funtions to warp the cursor explicitly and not during seat_set_focus_warp, but that will be left for another PR.

RyanDwyer

Have mouse_warping container in config
Open a tiling view and a floating view
With the tiling view focused, try to do mouse operations on the floating view (mod + drag, or resize a border)
The cursor jumps around undesirably

include/sway/tree/view.h

sway/desktop/xdg_shell.c

sway/input/cursor.c

RyanDwyer · 2018-10-16T07:45:02Z

sway/input/seat.c

+			cursor_warp_to_container(seat->cursor, container);
+			cursor_send_pointer_motion(seat->cursor, 0, true);
+		} else if (warp && new_output != last_output &&
+				   config->mouse_warping >= WARP_CONTAINER) {


I think you intend to use == WARP_OUTPUT instead of >= WARP_CONTAINER.

In the warping to output part, it should just warp to the workspace and not care about containers.

The warp condition exist in both statements, so it could be moved into the if (last_focus) one (err... why is this even conditional on having a last focus?).

I intended to use >= WARP_OUTPUT to ensure we also warp on output change if the WARP_CONTAINER is set. I'll need to read more code to understand whether it still works with == WARP_OUTPUT.

Current code warps to the container on output change, this may regress if we change it to warp only to workspaces

Yes, I'll unify this into the (last_focus) condition. I copied (last_focus) from previous code, I'll look into it.

Judging by git blame, last_focus was used to remove the focus from a view previously. It will probably cause no harm to remove here. What are your thoughts on this @RyanDwyer?

Yep, let's remove it. And by remove I mean replace it with warp.

sway/tree/view.c

Emantor · 2018-10-16T11:56:21Z

Hm, I can't reproduce with your instructions. However I found another bug:

If focus_follows_mouse is set to no and you click into a container, the mouse is warped to the middle of the container.

For mouse_warping cursor to correctly work on newly spawned containers, the workspace needs to be arranged before the cursor is warped. The shell functions each implement their own fullscreen and arrange checks, move them into the view_map function and pass their states via boolean arguments. Fixes swaywm#2819

The new functions allow a cursor to be warped without changing the focus. This is a preparation commit to handle cursor warping not only in seat_set_focus_warp.

If the cursor is warped during the destruction of the workspace, we end up in the wrong position. Warp the cursor after arrange_workspace() so we end up in the correct position.

ddevault · 2018-10-16T21:53:34Z

It doesn't seem that this fixes this issue:

#2815

Were you going to fix these together?

Emantor · 2018-10-17T13:05:54Z

No, I think this focus issue needs to be tackled a different way.
Tracking the relevant container on crossing a border sounds like a good idea, but is out of scope for this PR.

ddevault · 2018-10-17T13:57:09Z

Aight. Well, this patch works. Thanks!

Emantor force-pushed the fix-mouse-warping-container branch from 645f364 to d20ab8c Compare October 11, 2018 08:25

RyanDwyer self-requested a review October 11, 2018 08:27

Emantor mentioned this pull request Oct 13, 2018

Switching workspaces with focus_follows_mouse focuses the wrong window #2815

Closed

Emantor force-pushed the fix-mouse-warping-container branch from d20ab8c to 07be6fd Compare October 13, 2018 19:09

ddevault added this to the sway 1.0-beta.1 milestone Oct 14, 2018

ddevault mentioned this pull request Oct 14, 2018

Sway 1.0 #1735

Closed

Emantor force-pushed the fix-mouse-warping-container branch 2 times, most recently from 0b2d2ec to e711188 Compare October 15, 2018 14:44

RyanDwyer suggested changes Oct 16, 2018

View reviewed changes

Emantor force-pushed the fix-mouse-warping-container branch 3 times, most recently from 40ec9a3 to 082d7e9 Compare October 16, 2018 11:52

Emantor force-pushed the fix-mouse-warping-container branch 3 times, most recently from 6df429c to b3729ba Compare October 16, 2018 13:39

Emantor added 2 commits October 16, 2018 15:47

cursor: functions to warp cursor to container and workspace

0969bf7

The new functions allow a cursor to be warped without changing the focus. This is a preparation commit to handle cursor warping not only in seat_set_focus_warp.

Emantor added 2 commits October 16, 2018 15:47

seat: use new warping functions for cursor warping during focus warp

d69cf4c

view: rewarp cursor during view_unmap

1f0aeae

If the cursor is warped during the destruction of the workspace, we end up in the wrong position. Warp the cursor after arrange_workspace() so we end up in the correct position.

Emantor force-pushed the fix-mouse-warping-container branch from b3729ba to 1f0aeae Compare October 16, 2018 13:47

RyanDwyer approved these changes Oct 16, 2018

View reviewed changes

Emantor changed the title ~~[WIP] Fix mouse warping container~~ Fix mouse warping container Oct 16, 2018

ddevault merged commit 765c80e into swaywm:master Oct 17, 2018

Emantor deleted the fix-mouse-warping-container branch April 17, 2020 08:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mouse warping container #2820

Fix mouse warping container #2820

Emantor commented Oct 11, 2018 •

edited

Loading

RyanDwyer commented Oct 11, 2018

Emantor commented Oct 11, 2018 •

edited

Loading

RyanDwyer commented Oct 11, 2018

RyanDwyer commented Oct 11, 2018 •

edited

Loading

Emantor commented Oct 11, 2018

RyanDwyer commented Oct 12, 2018 •

edited

Loading

Emantor commented Oct 13, 2018 •

edited

Loading

RyanDwyer commented Oct 14, 2018

Emantor commented Oct 15, 2018 •

edited

Loading

RyanDwyer commented Oct 15, 2018

Emantor commented Oct 15, 2018

Emantor commented Oct 15, 2018

RyanDwyer left a comment

RyanDwyer Oct 16, 2018

Emantor Oct 16, 2018

Emantor Oct 16, 2018

RyanDwyer Oct 16, 2018

Emantor commented Oct 16, 2018

ddevault commented Oct 16, 2018

Emantor commented Oct 17, 2018

ddevault commented Oct 17, 2018

Fix mouse warping container #2820

Fix mouse warping container #2820

Conversation

Emantor commented Oct 11, 2018 • edited Loading

RyanDwyer commented Oct 11, 2018

Emantor commented Oct 11, 2018 • edited Loading

RyanDwyer commented Oct 11, 2018

RyanDwyer commented Oct 11, 2018 • edited Loading

Emantor commented Oct 11, 2018

RyanDwyer commented Oct 12, 2018 • edited Loading

Emantor commented Oct 13, 2018 • edited Loading

RyanDwyer commented Oct 14, 2018

Emantor commented Oct 15, 2018 • edited Loading

RyanDwyer commented Oct 15, 2018

Emantor commented Oct 15, 2018

Emantor commented Oct 15, 2018

RyanDwyer left a comment

Choose a reason for hiding this comment

RyanDwyer Oct 16, 2018

Choose a reason for hiding this comment

Emantor Oct 16, 2018

Choose a reason for hiding this comment

Emantor Oct 16, 2018

Choose a reason for hiding this comment

RyanDwyer Oct 16, 2018

Choose a reason for hiding this comment

Emantor commented Oct 16, 2018

ddevault commented Oct 16, 2018

Emantor commented Oct 17, 2018

ddevault commented Oct 17, 2018

Emantor commented Oct 11, 2018 •

edited

Loading

Emantor commented Oct 11, 2018 •

edited

Loading

RyanDwyer commented Oct 11, 2018 •

edited

Loading

RyanDwyer commented Oct 12, 2018 •

edited

Loading

Emantor commented Oct 13, 2018 •

edited

Loading

Emantor commented Oct 15, 2018 •

edited

Loading