-
Notifications
You must be signed in to change notification settings - Fork 56
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
base: master
Are you sure you want to change the base?
Conversation
8bfa708
to
519bfc4
Compare
b71c56c
to
7214db9
Compare
067324c
to
4f9ac2d
Compare
transaction_commit_dirty(); | ||
|
||
// Unset focus | ||
seat_set_focus_workspace(seat, NULL); |
There was a problem hiding this comment.
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
sway/desktop/render.c
Outdated
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; | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
sway/desktop/render.c
Outdated
// 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); | ||
} | ||
|
There was a problem hiding this comment.
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 😅
There was a problem hiding this comment.
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
sway/desktop/render.c
Outdated
pixman_region32_t *damage, struct sway_container *con, bool parent_focused, | ||
bool is_current_ws); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
sway/desktop/render.c
Outdated
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); | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 :)
Just to mention,
|
The errors here are:
|
Might have an idea of how to improve the config command syntax. Converting to draft |
Maybe something like this:
|
Looks good to me! |
I'll wait with refining this until the sway 1.9 rebase is merged :) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Y'all don't need to comment here to get notifications, just creates spam. The right side panel has a button for notifications :) |
Rebased from master |
Switch between workspaces with touchpad gestures
2024-01-21.01-40-39.mp4
TODO:
Animations?(We should wait until animations have landed in master)To test: