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

[Backport]: interface 'wl_output' has no event 4 #32487

Open
3 tasks done
FlorianFranzen opened this issue Jan 16, 2022 · 23 comments
Open
3 tasks done

[Backport]: interface 'wl_output' has no event 4 #32487

FlorianFranzen opened this issue Jan 16, 2022 · 23 comments

Comments

@FlorianFranzen
Copy link

@FlorianFranzen FlorianFranzen commented Jan 16, 2022

Preflight Checklist

Electron Version

16.0.6

What operating system are you using?

Other Linux

Operating System Version

NixOS 21.11

What arch are you using?

x64

Last Known Working Electron version

None

Expected Behavior

When electron is started using the ozone wayland platform it does not crash.

Actual Behavior

Under the latest wlroots or sway release, any electron app crashes at start with the error message in the title.

Testcase Gist URL

No response

Additional Information

This is an upstream issue in chromium and fixed with the following commit included with v99.0.4766.

However this chromium version is only uses in electron v18 nightly so far, so it might be worth back porting it to v17 and even v16.

@primeos
Copy link

@primeos primeos commented Jan 16, 2022

Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574 (Pri: 1).

I wrote a more detailed motivation for backporting here: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574#c19

This will be required when using Ozone/Wayland to prevent crashes on Wayland compositors that support version 4 of the wl_output protocol (rendering Ozone/Wayland unusable and thus requiring the use of XWayland). That version is available since Wayland 1.20.0 0 which was released on 2021-12-09 1. It already affects any Wayland compositor that is based on wlroots 0.15.0 but over time more compositors should follow until it eventually affects all (actively developed) Wayland compositors. Backporting this to M98 would also help a lot for Electron apps as Electron version 17 is currently based on that version 2 (although there is now also an independent Electron request for backporting 3).

Both fixes can be applied to M98 without any changes. For M97 I had to resolve a small merge conflict. It basically requires the following changes to dd4c3ddadbb9869f59cee201a38e9ca3b9154f4d [Chromium commit]:

@@ -233,9 +233,9 @@
  namespace ui {

  namespace {
--constexpr uint32_t kMaxSurfaceAugmenterVersion = 2;
+-constexpr uint32_t kMaxSurfaceAugmenterVersion = 1;
 +constexpr uint32_t kMinVersion = 1;
-+constexpr uint32_t kMaxVersion = 2;
++constexpr uint32_t kMaxVersion = 1;
  }

  // static

I've already tested backporting the fixes for both M97 and M98 and can confirm that it fixes the crashes (tested via Sway 1.7-rc2 which is based on wlroots 0.15).

@nornagon
Copy link
Member

@nornagon nornagon commented Jan 20, 2022

Is this the same as #32436?

@primeos
Copy link

@primeos primeos commented Jan 20, 2022

No, unfortunately not. #32436 is another issue that I can also reproduce and it's also present on "older" Wayland compositors that don't support version 4 of the wl_outupt protocol yet.

As an update on this issue: Both fixes are now backported to M98 by upstream:
https://bugs.chromium.org/p/chromium/issues/detail?id=1279574#c30

AFAIK that means they'll automatically land in Electron version 17 so I guess the question is not just whether to backport them to Electron version 16?

@nornagon
Copy link
Member

@nornagon nornagon commented Jan 20, 2022

Threw up a test cherry-pick: #32565

@nornagon
Copy link
Member

@nornagon nornagon commented Jan 20, 2022

Looks like there are some merge conflicts. I don't currently have the time to look deeper into it, but I'd be happy for someone to take over that cherry-pick PR.

@primeos
Copy link

@primeos primeos commented Jan 21, 2022

Thanks @nornagon. I did just try to backport the fixes to M96 (96.0.4664.99 to be precise) and there was luckily only a small and simple to resolve merge conflict when using Git:

diff --cc ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
index a9e723769879a,1b5585f7c18de..0000000000000
--- a/ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
+++ b/ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
@@@ -18,7 -19,8 +18,14 @@@
  namespace ui {

  namespace {
++<<<<<<< HEAD
 +constexpr uint32_t kMaxAuraShellVersion = 24;
++||||||| parent of dd4c3ddadbb98... [linux/wayland] Fixed terminate caused by binding to wrong version.
++constexpr uint32_t kMaxAuraShellVersion = 28;
++=======
+ constexpr uint32_t kMinVersion = 1;
+ constexpr uint32_t kMaxVersion = 28;
++>>>>>>> dd4c3ddadbb98... [linux/wayland] Fixed terminate caused by binding to wrong version.
  }

  // static
* Unmerged path ui/ozone/platform/wayland/host/surface_augmenter.cc

The other required change is that ui/ozone/platform/wayland/host/surface_augmenter.cc doesn't exist on M96 so I simply dropped that part from the patch:

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

        deleted by us:   ui/ozone/platform/wayland/host/surface_augmenter.cc
        both modified:   ui/ozone/platform/wayland/host/wayland_zaura_shell.cc

I've then proceeded to update your PR accordingly primeos@c5c69f1 (the diff is a bit large because I backported the original changes from M99 - I could also backport the cherry-picked commits from M98, if preferred). Those patches should now apply to M96 but I haven't done any testing. Feel free to use that changes if it helps (also applies if someone else is interested in opening a PR - I could also open a PR for a CI run but I cannot test Electron, unfortunately).

@nornagon
Copy link
Member

@nornagon nornagon commented Jan 24, 2022

@primeos if you want to throw up a PR and let our CI do the testing, that would be great!

primeos added a commit to primeos/electron that referenced this issue Jan 24, 2022
Those fixes are required to fix Ozone/Wayland on newer Wayland
compositors. The previous behaviour is incorrect but worked as long as
Wayland compositors didn't implement newer protocol versions that aren't
supported yet by Ozone/Wayland (e.g. wl_output version 4).

Fix electron#32487.
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574

Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
primeos added a commit to primeos/electron that referenced this issue Jan 24, 2022
Those patches are required to fix Ozone/Wayland on newer Wayland
compositors. The previous behaviour is incorrect but worked as long as
Wayland compositors didn't implement newer protocol versions that aren't
supported yet by Ozone/Wayland (e.g. wl_output version 4).

Fix electron#32487.
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574

Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
primeos added a commit to primeos/electron that referenced this issue Jan 24, 2022
Those patches are required to fix Ozone/Wayland on newer Wayland
compositors. The previous behaviour is incorrect but worked as long as
Wayland compositors didn't implement newer protocol versions that aren't
supported yet by Ozone/Wayland (e.g. wl_output version 4).

Fix electron#32487.
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574

Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
primeos added a commit to primeos/electron that referenced this issue Jan 24, 2022
Those patches are required to fix Ozone/Wayland on newer Wayland
compositors. The previous behaviour is incorrect but worked as long as
Wayland compositors didn't implement newer protocol versions that aren't
supported yet by Ozone/Wayland (e.g. wl_output version 4).

Fix electron#32487.
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574

Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
@primeos
Copy link

@primeos primeos commented Jan 24, 2022

@nornagon sure :) I've opened #32603 and the Linux builds already passed. However, I wasn't able to test/confirm if it works as expected (due to NixOS-specific issues / additional Electron issues). It would be awesome if you or someone else here could help with that. I've noted my results in #32603 (comment) (plus a link to the build artifact).

@erfanmola
Copy link

@erfanmola erfanmola commented Jan 26, 2022

I'm facing this issue too, using Arch Linux + Sway

@nnuel
Copy link

@nnuel nnuel commented Jan 27, 2022

Here as well with Chrome and Discord. Element defaults to Xwayland and with the wayland flags it gives the above error.

@theanonymousexyz
Copy link

@theanonymousexyz theanonymousexyz commented Jan 27, 2022

Same here on Artix + Sway. Bug reproduced with Brave and FreeTube. They work fine in XWayland.

@ChazyTheBest
Copy link

@ChazyTheBest ChazyTheBest commented Jan 27, 2022

Does this work for electron 13 too? Or does it need a different fix?

@Molytho
Copy link

@Molytho Molytho commented Jan 27, 2022

electron 13 is too different. I currently try to fix it there but wasn't able to build it yet.

@Molytho
Copy link

@Molytho Molytho commented Jan 28, 2022

The fix for electron 13 should work now. I had to make a small change in wl_keyboard related code too.

@ChazyTheBest
Copy link

@ChazyTheBest ChazyTheBest commented Jan 28, 2022

The fix for electron 13 should work now. I had to make a small change in wl_keyboard related code too.

Thank you so much.

zcbenz pushed a commit that referenced this issue Jan 31, 2022
This patch is required to fix Ozone/Wayland on recent compositors (e.g. wlroots 15).
Chromium tries to bind to interface versions it does not support. This wasn't a problem
until wl_output version 4 got released because chromium supported every version up to this.

Fix #32487.
Upstream chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574
@emvidi
Copy link

@emvidi emvidi commented Feb 3, 2022

On Arch + Sway until solved:
sudo pacman -U /var/cache/pacman/pkg/wlroots-0.14.1-3-x86_64.pkg.tar.zst && sudo pacman -U /var/cache/pacman/pkg/sway-1\:1.6.1-3-x86_64.pkg.tar.zst
and logout.

Update:
I switched to aur electron13-bin for the time being and got code-oss working again. (no need to downgrade anything)

@schmitmd
Copy link

@schmitmd schmitmd commented Feb 3, 2022

On Arch + Sway until solved: sudo pacman -U /var/cache/pacman/pkg/wlroots-0.14.1-3-x86_64.pkg.tar.zst && sudo pacman -U /var/cache/pacman/pkg/sway-1\:1.6.1-3-x86_64.pkg.tar.zst and logout.

Also can tack sway and wlroots into IgnorePkg in /etc/pacman.conf (until solved)

@emvidi
Copy link

@emvidi emvidi commented Feb 3, 2022

Yes, otherwise on upgrade is reinstalled again. I am using:
sudo pacman -Syu --ignore wlroots --ignore sway

@ckcr4lyf
Copy link

@ckcr4lyf ckcr4lyf commented Feb 8, 2022

I've version locked sway and wlroots in order to use chromium-wayland-vaapi (HW video decode support) chromium, which is basically Chrome/97.0.4692.99

Seems to work with the fix @emvidi posted!

@caleb-allen
Copy link

@caleb-allen caleb-allen commented Feb 14, 2022

I switched to aur electron13-bin for the time being and got code-oss working again

@emvidi is there anything special that needs to be done to get Code to use this electron version? In other words, does Code (and other electron apps) bundle its electron dependency with it, or does it use a system-wide version?

@jbethune
Copy link

@jbethune jbethune commented Feb 15, 2022

@emvidi
Copy link

@emvidi emvidi commented Feb 15, 2022

@caleb-allen on arch Code has a dep on electron13 (at the moment) so I have installed first the package from aur (electron13-bin) and then code. I am no expert neither on arch nor electron/code, this is what is working for me at the mom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet