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.

Sign up for GitHub

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

Jump to bottom

Fix hardware acceleration flag for Linux #4532

Merged
merged 3 commits into from
Jan 14, 2024
Merged

Fix hardware acceleration flag for Linux #4532

merged 3 commits into from
Jan 14, 2024

Conversation

Revival8697
Copy link
Contributor

@Revival8697 Revival8697 commented Jan 8, 2024 •
edited
Loading

Fix hardware acceleration flag for Linux

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

#961, already closed but issue still present.
#4526

Description

Screenshots

Here is the Before-After if the pull request is implemented as flags.

Before:
before

After:
after

Testing

Tested working (on my machine).
Test builds are available here.

Desktop

Additional context

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 8, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 8, 2024 10:56
@Revival8697
Copy link
Contributor Author

Revival8697 commented Jan 8, 2024 •
edited
Loading 

app.commandLine.appendSwitch('use-gl', 'angle')
app.commandLine.appendSwitch('use-angle', 'gl')

These two lines are not needed when tested but I included them because Chromium Docs said so.

You can remove them if they are enabled by default (by Chromium/Electron) or already implemented somewhere else.

Seems like they are, on Linux. I will remove them.

@absidue
Copy link
Member

absidue commented Jan 8, 2024

Did you test on Windows and macOS too before removing the existing flags?

If not please only add the new ones and stick them inside an if, so they only apply on Linux where they are needed.

@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jan 8, 2024
auto-merge was automatically disabled January 8, 2024 11:56

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 8, 2024 11:56
@Revival8697
Copy link
Contributor Author

Revival8697 commented Jan 8, 2024 •
edited
Loading

Did you test on Windows and macOS too before removing the existing flags?

Those flags are all deprecated. They do nothing.

#ignore-gpu-blacklist is now #ignore-gpu-blocklist but it is not really FreeTube's job to decide if it should bypass the blocklist. User can make that choice themselves or ask to remove their GPU from the blocklist.

You can add it back if you still decide to keep it.

The flag doesn't seem to cause any issue on Windows.

I can not test on MacOS since I do not own the hardware.

@Revival8697 Revival8697 changed the title Fix hardware acceleration flags Fix hardware acceleration flag for Linux Jan 8, 2024
@Revival8697
Copy link
Contributor Author

Revival8697 commented Jan 8, 2024 •
edited
Loading

If not please only add the new ones and stick them inside an if, so they only apply on Linux where they are needed.

Electron doesn't seem to have a native way to detect OS, there are examples/modules on the Internet but as I'm not very familiar with Electron, I will not attempt to do it.

If you deem it necessary to have such function, you can instruct me on how it should be done or find someone else to do it.

absidue
absidue reviewed Jan 8, 2024
View reviewed changes
src/main/index.js Outdated Show resolved Hide resolved
auto-merge was automatically disabled January 8, 2024 18:40

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 8, 2024 18:40
@absidue absidue added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Jan 9, 2024
kommunarr
kommunarr approved these changes Jan 14, 2024
View reviewed changes
Copy link
Collaborator

@kommunarr kommunarr left a comment

Choose a reason for hiding this comment

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

LGTM (tested with intel-gpu-tools like in the OP).
Screenshot_20240114_093543

@FreeTubeBot FreeTubeBot merged commit d2f14b0 into FreeTubeApp:development Jan 14, 2024
6 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 14, 2024
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Jan 15, 2024
@PikachuEXE
Merge branch 'development' into feature/playlist-2023-05-w-quick-book…
8d5f3b2 
…mark

* development: (53 commits)
  Add dearrow support for thumbnails (FreeTubeApp#4520)
  Translated using Weblate (French)
  Fix hardware acceleration flag for Linux (FreeTubeApp#4532)
  Translated using Weblate (Bengali)
  Translated using Weblate (Czech)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Turkish)
  Translated using Weblate (Spanish)
  Translated using Weblate (Arabic)
  Translated using Weblate (Italian)
  Translated using Weblate (Polish)
  Translated using Weblate (Russian)
  Translated using Weblate (French)
  Translated using Weblate (Chinese (Simplified))
  Add toggle to suppress sending additional args to external players (FreeTubeApp#4515)
  Translated using Weblate (English (United Kingdom))
  Translated using Weblate (Italian)
  Translated using Weblate (French)
  Translated using Weblate (Finnish)
  Translated using Weblate (Polish)
  ...

# Conflicts:
#	src/renderer/store/modules/settings.js
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Jan 15, 2024
@PikachuEXE
Merge branch 'feature/consistent-sharable-video-url-local' into custo…
346d6a1 
…m-builds/current

* feature/consistent-sharable-video-url-local: (30 commits)
  * Update places generating sharable YT video URLs to always return prefix https://youtu.be/
  Translated using Weblate (Chinese (Traditional))
  Translated using Weblate (Chinese (Simplified))
  Add dearrow support for thumbnails (FreeTubeApp#4520)
  Translated using Weblate (French)
  Fix hardware acceleration flag for Linux (FreeTubeApp#4532)
  Translated using Weblate (Bengali)
  Translated using Weblate (Czech)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Turkish)
  Translated using Weblate (Spanish)
  Translated using Weblate (Arabic)
  Translated using Weblate (Italian)
  Translated using Weblate (Polish)
  Translated using Weblate (Russian)
  Translated using Weblate (French)
  Translated using Weblate (Chinese (Simplified))
  Add toggle to suppress sending additional args to external players (FreeTubeApp#4515)
  Translated using Weblate (English (United Kingdom))
  Translated using Weblate (Italian)
  ...
@Revival8697 Revival8697 deleted the patch-1 branch January 15, 2024 12:51
@zestygrass zestygrass mentioned this pull request Jan 15, 2024
Closed
6 tasks
@zestygrass zestygrass mentioned this pull request Jan 30, 2024
Closed
3 tasks
@Revival8697 Revival8697 mentioned this pull request Apr 24, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@absidue absidue absidue approved these changes

@ChunkyProgrammer ChunkyProgrammer ChunkyProgrammer approved these changes

@kommunarr kommunarr kommunarr approved these changes

@PikachuEXE PikachuEXE Awaiting requested review from PikachuEXE

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc Awaiting requested review from efb4f5ff-1298-471a-8973-3d47447115dc

@MarmadileManteater MarmadileManteater Awaiting requested review from MarmadileManteater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Revival8697 @absidue @ChunkyProgrammer @kommunarr @FreeTubeBot