Show/Hide attached surfaces when entering/exiting from fullscreen

[tarek-y-ismail]

Closes #3274

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

1. Run mir with layer shell:

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

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

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

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

Scenario 2: Multiple outputs

1. Start mir with multiple virtual outputs:

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

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

Scenario 3: Fullscreen app and OSK

1. Run mir with all extensions:

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

2. Start a terminal via CTRL + ALT + t, run ubuntu-frame-osk
3. Start kate, open any text file. OSK should pop up.
4. Press F11 to fullscreen
5. 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

1. Run a different fullscreen app, a terminal via CTRL + ALT + t for example
2. Focus should switch to it, way bar should be visible once more
3. If you use the alt-tab switcher, waybar should hide and be restored as you focus on the fullscreened and non-fullscreened apps respectively.
4. 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

1. Repeat scenario 1, but with VLC.
2. With VLC fullscreened, open any entry from the menu bar
3. Waybar should remain hidden
4. Open a dialog via Media > Open File or Ctrl + o,
5. 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)

[AlanGriffiths]

No tests?

[AlanGriffiths]

No parameter names?

[tarek-y-ismail]

Are they necessary in this case?

[AlanGriffiths]

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.

[tarek-y-ismail]

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.

[mattkae]

An initial ask!

[tarek-y-ismail]

Something I've just realized is that I haven't taken into account cases where focus changes (or a surface is closed) with 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 closed 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 shouldn't be executed in the first place the active window is not different. Unfortunately, guarding against this breaks refocusing and clicking to focus.

---

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.