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.

Sign up for GitHub

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

Jump to bottom

fix: update GrabOp classification for GNOME 44 #1618

Merged
merged 2 commits into from
Apr 27, 2023
Merged

fix: update GrabOp classification for GNOME 44 #1618

merged 2 commits into from
Apr 27, 2023

Conversation

mooey5775
Copy link
Contributor

Fixes #1615, which is broken due to updates in Clutter's Grab handling behavior in GNOME/Mutter 44. This is described in this merge request (https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2683), and specifically caused by this commit (https://gitlab.gnome.org/GNOME/mutter/-/commit/12773cf8e2954082f346dee300af624b25baf23c)

More concretely, a new META_GRAB_OP_WINDOW_FLAG_UNCONSTRAINED flag is added, which should be ignored when determining if a grab op is a move op or not. Unfortunately, this flag is not exposed publicly, and is not backwards compatible with prior versions of GNOME.

The most principled way to fix this is to use these functions in mutter's display.c (https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/core/display.c#L1186), but these are also not public. I've reimplemented here in JS, with some more bit operations to recover the flags that are used. This may not be the most maintainable way to do this, am open to comments.

This works on Fedora 38 (GNOME 44) and Fedora 37 (GNOME 43), although I haven't tested anything older.

f38_postcommit.webm
f37_postcommit.webm

mooey5775
mooey5775 commented Apr 20, 2023
View reviewed changes
src/extension.ts
@@ -1411,7 +1411,7 @@ export class Ext extends Ecs.System<ExtEvent> {

/** Display an overlay indicating where the window will be placed if dropped */

if (overview.visible || !win || op !== 1) return
if (overview.visible || !win || is_keyboard_op(op) || is_resize_op(op)) return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried to replicate the semantics of the earlier code, but it's unclear to me what the condition here is. It doesn't seem to hurt to display overlays if we're doing a keyboard move op, but the previous code doesn't.

@mooey5775 mooey5775 marked this pull request as ready for review April 20, 2023 20:47
@emko emko mentioned this pull request Apr 21, 2023
Closed
5 tasks
@mooey5775
Copy link
Contributor Author

@mmstick bump on this if you have the chance - sorry for the noise

@mmstick mmstick requested a review from a team April 26, 2023 18:55
@n3m0-22 n3m0-22 self-assigned this Apr 26, 2023
@n3m0-22
Copy link
Contributor

n3m0-22 commented Apr 26, 2023

@mooey5775 Going over the steps listed in #1615 I have been unable to reproduce the issue this is meant to solve. I am able to move, stack, and un-stack windows using both the mouse and keyboard without this PR. Are you running Fedora?

@mmstick
Copy link
Member

mmstick commented Apr 27, 2023

@n3m0-22 Yes they're on Fedora 38 with GNOME 44. As long as this doesn't cause regression in Pop with GNOME 42, then I'm fine with merging it.

n3m0-22
n3m0-22 approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@n3m0-22 n3m0-22 left a comment

Choose a reason for hiding this comment

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

Tested with Pop 22.04 on lemp11 and gaze15. All regression testing passed.

pop-shell-1618-testing.txt

@mmstick mmstick merged commit b5accce into pop-os:master_jammy Apr 27, 2023
@mooey5775
Copy link
Contributor Author

Thanks!

For clarity, this problem only happens on GNOME 44 if windows are moved with the window action key (usually Super + [mouse drag]. Moving windows by dragging their titlebars would work either before or after this PR.

@flokli flokli mentioned this pull request Apr 28, 2023
Merged
12 tasks
@flokli
Copy link

flokli commented Apr 28, 2023

Confirmed this fixes the issue for me. Thanks!

piegamesde pushed a commit to NixOS/nixpkgs that referenced this pull request Apr 28, 2023
@flokli
gnomeExtensions.pop-shell: unstable-2023-04-05 -> unstable-2023-04-27
fdfea5f 
This contains pop-os/shell#1618, which should
fix grab handling behaviour in GNOME/Mutter 44.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmstick mmstick mmstick approved these changes

@n3m0-22 n3m0-22 n3m0-22 approved these changes

Assignees

@n3m0-22 n3m0-22

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot Manage Tiling With Mouse on GNOME 44
4 participants
@mooey5775 @n3m0-22 @mmstick @flokli