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

Fixes #134612: Added electron flags for wayland #135191

Closed
wants to merge 1 commit into from

Conversation

zzeebbii
Copy link

enable-features and the ozone-platform flag will be parsed from the runtime argv.json file.
This PR fixes #134612

@zzeebbii
Copy link
Author

@deepak1556 can you please at least start the workflows for this PR? :)

@deepak1556 deepak1556 added this to the November 2021 milestone Oct 29, 2021
Copy link
Collaborator

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, left some style comments

src/main.js Outdated Show resolved Hide resolved
src/vs/workbench/electron-sandbox/desktop.contribution.ts Outdated Show resolved Hide resolved
src/vs/workbench/electron-sandbox/desktop.contribution.ts Outdated Show resolved Hide resolved
@deepak1556
Copy link
Collaborator

Additionally it would be good to implement a auto value in Electron similar to what is implemented for chrome://flags https://bugs.chromium.org/p/chromium/issues/detail?id=1246928

@zzeebbii zzeebbii force-pushed the fix/electron-wayland branch 2 times, most recently from e67c2e8 to 2cc4453 Compare November 9, 2021 17:04
@zzeebbii
Copy link
Author

zzeebbii commented Nov 9, 2021

Additionally it would be good to implement a auto value in Electron similar to what is implemented for chrome://flags https://bugs.chromium.org/p/chromium/issues/detail?id=1246928

@deepak1556 I've fixed the things you mentioned but I couldn't understand what you said about auto. Can you please explain a little bit more?

src/main.js Outdated
else if (argvValue === true || argvValue === 'true') {
if (argvKey === 'disable-hardware-acceleration') {
app.disableHardwareAcceleration(); // needs to be called explicitly
} else {
app.commandLine.appendSwitch(argvKey);
}
}

// Other arguments with value type as 'string'
else if (typeof argvValue === 'string') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
else if (typeof argvValue === 'string') {
// ozone platform
else if (argvKey === 'ozone-platform') {
if (argvValue) {
app.commandLine.appendSwitch(argvKey, argvValue);
app.commandLine.appendSwitch('enable-features', 'UseOzonePlatform');
}
}

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@deepak1556
Copy link
Collaborator

auto flag is more of a note to me, it needs to be implemented in Electron similar to the work done in https://bugs.chromium.org/p/chromium/issues/detail?id=1246928 so that most of the time runtime picks the correct ozone platform.

ozone-platform flag will be parsed from runtime argv.json file.
@torhovland
Copy link

I would love to see this one merged!

@zzeebbii
Copy link
Author

@deepak1556 is there any update on this one? :)

@deepak1556 deepak1556 modified the milestones: February 2022, March 2022 Feb 24, 2022
@deepak1556 deepak1556 removed this from the March 2022 milestone Mar 23, 2022
@zzeebbii
Copy link
Author

Hi @deepak1556 is there any plan to merge this PR?

@deepak1556 deepak1556 added this to the May 2022 milestone May 2, 2022
@joyceerhl joyceerhl modified the milestones: May 2022, June 2022 Jun 3, 2022
@deepak1556 deepak1556 removed this from the June 2022 milestone Jun 30, 2022
@tobiasgrosser
Copy link

Hi, I would like to use this patch. Is there anything blocking the merge and can I help in some way with finalizing this patch?

@kowalski7cc
Copy link

Can be --enable-features=WaylandWindowDecorations added as well? It would help with decorations when not using the custom titlebar

else if (argvKey === 'ozone-platform') {
if (argvValue) {
app.commandLine.appendSwitch(argvKey, argvValue);
app.commandLine.appendSwitch('enable-features', 'UseOzonePlatform');
Copy link
Contributor

@vially vially Aug 4, 2022

Choose a reason for hiding this comment

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

The UseOzonePlatform feature no longer needs to be explicitly enabled since a couple of Electron releases ago, so this can be safely removed.

Suggested change
app.commandLine.appendSwitch('enable-features', 'UseOzonePlatform');

@@ -167,6 +167,9 @@ function configureCommandlineSwitchesSync(cliArgs) {

// Force enable screen readers on Linux via this flag
SUPPORTED_ELECTRON_SWITCHES.push('force-renderer-accessibility');

// Specify ozone platform implementation to use.
SUPPORTED_ELECTRON_SWITCHES.push('ozone-platform');
Copy link
Contributor

Choose a reason for hiding this comment

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

Since #154092, Visual Studio Code is using an Electron version that supports the --ozone-platform-hint flag. So it may be worth enabling this flag too (ozone-platform-hint) as part of this pull-request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vially unfortunately we cannot support ozone-platform-hint from application code since the flag detection happens before application code starts its execution which would be in ElectronBrowserMainParts::PreMainMessageLoopRun

@ReillyBrogan
Copy link

Posting this here for visibility. I updated Electron to 19.0.10 locally to experiment with the new --ozone-platform-hint flag and one thing that I noticed is that Electron running in Wayland sets the window app_id to the name of the desktop file (sans .desktop), as defined during the build. Unfortunately VSCode seems to set this to the name of the URL handler desktop file instead which means that the DE will not correctly identify vscode windows as belonging to the vscode application which means that they will not be associated with vscode in the "taskbar" (or whatever the given DE uses) and various other undesired behavior.

I'm not sure why VSCode sets it to the URL handler, as far as I can tell the URL handler works fine if it's set to the normal desktop file instead so I don't think that setting it to that is actually required for the URL handler.

Ref (read the Electron dev comments as well as my own): electron/electron#34855 (comment)
Code that sets it to the url handler:

packageJsonUpdates.desktopName = `${product.applicationName}-url-handler.desktop`;

I'm mentioning this as part of this PR because the individuals involved in this PR are clearly interested in Wayland on Linux and this will need to be resolved if Wayland is made the default (for Wayland users) or even if it's exposed as an option directly.

@deepak1556
Copy link
Collaborator

Sorry for the delay on this, looking into the changes again ozone platform initialization happens earlier https://github.com/electron/electron/blob/main/shell/browser/electron_browser_main_parts.cc#L220-L221 before entry point JS code of VSCode can run which makes it difficult to support this flag as an application configurable argument. Users are required to continue specifying either --ozone-platform=<wayland|x11> or --ozone-platform-hint=<wayland|x11|auto> when launching the application to get the desired backend.

@scratchmex
Copy link

scratchmex commented Aug 28, 2022

@deepak1556 When is going to be a default solution for people running wayland? Is there any issue tracking it?

edit: maybe using this? electron/electron#34937

@ghost
Copy link

ghost commented Sep 2, 2022

Sorry for the delay on this, looking into the changes again ozone platform initialization happens earlier https://github.com/electron/electron/blob/main/shell/browser/electron_browser_main_parts.cc#L220-L221 before entry point JS code of VSCode can run which makes it difficult to support this flag as an application configurable argument. Users are required to continue specifying either --ozone-platform=<wayland|x11> or --ozone-platform-hint=<wayland|x11|auto> when launching the application to get the desired backend.

wayland is quickly becoming the default on linux distros. is there a long term plan to address this?

@sedax90
Copy link

sedax90 commented Sep 6, 2022

vscode is the only reason I could not use fractional scaling in office with wayland...

@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Wayland Electron Arguments to argv.json