Configurable mouse focus

[skewballfox]

• resolves [Feature Request] Focus follows mouse for tiling mode #564

Still learning the codebase, and right now this is just some boilerplate. As mentioned in 564 there would be 3 mouse focus policies:

• focus requires click (current and default)
• focus changes when mouse moves to a new output
• focus follows mouse (sloppily or lazily)

The first requires 0 changes and the code for click to focus still needs to execute when following one of the other policies (if there is a delay before window focusing, clicking a window should still focus it.). So the only place code needs to be added is in the two todo!()s currently in src/input/mod.rs. I think I can probably extract the contents of this conditional into a function, with some minor changes to reuse the existing code for moving keyboard focus to a window or region.

[Drakulix]

Thanks for working on this!

Though after some internal discussions we have now a better idea of how this feature should exactly look like: #564 (comment)

[skewballfox]

Thanks for working on this!

Though after some internal discussions we have now a better idea of how this feature should exactly look like: #564 (comment)

no worries. Were you guys planning on doing the refactors separately or should I attempt to do those in this PR?

specifically these changes:

To facilitate this, we will need a bit of refactoring, as to what previously was the active_output of a seat. Parts of the code assume that output will contain a focused element. But with this change conceptionally the "focused output" and the "active output" (the one with the cursor on) will now be different things.

[Drakulix]

no worries. Were you guys planning on doing the refactors separately or should I attempt to do those in this PR?

specifically these changes:

To facilitate this, we will need a bit of refactoring, as to what previously was the active_output of a seat. Parts of the code assume that output will contain a focused element. But with this change conceptionally the "focused output" and the "active output" (the one with the cursor on) will now be different things.

If you want to, you may very well give this a go. Basically you can just grep the whole code base for seat.active_output() and see where it afterwards makes the assumption, that the returned output has a focused element.

I don't think most cases qualify, but those that do should instead do something like seat.get_keyboard().current_focus() to get the focused element and figure out the output by querying the Shell.

Otherwise I'll get around to it eventually.

[skewballfox]

One thing I'm fuzzy on is the places where I've encountered active_output() being used to get the current focused window, they generally involve filtering to a CosmicMapped instance(example: handle_shortcut_action)

Are variants like popups coercable to CosmicMapped or CosmicSurface?

Asking because right now I'm trying implement a function to call during Action::Close on KeyboardFocusTarget but PopupKind::InputMethod(PopupSurface) doesn't have a useful method for this.

[Drakulix]

One thing I'm fuzzy on is the places where I've encountered active_output() being used to get the current focused window, they generally involve filtering to a CosmicMapped instance(example: handle_shortcut_action)

Are variants like popups coercable to CosmicMapped or CosmicSurface?

Popups might be related to their parent:

cosmic-comp/src/shell/focus/target.rs

Line 163 in f02520c

get_popup_toplevel(&xdg).map(Cow::Owned)

Asking because right now I'm trying implement a function to call during Action::Close on KeyboardFocusTarget but PopupKind::InputMethod(PopupSurface) doesn't have a useful method for this.

However I don't think that method works for input-method. What you probably would need to do there (and what cosmic-comp likely fails to do in other places, IME support isn't well-tested yet..) is using get_parent and then shell.element_for_surface.

[skewballfox]

hey for the actions affecting a workspace, such as MigrateWorkspaceToNextOutput, those should also be switched to get the focused output rather than the active output correct?

[skewballfox]

I'm going to assume this is the case, though if I need to roll back changes for those specific actions let me know

[skewballfox]

Give me a few, tried to squash the commits and now it's including changes from other commits in the list of changes

[Drakulix]

hey for the actions affecting a workspace, such as MigrateWorkspaceToNextOutput, those should also be switched to get the focused output rather than the active output correct?

I think for that specific one it might make more sense to use the active output, though there might not be much point into moving an empty workspace? 😅
But my gut feeling would be to use the active_output, whenever an action concerns a whole workspace and use the output with the focused element, whenever the action concerns mostly that element or focus in some way.

I'm going to assume this is the case, though if I need to roll back changes for those specific actions let me know

Sure. Do you want me to review what's already there?

I am usually busy with other stuff and will only check comments every now and then, but if you feel like you want some input on the actual code, please explicitly ping me any time. Otherwise it isn't clear to me, what is just a rebase or warrants a full review.

[skewballfox]

@Drakulix If you could review primarily the changes to handle_shortcut_actions (starting on line 1645 of src/input/mod.rs) I'd appreciate it. mainly to see which actions should be rolled back or if I missed any that need to be switched.

There are a few functions I'm not sure whether to convert or not:

• map_window in the shell module. I suspect it should be.
• update for ContextMenu in src/shell/graphs/mod.rs, mainly because I'm lacking a bit of context of for what this is
• the stuff related to layers, but mainly because I'm not sure what a layer is in this context. I'm assuming it has to do with the z order or windows, but I can't see how a layer would have keyboard focus

edit: Outside of those I think I've moved all the relevant seat.active_output() calls to instead refer to the focused_output, and I added some functions to for handling focused output to seat. I tried running locally (replacing the instance of cosmic-comp in /usr/bin), and something is causing it to crash. I need to look into it more.

[Drakulix]

• ToggleWindowFloating should also use the focused output. (Like ToggleStacking and ToggleSticky, but that is delegated to Shell.)

map_window. I suspect it should be.

No windows should spawn where the cursor is, so active_output.

update for ContextMenu in src/shell/graps/mod.rs, mainly because I'm lacking a bit of context of for what this is

This is the context menu you get by right-clicking header-bars of apps. So the actions in there should always apply to the window, which the menu belongs to. As such that shouldn't need changing.

the stuff related to layers, but mainly because I'm not sure what a layer is in this context.

Those are wlr-layer-shell components, essentially shell-components like the panel or launcher. Those can have focus, but that should also remain on the active-output, so probably no changes necessary. (Though we should double check.)

[skewballfox]

would it be okay if I added a bool argument to set_focus to indicate whether the pointer should also be updated if focus changes?

If a focus change is caused by the pointer(movement or click), then even if pointer follows focus is set it shouldn't change position.

the alternative would be to check if the keyboard focus target is different from the active focus at the current call sites, prior to calling set_focus(along with filtering for positions that would cause set_focus to return early)

if there is another option you'd rather go with, let me know

Edit: Also, would it be okay if I moved the setting of focus data (active focus, keyboard focus) inside a check to make sure the keyboard focus has changed (if keyboard focus target != previous value of active focus)

[Drakulix]

would it be okay if I added a bool argument to set_focus to indicate whether the pointer should also be updated if focus changes?

If a focus change is caused by the pointer(movement or click), then even if pointer follows focus is set it shouldn't change position.

Makes sense. This is the kinda behaviour that isn't readable from the config, because it depends on the type of focus change. So imo a new argument is a good solution.

Edit: Also, would it be okay if I moved the setting of focus data (active focus, keyboard focus) inside a check to make sure the keyboard focus has changed (if keyboard focus target != previous value of active focus)

Setting the focus to the same value is harmless and shouldn't cause any issues.

[skewballfox]

also, I got focus follows mouse working, at least on the local system I've been using for testing

EDIT: update, found a bug with focus follows mouse where if a panel menu (like wifi or system panel) overlaps a window, moving the pointer over the region of the window occluded by the menu focuses the window. I'm guessing those menus don't show up as pointer focus targets. I think this might be relevant if they are subsurfaces. The most recent issue fixed it for overlays, but I haven't figured out how to determine if the keyboard focus target is the top level surface within it's own layer yet, so things like trying to hover over a docked app menu will still focus the underlying window.

also, cursor_follows_focus doensn't work at all, I'm guessing ptr.motion doesn't do what I was expecting.

[skewballfox]

so it turns out the issue might not be related to layer order, and instead a lack of delay for refocus. I can only hover over an overlay panel if I drag the mouse down the side of the output where the window isn't present, then onto the panel. I think the gap between the panel and the popup is stealing focus. I'll roll back that latest commit locally to retest and confirm in the morning.

[skewballfox]

So I looked a bit more into how to handle delaying focus enough to allow time to move the cursor to windows floating over the a focusable window. If we do go with the option to use a delay, we can likely do so with self.common.event_loop_handle.insert_source and a calloop::timer::Timer.

An alternative (and apparently preferred way is this comment about gnome's implementation is still accurate) is to delay changing window focus until the pointer has stopped moving. I haven't figured out how to detect that though.

[skewballfox]

@Drakulix This is ready for review. both settings are working. Thanks for all the help

just to help with you or anyone else who wants to test the new settings

echo true > ~/.config/cosmic/com.system76.CosmicComp/v1/focus_follows_cursor
echo true > ~/.config/cosmic/com.system76.CosmicComp/v1/cursor_follows_focus

[juarezr]

Adding a third possible mode:

• Mouse follows focus
• Useful especially with multiple monitors and keyboard-centric workflows (Alt+tab and Focus prev/next/last keys)

[skewballfox]

@juarezr that's actually what "cursor follows focus" is: the pointer warps to the top left corner of the newly focused window if that focus change was caused by a keyboard event.

it's been implemented in the PR.

[juarezr]

@juarezr that's actually what "cursor follows focus" is: the pointer warps to the top left corner of the newly focused window if that focus change was caused by a keyboard event.

it's been implemented in the PR.

@skewballfox ,

Amazing work!

IMHO, it is still possible to improve the "cursor follows focus" behavior a bit:

• You can move the cursor to the last position it was placed in the window instead of always moving to the top left corner.
• By doing this you take advantage of the locality.
• This is a very basic heuristic but it helps to place the cursor in areas where the user clicks most of the time or in the place it interrupted a previous workflow/task.
• This behavior is implemented in Feature: move mouse with focus to the last position in window forge-ext/forge#310. If by chance you want to try this behavior, just install this gnome extension.
• Of course, this is just a suggestion. ;)

[Drakulix]

A bunch of smaller comments/cleanups/suggestions. In general this seems almost ready to merge. I need to play around with this some more, but I am really liking the result both in feel/behaviour and code.

[skewballfox]

@Drakulix This is ready to be tested again. Let me know if I should keep or revert the cursor_follows_focus changes. No rush though.

[Drakulix]

A couple of issues I am still noticing using this PR:

• All three config options don't reload at runtime. This needs some changes here: https://github.com/pop-os/cosmic-comp/blob/master/src/config/mod.rs#L571
• Changing the active output, doesn't take away focus for empty outputs (as intended!), but the focus indicator still vanishes, this probably needs some changes in the rendering code.
• Toggling tiling/floating doesn't warp the cursor (should it?)
• Switching between workspaces doesn't warp the cursor, if the new target isn't below it (should it?)

On the latter two, I am not sure how aggressive people would expect "cursor_follows_focus" to be, but both scenarios feel like a significant enough change to me, that I would expect it to warp.

However I am certainly not the target audience of any of these config options, so I am very much open to discuss those.

[skewballfox]

EDIT: first two fixed

On the latter two, I am not sure how aggressive people would expect "cursor_follows_focus" to be, but both scenarios feel like a significant enough change to me, that I would expect it to warp.

If I'm being honest, also not the target audience for cursor_follows_focus, or at least I've never used it/seen it in another DE. I don't think it should trigger for events caused by the mouse (such as clicking to launch something from the doc), but that's something I could live with if it's the desired behavior.

I can try to extract, add a function to fire when the focus doesn't change, but for events like the focused window warps into another position

Switching between workspaces doesn't warp the cursor, if the new target isn't below it (should it?)

you mean below the cursor right? as in two windows in the new workspace, the last focused on that workspace was window y, but window x is now under the cursor after the switch?

[theshatterstone]

Hello. I am a Hyprland user, where I have mouse_follows_focus enabled and when it comes to the last 2 bullets by Draculix, toggling between tiling and floating does NOT warp the cursor or focus, and switching between workspaces also doesn't affect or change the focus. The only case in which mouse_follows_focus affects the mouse is when the focus changes via a keybind. So in terms of the way it should work, I don't think such things like points 3 and 4 should be the case.

[Drakulix]

LGTM! Could you do a cargo fmt run and squash that into the commit? I see a bunch of little style nitpicks, that I am sure that would catch. Otherwise this is good to go!

[skewballfox]

It's good to go now

[Drakulix]

Thanks again for all the work and feedback that went into this 🎉!

[KaKi87]

Hi,
How to know when this will be available from APT updates ?
Thanks

[leviport]

I can fit it into today's update release.

[KaKi87]

Wow, wonderful, thanks !

[theshatterstone]

Hello, I know this is far from the best place for it, but does anyone know where/how should I report that cosmic-comp at the COPR repo failed to build, so these don't work yet?

[Drakulix]

Hello, I know this is far from the best place for it, but does anyone know where/how should I report that cosmic-comp at the COPR repo failed to build, so these don't work yet?

https://pagure.io/fedora-cosmic/SIG

[theshatterstone]

I emailed the COPR maintainer and they fixed it, thanks! Though it does feel like there's a slight delay in focus switching.

[skewballfox]

@theshatterstone the delay is adjustable. the default is 250(milliseconds), you could set it to 100ms like this:

echo 100 > ~/.config/cosmic/com.system76.CosmicComp/v1/focus_follows_cursor_delay