Simplify BasicWindowManager::refocus. Add comments and fix typos

src/miral/application_selector.cpp

@@ -275,7 +275,14 @@ auto ApplicationSelector::advance(bool reverse, bool within_app) -> Window
 
     auto next_state_to_preserve =  tools.info_for(next_window.value()).state();
     tools.select_active_window(next_window.value());
+
+    // When using the application selector, the selection/raising of apps is
+    // more of a "preview" than actually selecting the application.
+    //
+    // So, apps should return to their previous state whenever we move on from
+    // them. Minimized apps should return to being minimized.
     restore_state = next_state_to_preserve;
+
     return next_window.value();
 }
 
@@ -285,4 +292,4 @@ auto ApplicationSelector::find(Window window) -> std::vector<Window>::iterator
     {
         return window == other;
     });
-}
+}

src/miral/basic_window_manager.cpp

@@ -290,28 +290,21 @@ void miral::BasicWindowManager::refocus(
 
     if (can_activate_window_for_session_in_workspace(application, workspaces_containing_window))
         return;
-
-    // Try to activate to recently active window of any application in a shared workspace
+    // Try to activate the recently active window of any application in a shared workspace
     {
         miral::Window new_focus;
+        auto workspaces_containing_window_mut = workspaces_containing_window;
+        std::sort(workspaces_containing_window_mut.begin(), workspaces_containing_window_mut.end());
 
-        mru_active_windows.enumerate([&](miral::Window& window)
+        mru_active_windows.enumerate(
+            [&](miral::Window& other_window)
             {
-                // select_active_window() calls set_focus_to() which updates mru_active_windows and changes window
-                auto const w = window;
-                if (!info_for(w).is_visible())
+                if (!info_for(other_window).is_visible())
                     return true;
 
-                for (auto const& workspace : workspaces_containing(w))
-                {
-                    for (auto const& ww : workspaces_containing_window)
-                    {
-                        if (ww == workspace)
-                        {
-                            return !(new_focus = select_active_window(w));
-                        }
-                    }
-                }
+                // select_active_window() calls set_focus_to() which updates mru_active_windows and changes window
+                if (window_workspaces_intersect(workspaces_containing_window_mut, other_window))
+                    return !(new_focus = select_active_window(other_window));
 
                 return true;
             });
@@ -322,7 +315,7 @@ void miral::BasicWindowManager::refocus(
     if (can_activate_window_for_session(application))
         return;
 
-    // Try to activate to recently active window of any application
+    // Try to activate the recently active window of any application
     {
         miral::Window new_focus;
 
@@ -1497,24 +1490,17 @@ void miral::BasicWindowManager::set_state(miral::WindowInfo& window_info, MirWin
 
             if (window == active_window() || !active_window())
             {
-                auto const workspaces_containing_window = workspaces_containing(window);
+                auto workspaces_containing_window = workspaces_containing(window);
+                std::sort(workspaces_containing_window.begin(), workspaces_containing_window.end());
 
                 // Try to activate to recently active window of any application
                 mru_active_windows.enumerate([&](Window& candidate)
                     {
                         if (candidate == window || !info_for(candidate).is_visible())
                             return true;
-                        auto const w = candidate;
-                        for (auto const& workspace : workspaces_containing(w))
-                        {
-                            for (auto const& ww : workspaces_containing_window)
-                            {
-                                if (ww == workspace)
-                                {
-                                    return !(select_active_window(w));
-                                }
-                            }
-                        }
+
+                        if(window_workspaces_intersect(workspaces_containing_window, candidate))
+                            select_active_window(candidate);
 
                         return true;
                     });
@@ -1526,8 +1512,7 @@ void miral::BasicWindowManager::set_state(miral::WindowInfo& window_info, MirWin
                 {
                     if (candidate == window || !info_for(candidate).is_visible())
                         return true;
-                    auto const w = candidate;
-                    return !(select_active_window(w));
+                    return !(select_active_window(candidate));
                 });
 
             if (window == active_window())
@@ -2997,3 +2982,21 @@ void miral::BasicWindowManager::move_cursor_to(mir::geometry::PointF point)
         });
     });
 }
+
+auto miral::BasicWindowManager::window_workspaces_intersect(std::vector<std::shared_ptr<Workspace>> const& w1_workspaces, Window const& w2) const -> bool
+{
+    auto w2_workspaces = workspaces_containing(w2);
+    std::sort(w2_workspaces.begin(), w2_workspaces.end());
+
+    auto intersection = std::vector<std::shared_ptr<Workspace>>();
+
+    std::set_intersection(
+        w1_workspaces.begin(),
+        w1_workspaces.end(),
+        w2_workspaces.begin(),
+        w2_workspaces.end(),
+        std::back_inserter(intersection));
+
+    return !intersection.empty();
+}
+

src/miral/basic_window_manager.h

@@ -332,6 +332,10 @@ class BasicWindowManager : public virtual mir::shell::WindowManager,
     void for_each_descendent_in(WindowInfo const& info, std::function<void(const Window&)> func);
     /// Gathers windows provided WindowInfo
     auto collect_windows(WindowInfo const& info) -> SurfaceSet;
+
+    // w1_workspaces must be sorted
+    auto window_workspaces_intersect(
+        std::vector<std::shared_ptr<Workspace>> const& w1_workspaces, Window const& w2) const -> bool;
 };
 }