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

Revert "feat: Set ELECTRON_OZONE_PLATFORM_HINT=auto as the default" #1438

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

castrojo
Copy link
Member

Reverts #1434

Kyle reporting Discord crashes, reverting for now.

@castrojo castrojo merged commit d52be58 into main Jun 24, 2024
2 of 6 checks passed
@castrojo castrojo deleted the revert-1434-main branch June 24, 2024 18:38
@polkaulfield
Copy link
Contributor

polkaulfield commented Jun 24, 2024

Damn. Discord worked fine here. Should we add it just for the nvidia flavors? Edit: I can reproduce the crash now on Nvidia. Weird.

@polkaulfield
Copy link
Contributor

I have been testing most of the commonly used apps and there's a crash with Slack too. With ELECTRON_OZONE_PLATFORM_HINT=x11 set up on flatseal both run fine.

@KyleGospo
Copy link
Member

KyleGospo commented Jun 24, 2024

I have been testing most of the commonly used apps and there's a crash with Slack too. With ELECTRON_OZONE_PLATFORM_HINT=x11 set up on flatseal both run fine.

Requiring users to have to manually go set environment arguments to make their applications stop breaking is a terrible experience, we can't ship this.

I would argue that any change to an application's behavior that isn't absolutely necessary should be closed and not accepted from here on.

Additionally, numerous other applications had broken borders or even wrong borders.

@polkaulfield
Copy link
Contributor

Damn... You are absolutely right.

Im using NVIDIA and everything was unusable almost without this flag. Thought it wouldn't create problems using auto instead of forcing wayland.

I tested a bunch of electron apps but there are way too many to check i guess, and yeah Discord is absolutely broken, i tested with Webcord and probably had a brain fart thinking i did with the official Discord app.

Maybe we could add this workaround in the documentation as a manual fix using flatseal for individual apps instead of setting it globally.

I apology for the breakage it has caused :(

@KyleGospo
Copy link
Member

KyleGospo commented Jun 24, 2024

You're good, you can't catch everything and it was for a good cause.

We caught it early so no harm. Appreciate the PR in the first place.

@polkaulfield
Copy link
Contributor

I was thinking of adding the flag just on vscode in the nvidia dx builds. That one doesn't break from my testing, it's unusable without it. What do you think? I can open a PR with that change.

@castrojo
Copy link
Member Author

Let's get a few people running it and get some feedback. Then after that let's put it in testing for a bit and get some more eyes on it. Lots of nvidia things churning right now.

@polkaulfield
Copy link
Contributor

Yeah, I was thinking that maybe an ujust script that allows you to do it on a per app basis might be the way (that adds the flag and wayland permission to the chosen problematic apps). I found out that most of the crashes on flatpak apps can be prevented toggling the wayland permission on when having the flag enabled.

@polkaulfield
Copy link
Contributor

I'll write one and send a PR to the testing branch when i have time if you think it's a good idea.

@castrojo
Copy link
Member Author

Should we wait for the next nvidia driver drop? I'm cognizant of adding a bunch of skaffolding and then having to throw it away later.

@polkaulfield
Copy link
Contributor

Well, I wrote a proof of concept script that adds the tweak for the flatpaks you choose.
https://gist.github.com/polkaulfield/f1c83ca70735edbeed2d52c78e8c9248
Maybe it's a good approach to have an ujust script with that

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.

3 participants