Skip to content

Conversation

@ChALkeR
Copy link

@ChALkeR ChALkeR commented Nov 17, 2020

This adds swiftshader dir and the vk_swiftshader_icd.json file into the package template, so that they would be included into the distribution.

Without swiftshader dir, Electron crashes at launch with "Failed to launch GPU process" error on some setups.

To reproduce: build an installer, run in a Windows VM with GPU acceleration disabled.
Alternatively: with no vendor GPU drivers installed, just the default Microsoft one.

cc @MarshallOfSound

This adds `swiftshader` dir and the `vk_swiftshader_icd.json` file
into the package template, so that they would be included into the
distribution.

Without `swiftshader` dir, Electron crashes with
"Failed to launch GPU process" error on some setups.
Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

I'd like to merge this but the tests are failing. Can you please fix those?

@basz
Copy link

basz commented Dec 14, 2020

is this stalling for a reason?

@malept
Copy link
Member

malept commented Dec 14, 2020

@basz see #367 (review)

@chetbox
Copy link

chetbox commented Dec 17, 2020

Can @ChALkeR or anyone else suggest why this is failing and what needs to be done? We're keen to see this fixed given it affects anyone installing an Electron 10 app on Windows, via the Squirrel Installer, which is the recommend installer. (See electron/forge#1955)

We've found this to be a affect users of VMs with no video hardware acceleration (a vanilla VirtualBox VM in this case) and also when remotely logged into machines via Teamviewer.

@martinellimarco
Copy link

As a random user, I've tested it and it does work. If you read the details of the failure it seems it has nothing to do with the actual change. My 2 cent is that this is a problem with the CI setup, maybe someone should check that or try to run the tests again.

@malept
Copy link
Member

malept commented Dec 23, 2020

I haven't looked in detail, but I suspect that this change will fail with older versions of Electron that do not contain the swiftshader files. This module doesn't currently specify the versions of Electron that it supports, so it effectively supports pretty much all versions. If we restrict that, this will require a major version bump. Otherwise the change will need to be fixed so that it works for versions without these files.

@zac-jacobson
Copy link

A workaround we found is, if you don't actually need hardware acceleration, to turn it off.
https://www.electronjs.org/docs/tutorial/offscreen-rendering#rendering-modes

@niik
Copy link
Contributor

niik commented Jan 19, 2021

👋 We (the GitHub Desktop team) is looking into upgrading to Electron 11 and came upon this issue which made us a tad bit nervous as it seems like it would affect Electon 11 as well (right?).

We'd be happy to help in any way we can to get this over the finish line but it's not immediately obvious how.

I haven't looked in detail, but I suspect that this change will fail with older versions of Electron that do not contain the swiftshader files. This module doesn't currently specify the versions of Electron that it supports, so it effectively supports pretty much all versions. If we restrict that, this will require a major version bump. Otherwise the change will need to be fixed so that it works for versions without these files.

@malept Do you have a sense of what your preferred approach would be?

In case the approach would be to special case versions prior to Electron 10 do you envision having two different nuspecttemplate files and conditionally including one or the other?

let templateData = await fs.readFile(path.join(__dirname, '..', 'template.nuspectemplate'), 'utf8');

Or would you rather attempt to add the entries to the existing template at runtime?

And last question (for now), what method would you prefer for detecting whether these files need to be included? Should we just check whether they exist in the appDirectory or would you like to specifically check the bundled electron version?

@malept
Copy link
Member

malept commented Jan 19, 2021

Or would you rather attempt to add the entries to the existing template at runtime?

I would prefer changing the existing template at runtime so that we don't have to potentially maintain two slightly different templates, but if it ends up being too much work, I guess we'll have to deal with it.

And last question (for now), what method would you prefer for detecting whether these files need to be included? Should we just check whether they exist in the appDirectory or would you like to specifically check the bundled electron version?

That is a good question. Try checking for existence first, and if that ends up being too complicated, let's talk about checking the Electron version.

@electron-bot
Copy link

🎉 This issue has been resolved in version 4.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants