Merge lp:~alan-griffiths/miral/workspaces-example into lp:miral

[Alan Griffiths]

[miral-shell] Example workspaces implementation

[miral-shell] Example workspaces implementation

Provides four workspaces:

  o Switch workspace . . . . . . . . . . : Meta-Alt-[F1|F2|F3|F4]
  o Switch workspace taking active window: Meta-Ctrl-[F1|F2|F3|F4]

[Gerry Boland (gerboland)]

Looks quite ok. The old state save/restore and window_state_hidden updates are a bit repetitive, but more belong in the policy implementation, so is ok

review: Approve

[lp:~alan-griffiths/miral/workspaces-example]

508. By Alan Griffiths on 2017-02-13

merge lp:~alan-griffiths/miral/workspaces/

509. By Alan Griffiths on 2017-02-14

merge lp:~alan-griffiths/miral/workspaces

510. By Alan Griffiths on 2017-02-14

Tidy up code

511. By Alan Griffiths on 2017-02-15

merge lp:~alan-griffiths/miral/workspaces

512. By Alan Griffiths on 2017-02-15

merge lp:~alan-griffiths/miral/workspaces

[Alan Griffiths (alan-griffiths)]

The active window tracking gets confused here.

Start miral-app

Start an app from the command line (e.g. mir_demo_client_eglplasma)

Move that app to another workspace (e.g. Ctrl-Meta-F4)

Switch back to first workspace (Alt-Meta-F4)

Move the terminal too (e.g. Ctrl-Meta-F4)

Now
1. Both apps show a "focussed" toolbar
2. Alt-Tab doesn't switch between apps

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

If a window in a hidden sets the state from the client then it overrides the hidden state imposed by the workspace.

review: Needs Fixing

[lp:~alan-griffiths/miral/workspaces-example]

513. By Alan Griffiths on 2017-02-16

Factor out common code

514. By Alan Griffiths on 2017-02-16

Fix handling of window state changes when on hidden workspace

[Alan Griffiths (alan-griffiths)]

> If a window in a hidden sets the state from the client then it overrides the
> hidden state imposed by the workspace.

Fixed

[lp:~alan-griffiths/miral/workspaces-example]

515. By Alan Griffiths on 2017-02-16

Don't lose unhiding windows from the MRU list

516. By Alan Griffiths on 2017-02-16

Remember the current intensity of *each* titlebar, not just the last one painted

[Alan Griffiths (alan-griffiths)]

> 1. Both apps show a "focussed" toolbar
> 2. Alt-Tab doesn't switch between apps

Fixed

review: Abstain

[Brandon Schaefer (brandontschaefer)]

An issues I can reproduce:
1) Open egl plasma demo
2) Open egl triangle demo
3) egl triangle should be on top
4) switch work spaces to 2, meta + alt + f2
5) switch back to work space 1, meta + alt + f1
6) (note that egl triangle is ontop still but no window appears to be focused, ie. toolbars are greyed out)
7) switch back to work space 2, meta + alt + f2
8) switch back to work space 1, meta + alt + f1

Expected:
  egl triangle to still be on top and focused

Result:
  egl triangle is below egl plasma and plasma has focus

You can test this by alt+tabbing to get triangle back on top.

[lp:~alan-griffiths/miral/workspaces-example]

517. By Alan Griffiths on 2017-02-17

Track active window history better when hiding windows

518. By Alan Griffiths on 2017-02-17

When switching workspaces, remember where focus was, so it doesn't randomly go to the first window we unhide

[Alan Griffiths (alan-griffiths)]

> An issues I can reproduce:
> 1) Open egl plasma demo
> 2) Open egl triangle demo
> 3) egl triangle should be on top
> 4) switch work spaces to 2, meta + alt + f2
> 5) switch back to work space 1, meta + alt + f1
> 6) (note that egl triangle is ontop still but no window appears to be focused,
> ie. toolbars are greyed out)
> 7) switch back to work space 2, meta + alt + f2
> 8) switch back to work space 1, meta + alt + f1
>
> Expected:
> egl triangle to still be on top and focused
>
> Result:
> egl triangle is below egl plasma and plasma has focus
>
> You can test this by alt+tabbing to get triangle back on top.

Fixed

[lp:~alan-griffiths/miral/workspaces-example]

519. By Alan Griffiths on 2017-02-17

merge :parent

520. By Alan Griffiths on 2017-02-17

merge lp:~alan-griffiths/miral/mru-window-visiblity

521. By Alan Griffiths on 2017-02-17

