From da1e2327577ac41e222953428e9a739173e24c86 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fernando=20Tarl=C3=A1=20Cardoso=20Lemos?= Date: Sat, 26 May 2012 17:37:45 -0300 Subject: [PATCH] Refined the fullscreen focus constraints. Basically, a focus change can't escape a fullscreen container. The only exception is per-output fullscreen containers, as you should be able to focus a container in a different workspace in this case. This is an improvement on 4eab046e, now considering the difference between global and per-output fullscreen and taking the tree structure into account to determine what escaping the fullscreen container means. It only affects targeted focus commands in the form "for_window [...] focus", but it lays the foundation for forthcoming fixes to all other focus commands. --- include/con.h | 23 +++++++ src/commands.c | 10 +-- src/con.c | 56 +++++++++++++++++ testcases/t/156-fullscreen-focus.t | 98 +++++++++++++++++++++++------- 4 files changed, 157 insertions(+), 30 deletions(-) diff --git a/include/con.h b/include/con.h index b14c477e..95726147 100644 --- a/include/con.h +++ b/include/con.h @@ -255,4 +255,27 @@ void con_set_layout(Con *con, int layout); */ Rect con_minimum_size(Con *con); +/** + * Returns true if changing the focus to con would be allowed considering + * the fullscreen focus constraints. Specifically, if a fullscreen container or + * any of its descendants is focused, this function returns true if and only if + * focusing con would mean that focus would still be visible on screen, i.e., + * the newly focused container would not be obscured by a fullscreen container. + * + * In the simplest case, if a fullscreen container or any of its descendants is + * fullscreen, this functions returns true if con is the fullscreen container + * itself or any of its descendants, as this means focus wouldn't escape the + * boundaries of the fullscreen container. + * + * In case the fullscreen container is of type CF_OUTPUT, this function returns + * true if con is on a different workspace, as focus wouldn't be obscured by + * the fullscreen container that is constrained to a different workspace. + * + * Note that this same logic can be applied to moving containers. If a + * container can be focused under the fullscreen focus constraints, it can also + * become a parent or sibling to the currently focused container. + * + */ +bool con_fullscreen_permits_focusing(Con *con); + #endif diff --git a/src/commands.c b/src/commands.c index 1e1ee0ff..b3c955e5 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1262,13 +1262,9 @@ void cmd_focus(I3_CMD) { if (!ws) continue; - /* Don't allow the focus switch if the focused and current - * containers are in the same workspace. */ - if (focused && - focused->type != CT_WORKSPACE && - focused->fullscreen_mode != CF_NONE && - con_get_workspace(focused) == ws) { - LOG("Cannot change focus while in fullscreen mode (same workspace).\n"); + /* Check the fullscreen focus constraints. */ + if (!con_fullscreen_permits_focusing(current->con)) { + LOG("Cannot change focus while in fullscreen mode (fullscreen rules).\n"); ysuccess(false); return; } diff --git a/src/con.c b/src/con.c index c24a379f..f969d056 100644 --- a/src/con.c +++ b/src/con.c @@ -1132,3 +1132,59 @@ Rect con_minimum_size(Con *con) { con->type, con->layout, con->orientation); assert(false); } + +/* + * Returns true if changing the focus to con would be allowed considering + * the fullscreen focus constraints. Specifically, if a fullscreen container or + * any of its descendants is focused, this function returns true if and only if + * focusing con would mean that focus would still be visible on screen, i.e., + * the newly focused container would not be obscured by a fullscreen container. + * + * In the simplest case, if a fullscreen container or any of its descendants is + * fullscreen, this functions returns true if con is the fullscreen container + * itself or any of its descendants, as this means focus wouldn't escape the + * boundaries of the fullscreen container. + * + * In case the fullscreen container is of type CF_OUTPUT, this function returns + * true if con is on a different workspace, as focus wouldn't be obscured by + * the fullscreen container that is constrained to a different workspace. + * + * Note that this same logic can be applied to moving containers. If a + * container can be focused under the fullscreen focus constraints, it can also + * become a parent or sibling to the currently focused container. + * + */ +bool con_fullscreen_permits_focusing(Con *con) { + /* No focus, no problem. */ + if (!focused) + return true; + + /* Find the first fullscreen ascendent. */ + Con *fs = focused; + while (fs && fs->fullscreen_mode == CF_NONE) + fs = fs->parent; + + /* The most common case is we hit the workspace level. In this + * situation, changing focus is also harmless. */ + assert(fs->fullscreen_mode != CF_NONE); + if (fs->type == CT_WORKSPACE) + return true; + + /* If fullscreen is per-output, the focus being in a different workspace is + * sufficient to guarantee that change won't leave fullscreen in bad shape. */ + if (fs->fullscreen_mode == CF_OUTPUT && + con_get_workspace(con) != con_get_workspace(fs)) { + return true; + } + + /* Allow it only if the container to be focused is contained within the + * current fullscreen container. */ + do { + if (con->parent == fs) + return true; + con = con->parent; + } while (con); + + /* Focusing con would hide it behind a fullscreen window, disallow it. */ + return false; +} diff --git a/testcases/t/156-fullscreen-focus.t b/testcases/t/156-fullscreen-focus.t index f9dc6dce..3a27c9ff 100644 --- a/testcases/t/156-fullscreen-focus.t +++ b/testcases/t/156-fullscreen-focus.t @@ -11,9 +11,9 @@ my $i3 = i3(get_socket_path()); my $tmp = fresh_workspace; -##################################################################### -# open the left window -##################################################################### +################################################################################ +# Open the left window. +################################################################################ my $left = open_window({ background_color => '#ff0000' }); @@ -21,23 +21,26 @@ is($x->input_focus, $left->id, 'left window focused'); diag("left = " . $left->id); -##################################################################### -# Open the right window -##################################################################### +################################################################################ +# Open the right window. +################################################################################ my $right = open_window({ background_color => '#00ff00' }); diag("right = " . $right->id); -##################################################################### -# Set the right window to fullscreen -##################################################################### +################################################################################ +# Set the right window to fullscreen. +################################################################################ + cmd 'nop setting fullscreen'; cmd 'fullscreen'; -##################################################################### -# Open a third window -##################################################################### +################################################################################ +# Open a third window. Since we're fullscreen, the window won't be # mapped, so +# don't wait for it to be mapped. Instead, just send the map request and sync +# with i3 to make sure i3 recognizes it. +################################################################################ my $third = open_window({ background_color => '#0000ff', @@ -51,13 +54,15 @@ sync_with_i3; diag("third = " . $third->id); -# move the fullscreen window to a different ws +################################################################################ +# Move the window to a different workspace, and verify that the third window now +# gets focused in the current workspace. +################################################################################ my $tmp2 = get_unused_workspace; cmd "move workspace $tmp2"; -# verify that the third window has the focus is($x->input_focus, $third->id, 'third window focused'); ################################################################################ @@ -87,20 +92,67 @@ is($nodes->[0]->{id}, $old_id, 'id unchanged'); is($nodes->[0]->{focused}, 1, 'fullscreen window focused'); ################################################################################ -# Make sure it's possible to focus a container in a different workspace even if -# we are currently focusing a fullscreen container. +# Ensure it's possible to change focus if it doesn't escape the fullscreen +# container with fullscreen global. We can't even focus a container in a +# different workspace. ################################################################################ -$tmp2 = fresh_workspace; -my $focusable_window = open_window; +cmd 'fullscreen'; +$tmp = fresh_workspace; cmd "workspace $tmp"; -cmd '[id="' . $focusable_window->id . '"] focus'; +my $diff_ws = open_window; -is(focused_ws(), $tmp2, 'focus went to a different workspace'); +$tmp2 = fresh_workspace; +cmd "workspace $tmp2"; +cmd 'split h'; -$nodes = get_ws_content($tmp2); -is(scalar @$nodes, 1, 'precisely one window'); -is($nodes->[0]->{focused}, 1, 'focusable window focused'); +$left = open_window; +my $right1 = open_window; +cmd 'split v'; +my $right2 = open_window; +$nodes = get_ws_content($tmp); + +cmd 'focus parent'; +cmd 'fullscreen global'; + +cmd '[id="' . $right1->id . '"] focus'; +is($x->input_focus, $right1->id, 'upper right window focused'); + +cmd '[id="' . $right2->id . '"] focus'; +is($x->input_focus, $right2->id, 'bottom right window focused'); + +cmd '[id="' . $left->id . '"] focus'; +is($x->input_focus, $right2->id, 'prevented focus change to left window'); + +cmd '[id="' . $diff_ws->id . '"] focus'; +is($x->input_focus, $right2->id, 'prevented focus change to different ws'); + +################################################################################ +# Same tests when we're in non-global fullscreen mode. We toggle fullscreen on +# and off to avoid testing whether focus level works in fullscreen for now. It +# should now be possible to focus a container in a different workspace. +################################################################################ + +cmd 'fullscreen global'; +cmd 'fullscreen global'; + +cmd '[id="' . $right1->id . '"] focus'; +is($x->input_focus, $right1->id, 'upper right window focused'); + +cmd 'focus parent'; +cmd 'fullscreen'; + +cmd '[id="' . $right1->id . '"] focus'; +is($x->input_focus, $right1->id, 'upper right window still focused'); + +cmd '[id="' . $right2->id . '"] focus'; +is($x->input_focus, $right2->id, 'bottom right window focused'); + +cmd '[id="' . $left->id . '"] focus'; +is($x->input_focus, $right2->id, 'prevented focus change to left window'); + +cmd '[id="' . $diff_ws->id . '"] focus'; +is($x->input_focus, $diff_ws->id, 'allowed focus change to different ws'); done_testing; -- 2.39.2