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

Shader parameters for shaders that use vulkan-only features are not included when exporting a project in headless mode #88187

Closed
nikitalita opened this issue Feb 11, 2024 · 3 comments · Fixed by #88409

Comments

@nikitalita
Copy link
Contributor

nikitalita commented Feb 11, 2024

Tested versions

v4.3.dev.custom_build [d335281]

System information

Godot v4.3.dev (d335281) - macOS 14.2.1 - Vulkan (Forward+) - integrated Apple M2 Pro - Apple M2 Pro (12 Threads)

Issue description

Related: #66842

While this issue was supposed to be fixed in #87392, it only does so if the shaders do not use any vulkan-specific features ( e.g. certain built-in functions, like findLSB()). If the shaders DO use vulkan-specific features, it fails to compile and obliterates any shader parameters.

before/after export:
image

Steps to reproduce

<path_to_editor_bin> --headless --path <retro-post-processing_path> --export-pack test_export_preset test_export.zip

should see shader compile errors in the output:

SHADER ERROR: Built-in function "findLSB(int)" is only supported on high-end platforms.
          at: (null) (:26)
ERROR: Shader compilation failed.
   at: shader_set_code (servers/rendering/dummy/storage/material_storage.cpp:90)

Open the zip, look at render_container.tscn, observe that the parameters (and the external resources that were used by those parameters) are removed.

Minimal reproduction project (MRP)

retro-post-processing.zip

@clayjohn
Copy link
Member

clayjohn commented Feb 11, 2024

The dummy rendering server should appear as a high end platform. I think there is just a single function overload for this.

edit: I think we can just change this line to false

@nikitalita
Copy link
Contributor Author

No, that won't work by itself; there's checks for rendering_method in ShaderLanguage, too:

if (OS::get_singleton()->get_current_rendering_method() == "gl_compatibility") {

This sort of thing happens in a couple of places; One other place is the ResourceImporterShaderFile:

ERR_FAIL_COND_V_EDMSG((OS::get_singleton()->get_current_rendering_method() == "gl_compatibility"), ERR_UNAVAILABLE, "Cannot import custom .glsl shaders when using the gl_compatibility rendering_method. Please switch to the forward_plus or mobile rendering methods to use custom shaders.");

I think what would work best is if there was some sort of override that the exporter could set for the rendering method that these various things look at when they need the rendering_method. Like, the desktop oses would set whatever the project config setting is for "rendering/renderer/rendering_method", the android exporter would set "rendering/renderer/rendering_method.mobile", etc.

@nikitalita
Copy link
Contributor Author

nikitalita commented Feb 11, 2024

After testing, just setting low_end will do for this particular issue, because we still set the renderer mode that’s in the project config. However, visual shaders should probably be recompiled based on what the actual renderer_method for that export is, and the resourceimportshaderfile compiles for whatever the device driver gives it for the Vulkan parameters (i.e. when it’s headless, it compiles for the lowest Vulkan API version). These are separate issues, though.

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