-
Notifications
You must be signed in to change notification settings - Fork 781
Add Slang shaders to additional samples #1406
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 Slang shaders to additional samples #1406
Conversation
This is needed to get proper support for SPIR-V compiled from Slang shaders
- instancing - conditional rendering - conservative rasterization
- dynamic uniform buffers - descriptor buffer basic - fragment shading rate
- hdr (+hpp) - graphics pipeline library - mesh shading (+hpp)
- texture loading (+hpp) - push descriptors (+hpp - open cl interop
- separte image sampler (+hpp) - texture mipmap generation (+hpp) - shader debug printf - texture compression basisu
- compute nbody (+hpp)
- dynamic primitive clipping - logic op dynamic state - sparse image
- color write enable - debug utils - dynamic line rasterization
- ray queries - ray tracing basics - ray tracing extended - ray tracing position fetch
- terrain tessellation - patch control points - synchronization 2 - vertex dynamic state
|
It looks like two issues are occurring here with Apple CI:
/Users/runner/work/Vulkan-Samples/Vulkan-Samples/shaders/sparse_image/slang/sparse.frag.slang(45): error 36107: entrypoint 'main' does not support compilation target 'spirv' with stage 'fragment' Also note that macOS CI does not access the VulkanSDK and will not try to compile any slang shaders, but would likely fail on the same sparse image shader if it did. |
|
It's a draft for a reason. |
|
Understood - just trying to help out re issue #1405 since this has nothing to do with your PR, only the Apple CI. |
|
You're welcome. We discussed that on one of our calls, and having each CI step rebuild the SPIR-V files that are already part of the repo doesn't make sense anyway. Esp. as the SDK tends to include broken/outdated compilers. |
I agree. However, the scope of the problem may be limited to iOS. At this point iOS seems to be the only CI target that requires and accesses the Vulkan SDK for static linkage to the MoltenVK framework. I believe all other CI targets depend on Vulkan-Headers only without need for the SDK (or compilers), assuming dynamic linkage to the Vulkan runtime library. |
|
Should put that in a separate issue. I'd like the comments section of this PR related to the actual PR if possible. |
- descriptor indexing
- dyamic blending
- time stamp queries (+hpp) - host image copy
- fragment shader barycentric - memory budget
- calibrated timestamps - dynamic multisample rasterization
- open gl interop
- portability
Use point size related built-ins
|
I tried to address all of the feedback. I would appreciate if we could merge this on our next call. Minor things can be fixed on separate PRs. |
Use getCount for const array size
|
Thanks for all the feedback. I hope I've it as far as possible for me. Seeing how this PR is growing (both in size and age), I'd like to have this merged in the current state. Minor things can easily be fixed afterwards. |
gary-sweet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just spotted some differences in the texture_loading shaders between glsl and both hlsl and slang. Those are clearly pre-existing issues and can easily be fixed after this goes in.
bc70e9d
|
I have addressed all open feedback. For those that reviewed it, please (re)approve so we can merge on the next call. |
Remove attributes not required by Slang
asuessenbach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more comments...
|
@asuessenbach: Please let me know when you're finished with your review. It's easier for me to go through the remaining issues one last time. |
asuessenbach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright now, those are my final comments on this PR!
|
@asuessenbach : Just a quick note that this PR is getting to point where github has issues loading it. Let me know when you're done with your review and I'll try to address all of feedback, but after that we should move remaining things to a separate issue. Looks like the comments are mostly cosmetic (and partially also wrong in GLSL/HLSL). I'd like to finish this PR at some point... |
Description
Important note: If you compile shaders locally, you have to use a very recent version of the slang compiler. The one shipped with the SDK is outdated and broken. Make sure to grab the latest one from https://github.com/shader-slang/slang/releases
This adds Slang shaders to additional samples:
Also enables SPIR-V 1.4 at framework level when using Slang.
Additional notes:
VKB_SKIP_SLANG_SHADER_COMPILATIONto skip compilation of Slang shaders (you can still use Slang shaders via the precompiled SPIR-V files)Refs #1385
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properly