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

Added Shaderc to tests in order to enable online shader builds #153

Closed
wants to merge 22 commits into from

Conversation

axsaucedo
Copy link
Member

@axsaucedo axsaucedo commented Feb 18, 2021

Edit: Decision was to go with #154

Fixes #150
Fixes #121

Aims to add shaderc in tests as dependency to enable all current tests to work in the CI.

This was also an attempt to explore whether shaderc could be adopted as a dependency, but it seems that shaderc has too many dependencies, including glslang, glslc, spirv-tools and many others. This increases the binary of the tests from 3mb to 11mb which is quite significant.

The existing PR #112 adds support only to GLSLang compiler from Khronos group, and seems to only add 2-3 mb, and only requires a single dependency making it more appealing. @alexander-g suggests that pyshaderc is faster to compile, but it seems that shaderc is based on glslc and hence glslang so it may be worth exploring further as it may be more feasible to just add glslang instead.

Outstanding:

  • Explore replacing shaderc with glslang after confirming tradeoffs on features and compilation speed
  • Remove all references to char and replace by uint32_t for inputs to data
  • Explore adding glslang as core dependency inside kompute to ensure it's well maintained

@axsaucedo axsaucedo added the c++ label Feb 18, 2021
@axsaucedo
Copy link
Member Author

After further assessment the decision is to go ahead with glslang #154 - the main reason is as this significantly reduces compilation times massively, as well as the maintenance burden, whilst providing the same base functionality as core. This means that the initial implementation won't have thread-safe support, however it will still support the #include supoprt, and this feature can be introduced later on potentially as more people adopt this functionality.

@axsaucedo axsaucedo closed this Feb 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant