Skip to content

GNOME 44 support#234

Merged
Leleat merged 8 commits intoubuntu:mainfrom
3v1n0:gnome-44-support
Mar 15, 2023
Merged

GNOME 44 support#234
Leleat merged 8 commits intoubuntu:mainfrom
3v1n0:gnome-44-support

Conversation

@3v1n0
Copy link
Copy Markdown
Collaborator

@3v1n0 3v1n0 commented Feb 24, 2023

GNOME removed grab op's manual handling support, but we can avoid them by just adapting the motion.

Tested in both wayland and X11, while I didn't test if older versions could still work using the same codepath (yet).

@3v1n0 3v1n0 marked this pull request as draft February 24, 2023 18:09
@3v1n0 3v1n0 marked this pull request as ready for review February 24, 2023 18:46
@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Feb 24, 2023

From what I can tell meta_display_get_grab_op also got removed (or replaced with meta_display_is_grabbed) in https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2683/diffs#20cc8d2a8631b82d45af18ada7cf50e191b61954_2079_1717.

So https://github.com/Leleat/Tiling-Assistant/blob/main/tiling-assistant%40leleat-on-github/src/extension/moveHandler.js#L489 also needs an update. Rest looks good and from quick testing (in a VM) it works well.

PS. Officially (a.k.a. according to the README) I only support the latest Shell version. In the past I occasionally supported multiple versions, if the the compatibility code wasn't too big. So feel free to drop 43 support, if you want. Ofc you can also leave it as is, up to you.

@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Feb 26, 2023

I've encountered a crash when testing on GNOME 44 (Fedora Rawhide). It might be related to the new grab mechanic. I haven't looked it deeply yet so I can't tell if it's an issue with T-A, mutter, or g-s.

STR with a (near?) 100% success rate:

  1. Be on X11 and have at least 2 windows open
  2. Tile a window so that the Tiling Popup appears
  3. Escape the Tiling Popup with the mouse (by clicking outside somewhere)
  4. Quickly try to open the overview with the hot corner. If you wait a few seconds or click/focus something else the crash won't occur.

@3v1n0 3v1n0 force-pushed the gnome-44-support branch from 4066e72 to abec297 Compare March 7, 2023 05:35
@3v1n0
Copy link
Copy Markdown
Collaborator Author

3v1n0 commented Mar 7, 2023

From what I can tell meta_display_get_grab_op also got removed (or replaced with meta_display_is_grabbed) in https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2683/diffs#20cc8d2a8631b82d45af18ada7cf50e191b61954_2079_1717.

So https://github.com/Leleat/Tiling-Assistant/blob/main/tiling-assistant%40leleat-on-github/src/extension/moveHandler.js#L489 also needs an update. Rest looks good and from quick testing (in a VM) it works well.

Good catch! Changed that too.

PS. Officially (a.k.a. according to the README) I only support the latest Shell version. In the past I occasionally supported multiple versions, if the the compatibility code wasn't too big. So feel free to drop 43 support, if you want. Ofc you can also leave it as is, up to you.

Well, changes were minor enough to be able to support both versions without many efforts I think.

@3v1n0
Copy link
Copy Markdown
Collaborator Author

3v1n0 commented Mar 7, 2023

STR with a (near?) 100% success rate:

Mh, I've not been able to reproduce that, but I may need to check it further.

@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Mar 11, 2023

STR with a (near?) 100% success rate:

Mh, I've not been able to reproduce that, but I may need to check it further.

I was able to reproduce the crash with the normal app switcher as well. So I've opened https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6491.

Since the crash isn't T-A's fault (probably), I think this is good to go once you've rebased.

@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Mar 12, 2023

Okay, one more thing: Meta.later_add needs replacing as well. But that really should be it 😅

@3v1n0
Copy link
Copy Markdown
Collaborator Author

3v1n0 commented Mar 14, 2023

Okay, one more thing: Meta.later_add needs replacing as well. But that really should be it sweat_smile

Yeah, i noticed that too... I've fixed it.

@3v1n0 3v1n0 force-pushed the gnome-44-support branch from abec297 to 28365a8 Compare March 14, 2023 19:48
@3v1n0
Copy link
Copy Markdown
Collaborator Author

3v1n0 commented Mar 14, 2023

I was able to reproduce the crash with the normal app switcher as well. So I've opened https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6491.

I've moved it to mutter, but personally I can't reproduce it in 44.rc at least... :(

@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Mar 14, 2023

I've moved it to mutter, but personally I can't reproduce it in 44.rc at least... :(

Hmm, so either it's a GNOME bug or it's specific to my setup. Either way, it's not T-A issue so whatever I guess.

Only found 2 remaining issues (or 1 issue and 1 nit). After you've fixed them, feel free to merge this. Thanks for the PR!

@3v1n0 3v1n0 force-pushed the gnome-44-support branch from 28365a8 to eceb707 Compare March 14, 2023 21:43
3v1n0 added 8 commits March 14, 2023 22:46
GNOME Shell 44 includes new Clutter Grabs, so we don't have to manually
perform the grab-end/grab-begin actions when the user tries to untile a
window by ungrabbing it.

We can just resize the window and start monitor for move events again.
Do not unconditionally reset the user settings, as these could be different,
instead save the user set old value and restore it if that's the case, or
restore the setting otherwise
…available

These have been removed in GNOME 44, so we need to check if they're
available before trying to get any value from it.
As other idles events these should be cancelled when destroyed.
We can just cancel it every time we require to hide the actor and stop any
eventual show later set in case
It can be used to add and remove laters in both 44 and 43.
@3v1n0 3v1n0 force-pushed the gnome-44-support branch from eceb707 to 150c42b Compare March 14, 2023 21:46
@Leleat Leleat merged commit 8f4cb3e into ubuntu:main Mar 15, 2023
@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Mar 15, 2023

Thanks!

@major
Copy link
Copy Markdown

major commented Mar 20, 2023

@Leleat This change looks great on Fedora 38 beta with GNOME 44.rc. 👏

@GuillaumeAmat
Copy link
Copy Markdown

Hey @Leleat, any idea on when the next extension's release would be available? Thanks 🙏

@Leleat
Copy link
Copy Markdown
Collaborator

Leleat commented Mar 23, 2023

@GuillaumeAmat I'll make a new release after #244.

@GuillaumeAmat
Copy link
Copy Markdown

Thanks for the heads-up 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants