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

Support GJS 1.72 Shell 42 #217

Merged
merged 9 commits into from
Apr 28, 2022
Merged

Support GJS 1.72 Shell 42 #217

merged 9 commits into from
Apr 28, 2022

Conversation

ettavolt
Copy link
Contributor

@ettavolt ettavolt commented Apr 7, 2022

Closes #212.

I'll go and ask Gnome team to open SecondaryMonitorDisplay along MonitorGroup and a couple of constants.
If they don't, we'll need to copy entire SecondaryMonitorDisplay and monkey-patch WorkspacesDisplay.

@@ -3,7 +3,8 @@ const Self = ExtensionUtils.getCurrentExtension();
const Util = Self.imports.util;
const OverviewControls = imports.ui.overviewControls;

const { SMALL_WORKSPACE_RATIO } = OverviewControls;
//const { SMALL_WORKSPACE_RATIO } = OverviewControls;
const SMALL_WORKSPACE_RATIO = 0.15;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait for Gnome Team to agree or disagree on opening this and other bits.

Copy link

Choose a reason for hiding this comment

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

Wait for Gnome Team to agree or disagree on opening this and other bits.

Did you open an issue or merge request at https://gitlab.gnome.org/GNOME/gnome-shell?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here

Or just a patch (apply with patch -p1).

@ettavolt
Copy link
Contributor Author

ettavolt commented Apr 7, 2022

With this and locally rebuilt shell (opening the above stuff) I now observe

apr 07 14:07:33 ptits-lt gnome-shell[16482]: clutter_actor_allocate: assertion '!isnan (real_allocation.x1) && !isnan (real_allocation.x2) && !isnan (real_allocation.y1) && !isnan (real_allocation.y2)' failed
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspacesView_WorkspacesDisplay>:0x55648e05d5c0] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspacesView_WorkspacesView>:0x55648faf3830] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspace_Workspace>:0x55648dff14a0] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspace_WorkspaceBackground>:0x55648dfd5880] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<ClutterActor>:0x55648e4e5f40] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<MetaBackgroundGroup>:0x55648cef3c60] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<MetaBackgroundActor>:0x55648efa6df0] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<ClutterActor>:0x55648ced7f70] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspace_Workspace>:0x55648de04020] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspace_WorkspaceBackground>:0x55648d2dac20] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<ClutterActor>:0x55648ced7930] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<MetaBackgroundGroup>:0x55648ced7610] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<MetaBackgroundActor>:0x55648d30f7d0] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<ClutterActor>:0x55648ced6fd0] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspace_Workspace>:0x55648de68c20] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<Gjs_ui_workspace_WorkspaceBackground>:0x55648e179210] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<ClutterActor>:0x55648ced6cb0] is on because it needs an allocation.
апр 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<MetaBackgroundGroup>:0x55648ced6990] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<MetaBackgroundActor>:0x55648d30fb00] is on because it needs an allocation.
apr 07 14:07:33 ptits-lt gnome-shell[16482]: Can't update stage views actor <unnamed>[<ClutterActor>:0x55648ced6670] is on because it needs an allocation.

Any ideas how to catch this NaN? :)

@pdecat
Copy link

pdecat commented Apr 9, 2022

Great work! I'd like to test this.

locally rebuilt shell (opening the above stuff)

Would you mind sharing precisely what changes you made?

@@ -9,21 +9,23 @@ var ControlsManagerLayout = class {
constructor() {
this.originalLayout = null;
this._overrideProperties = {
_computeWorkspacesBoxForState(state, workAreaBox, searchHeight, dashHeight, thumbnailsHeight) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an incompatible change. @mzur, would you please open a shell-42 branch (or take this as it)?

@ljanyst
Copy link

ljanyst commented Apr 13, 2022

Thanks for this!

There's a problem with the pop-up not disappearing when I release CTRL+ALT. I need to either select the workspace with enter or kill the popup with ESC.

@ljanyst
Copy link

ljanyst commented Apr 13, 2022

Correction. It does not change workspaces for me at all.

@ettavolt
Copy link
Contributor Author

ettavolt commented Apr 13, 2022

Correction. It does not change workspaces for me at all.

Some overrides for inaccessible Shell UI is not enough to cover everything (overview is broken), but it has to be disabled if you are running a patched shell.

Edit: I've just pushed a couple of changes that should make the last commit compatible with a patched shell.

@ljanyst
Copy link

ljanyst commented Apr 13, 2022

Thanks for the explanation! How fast do you think the patched shell will be available? Is it worth it to invest time in building custom Debian packages? Can you please point me to the patches?

@ettavolt
Copy link
Contributor Author

Give that I saw noone in the shell PR, not soon.
Debian packages are harder than Arch's, can't say.
Patch

@ljanyst
Copy link

ljanyst commented Apr 13, 2022

Thanks!

@zweif
Copy link

zweif commented Apr 16, 2022

I added a commit for drag and drop: zweif@60c0a0c
I also tried to get workspace selection by clicking to work, but without success. The click event doesn't seem to be noticed for clicks in lower rows. Clicks on windows in thumbnails are working somehow.

@esauvisky
Copy link
Contributor

esauvisky commented Apr 18, 2022

First of all thanks for your work on porting this over to Gnome 42.

I checked out this PR and most things work, besides having a bug in which when switching to any workspace without windows, the windows of a different workspace (couldn't determine which) would show up, as if they were on that workspace instead to begin with.
It's not possible to interact with the applications outside their actual workspace though.

I don't have much time and even less any knowledge on GJS but managed to pinpoint the issue via this journal error:

[...]
abr 18 08:06:30 gnome-shell[10586]: Called enable_unredirect_for_display while unredirection is enabled.
abr 18 08:06:30 gnome-shell[10586]: JS ERROR: TypeError: Object 0x389ef8fc1988 is not a subclass of GObject_Object, it's a Object
                                    _init@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceAnimation.js:91:14
                                    MonitorGroup@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceAnimation.js:19:4
                                    _prepareWorkspaceSwitch@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceAnimation.js:241:27
                                    animateSwitch@resource:///org/gnome/shell/ui/workspaceAnimation.js:371:14
                                    _switchWorkspace@resource:///org/gnome/shell/ui/windowManager.js:1632:34
                                    actionMoveWorkspace@resource:///org/gnome/shell/ui/windowManager.js:1836:23
                                    _showWorkspaceSwitcher@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceManagerOverride.js:426:25
                                    Caused by: Error: This JS object wrapper isn't wrapping a GObject. If this is a custom subclass, are you sure you chained up to the parent _init properly?

I assumed the subsequent code was probably not being executed and wrapped the instantiation of WorkspaceGroup in a try/catch block, which apparently worked around the issue for now:

        try {
            const stickyGroup = new WorkspaceGroup(null, monitor, movingWindow);
            this.add_child(stickyGroup);
        } catch (e) {}

Let me know if there is anything I can do to help with the porting.

@mzur
Copy link
Owner

mzur commented Apr 20, 2022

I added a commit for drag and drop: zweif@60c0a0c I also tried to get workspace selection by clicking to work, but without success. The click event doesn't seem to be noticed for clicks in lower rows. Clicks on windows in thumbnails are working somehow.

@zweif This issue was already observed for GNOME Shell 40 (see #177).

Copy link
Owner

@mzur mzur left a comment

Choose a reason for hiding this comment

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

As far as I can see, the only remaining issues are caused by the objects that can no longer be imported. I doubt that this issue will be processed quickly by the GNOME Shell core team. So if we want to release something for GNOME 42, we need to copy the core code. But we should do it in a way that is easily reversible once the objects can be imported again (probably for GNOME Shell 43).

Comment on lines +256 to +270
_syncStacking() {
const windowActors = global.get_window_actors().filter(w =>
this._shouldShowWindow(w.meta_window));

let lastRecord;

for (const windowActor of windowActors) {
const record = this._windowRecords.find(r => r.windowActor === windowActor);

if (record && lastRecord) {
this.set_child_above_sibling(record.clone, lastRecord ? lastRecord.clone : this._background);
lastRecord = record;
}
}
},
Copy link
Owner

@mzur mzur Apr 20, 2022

Choose a reason for hiding this comment

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

@ettavolt Could you please explain why this is override necessary? The (record && lastRecord) will never be true. Otherwise I see no difference to the upstream code.

@mzur
Copy link
Owner

mzur commented Apr 21, 2022

@ebeem Could you please explain what the SecondaryMonitorDisplay override was meant to do? The overview seems to look alright on all monitors if I disable the override (this is one of the things that can't be imported any more in GNOME 42).

@ebeem
Copy link
Collaborator

ebeem commented Apr 21, 2022

@mzur I think it overrides the app grid mode.
So if you have multi-monitors with the app grid view activates (double super), you should be able to see the thumbnails of the workspaces on the other monitors in a grid rather than a row.

@ljanyst
Copy link

ljanyst commented Apr 22, 2022

I had a look and it seems to mostly work. Thanks everyone for fixing it. There's one regression though. In the previous version, the grid stayed on the screen as long as you kept CTRL+ALT pressed. Now, you either have to confirm the workspace selection with enter or set up a timeout for it to disappear. Any chance the old behavior can be restored?

@mzur
Copy link
Owner

mzur commented Apr 22, 2022

@ebeem Thanks! This is what it looks like for me if the override is disabled:

Screenshot from 2022-04-22 15-55-10

I guess the override would fix the positioning of the workspaces on the second screen. But to be honest, I could live with the buggy behavior until the objects are made public again by GNOME Shell. Does anyone have objections?

The override can be enabled again once the following issue is
resolved and published in a new GNOME version:

https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2266
@mzur
Copy link
Owner

mzur commented Apr 22, 2022

@ljanyst Thanks, I can confirm this issue.

@ebeem Could you point out where the logic for the key event is located that lets the popup stay open while the keys are pressed? Then I can try to figure out why it no longer works in GNOME 42.

@ebeem
Copy link
Collaborator

ebeem commented Apr 22, 2022

The return state here is null for some reason, I will try to debug it more, but this is either an upstream issue or there is some update that breaks this functionality

@zweif
Copy link

zweif commented Apr 22, 2022

I added a commit for drag and drop: zweif@60c0a0c I also tried to get workspace selection by clicking to work, but without success. The click event doesn't seem to be noticed for clicks in lower rows. Clicks on windows in thumbnails are working somehow.

@zweif This issue was already observed for GNOME Shell 40 (see #177).

I wasn't aware of this issue and it seems to match my observations, but drag and drop works for me with this change.

@ahopkins
Copy link

Eagerly awaiting this... Anything that I can help with?

@mzur mzur changed the base branch from master to gnome-42 April 28, 2022 19:37
@mzur mzur merged commit dc8bf5f into mzur:gnome-42 Apr 28, 2022
@mzur mzur mentioned this pull request Apr 28, 2022
2 tasks
@mzur
Copy link
Owner

mzur commented Apr 28, 2022

This can continue in #218. I'll create a beta release from that branch.

mzur added a commit that referenced this pull request Apr 29, 2022
It didn't seem to have any effect.

References #217 (comment)
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.

GNOME 42 and GJS >= 1.71 support
8 participants