Skip to content

Conversation

@vikke1234
Copy link

This PR moves the SPIRV/ into glslang/.

Currently, when using CMake's FetchContent1 to fetch SPIRV as a dependency, the headers are only generated upon installation. This results in inconsistent paths for including the headers. For example, if you use FIND_PACKAGE_ARGS to first search for system packages and fall back to compiling from source, it complicates the compilation process

A sample application that shows this behaviour can be found here: https://github.com/vikke1234/glslang-demo
The master branch is the current behaviour, while fix cantains this change.

Footnotes

  1. https://cmake.org/cmake/help/latest/module/FetchContent.html

@CLAassistant
Copy link

CLAassistant commented Jan 16, 2025

CLA assistant check
All committers have signed the CLA.

@arcady-lunarg arcady-lunarg added the kokoro:run Trigger Google bot runs label Jan 22, 2025
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Jan 22, 2025
@vikke1234
Copy link
Author

I'll try to fix these ASAP, I'm in the middle of a move so might take a moment. Seems only a -I is missing

@AnyOldName3
Copy link
Contributor

This will fix part of #3509. Because the text of the original report got mangled when I edited it, I've lost track of whether this is the only remaining problem 3509 is about, but it's definitely one of the things it's about.

In order to keep paths the same as post install, move the
SPIRV to glslang.
@vikke1234
Copy link
Author

Hi,

Sorry for the delay, should work now.

@arcady-lunarg arcady-lunarg added the kokoro:run Trigger Google bot runs label Feb 3, 2025
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Feb 3, 2025

if(ENABLE_SPIRV)
add_subdirectory(SPIRV)
add_subdirectory(glslang/SPIRV)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this to glslang/CMakeLists.txt?

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

Successfully merging this pull request may close these issues.

6 participants