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

disable/enable gpu when running under electron #3226

Closed
BKeyport opened this issue Oct 9, 2023 · 18 comments
Closed

disable/enable gpu when running under electron #3226

BKeyport opened this issue Oct 9, 2023 · 18 comments

Comments

@BKeyport
Copy link
Contributor

BKeyport commented Oct 9, 2023

Does anyone know what this is about, and how to fix it?

[28358:1008/184025.188520:ERROR:object_proxy.cc(590)] Failed to call method: org.freedesktop.portal.Settings.Read: object_path= /org/freedesktop/portal/desktop: org.freedesktop.DBus.Error.ServiceUnknown: The name org.freedesktop.portal.Desktop was not provided by any .service files

-- while we're at it, can we modify base code to start up with the mesa-loader error disabled?

Thanks!

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 9, 2023

for the mesa loader thing add

export ELECTRON_DISABLE_GPU=1

in from of the npm start
if you use my install script, that's

~/MagicMirror/installers/mm.sh

the other error is an assumption on electrons part that there is a desktop control file there but it doesn't.exist on most of our systems anymore. nothing we can do about it

@BKeyport
Copy link
Contributor Author

BKeyport commented Oct 9, 2023

I'm aware how to fix it after using your installer - I'm suggesting we come up with some way to proactively disable it.

As for the other one, good to know it's something not able to be fixed. Darn assumptions...

@khassel
Copy link
Collaborator

khassel commented Oct 9, 2023

this is coded here

I introduced this with the defensive approach, I'm open to doing it the other way around (e.g. with a new env var ELECTRON_ENABLE_GPU).

Opinions @sdetweil @rejas

@sdetweil
Copy link
Collaborator

@khassel I'm ok with enable_gpu
most wouldn't need it anyhow.

rejas pushed a commit that referenced this issue Dec 13, 2023
can be overriden with env var ELECTRON_ENABLE_GPU=1.

see #3226 

Tests will fail as long as
#3289 is not merged.
MichMich added a commit that referenced this issue Jan 1, 2024
## [2.26.0] - 01-01-2024

Thanks to: @bnitkin, @bugsounet, @dependabot, @jkriegshauser,
@kaennchenstruggle, @KristjanESPERANTO and @Ybbet.

Special thanks to @khassel, @rejas and @sdetweil for taking over most
(if not all) of the work on this release as project collaborators. This
version would not be there without their effort. Thank you guys! You are
awesome!

