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

Add support for GNOME 42 (Ubuntu 22.04) #189

Merged
merged 3 commits into from
May 22, 2022
Merged

Add support for GNOME 42 (Ubuntu 22.04) #189

merged 3 commits into from
May 22, 2022

Conversation

ankurkotwal
Copy link
Contributor

Ensure that GNOME 42 is supported. This also adds support for Ubuntu 22.04, which includes GNOME 42 out of the box.

Ensure that GNOME 42 is supported. This also adds support for Ubuntu 22.04, which includes GNOME 42 out of the box.
@ankurkotwal
Copy link
Contributor Author

Fixes #188

@MrSquaare
Copy link

Confirms that it works for me, Ubuntu 22.04

@fin-ger
Copy link
Member

fin-ger commented Apr 24, 2022

Thanks for your testing!

For such a version increment to happen, I need testing in the most popular Linux distributions, including

  • all supported Ubuntu LTS releases
    • 20.04 (Gnome 3.36)
    • 18.04 (Gnome 3.28)
  • latest non-LTS release of Ubuntu
    • 22.04 (Gnome 42)
  • latest debian testing, stable, and old-stable
    • testing (bookworm) (Gnome 42)
    • stable (bullseye) (Gnome 3.38)
    • old-stable (buster) (Gnome 3.30)
  • Fedora stable and old-stable
    • Fedora 34 (soon EOL, maybe skip?) (Gnome 40)
    • Fedora 35 (Gnome 41)
    • Fedora 36 (currently testing, but stable soon, maybe also test?) (Gnome 42)
  • latest Arch (Gnome 42)
  • latest PopOS (22.04, Gnome 42)

I currently don't have the time to do the testing, but if someone can confirm the following points for one of these distros, it would help a lot in getting a Gnome 42 (and 41) release done:

Testing procedure

Run

$ make install PREFIX=$HOME/.local
$ dbus-run-session -- gnome-shell --nested --wayland

