[PS-1196] Add option to disable hardware acceleration #2896

evelez7 · 2022-06-11T17:32:05Z

Type of change

- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Address #2615 which requests an option to disable hardware acceleration in the desktop application.

Code changes

Desktop

/accounts/settings.component.html - Add checkbox to toggle hardware acceleration in application settings
/accounts/settings.component.ts - Add enableHardwareAcceleration member boolean and text variables. Also initialize along with other members and add a save function for when checkbox is used in settings. It is initialized as true by default.
locales/en(/en_GB/en_IN)/messages.json - Add text for the checkbox option
main.ts - Add an async function to toggle hardware acceleration. The function must wait for the Promise from stateService for the setting on hardware acceleration. It is called as the first line in main.bootstrap().

Common

/abstractions/state.service.ts - Add get/set enableHardwareAcceleration declarations.
/models/domain/globalState.ts - Add enableHardwareAcceleration boolean.
/services/stateMigration.service.ts - Add enableHardwareDeclaration string key and line to initialize the variable in globals.
/services/state.service.ts - Define get/set enableHardwareAcceleration, similar to other functions in that file.

Screenshots

Before you submit

- [x] I have checked for **linting** errors (`npm run lint`) (required)
- [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required)
- [ ] This change requires a **documentation update** (notify the documentation team)
- [ ] This change has particular **deployment requirements** (notify the DevOps team)

begin to address #2615 * add HTML portion for checkbox * add text for EN variations

see #2615 * add listener function for the toggle * add hardware acceleration member to state classes

* toggle hardware acceleration in main.bootstrap if set to false * change en descriptions to indicate restart is required

Hinton · 2022-10-07T06:49:07Z

Hi @evelez7,

Apologies for the late review, I think this change in general is appriciated but the number of people that have issuse with hardware acceleration is far outnumbered by those it helps. Can we invert the condition, and have HW acceleration be enabled by default.

also set the enableHardwareAcceleration initialization to false like the rest of the variables

evelez7 · 2022-10-07T23:47:46Z

Can we invert the condition, and have HW acceleration be enabled by default.

Just pushed a commit to fix this @Hinton.

Hinton

We'll need to rebase this on master, and here are some additional review feedback.

Hinton · 2022-10-10T08:38:37Z

apps/desktop/src/app/accounts/settings.component.ts

+  enableHardwareAccelerationText: string;
+  enableHardwareAccelerationDescText: string;


These should be unnecessary since we don't need to conditionally change what text is shown.

Hinton · 2022-10-10T08:38:53Z

apps/desktop/src/app/accounts/settings.component.ts

+    const enableHardwareAccelerationKey = "enableHardwareAcceleration";
+    this.enableHardwareAccelerationText = this.i18nService.t(enableHardwareAccelerationKey);
+    this.enableHardwareAccelerationDescText = this.i18nService.t(
+      enableHardwareAccelerationKey + "Desc"
+    );
+


Once 60-61 is removed, this also needs to be removed.

Hinton · 2022-10-10T08:39:27Z

apps/desktop/src/locales/en_GB/messages.json

@@ -889,6 +889,12 @@
  "enableTrayDesc": {
    "message": "Always show an icon in the system tray."
  },
+  "enableHardwareAcceleration": {


Please only change the en locale. The other locales are generated by the crowdin integration.

Hinton · 2022-10-10T08:39:31Z

apps/desktop/src/locales/en_IN/messages.json

@@ -930,6 +930,12 @@
  "enableTrayDesc": {
    "message": "Always show an icon in the system tray."
  },
+  "enableHardwareAcceleration": {


Please only change the en locale. The other locales are generated by the crowdin integration.

Hinton · 2022-10-10T08:40:12Z

libs/common/src/services/stateMigration.service.ts

@@ -59,6 +59,7 @@ const v1Keys: { [key: string]: string } = {
  enableMinimizeToTray: "enableMinimizeToTray",
  enableStartToTray: "enableStartToTrayKey",
  enableTray: "enableTray",
+  enableHardwareAcceleration: "enableHardwareAcceleration",


I don't believe we'll need a state migration for this. It's a new option that wasn't available before the migration.

Hinton · 2022-10-10T08:41:31Z

apps/desktop/src/locales/en/messages.json

@@ -930,6 +930,12 @@
  "enableTrayDesc": {
    "message": "Always show an icon in the system tray."
  },
+  "enableHardwareAcceleration": {
+    "message": "Enable hardware acceleration"


Suggested change

"message": "Enable hardware acceleration"

"message": "Use hardware acceleration"

Hinton · 2022-10-10T08:41:45Z

apps/desktop/src/locales/en/messages.json

+    "message": "Enable hardware acceleration"
+  },
+  "enableHardwareAccelerationDesc": {
+    "message": "Enable the application to make use of a discrete GPU. Restart is required."


Suggested change

"message": "Enable the application to make use of a discrete GPU. Restart is required."

"message": "By default this setting is on. Turn off only if you experience graphical issues. Restart is required."

github-actions · 2023-01-10T05:25:44Z

We can’t merge your pull request until you make the changes we’ve requested. As we haven’t heard from you recently, this pull request will be closed.

If you’re still working on this, please respond here after you’ve made the changes we’ve requested and our team will re-open it for further review.

Please make sure to resolve any conflicts with the master branch before requesting another review.

evelez7 added 5 commits June 10, 2022 22:08

add option to toggle hardware acceleration

3fac86a

begin to address #2615 * add HTML portion for checkbox * add text for EN variations

persist hardware acceleration changes

afb44b1

see #2615 * add listener function for the toggle * add hardware acceleration member to state classes

toggle hardware acceleration in desktop main

1d5a4af

* toggle hardware acceleration in main.bootstrap if set to false * change en descriptions to indicate restart is required

Add linter/prettier changes

078ea70

Remove period from option title

d9d74aa

djsmith85 added the community-pr label Jun 13, 2022

djsmith85 added the desktop Desktop Application label Jul 26, 2022

djsmith85 changed the title ~~Add option to disable hardware acceleration~~ [PS1196] Add option to disable hardware acceleration Jul 26, 2022

djsmith85 changed the title ~~[PS1196] Add option to disable hardware acceleration~~ [PS-1196] Add option to disable hardware acceleration Jul 26, 2022

evelez7 mentioned this pull request Jul 27, 2022

Feature request: disable GPU render accelerate please. #2615

Closed

Hinton self-assigned this Oct 7, 2022

enable hardware acceleration by default

ed43f82

also set the enableHardwareAcceleration initialization to false like the rest of the variables

Hinton reviewed Oct 10, 2022

View reviewed changes

djsmith85 added the needs-changes Closes PR if no further changes after 21 days label Dec 19, 2022

github-actions bot closed this Jan 10, 2023

prithvi2k2 mentioned this pull request Aug 5, 2023

[PM-3316] Feature addition - Toggle Hardware Acceleration [Desktop] #5968

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PS-1196] Add option to disable hardware acceleration #2896

[PS-1196] Add option to disable hardware acceleration #2896

evelez7 commented Jun 11, 2022 •

edited

Loading

Hinton commented Oct 7, 2022

evelez7 commented Oct 7, 2022 •

edited

Loading

Hinton left a comment

Hinton Oct 10, 2022

Hinton Oct 10, 2022

Hinton Oct 10, 2022

Hinton Oct 10, 2022

Hinton Oct 10, 2022

Hinton Oct 10, 2022

Hinton Oct 10, 2022

github-actions bot commented Jan 10, 2023

		enableHardwareAccelerationText: string;
		enableHardwareAccelerationDescText: string;

	"message": "Enable hardware acceleration"
	"message": "Use hardware acceleration"

	"message": "Enable the application to make use of a discrete GPU. Restart is required."
	"message": "By default this setting is on. Turn off only if you experience graphical issues. Restart is required."

[PS-1196] Add option to disable hardware acceleration #2896

[PS-1196] Add option to disable hardware acceleration #2896

Conversation

evelez7 commented Jun 11, 2022 • edited Loading

Type of change

Objective

Code changes

Desktop

Common

Screenshots

Before you submit

Hinton commented Oct 7, 2022

evelez7 commented Oct 7, 2022 • edited Loading

Hinton left a comment

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

Hinton Oct 10, 2022

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2023

evelez7 commented Jun 11, 2022 •

edited

Loading

evelez7 commented Oct 7, 2022 •

edited

Loading