merge lp:~alan-griffiths/miral/workspace-examples-prerequisites

522. By Alan Griffiths on 2017-02-17

Add workspace keys

523. By Alan Griffiths on 2017-02-17

merge lp:~alan-griffiths/miral/workspace-examples-prerequisites

524. By Alan Griffiths on 2017-02-17

Fix logging

525. By Alan Griffiths on 2017-02-17

Fix

526. By Alan Griffiths on 2017-02-17

Fix logging

527. By Alan Griffiths on 2017-02-17

Better solution

528. By Alan Griffiths on 2017-02-20

Comments to clarify code I can't simplify

529. By Alan Griffiths on 2017-02-20

DRY - reduce repetitive workspace visible/hidden code

530. By Alan Griffiths on 2017-02-20

merge :parent

531. By Alan Griffiths on 2017-02-20

merge lp:~alan-griffiths/miral/workspace-examples-prerequisites

Unmerged revisions

=== modified file 'miral-shell/titlebar_provider.cpp'
--- miral-shell/titlebar_provider.cpp	2017-02-14 11:49:59 +0000
+++ miral-shell/titlebar_provider.cpp	2017-02-17 12:58:48 +0000
@@ -222,10 +222,10 @@
 
 void TitlebarProvider::paint_titlebar_for(miral::WindowInfo const& info, int intensity)
 {
-    this->intensity = intensity;
-
     if (auto data = find_titlebar_data(info.window()))
     {
+        data->intensity = intensity;
+
         auto const title = info.name();
 
         if (auto surface = data->titlebar.load())
@@ -339,7 +339,10 @@
         auto const title = window_info.name();
 
         if (auto surface = data->titlebar.load())
-            enqueue_work([this, surface, title]{ paint_surface(surface, title, intensity); });
+        {
+            enqueue_work([this, surface, title, intensity=data->intensity.load()]
+                             { paint_surface(surface, title, intensity); });
+        }
     }
 }
 

