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 NVIDIA's threaded OpenGL optimization on Windows #71472

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

EIREXE
Copy link
Contributor

@EIREXE EIREXE commented Jan 15, 2023

Disables NVIDIA's threaded optimization which causes random stuttering on Windows with NVIDIA GPUs, virtually all performant OpenGL programs (like ryujinx) disable this to prevent stuttering and it's a well documented issue.

Should fix #33969

@EIREXE EIREXE requested review from a team as code owners January 15, 2023 18:13
@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch from e3ddbe3 to 711b0ce Compare January 15, 2023 18:22
thirdparty/README.md Outdated Show resolved Hide resolved
platform/windows/gl_manager_windows.cpp Outdated Show resolved Hide resolved
platform/windows/gl_manager_windows.cpp Outdated Show resolved Hide resolved
platform/windows/gl_manager_windows.cpp Outdated Show resolved Hide resolved
platform/windows/gl_manager_windows.cpp Outdated Show resolved Hide resolved
@EIREXE
Copy link
Contributor Author

EIREXE commented Jan 15, 2023

@bruvzg Should be OK now, I also added a fallback profile name for use in the Project Manager

@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch from 8bc071e to 7dd4110 Compare January 15, 2023 19:03
@clayjohn
Copy link
Member

Seems like a shame to add 25,000 lines of code to disable a driver setting only on one brand of GPUs, but looking at the build artifacts, overall size has changed by less than 1 mb (the size for windows build artifacts in this PR are reported at the same size as master) so maybe it isn't a big deal

@aaronfranke
Copy link
Member

Why does disabling a driver setting require 25k lines of third-party code?

@EIREXE
Copy link
Contributor Author

EIREXE commented Jan 16, 2023

Why does disabling a driver setting require 25k lines of third-party code?

ask nvidia, although we could just copy some nvidia types over and use only those

@Riteo
Copy link
Contributor

Riteo commented Jan 17, 2023

@EIREXE would it be possible (aka without too much trouble) to trim everything that isn't the threaded optimization part from the thirdparty code or is it too jumbled up for that?

I can't imagine that most big graphics programs just import this huge gigantic 25k lines header for this idiotic problem, it's silly.

I swear most weird graphics problems we stumble upon here are NVIDIA's fault.

Edit: obv I misread your comment and you already commented it as a possibility, sorry. Obviously try doing this if it's humanly feasible, knowing nvidia I wouldn't be surprised if it were a jumbled mess.

@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch from 7dd4110 to 2d4270b Compare January 17, 2023 17:12
@EIREXE
Copy link
Contributor Author

EIREXE commented Jan 17, 2023

@EIREXE would it be possible (aka without too much trouble) to trim everything that isn't the threaded optimization part from the thirdparty code or is it too jumbled up for that?

I can't imagine that most big graphics programs just import this huge gigantic 25k lines header for this idiotic problem, it's silly.

I swear most weird graphics problems we stumble upon here are NVIDIA's fault.

Edit: obv I misread your comment and you already commented it as a possibility, sorry. Obviously try doing this if it's humanly feasible, knowing nvidia I wouldn't be surprised if it were a jumbled mess.

I've done just that, seems to work fine.

@clayjohn
Copy link
Member

@EIREXE I guess you can remove the original NVAPI file right?

@EIREXE
Copy link
Contributor Author

EIREXE commented Jan 17, 2023

@EIREXE I guess you can remove the original NVAPI file right?

oh my b, I forgot to remove it

@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch from 2d4270b to c1fabd4 Compare January 17, 2023 18:37
@clayjohn clayjohn modified the milestones: 4.0, 4.x Jan 19, 2023
@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch 2 times, most recently from 0dad4ec to 947e562 Compare April 18, 2023 23:31
thirdparty/README.md Outdated Show resolved Hide resolved
@akien-mga akien-mga modified the milestones: 4.x, 4.1 Jun 13, 2023
@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch from 0c72d3a to 3fba270 Compare June 13, 2023 09:44
@EIREXE
Copy link
Contributor Author

EIREXE commented Jun 13, 2023

Sorry for the many mess-ups, should be fine now I hope.

@EIREXE EIREXE force-pushed the nvapi-threaded-optimization branch from 3fba270 to 938a837 Compare June 13, 2023 09:47
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks great!

@akien-mga akien-mga merged commit 5288a69 into godotengine:master Jun 13, 2023
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 13, 2023
@akien-mga akien-mga changed the title Disable NVIDIA's threaded optimization on Windows Disable NVIDIA's threaded OpenGL optimization on Windows Jul 5, 2023
@Calinou
Copy link
Member

Calinou commented Sep 2, 2023

For context, this is not necessary on Linux as threaded optimizations are disabled by default there.

aitorciki added a commit to aitorciki/godot that referenced this pull request Sep 22, 2023
An NVIDIA profile is applied to the current executable to disable
threaded OpenGL optimizations on Windows (see godotengine#71472). But because the
application is only added to the profile upon the profile creation,
newer executables won't be added to the profile (e.g. if the profile is
created on first launch of Godot_v4.1-stable_win64.exe, when users
update the editor and launch Godot_v4.2-stable_win64.exe, the profile
will never be applied to this new executable).
This patch fixes that scenario by splitting creating the profile (if it
doesn't exist) and adding the application (if it doesn't have a profile
applied) into two separate steps.
Applications that have been manually added to a different profile aren't
overriden to avoid confusing users who know what they're doing.
mandryskowski pushed a commit to mandryskowski/godot that referenced this pull request Oct 11, 2023
An NVIDIA profile is applied to the current executable to disable
threaded OpenGL optimizations on Windows (see godotengine#71472). But because the
application is only added to the profile upon the profile creation,
newer executables won't be added to the profile (e.g. if the profile is
created on first launch of Godot_v4.1-stable_win64.exe, when users
update the editor and launch Godot_v4.2-stable_win64.exe, the profile
will never be applied to this new executable).
This patch fixes that scenario by splitting creating the profile (if it
doesn't exist) and adding the application (if it doesn't have a profile
applied) into two separate steps.
Applications that have been manually added to a different profile aren't
overriden to avoid confusing users who know what they're doing.
YuriSizov pushed a commit to YuriSizov/godot that referenced this pull request Oct 24, 2023
An NVIDIA profile is applied to the current executable to disable
threaded OpenGL optimizations on Windows (see godotengine#71472). But because the
application is only added to the profile upon the profile creation,
newer executables won't be added to the profile (e.g. if the profile is
created on first launch of Godot_v4.1-stable_win64.exe, when users
update the editor and launch Godot_v4.2-stable_win64.exe, the profile
will never be applied to this new executable).
This patch fixes that scenario by splitting creating the profile (if it
doesn't exist) and adding the application (if it doesn't have a profile
applied) into two separate steps.
Applications that have been manually added to a different profile aren't
overriden to avoid confusing users who know what they're doing.

(cherry picked from commit 6263774)
}

// On windows we have to disable threaded optimization when using NVIDIA graphics cards
// to avoid stuttering, see https://github.com/microsoft/vscode-cpptools/issues/6592
Copy link
Contributor

Choose a reason for hiding this comment

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

The link to vscode's issue seems irrelevant, am I understand it wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@jsjtxietian jsjtxietian Nov 15, 2023

Choose a reason for hiding this comment

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

Thanks ! I will take a look, Can I help you make a pr correct this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm very busy lately, so I would be very thankful if you could do it for me

setting.isCurrentPredefined = 0;
setting.isPredefinedValid = 0;
int thread_control_val = OGL_THREAD_CONTROL_DISABLE;
if (!GLOBAL_GET("rendering/gl_compatibility/nvidia_disable_threaded_optimization")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we check nvidia_disable_threaded_optimization at the beginning of the function to avoid crash in #84880 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants