Show/Hide attached surfaces when entering/exiting from fullscreen by tarek-y-ismail · Pull Request #4664 · canonical/mir

tarek-y-ismail · 2026-02-04T11:29:22Z

What's new?

Adds code BasicWindowManager to hide/restore attached surfaces in the "above" layer when a surface is fullscreened, unfullscreened, and removed.

How to test

Scenario 1: waybar on the above layer + fullscreen app

Run mir with layer shell:

miral-app --add-wayland-extensions=zwlr_layer_shell_v1

Set waybar to be on the "bottom" layer (~/.config/waybar/config):

{
    "layer": "bottom", // Waybar at top layer
   ...
}

Start an application, a terminal for example
Maximize it, it should take up all screen area until it hits waybar.
Fullscreen it, it should take up the whole output
Change the layer configuration waybar's config to be "top"
Repeat step 4. Behavior should be identical
Repeat step 5. Behavior should be identical

Scenario 2: Multiple outputs

Start mir with multiple virtual outputs:

   miral-app --add-wayland-extensions=zwlr_layer_shell_v1  --x11-output=960x500:400x400

Start a terminal via CTRL + ALT + t
Make sure waybar is configured on the top layer and start it
Fullscreen the terminal via F11
Waybar should be hidden on the output where the terminal is fullscreened, and still visible on the other output
Un-fullscreen and move the terminal to the other output, then re-fullscreen
Behavior should be similar to step 5.

Scenario 3: Fullscreen app and OSK

Run mir with all extensions:

miral-app --add-wayland-extensions=all

Start a terminal via CTRL + ALT + t, run ubuntu-frame-osk
Start kate, open any text file. OSK should pop up.
Press F11 to fullscreen
OSK should be hidden.

Preferably, we'd like the OSK to be visible when an app is fullscreened, but since it's placed in the "above" layer, and we have no way in the layer shell protocol to identify it, it will be broken for now.

Scenario 4: Waybar + Fullscreen app + non-fullscreen app
This follows scenario 1, with the following extra steps

Run a different fullscreen app, a terminal via CTRL + ALT + t for example
Focus should switch to it, way bar should be visible once more
If you use the alt-tab switcher, waybar should hide and be restored as you focus on the fullscreened and non-fullscreened apps respectively.
Changing focus via the pointer should behave similarly to step 3

Scenario 5: Waybar + Fullscreen app exits
Repeat scenario 1. Then close the app via a shortcut. If using a terminal, CTRL + D. Waybar should pop into view once more.

Scenario 5: Waybar + Fullscreen app with child surfaces

Repeat scenario 1, but with VLC.
With VLC fullscreened, open any entry from the menu bar
Waybar should remain hidden
Open a dialog via Media > Open File or Ctrl + o,
Way bar should remain hidden.

Scenario 6:
Same as scenario 4, but instead of switching focus between the two apps, you exit the non-fullscreen app first.

Focus should switch to the fullscreen app, and waybar should remain hidden.

Checklist

Tests added and pass (coming (if possible) once this approach is approved)
Adequate documentation added (coming (if possible) once this approach is approved)

src/miral/basic_window_manager.cpp

AlanGriffiths

No tests?

src/miral/basic_window_manager.cpp

src/miral/basic_window_manager.h

src/miral/basic_window_manager.cpp

AlanGriffiths · 2026-02-04T15:49:11Z

src/miral/basic_window_manager.cpp

+    bool const was_fullscreen{info.state() == mir_window_state_fullscreen};
+    if (was_fullscreen)


No need for a variable (especially with scope beyond the if statement).

But... does this code belong here anyway? I mean, if the focus shifts to another fullscreen window we need to undo this work.

Instead, when a window (or no window) gets focus we should be checking the state of the attached windows and acting accordingly.

I looked into it. This is the latest we can do this check. It was added specifically for the case where you have a fullscreen window, and you close it without exiting fullscreen first.

In that case, a few lines later, the info for this window will be erased before we reach the focus checking code, so we won't be able to move this code there.

src/miral/basic_window_manager.cpp

mattkae

An initial ask!

src/miral/basic_window_manager.cpp

states They are attached! The previous state is constant for all of them!

Mir saw that the buffer is not mapped, but the client submitted a buffer with a size, so it sets its status to restored. `place_and_size_for_state` then sees that the attached client is not overlapping with the application zone. Which is correct because application zones shrink for attached surfaces with exclusion zones. It then tries to center the surface in the application zone, causing clients like waybar, ironbar, and gbar to appear in the middle of the screen when they try submitting a buffer while hidden under a fullscreen app.

…p to a fullscreen app

surfaces

tarek-y-ismail · 2026-02-06T18:31:05Z

Something I've just realized is that I haven't taken into account cases where focus changes (or a surface is closed) with the closed surface and the newly focused surface on different outputs/display areas. So far I've assumed both are on the same display area.

Another thing I should mention is that focus handling feels quite messy at the moment. For example, when we have two apps open. Let's call them A and B. If we close A, remove_window gets called which removes the window and updates the MRU window stack among other things. active_window() now returns B .

Context:

mir/src/miral/basic_window_manager.cpp

Lines 327 to 349 in 38d8e69

    
           policy->advise_delete_window(info); 
        
           info_for(application).remove_window(info.window()); 
        
           mru_active_windows.erase(info.window()); 
        
           fullscreen_surfaces.erase(info.window()); 
        
           for (auto& area : display_areas) 
        
           { 
        
               area->attached_windows.erase(info.window()); 
        
               std::erase(area->hidden_attached_windows, info.window()); 
        
           } 
        
           if (info.state() == mir_window_state_attached && 
        
               info.exclusive_rect().is_set()) 
        
           { 
        
               update_application_zones_and_attached_windows(); 
        
           } 
        
           application->destroy_surface(info.window()); 
        
           bool const prev_was_fullscreen{info.state() == mir_window_state_fullscreen}; 
        
           // NB erase() invalidates info, but we want to keep access to "parent". 
        
           auto const parent = info.parent(); 
        
           auto const prev_display_area = display_area_for(info); 
        
           erase(info);

mir/src/miral/basic_window_manager.cpp

Lines 578 to 581 in 38d8e69

    
           auto miral::BasicWindowManager::active_window() const -> Window 
        
           { 
        
               return allow_active_window ? mru_active_windows.top() : Window{}; 
        
           }

We also call refocus, for which itself traverses the MRU window stack from top to bottom, starting with B.

mir/src/miral/basic_window_manager.cpp

Lines 385 to 402 in 38d8e69

    
           mru_active_windows.enumerate([&](miral::Window& window) 
        
               { 
        
                   // select_active_window() calls set_focus_to() which updates mru_active_windows and changes window 
        
                   auto const w = window; 
        
                   for (auto const& workspace : workspaces_containing(w)) 
        
                   { 
        
                       for (auto const& ww : workspaces_containing_window) 
        
                       { 
        
                           if (ww == workspace) 
        
                           { 
        
                               return !(new_focus = select_active_window(w)); 
        
                           } 
        
                       } 
        
                   } 
        
                   return true; 
        
               });

Then that calls set_active_window, and gets the active window, which is B.

mir/src/miral/basic_window_manager.cpp

Line 1703 in 38d8e69

auto const prev_window = active_window();

Then we execute the rest of set_active_window, which at that point we don't have information about the window we just removed!

Edit: I just realized that I've worked on this alone for 120% of my time today. So there might be a solution to the mini-rant above that my neurons are just too fried to see right now.

Should appease GCC

When there would be a layer shell surface with and a fullscreen surface on one output, and we click on empty space in the other output, this would cause the hidden layer shell surface to erroneously be restored.

its own private method

…al to its own private method

fullscreen

AlanGriffiths

This seems messy.

I don't have a clear solution to offer, but trying to model a window state of "attached and hidden" as "hidden and in a list" introduces a lot of corner cases that might be avoided with another approach. (A separate "hidden" property or a new "attached and hidden" state.)

src/miral/basic_window_manager.cpp

AlanGriffiths · 2026-02-11T10:31:04Z

src/miral/basic_window_manager.cpp

+    // If a layer shell surface (waybar) tries to submit a buffer while
+    // its hidden (in favor of another fullscreen surface), don't try
+    // to modify its placement.
+    if (display_area_for(info)->is_hidden_attached(info))
+        mods.state().consume();


This seems overbroad: it bans any transitions. E.g. a panel that decides to transition from "attached" to "restored" (and, maybe, change layer too).

The problem is that some bars (waybar, ironbar, etc...) update themselves even after being set to hidden. This thread on matrix has all the details.

I don't see much detail:

tarek-y-ismail
I also may have discovered a Waybar bug...
tarek-y-ismail
Doesn't happen with Yambar, happens with ironbar
tarek-y-ismail
Also breaks with gBar

What are these components actually doing?

Oops, I may have supplied the wrong link. Please try this

It may be the trouble I have following these links

Ok I think I figured out what's happening.

When we enter fullscreen, we set the window's state to hidden, which works fine.

The bug occurs when the bar submits a frame. This piece of code here sees that the surface is not mapped, but it submitted a buffer, so it should be mapped. It then sets its state to restored: https://github.com/canonical/mir/blob/7e7e41f259a0cba81e870467d488d1dda37913a2/src/server/frontend_xwayland/xwayland_surface_role.cpp#L136-L141

Execution reaches this piece of code, where it sees the state as restored and sees that the restore rect does not overlap the application zone, because layer shell zones may have an exclusion zone, which shrinks the application zone: https://github.com/canonical/mir/blob/7e7e41f259a0cba81e870467d488d1dda37913a2/src/miral/basic_window_manager.cpp#L1445-L1452

Then clamp_top actually does not clamp, it centers the surface vertically: https://github.com/canonical/mir/blob/7e7e41f259a0cba81e870467d488d1dda37913a2/src/miral/basic_window_manager.cpp#L48-L51

Which leads to the bars (see next thread) appearing in the middle of the screen.

OK so there's a mess: this logic is clearly simplistic...

bool const is_mapped = surface.value()->visible(); bool const should_be_mapped = static_cast<bool>(wl_surface->buffer_size()); if (!is_mapped && should_be_mapped && surface.value()->state() == mir_window_state_hidden) { spec.state = mir_window_state_restored; } else if (is_mapped && !should_be_mapped) { spec.state = mir_window_state_hidden; }

...not only does it ignore the existence of attached windows, it ignores fullscreen, maximized, vertmaximized,, etc.

For this approach to work we should maintain the correct state to "unhide" to. (The example FloatingWindowManagerPolicy does something along these lines with PolicyData::old_state.)

But that isn't the end of the story.

In BasicWindowManager::modify_surface() we should recognise that the client is trying to show a window we want hidden and attached and prevent that. (I.e. we should not get to BasicWindowManager::update_attached_and_fullscreen_sets() in this scenario.)

Summarising conversation from #4694:

The code quoted above does not distinguish between the window manager hiding the window and the client hiding the window. The current behaviour is correct for the client hiding the window, but wrong for the window manager hiding the window in the case of shell surfaces hidden when an app fullscreens.

SurfaceStateTracker has logic covering the current case but cannot be applied for everything: we shouldn't be breaking the behaviour client expects. (In particular, losing the "attached" status as described above.)

tl;dr: this still needs fixing

Before I forget all about this.

set_state is called in three locations: in modify_window, and when hiding/reattaching windows.

Right now what we do should work, as attached windows are returned to attached when visible once again. This covers set_state calls from hiding/reattaching code. If we really want to fix this properly for all other cases, we'll have to distinguish modify_window calls from the client and internally from the compositor. And once that's done, we should be able to apply a solution similar to the on in #4694

In any case, this PR should move forward, and the proper fix should be in its own PR since this issue is pre-existing.

Added `BasicWindowManager::hide_attached_windows_for_fullscreen` as a helper methods as well. Co-authored-by: Alan Griffiths <[email protected]>

…wManager`

`BasicWindowManager`

src/miral/basic_window_manager.cpp

…ows`

…-fullscreened

Just some cleanups that I found while working on #4664 and don't quite fit there.

…-fullscreened

AlanGriffiths

I think there's a cascade of problems caused by conflating "not visible" with mir_window_state_hidden.

In the general case, "hidden" is an aspect of window state orthogonal to "fullscreen", "maximized", "attached", etc. (Yes, it is implied by mir_window_state_hidden but can co-exist with the others.)

Maybe things would be simpler if we were only be setting BasicSurface::State::hidden without changing the state from attached.

AlanGriffiths · 2026-02-17T09:46:44Z

src/miral/basic_window_manager.cpp

+    // If a layer shell surface (waybar) tries to submit a buffer while
+    // its hidden (in favor of another fullscreen surface), don't try
+    // to modify its placement.
+    if (display_area_for(info)->is_hidden_attached(info))
+        mods.state().consume();


OK so there's a mess: this logic is clearly simplistic...

bool const is_mapped = surface.value()->visible(); bool const should_be_mapped = static_cast<bool>(wl_surface->buffer_size()); if (!is_mapped && should_be_mapped && surface.value()->state() == mir_window_state_hidden) { spec.state = mir_window_state_restored; } else if (is_mapped && !should_be_mapped) { spec.state = mir_window_state_hidden; }

...not only does it ignore the existence of attached windows, it ignores fullscreen, maximized, vertmaximized,, etc.

For this approach to work we should maintain the correct state to "unhide" to. (The example FloatingWindowManagerPolicy does something along these lines with PolicyData::old_state.)