=== modified file 'miral-shell/titlebar_provider.h'
--- miral-shell/titlebar_provider.h	2017-02-14 11:49:59 +0000
+++ miral-shell/titlebar_provider.h	2017-02-17 12:58:48 +0000
@@ -79,6 +79,7 @@
     struct Data
     {
         std::atomic<MirWindow*> titlebar{nullptr};
+        std::atomic<int> intensity{0xff};
         std::function<void(MirWindow* surface)> on_create{[](MirWindow*){}};
         miral::Window window;
 
@@ -92,7 +93,6 @@
     std::mutex mutable mutex;
     mir::client::Connection connection;
     std::weak_ptr<mir::scene::Session> weak_session;
-    std::atomic<int> intensity{0xff};
 
     SurfaceMap window_to_titlebar;
     TitleMap windows_awaiting_titlebar;

=== modified file 'miral-shell/titlebar_window_manager.cpp'
--- miral-shell/titlebar_window_manager.cpp	2017-02-15 16:54:12 +0000
+++ miral-shell/titlebar_window_manager.cpp	2017-02-17 12:58:48 +0000
@@ -32,6 +32,44 @@
 namespace
 {
 int const title_bar_height = 12;
+
+struct PolicyData
+{
+    bool in_hidden_workspace{false};
+
+    void update_for_active_workspace(WindowManagerTools& tools, WindowInfo const& window_info)
+    {
+        if (in_hidden_workspace)
+        {
+            in_hidden_workspace = false;
+            WindowSpecification modifications;
+            modifications.state() = old_state;
+            tools.place_and_size_for_state(modifications, window_info);
+            tools.modify_window(window_info.window(), modifications);
+        }
+    }
+
+    void update_for_hidden_workspace(WindowManagerTools& tools, WindowInfo const& window_info)
+    {
+        if (!in_hidden_workspace)
+        {
+            in_hidden_workspace = true;
+            old_state = window_info.state();
+
+            WindowSpecification modifications;
+            modifications.state() = mir_window_state_hidden;
+            tools.place_and_size_for_state(modifications, window_info);
+            tools.modify_window(window_info.window(), modifications);
+        }
+    }
+
+    MirWindowState old_state;
+};
+
+inline PolicyData& policy_data_for(WindowInfo const& info)
+{
+    return *std::static_pointer_cast<PolicyData>(info.userdata());
+}
 }
 
 TitlebarWindowManagerPolicy::TitlebarWindowManagerPolicy(
@@ -43,6 +81,11 @@
     titlebar_provider{std::make_unique<TitlebarProvider>(tools)}
 {
     launcher.launch("decorations", *titlebar_provider);
+
+    for (auto key : {KEY_F1, KEY_F2, KEY_F3, KEY_F4})
+        key_to_workspace[key] = this->tools.create_workspace();
+
+    active_workspace = key_to_workspace[KEY_F1];
 }
 
 TitlebarWindowManagerPolicy::~TitlebarWindowManagerPolicy() = default;
@@ -267,18 +310,31 @@
     CanonicalWindowManagerPolicy::advise_new_window(window_info);
 
     auto const application = window_info.window().application();
-
-    if (application == titlebar_provider->session())
+    auto const parent = window_info.parent();
+
+    auto const is_titlebar = application == titlebar_provider->session();
+
+    if (is_titlebar)
     {
         titlebar_provider->advise_new_titlebar(window_info);
 
-        auto const parent = window_info.parent();
-
         if (tools.active_window() == parent)
             titlebar_provider->paint_titlebar_for(tools.info_for(parent), 0xFF);
         else
             titlebar_provider->paint_titlebar_for(tools.info_for(parent), 0x3F);
     }
+
+    if (!parent)
+        tools.add_tree_to_workspace(window_info.window(), active_workspace);
+    else
+    {
+        auto& pdata = policy_data_for(window_info);
+
+        pdata.in_hidden_workspace = policy_data_for(tools.info_for(parent)).in_hidden_workspace;
+
+        if (policy_data_for(tools.info_for(parent)).in_hidden_workspace)
+            pdata.update_for_hidden_workspace(tools, window_info);
+    }
 }
 
 void TitlebarWindowManagerPolicy::handle_window_ready(WindowInfo& window_info)
@@ -339,6 +395,36 @@
     auto const scan_code = mir_keyboard_event_scan_code(event);
     auto const modifiers = mir_keyboard_event_modifiers(event) & modifier_mask;
 
+    // Switch workspaces
+    if (action == mir_keyboard_action_down &&
+        modifiers == (mir_input_event_modifier_alt | mir_input_event_modifier_meta))
+    {
+        switch (scan_code)
+        {
+        case KEY_F1:
+        case KEY_F2:
+        case KEY_F3:
+        case KEY_F4:
+            switch_workspace_to(key_to_workspace[scan_code]);
+            return true;
+        }
+    }
+
+    // Switch workspace taking the active window
+    if (action == mir_keyboard_action_down &&
+        modifiers == (mir_input_event_modifier_ctrl | mir_input_event_modifier_meta))
+    {
+        switch (scan_code)
+        {
+        case KEY_F1:
+        case KEY_F2:
+        case KEY_F3:
+        case KEY_F4:
+            switch_workspace_to(key_to_workspace[scan_code], tools.active_window());
+            return true;
+        }
+    }
+
     if (action != mir_keyboard_action_repeat)
         end_resize();
 
@@ -364,7 +450,7 @@
     }
     else if (action == mir_keyboard_action_down && scan_code == KEY_F4)
     {
-        switch (modifiers & modifier_mask)
+        switch (modifiers)
         {
         case mir_input_event_modifier_alt|mir_input_event_modifier_shift:
             if (auto const& window = tools.active_window())
@@ -571,5 +657,96 @@
     if (app_info.application() == titlebar_provider->session())
         titlebar_provider->place_new_titlebar(parameters);
 
+    parameters.userdata() = std::make_shared<PolicyData>();
     return parameters;
 }
+
+void TitlebarWindowManagerPolicy::advise_adding_to_workspace(
+    std::shared_ptr<Workspace> const& workspace, std::vector<Window> const& windows)
+{
+    if (windows.empty())
+        return;
+
+    for (auto const& window : windows)
+    {
+        auto const& window_info = tools.info_for(window);
+        auto& pdata = policy_data_for(window_info);
+
+        if (workspace == active_workspace)
+        {
+            pdata.update_for_active_workspace(tools, window_info);
+        }
+        else
+        {
+            pdata.update_for_hidden_workspace(tools, window_info);
+        }
+    }
+}
+
+void TitlebarWindowManagerPolicy::switch_workspace_to(
+    std::shared_ptr<Workspace> const& workspace,
+    Window const& window)
+{
+    if (workspace == active_workspace)
+        return;
+
+    auto const old_active = active_workspace;
+    active_workspace = workspace;
+
+    // Remember active_window when we switch away
+    workspace_to_active[old_active] = tools.active_window();
+
+    tools.add_tree_to_workspace(window, active_workspace);
+
+    if (!window)
+    {
+        if (auto const ww = workspace_to_active[workspace])
+        {
+            tools.for_each_workspace_containing(ww, [&](std::shared_ptr<miral::Workspace> const& ws)
+                {
+                    if (ws == workspace)
+                    {
+                        auto const& window_info = tools.info_for(ww);
+                        auto& pdata = policy_data_for(window_info);
+                        pdata.update_for_active_workspace(tools, window_info);
+                    }
+                });
+        }
+    }
+
+    tools.for_each_window_in_workspace(active_workspace, [&](Window const& window)
+        {
+            if (window.application() == titlebar_provider->session())
+                return; // titlebars are taken care of automatically
+
+            auto const& window_info = tools.info_for(window);
+            auto& pdata = policy_data_for(window_info);
+            pdata.update_for_active_workspace(tools, window_info);
+        });
+
+    tools.remove_tree_from_workspace(window, old_active);
+
+    tools.for_each_window_in_workspace(old_active, [&](Window const& window)
+        {
+            if (window.application() == titlebar_provider->session())
+                return; // titlebars are taken care of automatically
+
+            auto const& window_info = tools.info_for(window);
+            auto& pdata = policy_data_for(window_info);
+
+            pdata.update_for_hidden_workspace(tools, window_info);
+        });
+}
+
+void TitlebarWindowManagerPolicy::handle_modify_window(WindowInfo& window_info, WindowSpecification const& modifications)
+{
+    auto mods = modifications;
+
+    auto& pdata = policy_data_for(window_info);
+
+    if (pdata.in_hidden_workspace && mods.state().is_set())
+        pdata.old_state = mods.state().consume();
+
+    CanonicalWindowManagerPolicy::handle_modify_window(window_info, mods);
+}
+

=== modified file 'miral-shell/titlebar_window_manager.h'
--- miral-shell/titlebar_window_manager.h	2017-02-15 16:54:12 +0000
+++ miral-shell/titlebar_window_manager.h	2017-02-17 12:58:48 +0000
@@ -20,10 +20,12 @@
 #define MIRAL_SHELL_TITLEBAR_WINDOW_MANAGER_H
 
 #include <miral/canonical_window_manager.h>
+#include <miral/workspace_policy.h>
 
 #include "spinner/splash.h"
 
 #include <chrono>
+#include <map>
 
 namespace miral { class InternalClientLauncher; }
 
@@ -31,7 +33,7 @@
 
 class TitlebarProvider;
 
-class TitlebarWindowManagerPolicy : public miral::CanonicalWindowManagerPolicy
+class TitlebarWindowManagerPolicy : public miral::CanonicalWindowManagerPolicy, miral::WorkspacePolicy
 {
 public:
     TitlebarWindowManagerPolicy(miral::WindowManagerTools const& tools, SpinnerSplash const& spinner, miral::InternalClientLauncher const& launcher);
@@ -48,6 +50,8 @@
      *  o Maximize/restore current window (to display size): Alt-F11
      *  o Maximize/restore current window (to display height): Shift-F11
      *  o Maximize/restore current window (to display width): Ctrl-F11
+     *  o Switch workspace . . . . . . . . . . : Meta-Alt-[F1|F2|F3|F4]
+     *  o Switch workspace taking active window: Meta-Ctrl-[F1|F2|F3|F4]
      *  @{ */
     bool handle_pointer_event(MirPointerEvent const* event) override;
     bool handle_touch_event(MirTouchEvent const* event) override;
@@ -63,6 +67,8 @@
     void advise_state_change(miral::WindowInfo const& window_info, MirWindowState state) override;
     void advise_resize(miral::WindowInfo const& window_info, Size const& new_size) override;
     void advise_delete_window(miral::WindowInfo const& window_info) override;
+
+    void handle_modify_window(miral::WindowInfo& window_info, miral::WindowSpecification const& modifications) override;
     /** @} */
 
 protected:
@@ -104,6 +110,19 @@
 
     // Workaround for lp:1627697
     std::chrono::steady_clock::time_point last_resize;
+
+    void advise_adding_to_workspace(
+        std::shared_ptr<miral::Workspace> const& workspace,
+        std::vector<miral::Window> const& windows) override;
+
+    // Switch workspace, taking window (if not null)
+    void switch_workspace_to(
+        std::shared_ptr<miral::Workspace> const& workspace,
+        miral::Window const& window = miral::Window{});
+
+    std::shared_ptr<miral::Workspace> active_workspace;
+    std::map<int, std::shared_ptr<miral::Workspace>> key_to_workspace;
+    std::map<std::shared_ptr<miral::Workspace>, miral::Window> workspace_to_active;
 };
 
 #endif //MIRAL_SHELL_TITLEBAR_WINDOW_MANAGER_H

=== modified file 'miral/basic_window_manager.cpp'
--- miral/basic_window_manager.cpp	2017-02-13 16:23:29 +0000
+++ miral/basic_window_manager.cpp	2017-02-17 12:58:48 +0000
@@ -1041,16 +1041,11 @@
                             window_info.state() == mir_window_state_minimized;
 
     policy->advise_state_change(window_info, value);
-    window_info.state(value);
-
-    mir_surface->configure(mir_window_attrib_state, value);
 
     switch (value)
     {
     case mir_window_state_hidden:
     case mir_window_state_minimized:
-        mir_surface->hide();
-
         if (window == active_window())
         {
             auto const workspaces_containing_window = workspaces_containing(window);
@@ -1087,15 +1082,23 @@
 
             if (window == active_window())
                 select_active_window({});
-
-            mru_active_windows.erase(window);
         }
+
+        window_info.state(value);
+        mir_surface->configure(mir_window_attrib_state, value);
+        mir_surface->hide();
+
         break;
 
     default:
+        auto const none_active = !active_window();
+        window_info.state(value);
+        mir_surface->configure(mir_window_attrib_state, value);
         mir_surface->show();
-        if (was_hidden && !active_window())
+        if (was_hidden && none_active)
+        {
             select_active_window(window);
+        }
     }
 }
 
@@ -1920,13 +1923,13 @@
 
     std::function<void(WindowInfo const& info)> const add_children =
         [&,this](WindowInfo const& info)
+        {
+            for (auto const& child : info.children())
             {
-                for (auto const& child : info.children())
-                {
-                    windows.push_back(child);
-                    add_children(info_for(child));
-                }
-            };
+                windows.push_back(child);
+                add_children(info_for(child));
+            }
+        };
 
     windows.push_back(root);
     add_children(*info);

=== modified file 'miral/mru_window_list.cpp'
--- miral/mru_window_list.cpp	2017-02-02 11:31:52 +0000
+++ miral/mru_window_list.cpp	2017-02-17 12:58:48 +0000
@@ -18,8 +18,27 @@
 
 #include "mru_window_list.h"
 
+#include <mir/scene/surface.h>
+
 #include <algorithm>
 
+namespace
+{
+bool visible(miral::Window const& window)
+{
+    std::shared_ptr<mir::scene::Surface> const& surface{window};
+
+    switch (surface->state())
+    {
+    case mir_window_state_hidden:
+    case mir_window_state_minimized:
+        return false;
+    default:
+        return surface->visible();
+    }
+}
+}
+
 void miral::MRUWindowList::push(Window const& window)
 {
     windows.erase(remove(begin(windows), end(windows), window), end(windows));
@@ -33,12 +52,14 @@
 
 auto miral::MRUWindowList::top() const -> Window
 {
-    return (!windows.empty()) ? windows.back() : Window{};
+    auto const& found = std::find_if(rbegin(windows), rend(windows), visible);
+    return (found != rend(windows)) ? *found: Window{};
 }
 
 void miral::MRUWindowList::enumerate(Enumerator const& enumerator) const
 {
     for (auto i = windows.rbegin(); i != windows.rend(); ++i)
-        if (!enumerator(const_cast<Window&>(*i)))
-            break;
+        if (visible(*i))
+            if (!enumerator(const_cast<Window&>(*i)))
+                break;
 }

=== modified file 'test/mru_window_list.cpp'
--- test/mru_window_list.cpp	2016-08-05 08:53:45 +0000
+++ test/mru_window_list.cpp	2017-02-17 12:58:48 +0000
@@ -24,17 +24,21 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 
-using StubSurface = mir::test::doubles::StubSurface;
 using namespace testing;
 
 namespace
 {
+struct StubSurface : mir::test::doubles::StubSurface
+{
+    bool visible() const override { return true; }
+};
+
 struct StubSession : mir::test::doubles::StubSession
 {
     StubSession(int number_of_surfaces)
     {
         for (auto i = 0; i != number_of_surfaces; ++i)
-            surfaces.push_back(std::make_shared<mir::test::doubles::StubSurface>());
+            surfaces.push_back(std::make_shared<StubSurface>());
     }
 
     std::shared_ptr<mir::scene::Surface> surface(mir::frontend::SurfaceId surface) const override