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

Allow building using external dependencies #5076

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

TheJackiMonster
Copy link

I had issues building slang as submodule without fetching its dependencies recursively. So I added options to allow building slang fully with externally included targets and headers.

One example for this here:
https://github.com/TheJackiMonster/vkcv-framework/blob/develop/modules/shader_compiler/config/SLANG.cmake

@csyonghe
Copy link
Collaborator

@expipiplus1 can you review this? I remember we have options for selecting where to source the spirv headers etc.
Is it possible to consolidate these options a bit further?

Copy link
Collaborator

@expipiplus1 expipiplus1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a very useful improvement! thanks!

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
external/CMakeLists.txt Outdated Show resolved Hide resolved
@csyonghe
Copy link
Collaborator

@expipiplus1 can you please do another round of review, thanks.

@expipiplus1
Copy link
Collaborator

expipiplus1 commented Sep 19, 2024

@TheJackiMonster I made some cleanups and changes, and merged the latest version of slang, it's in my branch here,
https://github.com/expipiplus1/slang/tree/external

If it works for you, please merge it into your PR here and we'll merge if CI passes

@TheJackiMonster
Copy link
Author

Should work for me. I just added an option to disable slang-rhi because it's only used for the render-test target. So from my understanding it should not be required for the rest of the build.

But feel free to correct me if I'm wrong with this.

@expipiplus1 expipiplus1 added the pr: non-breaking PRs without breaking changes label Sep 19, 2024
@expipiplus1
Copy link
Collaborator

Ok good, I'll take care of fixing the CI and merging from here.\

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants