Fix "vswitch" plugin: undesired 'last' direction

plugins/scale/scale.cpp

@@ -91,7 +91,7 @@ struct view_scale_data
  * BTN_MIDDLE:
  * - If middle_click_close is true, closes the view clicked
  */
-class wayfire_scale : public wf::per_output_plugin_instance_t,
+class wayfire_scale : public wf::vswitch::per_output_vswitch_handler_t,
     public wf::keyboard_interaction_t,
     public wf::pointer_interaction_t,
     public wf::touch_interaction_t
@@ -161,38 +161,40 @@ class wayfire_scale : public wf::per_output_plugin_instance_t,
     void setup_workspace_switching()
     {
         workspace_bindings = std::make_unique<wf::vswitch::control_bindings_t>(output);
-        workspace_bindings->setup([&] (wf::point_t delta, wayfire_toplevel_view view, bool only_view)
+        workspace_bindings->setup(this);
+    }
+
+    bool handle_transition(wf::point_t delta, wayfire_toplevel_view view, bool only_view) override
+    {
+        if (!output->is_plugin_active(grab_interface.name))
         {
-            if (!output->is_plugin_active(grab_interface.name))
-            {
-                return false;
-            }
+            return false;
+        }
 
-            if (delta == wf::point_t{0, 0})
-            {
-                // Consume input event
-                return true;
-            }
+        if (delta == wf::point_t{0, 0})
+        {
+            // Consume input event
+            return true;
+        }
 
-            if (only_view)
-            {
-                // For now, scale does not let you move views between workspaces
-                return false;
-            }
+        if (only_view)
+        {
+            // For now, scale does not let you move views between workspaces
+            return false;
+        }
 
-            auto ws = output->wset()->get_current_workspace() + delta;
+        auto ws = output->wset()->get_current_workspace() + delta;
 
-            // vswitch picks the top view, we want the focused one
-            std::vector<wayfire_toplevel_view> fixed_views;
-            if (view && current_focus_view && !all_workspaces)
-            {
-                fixed_views.push_back(current_focus_view);
-            }
+        // vswitch picks the top view, we want the focused one
+        std::vector<wayfire_toplevel_view> fixed_views;
+        if (view && current_focus_view && !all_workspaces)
+        {
+            fixed_views.push_back(current_focus_view);
+        }
 
-            output->wset()->request_workspace(ws, fixed_views);
+        output->wset()->request_workspace(ws, fixed_views);
 
-            return true;
-        });
+        return true;
     }
 
     /* Add a transformer that will be used to scale the view */
@@ -1132,6 +1134,7 @@ class wayfire_scale : public wf::per_output_plugin_instance_t,
         }
 
         layout_slots(get_views());
+        last_ws = ev->old_viewport;
     };
 
     wf::signal::connection_t<wf::workarea_changed_signal> workarea_changed =

plugins/vswitch/vswitch.cpp

@@ -351,7 +351,7 @@ class workspace_switch_t
 }
 }
 
-class vswitch : public wf::per_output_plugin_instance_t
+class vswitch : public wf::vswitch::per_output_vswitch_handler_t
 {
   private:
 
@@ -400,40 +400,42 @@ class vswitch : public wf::per_output_plugin_instance_t
             [=] () { output->deactivate_plugin(&grab_interface); });
 
         bindings = std::make_unique<wf::vswitch::control_bindings_t>(output);
-        bindings->setup([this] (wf::point_t delta, wayfire_toplevel_view view, bool only_view)
+        bindings->setup(this);
+    }
+
+    bool handle_transition(wf::point_t delta, wayfire_toplevel_view view, bool only_view) override
+    {
+        // Do not switch workspace with sticky view, they are on all
+        // workspaces anyway
+        if (view && view->sticky)
         {
-            // Do not switch workspace with sticky view, they are on all
-            // workspaces anyway
-            if (view && view->sticky)
+            view = nullptr;
+        }
+
+        if (this->set_capabilities(wf::CAPABILITY_MANAGE_DESKTOP))
+        {
+            if (delta == wf::point_t{0, 0})
             {
-                view = nullptr;
+                // Consume input event
+                return true;
             }
 
-            if (this->set_capabilities(wf::CAPABILITY_MANAGE_DESKTOP))
+            if (only_view && view)
             {
-                if (delta == wf::point_t{0, 0})
+                if (!view->get_wset())
                 {
-                    // Consume input event
-                    return true;
+                    return false;
                 }
 
-                if (only_view && view)
-                {
-                    if (!view->get_wset())
-                    {
-                        return false;
-                    }
-
-                    wf::vswitch::move_view(view, delta, true);
-                    return true;
-                }
-
-                return add_direction(delta, view);
-            } else
-            {
-                return false;
+                wf::vswitch::move_view(view, delta, true);
+                return true;
             }
-        });
+
+            return add_direction(delta, view);
+        } else
+        {
+            return false;
+        }
     }
 
     inline bool is_active()
@@ -523,6 +525,7 @@ class vswitch : public wf::per_output_plugin_instance_t
             return;
         }
 
+        last_ws = ev->old_viewport;
         if (is_active())
         {
             ev->carried_out = add_direction(ev->new_viewport - ev->old_viewport);

plugins/vswitch/wayfire/plugins/vswitch.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "wayfire/per-output-plugin.hpp"
 #include "wayfire/scene-input.hpp"
 #include "wayfire/seat.hpp"
 #include "wayfire/core.hpp"
@@ -23,6 +24,63 @@ namespace wf
 {
 namespace vswitch
 {
+class per_output_vswitch_handler_t : public wf::per_output_plugin_instance_t
+{
+  public:
+    wf::point_t last_ws = {.x = 0, .y = 0};
+
+    bool handle_dir(wf::point_t delta, wayfire_toplevel_view view, bool window_only, bool wraparound)
+    {
+        if (!view && window_only)
+        {
+            // Maybe there is no view, in any case, no need to do anything
+            return false;
+        }
+
+        auto ws = output->wset()->get_current_workspace();
+        auto target_ws = ws + delta;
+        if (!output->wset()->is_workspace_valid(target_ws))
+        {
+            if (wraparound)
+            {
+                auto grid_size = output->wset()->get_workspace_grid_size();
+                target_ws.x = (target_ws.x + grid_size.width) % grid_size.width;
+                target_ws.y = (target_ws.y + grid_size.height) % grid_size.height;
+            } else
+            {
+                target_ws = ws;
+            }
+        }
+
+        // Remember the direction we are moving now so that we can potentially
+        // move back. Only remember when we are actually changing the workspace
+        // and not just move a view around.
+        if (!window_only)
+        {
+            if (target_ws != ws)
+            {
+                // this->last_dir = target_ws - ws;
+                this->last_ws = ws;
+            }
+        }
+
+        return handle_transition(target_ws - ws, view, window_only);
+    }
+
+    wf::point_t get_last_dir(wf::point_t current)
+    {
+        return this->last_ws - current;
+    }
+
+    bool handle_last(wayfire_toplevel_view view, bool window_only)
+    {
+        return handle_dir(get_last_dir(output->wset()->get_current_workspace()), view, window_only, false);
+    }
+
+    // Override in actual plugin to do 'things' on workspace change transition
+    virtual bool handle_transition(wf::point_t delta, wayfire_toplevel_view view, bool only_view) = 0;
+};
+
 /**
  * A simple class to register the vswitch bindings and get a custom callback called.
  */
@@ -58,8 +116,8 @@ class control_bindings_t
      *
      * @param delta The difference between current and target workspace.
      * @param view The view to be moved together with the switch, or nullptr.
-     * @param window_only Move only the view to the given workspace. It is
-     *   guaranteed that @view will not be nullptr if this is true.
+     * @param window_only Move only the view to the given workspace. It is guaranteed that @view will not be
+     * nullptr if this is true.
      */
     using binding_callback_t = std::function<bool (
         wf::point_t delta, wayfire_toplevel_view view, bool window_only)>;
@@ -69,10 +127,10 @@ class control_bindings_t
      *
      * @param callback The callback to execute on each binding
      */
-    void setup(binding_callback_t callback)
+    void setup(per_output_vswitch_handler_t *handler)
     {
         tear_down();
-        this->user_cb = callback;
+        this->handler = handler;
 
         // Setup a new binding on the output.
         //
@@ -87,7 +145,7 @@ class control_bindings_t
         activator_cbs.push_back(std::make_unique<activator_callback>()); \
         *activator_cbs.back() = [=] (const wf::activator_data_t&) \
         {\
-            return handle_dir({dx, dy}, view, only, callback); \
+            return handler->handle_dir({dx, dy}, view, only, wraparound); \
         };\
         output->add_activator(binding_##name, activator_cbs.back().get());
 
@@ -114,7 +172,7 @@ class control_bindings_t
         activator_cbs.push_back(std::make_unique<activator_callback>()); \
         *activator_cbs.back() = [=] (const wf::activator_data_t&) \
         {\
-            return handle_dir(-get_last_dir(), view, only, callback); \
+            return handler->handle_last(view, only); \
         };\
         output->add_activator(binding_##name, activator_cbs.back().get());
 
@@ -154,7 +212,7 @@ class control_bindings_t
                 wf::point_t current = output->wset()->get_current_workspace();
 
                 auto view = (grab_view ? get_target_view() : nullptr);
-                return handle_dir(target - current, view, only_view, callback);
+                return handler->handle_dir(target - current, view, only_view, wraparound);
             };
 
             output->add_activator(wf::create_option(binding),
@@ -191,7 +249,7 @@ class control_bindings_t
     }
 
   protected:
-    binding_callback_t user_cb;
+    per_output_vswitch_handler_t *handler;
     std::vector<std::unique_ptr<wf::activator_callback>> activator_cbs;
 
     wf::point_t last_dir = {0, 0};
@@ -204,9 +262,9 @@ class control_bindings_t
         {
             // Reload only if the plugin has already setup bindings once,
             // otherwise, we do not have any callbacks to register.
-            if (user_cb)
+            if (handler)
             {
-                setup(user_cb);
+                setup(handler);
             }
         });
     };
@@ -240,9 +298,8 @@ class control_bindings_t
     }
 
     /**
-     * Handle binding in the given direction. The next workspace will be
-     * determined by the current workspace, target direction and wraparound
-     * mode.
+     * Handle binding in the given direction. The next workspace will be determined by the current workspace,
+     * target direction and wraparound mode.
      */
     virtual bool handle_dir(wf::point_t dir, wayfire_toplevel_view view, bool window_only,
         binding_callback_t callback)