swaywm / sway

i3-compatible Wayland compositor
https://swaywm.org
MIT License
14.56k stars 1.11k forks source link

Sway does no properly update when a window moves into an existing container #8292

Open WhyNotHugo opened 2 months ago

WhyNotHugo commented 2 months ago

Version

sway version 1.10-dev-40ca4150b (Jun 10 2024, branch 'master')

Configuration

Run sway -c test.conf with the following as test.conf:

workspace_layout tabbed
exec sh test.sh

And test.sh is:

# Open five windows
foot &
foot &
foot &
foot &
foot &

sleep 1

# focus the second one
swaymsg focus left
swaymsg focus left
swaymsg focus left

sleep 1
# split it vertically
swaymsg split v

sleep 1
# focus the rightmost one
swaymsg focus right
swaymsg focus right
swaymsg focus right

sleep 1
# move it left three times
swaymsg move left
sleep 1
swaymsg move left
sleep 1
swaymsg move left

# At this point, you'll note that we should be seeing the moved window inside
# the split container, but we're actually seeing the window on the right of the
# tabbed container. The titlebar on the other hand, shows the fourth window as
# active.
sleep 5

# This properly moves the window that we've been moving all along. It will now
# be on the left of the split container.
# Everything renders correctly again at this point.
swaymsg move left

The sleeps are there to ensure that all commands execute before proceeding. They also help visualise the issue.

Description

When I move a window into a split container, the view doesn't properly update, and I see the wrong window, while sway shows the wrong titlebar as active. Moving that window again restores sway to a consistent state.

I recommend running the above reproductions script while reading through the script as actions progress.

Additional notes

I previously mentioned this in https://github.com/swaywm/sway/issues/8205, but it turns out it is a separate issue.

WhyNotHugo commented 3 days ago

I've bisected this further:

5e18ed3cf03eee9e83909fede46dd98dff652647 is the first bad commit
commit 5e18ed3cf03eee9e83909fede46dd98dff652647 (HEAD)
Author: Ronan Pigott <ronan@rjp.ie>
Date:   Wed Feb 28 17:51:03 2024 -0700

    commands/move: do not force focus on the moved container

    My code archaeology isn't good enough to determine what this is here
    for, but it isn't correct. We should be able to move containers in a
    direction without focusing them. AFAICT i3 doesn't do this, so we
    shouldn't either.

    This fixes ipc commands like move <dir> with criteria that apply to
    containers which are not the current focus.

 sway/commands/move.c | 9 ---------
 1 file changed, 9 deletions(-)

Using wlroots 2a897af7dc532a3585401ae317d586a69c1af1d3

WhyNotHugo commented 3 days ago

cc: @rpigott

WhyNotHugo commented 3 days ago

I didn't properly clarify this above: when this issue is triggered, the tab bar shows the right-most window as focused, but another window has focus (and responds to keyboard input and to swaymsg move commands).

However, the window that is rendered inside the tabbed container is different from the focused one.

WhyNotHugo commented 3 days ago

Restoring the following two lines fixes the issue:

    seat_set_raw_focus(config->handler_context.seat, &new_ws->node);
    seat_set_focus_container(config->handler_context.seat, container);