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

Initial exploration: Include explicit GLSL to SPIRV compilation #107

Closed
alexander-g opened this issue Jan 2, 2021 · 6 comments
Closed

Comments

@alexander-g
Copy link
Contributor

Currently raw GLSL source code is directly passed to vkCreateShaderModule. I believe this works only with the Nvidia driver as indictated by this extension, which is deprecated without replacement. It does not work for other drivers, I have tested it with the default Intel driver on my Microsoft Surface and got a segfault. (Works fine with SPIRV).
Additionally if the GLSL code is incorrect only a VK_ERROR_INVALID_SHADER_NV exception is thrown without additional information what is wrong.
Both could be fixed by including the glslang compiler.

@axsaucedo
Copy link
Member

That is absolutely right - it has actually come up several times, and utliamtely it only works if the glslang libraries are installed in the respective machine. I don't think we would want to include this library, but I agree that potentially adding this as part of the documentation could be enough to ensure people know why there may be a segfault in place.

To provide further detail, some issues that show this behaviour include #100 and #84.

@alexander-g
Copy link
Contributor Author

Why not include it? The overhead is small and it helps a lot during development when you quickly want to change a shader, especially with python.

@axsaucedo
Copy link
Member

@alexander-g the main reason I woudl think against it would be adding a dependency that would only be required when explicitly needed. All other dependencies are either built from source (with the submodule + cmake) installing all relevant dependencies or using static libs. Not adding it would not block people from being able to just install it themselves by following the makefile target, so it basically would be whether Kompute should take the complexity of installing it, which would seem as more maintenance overhead than benefit (vs documentation that just points people to make install glsl itself).

@alexander-g
Copy link
Contributor Author

alexander-g commented Jan 15, 2021

The GLSL compiler from Khronos is exactly simply a submodule that can be compiled to a static lib.
I will try to implement it and write a PR, then you can decide.

@axsaucedo
Copy link
Member

Totally agree it can be done, it's just more of the overhead to maintain (making sure it's upgraded, tested) - the less deps the better. Having said that, if you're trying it, it woudl still be worth assessing, including the size increase of binary, etc. Thanks @alexander-g !

@axsaucedo
Copy link
Member

Closing this issue to continue discussion on #150

@axsaucedo axsaucedo changed the title Include explicit GLSL to SPIRV compilation Initial exploration: Include explicit GLSL to SPIRV compilation Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants