Why does your commit remove VSG_SUPPORTS_ShaderCompiler? That's an important requirement for some use cases.

Why does your commit remove VSG_SUPPORTS_ShaderCompiler? That's an important requirement for some use cases.

Yeah, changing ABI out of the blue is bad, but look at this way, it's always on now, so no need test for its support.

@robertosfield what should the policy for deprecation be?

Why does your commit remove VSG_SUPPORTS_ShaderCompiler? That's an important requirement for some use cases.

Yeah, changing ABI out of the blue is bad, but look at this way, it's always on now, so no need test for its support.

My point is this: why do you think it should be deprecated, especially as part of a PR that does something else? Running VSG in an environment where there are only precompiled SPIRV files available should be supported.

A case of one step forward, ten steps back. This is an easy reject.

Forcing an update to a much later CMake version is something I really want to avoid unless we really have to. This change could break the build where the VSG compiles fine yet now. It's changes like this that could create a lot of work for us downstream.

The change for VSG_SUPPORTS_ShaderCompiler is really bad too. We still want to be able to compile the VSG without glslang. It's perfectly possible to use the VSG without glslang, it's just some usage cases that rely upon shader compilation.

I understand the desire to make things more flexible but this is huge step back in flexibility.

Just a sidebar: I don't think a recent version of CMake is an onerous requirement. I just tried installing CMake on Ubuntu 22.04.2 using snap and it happily installed cmake v3.27. 8. The FetchContent support is better in 3.24 and directly supports using find_package as an alternative to downloading and building the content.

I also don't think that there's anything inherently wrong with building against a custom version of glslang, but I would prefer using FetchContent to pull it from your repo.

Any extra hurdle that folks need to jump has to be judged by the benefit it provides. Slightly better FetchContent support is not enough reason. find_package with glslang has been problematic in the past as well so it's a potentially reintroducing more problems than is solves. We have to be very careful about any of these changes. As I've said too often now, this is the far from the first time around trying to get glslang to work cleanly, trying stuff I've already tried and failed with is really not helpful at moving us forward.

Why reject? Just say what needs to change? :)

I would say that explicit is better than implicit, and that using a system lib should be preferred over pulling it in from source. So VSG_USE_SYSTEM_GLSLANG defaults to ON.

FetchContent works, it requires 3.17 however 3.24 has additional creature comforts that require less hacky cmake.

I believe the main concern was: VSG_SUPPORTS_ShaderCompiler

Sure, we can still keep this. This would imply that we allow compiling without glslang support which is fine as well.

Would adding the above back allow this PR to be reopened? :)

Right now, I'm on my private repo making sure all of CI compiles, there is some last bits on the android side that need fixing.

I tried FetchContent before I tried just pulling stuff in directly. I resorted to the solution we have because FetchContent and the other solutions I tried didn't work well enough in all the different situations that users use the VSG. How many times do I need to say the same thing? I've spent many days trying many different combinations of ways to get glslang to work well within the VSG.

I don't like having glslang built as part of the VSG, it's cludge to get around the fact that gslang was breaking build/runtime on too many different user systems.

The only thing that might change things is if the glslang project itself changes things - allow us to specify the version requirements easily and robustly without breaking when an older, problematic in various ways, version is available but not usable. If glslang starts behaving itself sufficiently as a standalone project then we can ditch the internal glslang completely and just use find_package().

If you'd prefer find_package, that's fine too as it works fine on macos and ubuntu. The odd one out would be windows I think.

What bothers me is that there is an implicit attempt to grab glslang and if it can't do that, it's ignored. It's a best effort attempt.

Can this go away?

Or do you still wish to do a best-effort attempt to fetch it if it's not found?

I'm asking because I'd prefer it to be an optional param that is off by default, as you said, some projects don't want it. Rather be explicit than implicit.

If the attempt to pill in glslang fails it very much isn't ignored.

The VSG sets #define about support for glslang in the automatically generated headers so it can be checked at compile time, as well as a function that enables a check at runtime. As Tim pointed out your PR actually broke this...

If the glslang project has fixed the problems that prevent using it via find_package then we may be able to ditch our internal build and just link to it as we do Vulkan.