libsdl-org / SDL_shadercross

Shader translation library for SDL's GPU API.
zlib License
57 stars 18 forks source link

Link SPIRV-Cross instead of using SDL_LoadObject #46

Closed thatcosmonaut closed 1 week ago

thatcosmonaut commented 1 week ago

Partially resolves #44

We can probably stop vendoring spirv_cross_c.h and spirv.h now that we're linking SPIRV-Cross directly.

madebr commented 1 week ago

I added spirv-cross as a git submodule instead of using cmake's FetchContent to do this. I believe this keeps complexity low.

The following CMake options are relevant:

I also added -Werror when building the shadercross cli util, which errors now.

thatcosmonaut commented 1 week ago

Thanks, I'll look into that. While I'm at it do you have any insight on the best way to link DirectXShaderCompiler?

madebr commented 1 week ago

Thanks, I'll look into that. While I'm at it do you have any insight on the best way to link DirectXShaderCompiler?

SDLGPUSHADERCROSS_VENDORED assumes the sources of spirv-cross are already available (checked out as a git submodule). I think we can do something similar here.

For dependencies that don't provide CMake support (ignore pkg-config because that is not available by default on Windows), I usually write custom FindXXX.cmake modules. Building it as a subproject is probably out of the question (LLVM is not fast to build).

Let's create a (cmake) script that downloads the prebuilt .so/dll binaries from the release binaries to a known location (e.g. external/dxcompiler) Then by building with SDLGPUSHADERCROSS_VENDORED=ON, we will look in external/dxcompiler for the binaries. If SDLGPUSHADERCROSS_VENDORED=OFF, then CMake will look in the default locations (and fail if it cannot find any).

madebr commented 1 week ago

I created a poc at https://github.com/madebr/SDL_gpu_shadercross/commit/e0df3340d5b626e17efc4274251c3393ea7d4b12 if you want to try things out.

thatcosmonaut commented 1 week ago

Awesome, I will check that out shortly.

thatcosmonaut commented 1 week ago

-Werror issue is fixed, it looks like verifying CMake files fails for SLR though.

madebr commented 1 week ago

Final note, I simply added the current main branch of spirv-cross as a submodule. For SDL_image and SDL_mixer we use libsdl-org forks of tagged releases.

thatcosmonaut commented 1 week ago

I think it's safe to use spirv-cross main, they don't do ABI breaking changes and main branch is generally bugfixes and improvements.

madebr commented 1 week ago

Adding it as a submodule "freezes" it though. So you might want to regularly update it to get latest bugfixes.

thatcosmonaut commented 1 week ago

Ah that's right. Makes sense.