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

Check for GLES3 shader cache support before using it #80680

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bitsawer
Copy link
Member

Experimental draft PR for logging and possibly fixing some GLES3 shader cache related issues on older graphics cards and drivers. See #79115 and #78888 This is mostly for providing test PR and possibly builds for those who can't build their own Godot engine.

This PR adds heavy logging as it seems some drivers seem to blow up when attempting to use their shader cache. Also adds is_shader_cache_supported() function which tries to test if some cache related values are available before attempting to cache shader data. It seems that many reported issues are related to devices that report GL_NUM_PROGRAM_BINARY_FORMATS to be zero, see https://registry.khronos.org/OpenGL/extensions/ARB/ARB_get_program_binary.txt, so in these cases it might be best not to use cache at all.

@bitsawer bitsawer added this to the 4.x milestone Aug 16, 2023
@bitsawer bitsawer force-pushed the opengl_cache_adventures branch from 491bdca to 5908a58 Compare August 16, 2023 11:39
Comment on lines +626 to +635
print_error("Test error redirect");
print_verbose("Test verbose enabled");
Copy link
Member

Choose a reason for hiding this comment

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

I guess you can remove these before users try this out

Copy link
Member Author

Choose a reason for hiding this comment

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

I left them there for now to make sure that if people post their logs they have used --verbose flag and 2>&1 error redirect on Windows, I'll remove them with other logging if we want to merge some version of this PR at some point.

@bitsawer bitsawer force-pushed the opengl_cache_adventures branch from 5908a58 to bd93053 Compare August 16, 2023 11:56
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

This looks super helpful.

I guess what we should do is get it to a place where it provides a ton of info for debugging. distribute binaries to affected users, then in a final version trim down the debug information to just what is most useful and use print_verbose. Is that what you have in mind?

Comment on lines +524 to +541
GLint max_program_binary_formats = 0;
glGetIntegerv(GL_NUM_PROGRAM_BINARY_FORMATS, &max_program_binary_formats);
CACHE_LOG("GL_NUM_PROGRAM_BINARY_FORMATS", max_program_binary_formats);

bool supported = max_program_binary_formats > 0;
CACHE_LOG("Shader cache supported", supported);

return supported;
Copy link
Member

Choose a reason for hiding this comment

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

I guess this can also be calculated at startup (and then cached) here

{
String shader_cache_dir = Engine::get_singleton()->get_shader_cache_path();
if (shader_cache_dir.is_empty()) {
shader_cache_dir = "user://";
}
Ref<DirAccess> da = DirAccess::open(shader_cache_dir);
if (da.is_null()) {
ERR_PRINT("Can't create shader cache folder, no shader caching will happen: " + shader_cache_dir);
} else {
Error err = da->change_dir("shader_cache");
if (err != OK) {
err = da->make_dir("shader_cache");
}
if (err != OK) {
ERR_PRINT("Can't create shader cache folder, no shader caching will happen: " + shader_cache_dir);
} else {
shader_cache_dir = shader_cache_dir.path_join("shader_cache");
bool shader_cache_enabled = GLOBAL_GET("rendering/shader_compiler/shader_cache/enabled");
if (!Engine::get_singleton()->is_editor_hint() && !shader_cache_enabled) {
shader_cache_dir = String(); //disable only if not editor
}
if (!shader_cache_dir.is_empty()) {
ShaderGLES3::set_shader_cache_dir(shader_cache_dir);
}
}
}
}

But that wouldn't be necessary for a testing build of course.

@bitsawer
Copy link
Member Author

Yep, I added some excessive logging as debugging this is partly shooting in the dark, Foxysen provided very good info in #79115 and seemed to pinpoint at least one place where things crash, so I'll try to ping them once the builds are ready in case they can test this. Finding other people to test this who suffer from the issue would also be great.

In the final version we probably don't need any extra logging at all, so let's first see if this manages to clear things out in any way.

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

Successfully merging this pull request may close these issues.

2 participants