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

Add device info to GLES3 shader cache key hash #82359

Merged
merged 1 commit into from Sep 26, 2023

Conversation

bitsawer
Copy link
Member

Adds some extra device info to the shader base cache key hash so that toggling between two different GPU's gives each device their own cache files. I also added the driver version, so driver updates will now also invalidate the cache. On my device, these values are:

GL_VENDOR: NVIDIA Corporation
GL_RENDERER: NVIDIA GeForce GTX 970/PCIe/SSE2
GL_VERSION: 3.3.0 NVIDIA 536.23

@bitsawer bitsawer added this to the 4.2 milestone Sep 26, 2023
@bitsawer bitsawer requested a review from a team as a code owner September 26, 2023 10:12
@RandomShaper
Copy link
Member

I'd like to elaborate on the different caches:

  • RenderingDevice (namely, Vulkan):
    1. We're caching SPIR-V representation of shaders so they don't need to be recompiled from GLSL. (GPU-agnostic.)
    2. We're also caching pipelines so they don't need to be created from SPIR-V. (GPU-specific.)
  • GL ES 3:
    3. We are caching shader binaries. That's more similar to 2) than 1). It's GPU-specific.

Where I'm getting with this is simply that the file name of the GL ES 3 shader cache could include the GPU device name just as fine as the Vulkan RD one does. For your reference:

pipelines_cache.file_path = "user://vulkan/pipelines";
pipelines_cache.file_path += "." + context->get_device_name().validate_filename().replace(" ", "_").to_lower();
if (Engine::get_singleton()->is_editor_hint()) {
pipelines_cache.file_path += ".editor";
}
pipelines_cache.file_path += ".cache";

@bitsawer
Copy link
Member Author

I could add the device name prefix to the cached files too, but I'm not really sure its all that useful compared to the hashing approach where the device name is rolled into the file name hash. I consider it a bit more reliable this way, we don't have to worry about what kind of weird or possibly long name the driver gives us, especially as the user directory+cache directory+file hashes start to get pretty long already and can already approach Windows default path limit of 260. The name format is not really even standardized, most are similar but there are some pretty weird out there.

@RandomShaper
Copy link
Member

No big deal. I was just thinking that it could be good for feature parity, but switching GPUs may not be as frequent with GL ES 3.

The path length issues would already be a (very real) matter for Vulkan and should be tackled anyway.

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.

Looks good to me.

I'm happy to hash these instead of including in the filename as the filename includes the hash anyway.

@akien-mga akien-mga merged commit 36945da into godotengine:master Sep 26, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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