view: try to reduce confusion in focused_view tracking

Our current approach to handling the focused/active view is a bit
confusing. In particular, it's hard to be sure when server->focused_view
is or isn't in sync with the real wlroots keyboard focus.

Try to clean things up a bit. In particular:

- Add comments to server->focused_view and desktop_focused_view() to
  clarify that they should match, but it's not guaranteed.

- desktop_focused_view() now prints a warning if it detects that
  server->focused_view is out of sync. We should keep an eye out for
  this warning, and if we see it, try to figure out why it happened.

- For consistency, use only "focus/defocus" as the verbs in function
  names rather than "activate". This is a bit arbitrary, but the idea is
  that focus is the primary action while the active/inactive state is a
  side effect.

- view_focus/defocus() replace view_set_activated() and now update both
  focus and active/inactive state, to try to keep them in sync.

- Add comments at view_focus/defocus() to warn against calling them
  directly (we should generally call the desktop.c functions).

- desktop_focus_view(NULL) is now forbidden and is no longer handled as
  a special case to clear the focus. This was (at least to me) a
  surprising behavior and caused trouble when working on another change.

- To maintain existing behavior, desktop_focus_topmost_mapped_view() now
  explicitly clears the focus if there are no mapped views.

There should be no behavioral change here.
This commit is contained in:
John Lindgren 2023-09-26 01:35:36 -04:00 committed by Johan Malm
parent 003d1fd494
commit 3022985ba7
11 changed files with 100 additions and 60 deletions

View file

@ -187,19 +187,50 @@ _view_set_activated(struct view *view, bool activated)
}
}
/*
* Give the view keyboard focus and mark it active. This function should
* only be called by desktop_focus_view(), which contains additional
* checks to make sure it's okay to give focus.
*/
void
view_set_activated(struct view *view)
view_focus(struct view *view)
{
assert(view);
struct view *last = view->server->focused_view;
if (last == view) {
return;
/* Update seat focus */
struct seat *seat = &view->server->seat;
if (view->surface != seat->seat->keyboard_state.focused_surface) {
seat_focus_surface(seat, view->surface);
}
if (last) {
_view_set_activated(last, false);
/* Update active view */
if (view != view->server->focused_view) {
if (view->server->focused_view) {
_view_set_activated(view->server->focused_view, false);
}
_view_set_activated(view, true);
view->server->focused_view = view;
}
}
/*
* Take keyboard focus from the view and mark it inactive. It's rarely
* necessary to call this function directly; usually it's better to
* focus a different view instead by calling something like
* desktop_focus_topmost_mapped_view().
*/
void
view_defocus(struct view *view)
{
assert(view);
/* Update seat focus */
struct seat *seat = &view->server->seat;
if (view->surface == seat->seat->keyboard_state.focused_surface) {
seat_focus_surface(seat, NULL);
}
/* Update active view */
if (view == view->server->focused_view) {
_view_set_activated(view, false);
view->server->focused_view = NULL;
}
_view_set_activated(view, true);
view->server->focused_view = view;
}
void
@ -377,18 +408,7 @@ _minimize(struct view *view, bool minimized)
view->minimized = minimized;
if (minimized) {
view->impl->unmap(view, /* client_request */ false);
_view_set_activated(view, false);
if (view == view->server->focused_view) {
/*
* Prevents clearing the active view when
* we don't actually have keyboard focus.
*
* This may happen when using a custom mouse
* focus configuration or by using the foreign
* toplevel protocol via some panel.
*/
view->server->focused_view = NULL;
}
view_defocus(view);
} else {
view->impl->map(view);
}