and check for errors and warnings regarding cpupower in the console output while doing the following steps:

  1. Do clean installation (via the dropdown)
  2. Moving both sliders in the dropdown with
    1. Mouse
    2. Arrow keys
    3. Mouse wheel
  3. Toggle Turbo Boost
  4. Change Profile
  5. Toggle Auto-Switch (testing maybe not possible as VMs don't fire AC adapter events)
  6. Open Preferences and toggle all options
  7. Do 2-4 for all available backends
  8. Create a new profile and change its position (move up and down in the list)
  9. Set all values in that new profile
  10. Select it in the extensions dropdown menu
  11. Remove the profile
  12. Open About Window
  13. Uninstall tool and polkit rules (this is known to be flaky, if this fails this is not a stopper)
  14. Install latest version from Gnome Extensions
  15. Update with this version
  16. Check if update procedure is working (Note: this requires a version increment which is only done by make release which I have not done yet, so version in installer.sh and cpufreqctl needs to be incremented manually for testing)
  17. Native installs
    1. For debian-based distros
      1. do a clean install of the latest .deb for this extension
      2. build a deb for the new version and update with the package manager and check if the update is working
    2. For red-hat based distros
      1. do a clean install of the latest .rpm for this extension
      2. build a rpm for the new version and update with the package manager and check if the update is working

Global installation method and all other installation methods are not tested (at least by me). You can do it if you want and if you encounter any issues, I will do my best to fix them.

Orca (screen reader) support can only be tested in "real" gnome-shells (non-nested). At least I couldn't get Orca running in the nested gnome-shell, but it would be nice to have basic testing for screen-readers in all distros (profile management in preferences dialog is known to be hard-to-impossible to use with orca, this will be fixed when redesigning the preferences dialog and not a stopper for a new release).

@fin-ger
Copy link
Member

fin-ger commented Apr 24, 2022

Some other things to consider:

  • Which distros are using which Gnome version?
  • Can we remove Gnome 3.28 support?
    • Ubuntu was the last distro shipping this, I think
  • Add Gnome 41 as well
  • Is there any code specific to one of the Gnome versions removed?
    • Remove that code from the codebase
  • Gnome 3.30 for debian old-stable was missing in the old version (if test is successful -> re-add, otherwise don't care)
  • Maybe remove Gnome 40 as Fedora 34 goes EOL soon

@JustCryen
Copy link

So I downloaded the current AUR git version (like the previous one) and added 41 and 42 to the metadata.json
I haven't noticed any issues with 41 and it's mostly fine on 42 but I now noticed that both sliders get "stuck" to the mouse when I interact with them.
Essentially I'm able to move them and the frequency limit does change but now when I hover over the slider the limit gets moved to where I currently have my cursor located.
This happens on both of the sliders, it can also happen to both of them at the same time.

@fin-ger fin-ger mentioned this pull request Apr 29, 2022
@fin-ger
Copy link
Member

fin-ger commented Apr 29, 2022

@JustCryen Can you test the code of this PR and follow the steps of #189 (comment)?

$ git clone https://github.com/ankurkotwal/cpupower.git -b fix/gnome42
$ cd cpupower
$ make install PREFIX=/home/<username>/.local

Critical points are the preferences window and the installation process of the polkit-rules and the background tool. So, it is best to completely uninstall any pre-existing installation before testing.

Edit:

As pointed out by @tnfru the extension needs to be enabled:

$ gnome-extensions enable cpupower@mko-sl.de

@fin-ger fin-ger mentioned this pull request Apr 29, 2022
Copy link

@adityatelange adityatelange left a comment

Choose a reason for hiding this comment

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

Works fine on Pop!_OS 22.04 LTS
image

@TJonCooper
Copy link

On Ubuntu 22.04 (Gnome Shell 42) - the preferences window pane does not open. I installed cpu-power-manager using the PPA, and then altered the metadata.json file to include "42" as this PR suggests. Everything else appears to work.

@fin-ger
Copy link
Member

fin-ger commented Apr 30, 2022

OK, looks like I have to do the testing myself. Those are contradicting results. PopOS should behave exactly the same as Ubuntu 22.04.

Also, forgive me when sounding harsh, I know you are just wanting to help. Can you please use the installation method above and stop editing the metadata on your own. I don't know whether you actually installed the correct version from the ppa where the metadata is the only change. The preferences window issue has appeared in previous versions. Also, if such an issue appears, please provide logs (as described in the Readme).

I'm sorry that such "simple" updates always take so long, but GNOME and distros have the tendency to always break our code with every new release. Therefore I switched to exhaustively testing each release on every single platform. If it turns out that PopOS behaves differently to Ubuntu 22.04, I'm afraid we have to include PopOS in our tests explicitely in the future, as it is a popular distro used by many users.

We have passed such version increments in the past and had users reporting complete failure of the extension with the issue tracker overwhelmed with reports. As long as the state for GNOME 41 and 42 of this extension is not clear, it is better to have a clear sign that these versions are not supported, yet. Most users understand that it just does not work for now.

As Ubuntu folks are now in the new position of running a bleeding edge GNOME version, you have to learn that it usually takes extensions half a year or so to adopt new GNOME releases. Ubuntu this time decided to go for GNOME 42 instead of an older release, so this is what all the Arch folks are experiencing since day one 😄

@TJonCooper
Copy link

TJonCooper commented Apr 30, 2022

Ouff - my apologies. I somehow managed to overlook the installation method above. I can confirm on Ubuntu 22.04 (GNOME version = 42.0, Windowing System = X11) that, when installing via this method:

$ git clone https://github.com/ankurkotwal/cpupower.git -b fix/gnome42
$ cd cpupower
$ make install PREFIX=/home/<username>/.local

Everything is functioning as intended - including the preferences pane. Thank you for the great work and please do not feel rushed to release a version for Gnome 42.

@fin-ger
Copy link
Member

fin-ger commented Apr 30, 2022

So I downloaded the current AUR git version (like the previous one) and added 41 and 42 to the metadata.json I haven't noticed any issues with 41 and it's mostly fine on 42 but I now noticed that both sliders get "stuck" to the mouse when I interact with them. Essentially I'm able to move them and the frequency limit does change but now when I hover over the slider the limit gets moved to where I currently have my cursor located. This happens on both of the sliders, it can also happen to both of them at the same time.

Hmm, I can't confirm that behavior. Although, I am currently an an AMD system. Does anyone else see this issue, too?

Nevertheless, I'll check Arch as working, as this is not blocking any core functionality. It's "just" annoying, I think.

@tnfru
Copy link

tnfru commented May 2, 2022

I'm on Pop!_OS 22.04 LTS x86_64 I ran the code snippet and the extensions seems to be installed and I can go to the settings via the extensions list, but it does not show up in the top bar. I triggered the option but that does not change anything. Gnome 42.

I relogged and did alt + f2 -> r

@fin-ger
Copy link
Member

fin-ger commented May 2, 2022

Hi @tnfru, thank you for your report!

Have you uninstalled any previous installation first?

Also, can you provide logs as described in the Readme?

$ journalctl /usr/bin/gnome-shell

Make sure to remove sensitive information from the logs .

@tnfru
Copy link

tnfru commented May 2, 2022

Hey I figured the application was inactive, fixed it with:
gnome-extensions enable cpupower@mko-sl.de

Works fine now

@JustCryen
Copy link

JustCryen commented May 3, 2022

@JustCryen Can you test the code of this PR and follow the steps of #189 (comment)?

$ git clone https://github.com/ankurkotwal/cpupower.git -b fix/gnome42
$ cd cpupower
$ make install PREFIX=/home/<username>/.local

Critical points are the preferences window and the installation process of the polkit-rules and the background tool. So, it is best to completely uninstall any pre-existing installation before testing.

Sorry for late reply.
I've tested the PR you requested and followed the testing procedures but I excluded anything after and including 14th instruction. (because the newest one in the GSE is not compatible with gnome 42 and I would need to make changes myself which you don't want during tests)

And it's basically the same as before, this means all seems to be working but the sliders still behave "sticky" after first interaction.

(gnome-shell:38543): Gjs-CRITICAL **: 22:14:48.666: JS ERROR: TypeError: device.grab is not a function
startDragging@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:89:20
vfunc_button_press_event@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:73:21


(gnome-shell:38543): Gjs-CRITICAL **: 22:14:48.960: JS ERROR: TypeError: this._grabbedDevice is undefined
_endDragging@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:115:17
vfunc_button_release_event@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:129:25


(gnome-shell:38543): Gjs-CRITICAL **: 22:14:55.166: JS ERROR: TypeError: device.grab is not a function
startDragging@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:89:20
vfunc_button_press_event@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:73:21


(gnome-shell:38543): Gjs-CRITICAL **: 22:14:55.976: JS ERROR: TypeError: this._grabbedDevice is undefined
_endDragging@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:115:17
vfunc_button_release_event@/home/cryen/.local/share/gnome-shell/extensions/cpupower@mko-sl.de/src/slider2.js:129:25

*** BUG ***
In pixman_region32_init_rect: Invalid rectangle passed
Set a breakpoint on '_pixman_log_error' to debug

*** BUG ***
In pixman_region32_init_rect: Invalid rectangle passed
Set a breakpoint on '_pixman_log_error' to debug

*** BUG ***
In pixman_region32_init_rect: Invalid rectangle passed
Set a breakpoint on '_pixman_log_error' to debug

polkit-rules seemed to install correctly but I'm not sure what you mean by "background tool"

So I downloaded the current AUR git version (like the previous one) and added 41 and 42 to the metadata.json I haven't noticed any issues with 41 and it's mostly fine on 42 but I now noticed that both sliders get "stuck" to the mouse when I interact with them. Essentially I'm able to move them and the frequency limit does change but now when I hover over the slider the limit gets moved to where I currently have my cursor located. This happens on both of the sliders, it can also happen to both of them at the same time.

Hmm, I can't confirm that behavior. Although, I am currently an an AMD system. Does anyone else see this issue, too?

Nevertheless, I'll check Arch as working, as this is not blocking any core functionality. It's "just" annoying, I think.

Yeah, I guess.
I'm using Nvidia but I'm not sure how relevant that is in this case.

@fin-ger
Copy link
Member

fin-ger commented May 4, 2022

@JustCryen Thank you for your detailed report!

@mastercaution Do you know what the error in slider2.js relates to? Do we need a special code in the slider for Gnome 42 onwards?

@fin-ger
Copy link
Member

fin-ger commented May 4, 2022

Hey I figured the application was inactive, fixed it with: gnome-extensions enable cpupower@mko-sl.de

Works fine now

@tnfru Ah, thank you for the hint. I will include this in the instructions!

@mastercaution
Copy link
Contributor

mastercaution commented May 4, 2022

@mastercaution Do you know what the error in slider2.js relates to? Do we need a special code in the slider for Gnome 42 onwards?

We can probably fix the sticky behavior by applying the following commit to our slider: https://gitlab.gnome.org/GNOME/gnome-shell/-/commit/88a8ba086983ffa2669facb3a6e0c4e1e540a14f

Looks like this is the only change needed for Gnome 42 regarding the slider ... I hope so ^^

EDIT: Patch for slider2.js (not tested):

diff --git a/src/slider2.js b/src/slider2.js
index 8707a8b..d8d529d 100644
--- a/src/slider2.js
+++ b/src/slider2.js
@@ -83,11 +83,7 @@ var Slider2 = GObject.registerClass({
         let device = event.get_device();
         let sequence = event.get_event_sequence();
 
-        if (sequence) {
-            device.sequence_grab(sequence, this);
-        } else {
-            device.grab(this);
-        }
+        this._grab = global.stage.grab(this);
 
         this._grabbedDevice = device;
         this._grabbedSequence = sequence;
@@ -109,10 +105,9 @@ var Slider2 = GObject.registerClass({
                 this._releaseId = 0;
             }
 
-            if (this._grabbedSequence) {
-                this._grabbedDevice.sequence_ungrab(this._grabbedSequence);
-            } else {
-                this._grabbedDevice.ungrab();
+            if (this._grab) {
+                this._grab.dismiss();
+                this._grab = null;
             }
 
             this._grabbedSequence = null;
@@ -134,14 +129,14 @@ var Slider2 = GObject.registerClass({
 
     vfunc_touch_event() {
         let event = Clutter.get_current_event();
-        let device = event.get_device();
         let sequence = event.get_event_sequence();
 
         if (!this._dragging &&
             event.type() === Clutter.EventType.TOUCH_BEGIN) {
             this.startDragging(event);
             return Clutter.EVENT_STOP;
-        } else if (device.sequence_get_grabbed_actor(sequence) === this) {
+        } else if (this._grabbedSequence && 
+                   sequence.get_slot() === this._grabbedSequence.get_slot()) {
             if (event.type() === Clutter.EventType.TOUCH_UPDATE) {
                 return this._motionEvent(this, event);
             } else if (event.type() === Clutter.EventType.TOUCH_END) {

@JustCryen
Copy link

Yup, this did the trick!
Also, no more errors.

@fin-ger
Copy link
Member

fin-ger commented May 4, 2022

Then, we have to make sure this does not break older Gnome versions.

@fin-ger
Copy link
Member

fin-ger commented May 5, 2022

Alright, I prepared a patch which should introduce the new slider code only for Gnome 42. I am currently testing distros.

Ubuntu 18.04, Ubuntu 20.04, Fedora 35 and Fedora 36 are successful so far. I am continuing tests with Debian now and will include an additional test for latest PopOS.

@fin-ger fin-ger merged commit 3a588d6 into deinstapel:master May 22, 2022
@ankurkotwal ankurkotwal deleted the fix/gnome42 branch July 23, 2022 00:53
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.

8 participants