This release also marks the latest release by Michael Teeuw. For more
info, please read the following post: [A New Chapter for MagicMirror:
The Community Takes the
Lead](https://forum.magicmirror.builders/topic/18329/a-new-chapter-for-magicmirror-the-community-takes-the-lead).

### Added

- Added update notification updater (for 3rd party modules)
- Added node 21 to the test matrix
- Added transform object to calendar:customEvents
- Added ESLint rules for jest (including jest/expect-expect and
jest/no-done-callback)

### Removed

- Removed Codecov workflow (not working anymore, other workflow
required) (#3107)
- Removed titleReplace from calendar, replaced + extended by
customEvents (backward compatibility included) (#3249)
- Removed failing unit test (#3254)
- Removed some unused variables

### Updated

- Update electron to v27 and update other dependencies as well as github
actions
- Update newsfeed: Use `html-to-text` instead of regex for transform
description
- Review ESLint config (#3269)
- Updated dependencies
- Clock module: optionally display current moon phase in addition to
rise/set times
- electron is now per default started without gpu, if needed it must be
enabled with new env var `ELECTRON_ENABLE_GPU=1` on startup (#3226)
- Replace prettier by stylistic in ESLint config to lint JavaScript (and
disable some rules for `config/config.js*` files)
- Update node-ical to v0.17.1 and fix tests

### Fixed

- Avoid fade out/in on updateDom when many calendars are used
- Fix the option eventClass on customEvents.
- Fix yr API version in locationforecast and sunrise call (#3227)
- Fix cloneObject() function to respect RegExp (#3237)
- Fix newsfeed module for feeds using "a10:updated" tag (#3238)
- Fix issue template (#3167)
- Fix #3256 filter out bad results from rrule.between
- Fix calendar events sometimes not respecting deleted events (#3250)
- Fix electron loadurl locally on Windows when address "0.0.0.0" (#2550)
- Fix updatanotification (update_helper.js): catch error if reponse is
not an JSON format (check PM2)
- Fix missing typeof in calendar module
- Fix style issues after prettier update
- Fix calendar test (#3291) by moving "Exdate check" from e2e to
electron to run on a Thursday
- Fix calendar config params `fetchInterval` and `excludedEvents` were
never used from single calendar config (#3297)
- Fix MM_PORT variable not used in electron and allow full path for
MM_CONFIG_FILE variable (#3302)

---------

Signed-off-by: naveen <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Karsten Hassel <[email protected]>
Co-authored-by: Malte Hallström <[email protected]>
Co-authored-by: Veeck <[email protected]>
Co-authored-by: veeck <[email protected]>
Co-authored-by: dWoolridge <[email protected]>
Co-authored-by: Johan <[email protected]>
Co-authored-by: Dario Mratovich <[email protected]>
Co-authored-by: Dario Mratovich <[email protected]>
Co-authored-by: Magnus <[email protected]>
Co-authored-by: Naveen <[email protected]>
Co-authored-by: buxxi <[email protected]>
Co-authored-by: Thomas Hirschberger <[email protected]>
Co-authored-by: Kristjan ESPERANTO <[email protected]>
Co-authored-by: Andrés Vanegas Jiménez <[email protected]>
Co-authored-by: Dave Child <[email protected]>
Co-authored-by: grenagit <[email protected]>
Co-authored-by: Grena <[email protected]>
Co-authored-by: Magnus Marthinsen <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: Piotr Rajnisz <[email protected]>
Co-authored-by: Suthep Yonphimai <[email protected]>
Co-authored-by: CarJem Generations (Carter Wallace) <[email protected]>
Co-authored-by: Nicholas Fogal <[email protected]>
Co-authored-by: JakeBinney <[email protected]>
Co-authored-by: OWL4C <[email protected]>
Co-authored-by: Oscar Björkman <[email protected]>
Co-authored-by: Ismar Slomic <[email protected]>
Co-authored-by: Jørgen Veum-Wahlberg <[email protected]>
Co-authored-by: Eddie Hung <[email protected]>
Co-authored-by: Bugsounet - Cédric <[email protected]>
Co-authored-by: bugsounet <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Knapoc <[email protected]>
Co-authored-by: sam detweiler <[email protected]>
Co-authored-by: veeck <[email protected]>
Co-authored-by: Paranoid93 <[email protected]>
Co-authored-by: NolanKingdon <[email protected]>
Co-authored-by: J. Kenzal Hunter <[email protected]>
Co-authored-by: Teddy <[email protected]>
Co-authored-by: TeddyStarinvest <[email protected]>
Co-authored-by: martingron <[email protected]>
Co-authored-by: dgoth <[email protected]>
Co-authored-by: kaennchenstruggle <[email protected]>
Co-authored-by: jkriegshauser <[email protected]>
Co-authored-by: Ben Nitkin <[email protected]>
@khassel khassel closed this as completed Jan 1, 2024
@jalibu
Copy link

jalibu commented Jan 24, 2024

Modules with CSS transition animations (like stock tickers) still need it.
Some MMM-Jast users report choppy rendering since 2.26.0

I would welcome a more user-friendly way to activate this option. Not all users know how to customize the startup commands and how to set ENV variables.

What about introducing .env files for configuration?

@khassel
Copy link
Collaborator

khassel commented Jan 24, 2024

What about introducing .env files for configuration?

AFAIS this is built-in with nodejs version v20.6.0, electron is still on v18 so we would need an extra npm package for this (e.g. dotenv).

Beside this we have only 3 env vars: ELECTRON_DISABLE_GPU, MM_CONFIG_FILE, MM_PORT and the 2 last one are for running 2 mm instances from the same mm dir so it makes no sense to put them in an .env file.

So as long as we are talking about 1 variable I would prefer to add another line in package.json

        "scripts": {
                "start": "DISPLAY=\"${DISPLAY:=:0}\" ./node_modules/.bin/electron js/electron.js",
                "start:gpu": "ELECTRON_ENABLE_GPU=1 DISPLAY=\"${DISPLAY:=:0}\" ./node_modules/.bin/electron js/electron.js",
                "start:dev": "DISPLAY=\"${DISPLAY:=:0}\" ./node_modules/.bin/electron js/electron.js dev",

so users could start mm with npm run start:gpu if they need the gpu stuff. But maybe there are better ideas ...

@khassel khassel reopened this Jan 24, 2024
@khassel khassel changed the title Error on startup question... disable/enable gpu when running under electron Jan 24, 2024
@KristjanESPERANTO
Copy link
Contributor

So as long as we are talking about 1 variable I would prefer to add another line in package.json

I like this idea 🙂 I just have a small change suggestion to avoid redundancy:

    "start:gpu": "ELECTRON_ENABLE_GPU=1 && npm run start",

@sdetweil
Copy link
Collaborator

this && won't work on windows. it's not a supported platform but we give instructions for it

@KristjanESPERANTO
Copy link
Contributor

KristjanESPERANTO commented Jan 25, 2024

this && won't work on windows. it's not a supported platform but we give instructions for it

Does ELECTRON_ENABLE_GPU=1 (or export ELECTRON_ENABLE_GPU=1) work on Windows? If not I see no problem in using && here.

In the Windows part of the documentation we could add how to set the variable. It think it should be like this:

  • PowerShell: $env:ELECTRON_ENABLE_GPU=1
  • CMD: set ELECTRON_ENABLE_GPU=1

@khassel
Copy link
Collaborator

khassel commented Jan 25, 2024

this works:

"start:gpu": "ELECTRON_ENABLE_GPU=1 npm start",

@khassel
Copy link
Collaborator

khassel commented Jan 27, 2024

@jalibu is the above solution o.k. for you?

@jalibu
Copy link

jalibu commented Jan 28, 2024

@khassel to be honest, I did not really understand this issue with the GPU.
Does it occur with everyone who uses a Raspi with Bullseye? Is it an annoying warning or is it an error?

I'm asking because unfortunately, I don't have an MM myself at the moment and have never seen the problem.

Regardless of whether we opt-in or opt-out, I think the solution is ok. It would be great if we had a selection option in the installer scripts @sdetweil

@sdetweil
Copy link
Collaborator

@jalibu what would be the prompt for the user. 90% are installing mm for the first time, and don't know what a GPU is, or if they might use it. I am 7 years into using mm and not sure myself.

@codac
Copy link
Contributor

codac commented Jun 28, 2024

@khassel Regarding Node_Version: 18.17.0 and YARN_Version 1.22.19 when starting up the docker I get the following message in the protocol:
ELECTRON_ENABLE_GPU: undefined
followed by
'VERSIONS: electron: undefined; used node: 20.14.0; installed node: 20.14.0; npm: 10.7.0; pm2: '
with parameter
ELECTRON_DISABLE_GPU = 1

How to fix that protocol message?

@sdetweil
Copy link
Collaborator

we changed the GPU setting to OFF by default. so the env string is
ELECTRON_ENABLE_GPU =1

@codac
Copy link
Contributor

codac commented Jun 28, 2024

I have removed the "ELECTRON_DISABLE_GPU = 1" parameter (as you said its default now, but the protocol messages remain the same.

@sdetweil
Copy link
Collaborator

correct, you have to add

export ELECTRON_ENABLE_GPU=1

notice the output says enable

@sdetweil sdetweil reopened this Jun 28, 2024
@sdetweil
Copy link
Collaborator

the docker deployment doesn't use electron
so it doesn't matter

@khassel khassel closed this as completed Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants