Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workspace movement with touchpad gestures #269

Draft
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

ErikReider
Copy link
Collaborator

@ErikReider ErikReider commented Jan 21, 2024

Switch between workspaces with touchpad gestures

2024-01-21.01-40-39.mp4

TODO:

  • Workspace logic
    • Switch between fullscreen/non-fullscreen views
    • Clip floating views to the output while switching
    • Clip the floating views to the workspace bounds while switching
  • Touchpad gestures
    • Snap to the nearest workspace on release (without animations for now)
  • Horizontal support
  • Vertical support
  • Animations? (We should wait until animations have landed in master)
  • Bounce back feedback (spring effect when trying to go past the last workspace)
  • Bounce back ease-out
  • Small but fast swipes should switch workspace
  • Configurables
    • Threshold
    • Speed. The default probably needs some refinement as well
    • Needed speed
    • wrap around workspaces
    • Bounce back size in px (scaled)

To test:

bindworkspacegesture 4:horizontal
# bindworkspacegesture --inverted 4:horizontal
# bindworkspacegesture 4:vertical
# bindworkspacegesture --inverted 4:vertical

@ErikReider ErikReider added the enhancement New feature or request label Jan 21, 2024
@ErikReider ErikReider self-assigned this Jan 21, 2024
@ErikReider ErikReider marked this pull request as ready for review January 24, 2024 01:34
transaction_commit_dirty();

// Unset focus
seat_set_focus_workspace(seat, NULL);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch - this was a pain in fade out animations haha

Comment on lines 118 to 133
int x = 0, y = 0;

int ws_dimen;
int *coord;
switch (output->workspace_scroll.direction) {
case SWIPE_GESTURE_DIRECTION_NONE:
return;
case SWIPE_GESTURE_DIRECTION_HORIZONTAL:
ws_dimen = output->width;
coord = &x;
break;
case SWIPE_GESTURE_DIRECTION_VERTICAL:
ws_dimen = output->height;
coord = &y;
break;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this bit is confusing to me, may need some clarification - what do x and y represent if they remain at 0 - and why does coord depend on them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that is a bit confusing… xD

It's mostly there to remove duplicate code due to the gesture either needing to adjust the X or Y coordinates depending on the gesture direction. Do you have any other way of solving this without duplicating the code?

Maybe just add the x and y coords as args?

Comment on lines 112 to 150
// Clips the rendered damage to the workspace region.
// Fixes containers being rendered across workspaces while switching.
static void adjust_damage_to_workspace_bounds(pixman_region32_t *damage,
bool on_focused_workspace, struct sway_output *output) {
float scale = output->wlr_output->scale;
float scroll_percent = output->workspace_scroll.percent;
int x = 0, y = 0;

int ws_dimen;
int *coord;
switch (output->workspace_scroll.direction) {
case SWIPE_GESTURE_DIRECTION_NONE:
return;
case SWIPE_GESTURE_DIRECTION_HORIZONTAL:
ws_dimen = output->width;
coord = &x;
break;
case SWIPE_GESTURE_DIRECTION_VERTICAL:
ws_dimen = output->height;
coord = &y;
break;
}

*coord = round(-ws_dimen * scroll_percent);
if (!on_focused_workspace) {
if (scroll_percent > 0) {
*coord += ws_dimen;
} else if (scroll_percent < 0) {
*coord -= ws_dimen;
}
}

struct wlr_box monitor_box = get_monitor_box(output->wlr_output);
pixman_region32_intersect_rect(damage, damage,
monitor_box.x, monitor_box.y,
monitor_box.width, monitor_box.height);
pixman_region32_translate(damage, x * scale, y * scale);
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't clipping to the output size solve this? Maybe this comment is my lack of understanding of the purpose of x and y coming out 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's exactly that it's doing, except that we're translating the damage to the workspaces translated bounds

Comment on lines 1531 to 1532
pixman_region32_t *damage, struct sway_container *con, bool parent_focused,
bool is_current_ws);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parent_focused, is_current_ws: can we combine these? I'd think they'd indicate the same thing and would clean up quite a few function calls

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this. The parent_focused sometimes indicates if the container is focused or not. Could result in some cases where an unfocused container in the focused workspace would be incorrectly adjusted as if it was located on the sibling workspace

Comment on lines 2011 to 2017
for (int i = 0; i < workspace->current.floating->length; ++i) {
struct sway_container *floater =
workspace->current.floating->items[i];
if (container_is_transient_for(floater, fullscreen_con)) {
render_floating_container(output, damage, floater, on_focused_workspace);
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we render floating containers in this function? Wouldn't the fullscreen view be above floating cons?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for floating containers that are transient for the fullscreened application. It's what upstream does :)

@ozwaldorf
Copy link
Contributor

ozwaldorf commented Jan 26, 2024

Just to mention, nix build seems to be broken:

error: builder for '/nix/store/9sg0wwdj4wdkxskw6wjp2qh0mvcjrydn-sway-unwrapped-1.8.1.drv' failed with exit code 1;
       last 10 log lines:
       > [137/344] Compiling C object sway/sway.p/commands_blur_contrast.c.o
       > [138/344] Compiling C object sway/sway.p/commands_blur_radius.c.o
       > [139/344] Compiling C object sway/sway.p/commands_blur_saturation.c.o
       > [140/344] Compiling C object sway/sway.p/commands_border.c.o
       > [141/344] Compiling C object sway/sway.p/commands_blur_noise.c.o
       > [142/344] Compiling C object sway/sway.p/config_output.c.o
       > [143/344] Compiling C object sway/sway.p/commands_corner_radius.c.o
       > [144/344] Compiling C object sway/sway.p/commands_bind.c.o
       > [145/344] Compiling C object sway/sway.p/commands_client.c.o
       > ninja: build stopped: subcommand failed.
       For full logs, run 'nix log /nix/store/9sg0wwdj4wdkxskw6wjp2qh0mvcjrydn-sway-unwrapped-1.8.1.drv'.

@WillPower3309
Copy link
Owner

Just to mention, nix build seems to be broken:

error: builder for '/nix/store/9sg0wwdj4wdkxskw6wjp2qh0mvcjrydn-sway-unwrapped-1.8.1.drv' failed with exit code 1;
       last 10 log lines:
       > [137/344] Compiling C object sway/sway.p/commands_blur_contrast.c.o
       > [138/344] Compiling C object sway/sway.p/commands_blur_radius.c.o
       > [139/344] Compiling C object sway/sway.p/commands_blur_saturation.c.o
       > [140/344] Compiling C object sway/sway.p/commands_border.c.o
       > [141/344] Compiling C object sway/sway.p/commands_blur_noise.c.o
       > [142/344] Compiling C object sway/sway.p/config_output.c.o
       > [143/344] Compiling C object sway/sway.p/commands_corner_radius.c.o
       > [144/344] Compiling C object sway/sway.p/commands_bind.c.o
       > [145/344] Compiling C object sway/sway.p/commands_client.c.o
       > ninja: build stopped: subcommand failed.
       For full logs, run 'nix log /nix/store/9sg0wwdj4wdkxskw6wjp2qh0mvcjrydn-sway-unwrapped-1.8.1.drv'.

The errors here are:

p/desktop_render.c.o -c ../sway/desktop/render.c
../sway/desktop/render.c: In function 'adjust_box_to_workspace_offset':
../sway/desktop/render.c:102:9: error: 'box_coord' may be used uninitialized [-Werror=maybe-uninitialized]
  102 |         *box_coord -= ws_dimen * scroll_percent;
      |         ^~~~~~~~~~
../sway/desktop/render.c:88:14: note: 'box_coord' was declared here
   88 |         int *box_coord;
      |              ^~~~~~~~~
../sway/desktop/render.c:102:32: error: 'ws_dimen' may be used uninitialized [-Werror=maybe-uninitialized]
  102 |         *box_coord -= ws_dimen * scroll_percent;
      |                       ~~~~~~~~~^~~~~~~~~~~~~~~~
../sway/desktop/render.c:87:13: note: 'ws_dimen' was declared here
   87 |         int ws_dimen;
      |             ^~~~~~~~
In function 'adjust_damage_to_workspace_bounds',
    inlined from 'render_fullscreen_con' at ../sway/desktop/render.c:1985:3:
../sway/desktop/render.c:135:16: error: 'coord' may be used uninitialized [-Werror=maybe-uninitialized]
  135 |         *coord = round(-ws_dimen * scroll_percent);
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../sway/desktop/render.c: In function 'render_fullscreen_con':
../sway/desktop/render.c:121:14: note: 'coord' was declared here
  121 |         int *coord;
      |              ^~~~~
In function 'adjust_damage_to_workspace_bounds',
    inlined from 'render_fullscreen_con' at ../sway/desktop/render.c:1985:3:
../sway/desktop/render.c:138:32: error: 'ws_dimen' may be used uninitialized [-Werror=maybe-uninitialized]
  138 |                         *coord += ws_dimen;
      |                                ^~
../sway/desktop/render.c: In function 'render_fullscreen_con':
../sway/desktop/render.c:120:13: note: 'ws_dimen' was declared here
  120 |         int ws_dimen;
      |             ^~~~~~~~
cc1: all warnings being treated as errors

@ErikReider
Copy link
Collaborator Author

Might have an idea of how to improve the config command syntax. Converting to draft

@ErikReider ErikReider marked this pull request as draft January 27, 2024 22:30
@ErikReider
Copy link
Collaborator Author

ErikReider commented Jan 27, 2024

Might have an idea of how to improve the config command syntax. Converting to draft

Maybe something like this:

bindgesture --invert fluid_swipe:3:horizontal workspace_switch

@WillPower3309
Copy link
Owner

Might have an idea of how to improve the config command syntax. Converting to draft

Maybe something like this:

bindgesture --invert fluid_swipe:3:horizontal workspace_switch

Looks good to me!

@ErikReider
Copy link
Collaborator Author

I'll wait with refining this until the sway 1.9 rebase is merged :)

@parmjotsinghrobot

This comment was marked as off-topic.

@swwwee

This comment was marked as off-topic.

@ErikReider
Copy link
Collaborator Author

Y'all don't need to comment here to get notifications, just creates spam. The right side panel has a button for notifications :)

@ErikReider
Copy link
Collaborator Author

Rebased from master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1:1 trackpad gestures for switching desktops
5 participants