But that isn't the end of the story.

In BasicWindowManager::modify_surface() we should recognise that the client is trying to show a window we want hidden and attached and prevent that. (I.e. we should not get to BasicWindowManager::update_attached_and_fullscreen_sets() in this scenario.)

AlanGriffiths · 2026-02-17T17:21:07Z

In the general case, "hidden" is an aspect of window state orthogonal to "fullscreen", "maximized", "attached", etc. (Yes, it is implied by mir_window_state_hidden but can co-exist with the others.)

Maybe things would be simpler if we were only be setting BasicSurface::State::hidden without changing the state from attached.

I just came across SurfaceStateTracker - which sounds like it is doing what we need for this.

AlanGriffiths · 2026-02-17T18:03:18Z

I just came across SurfaceStateTracker - which sounds like it is doing what we need for this.

Introduced here: #2015

tarek-y-ismail · 2026-02-18T12:13:11Z

@AlanGriffiths spawned a few PRs off of your feedback as it didn't seem directly applicable to this PR. The commit history needs some cleaning up here. Will do that once you give it the thumbs up.

fullscreening and unfullscreening a normal surface.

stale

…-fullscreened

AlanGriffiths · 2026-02-27T11:23:19Z

src/miral/basic_window_manager.cpp

+    // If a layer shell surface (waybar) tries to submit a buffer while
+    // its hidden (in favor of another fullscreen surface), don't try
+    // to modify its placement.
+    if (display_area_for(info)->is_hidden_attached(info))
+        mods.state().consume();


Summarising conversation from #4694:

The code quoted above does not distinguish between the window manager hiding the window and the client hiding the window. The current behaviour is correct for the client hiding the window, but wrong for the window manager hiding the window in the case of shell surfaces hidden when an app fullscreens.

SurfaceStateTracker has logic covering the current case but cannot be applied for everything: we shouldn't be breaking the behaviour client expects. (In particular, losing the "attached" status as described above.)

tl;dr: this still needs fixing

src/miral/basic_window_manager.cpp

AlanGriffiths · 2026-02-27T11:37:39Z

src/miral/basic_window_manager.cpp

        policy->advise_focus_gained(info_for_hint);
+
+        if(prev_window != hint)
+            handle_attached_surfaces_for_focus_change(prev_window, hint);


There's an apparent mismatch between the supplied "prev_window, hint" and the parameter names "prev, current". But I guess that's pre-existing.

Co-authored-by: Alan Griffiths <[email protected]>

github-actions · 2026-03-06T14:06:24Z

TICS Quality Gate

✔️ Passed

mir

Coding Standards: ✔️ Passed

✔️ Condition “No new Coding Standard Violations for level 1, 2, 3 with respect to Previous analysis” passed.

See the results in the TICS Viewer

The following files have been checked for this project

src/miral/basic_window_manager.cpp
src/miral/basic_window_manager.h

TICS / TICS / Run TICS analysis

tarek-y-ismail commented Feb 4, 2026

View reviewed changes

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

Show/Hide attached surfaces when entering/exiting from fullscreen

4a67dd1

tarek-y-ismail force-pushed the MIRENG-457/hide-attached-above-surfaces-when-fullscreened branch from e76e32d to 4a67dd1 Compare February 4, 2026 11:37

tarek-y-ismail requested review from AlanGriffiths and mattkae February 4, 2026 11:37

tarek-y-ismail marked this pull request as ready for review February 4, 2026 11:37

tarek-y-ismail requested a review from a team as a code owner February 4, 2026 11:37

AlanGriffiths requested changes Feb 4, 2026

View reviewed changes

mattkae previously requested changes Feb 5, 2026

View reviewed changes

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

tarek-y-ismail added 2 commits February 6, 2026 09:27

Remove accidental whitespace change

8cfa385

Skip restoring window if it becomes invalid

92d2eef

tarek-y-ismail mentioned this pull request Feb 6, 2026

Basic window manager cleanups #4669

Merged

tarek-y-ismail added 9 commits February 6, 2026 17:41

Use set_state, and only store attached windows and not their previous

aaceb4a

states They are attached! The previous state is constant for all of them!

Take hidden attached windows into account for display_area_for

93fbe75

Add a fallback for restoring attached windows in select_active_window

345a39a

Move logic for handling hiding/restoring attached surface to set_state

5530ce8

Hide attached windows when focus goes from a closed non-fullscreen ap…

55fede6

…p to a fullscreen app

Add pre and post conditions to {hide,restore}_all_attached

a493ab2

Handle more cases of windows closing with hidden or visible attached

734012e

surfaces

Remove removed windows from hidden_attached_windows

38d8e69

tarek-y-ismail added 4 commits February 10, 2026 10:54

Add [[fallthrough]] for the fullscreen case in set_state

239641c

Should appease GCC

Don't restore layer shell surfaces when select_active_window fails

31297ff

When there would be a layer shell surface with and a fullscreen surface on one output, and we click on empty space in the other output, this would cause the hidden layer shell surface to erroneously be restored.

Move code to handle attached surface hiding/restoring on focus change to

9c5da9f

its own private method

Move code to handle attached surface hiding/restoring on window remov…

e66137e

…al to its own private method

tarek-y-ismail self-assigned this Feb 10, 2026

Add tests for hiding layer shell windows when applications are

2afe488

fullscreen

tarek-y-ismail requested review from AlanGriffiths and mattkae February 10, 2026 15:32

Add a name to the parameter of {hide,restore}_all_attached

bf26f01

AlanGriffiths requested changes Feb 11, 2026

View reviewed changes

tarek-y-ismail and others added 6 commits February 12, 2026 15:13

Rewrite DisplayArea::hide_all_attached to DisplayArea::hide_attached

3c1ee4a

Added `BasicWindowManager::hide_attached_windows_for_fullscreen` as a helper methods as well. Co-authored-by: Alan Griffiths <[email protected]>

Rewrite is_window_or_parent_fullscreen as a concise for loop

6bd2edf

Make is_window_or_parent_fullscreen a private method of `BasicWindo…

38a822a

…wManager`

Remove redunant window validity check

1fce66d

s/restore_all_attached/attach_all_hidden

c5b5a19

Rework how hidden attached windows are re-attached to happen in

3dc2aa2

`BasicWindowManager`

AlanGriffiths requested changes Feb 12, 2026

View reviewed changes

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

src/miral/basic_window_manager.cpp Outdated Show resolved Hide resolved

tarek-y-ismail added 4 commits February 12, 2026 17:04

Don't need to check both attached_windows and `hidden_attached_wind…

d57247a

…ows`

Capture prev_display_area by reference

dbf54f0

Merge branch 'main' into MIRENG-457/hide-attached-above-surfaces-when…

cd5b4d1

…-fullscreened

Fix crashes in is_window_or_parent_fullscreen

f5a4a29

tarek-y-ismail requested a review from AlanGriffiths February 13, 2026 07:43

AlanGriffiths added a commit that referenced this pull request Feb 13, 2026

Basic window manager cleanups (#4669)

faf16fb

Just some cleanups that I found while working on #4664 and don't quite fit there.

Merge branch 'main' into MIRENG-457/hide-attached-above-surfaces-when…

bc69a76

…-fullscreened

AlanGriffiths requested changes Feb 17, 2026

View reviewed changes

This was referenced Feb 18, 2026

Use SurfaceStateTracker to hide/restore windows to the proper state #4694

Closed

Add test to make sure that layer shell surfaces are not remapped with the wrong position when hidden by fullscreen surfaces. canonical/wlcs#405

Closed

tarek-y-ismail requested a review from AlanGriffiths February 18, 2026 12:12

Add test for hiding/restoring attached surfaces in the above layer when

bd22724

fullscreening and unfullscreening a normal surface.

tarek-y-ismail mentioned this pull request Feb 19, 2026

Need a regression test for layer shell surfaces submitting buffers while hidden #4702

Open

Merge branch 'main' into MIRENG-457/hide-attached-above-surfaces-when…

4038e64

…-fullscreened

AlanGriffiths requested changes Feb 27, 2026

View reviewed changes

Fix whitespace

e40cd7d

Co-authored-by: Alan Griffiths <[email protected]>

		bool const was_fullscreen{info.state() == mir_window_state_fullscreen};
		if (was_fullscreen)

Conversation

tarek-y-ismail commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's new?

How to test

Checklist

Uh oh!

Uh oh!

AlanGriffiths left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattkae left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tarek-y-ismail commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlanGriffiths left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarek-y-ismail Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlanGriffiths left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanGriffiths commented Feb 17, 2026

Uh oh!

AlanGriffiths commented Feb 17, 2026

Uh oh!

tarek-y-ismail commented Feb 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 6, 2026

TICS Quality Gate

tarek-y-ismail commented Feb 4, 2026 •

edited

Loading

tarek-y-ismail commented Feb 6, 2026 •

edited

Loading

tarek-y-ismail Mar 6, 2026 •

edited

Loading