SnowflakePowered / glslang-rs

Rust bindings to glslang
Other
3 stars 2 forks source link

[Suggestion] Add target_language to CompilerOptions #3

Closed GeForceLegend closed 7 months ago

GeForceLegend commented 7 months ago

Currently target_language is hardcoded as glslang_target_language_t::SPIRV in this line: https://github.com/SnowflakePowered/glslang-rs/blob/master/glslang/src/input.rs#L75

But sometimes, we may want to set it to glslang_target_language_t::None, like implementing language server for non-Vulkan shaders (as lots of them cannot pass compile targets SPIR-V). Setting it to None can correctly check OpenGL shaders on all GLSL versions, letting this crate can be used at much wider situation.

chyyran commented 7 months ago

I wouldn’t mind adding it to compiler options but the design of the API (which is strictly limited to whats available from the glslang C API) right now doesn’t really lend itself to language server usage.

Allowing TargetLanguage::None also opens up the compile API to soundness bugs so the validation use case has to be done very carefully here. Do you have an example usage of how the current API or the glslang C API can be used as you describe?

GeForceLegend commented 7 months ago

This is my language server, currently validate shaders by creating a 1*1 boardless window and calling gl::CompileShader. If I can using glslang to validate shaders, then there is no need dealing with fukin gl and different graphics card drivers. https://github.com/GeForceLegend/vscode-mcshader

chyyran commented 7 months ago

I just checked glslang_target_language_t::None for you and it definitely does not do what you expect, just throws

Unable to parse built-ins
ERROR: 0:14: '' :  syntax error, unexpected TEXTURE2D, expecting COMMA or SEMICOLON
INTERNAL ERROR: Unable to parse built-ins

and dumps internal builtins.

I think what you want is setting target in CompilerOptions to TargetEnv::None, which is already available.

GeForceLegend commented 7 months ago
ERROR: 0:14: '' :  syntax error, unexpected TEXTURE2D, expecting COMMA or SEMICOLON

This is just what I expect, which means compile failed due to line 14 in file 0, and then I can send a diagnostic containing the info to client.

And there are 2 images showing some shader that can pass compile when target language is none but cannot pass on SPIR-V. I know version 120 is very old, but this will happen on most used version of Minecraft Java Edition shaderpacks too (330 compatibility) 20240208160046 20240208160154

chyyran commented 7 months ago

0.3 should support this now, see Target. For your equivalent you want to set Target::None(None).

GeForceLegend commented 7 months ago

It might be better be able to switch messages types, as CASCADING_ERRORS and KEEP_UNCALLED are useful for lsp.