humbletim / setup-vulkan-sdk

github action that provisions the Vulkan SDK and configures VULKAN_SDK environment variable
MIT License
48 stars 12 forks source link

Added Shaderc Implementation #10

Open sjcobb2022 opened 2 years ago

sjcobb2022 commented 2 years ago

Hi there,

I needed shaderc for building my project so I implemented it here. It took a lot of attempts so I made this new branch with a nice clean commit tree.

There is one caveat however, being that llvm on macOS doesn't compile. I guess this isn't a problem yet though, since MoltenVK isn't implemented yet.

That's it really, it works fine now in my projects.

sjcobb2022 commented 2 years ago

After some quick debugging the implementation now compiles to debug. Added additional input to specify Release or Debug

humbletim commented 2 years ago

hi samuel, thanks for the PR and your ongoing work on shaderc integration!

i've noticed a lot of commits coming in -- is there anything i can do to help test?

also if you haven't tried https://github.com/mxschmitt/action-tmate yet, i highly recommend it for rapid fire iteration and troubleshooting complex actions -- just drop it into your workflow (like before the first failing step) and at it will pause and offer an ssh endpoint in the log that you can use to remote into the runner container (at which point you can make temp changes to cmake files using nano etc. and try ideas out at the console prompt as part of a single action run).

sjcobb2022 commented 2 years ago

Hi @humbletim. Yes, sorry. I do apologies for all the commits I am a bit messy when it comes to committing and I've been using each push as a way to test my code which definitely isn't the best method. So thanks for recommending tmate!

My current issue is linker errors with shaderc_combined.lib / libshader_combined.a. It appears that whilst the library compiles, it does not link to SPIRV-Tools, or glslang correctly and spits out a bunch of undefined symbol errors. I'm not sure exactly why it's doing this. My theory is that there is an error somewhere in my changes which is not linking SPIRV-Tools/glslang correctly meaning it can't identify the symbols.

I do have a workaround for this (pointing shaderc directly to the source of SPIRV-Tools/glslang from ExternalProject_Add) but this takes forever to compile - as it recompiles both libraries - and isn't very elegant. My approach so far has been to include all the necessary libs required by libshaderc and then try and compile, which obviously hasn't worked. IMHO Google's code is really messy and segmenting shaderc into so many different parts is an absolute nightmare.

I do believe that my issue right now is that I need to link both: glslang/glslang/glslang.a AND glslang/libglslang.a. Which is a weird artifact in the documentation and I really don't know anything about it.

If you would like me to close the pull request I can do so if you would like. I'm very sorry I pushed this request without thorough testing.

humbletim commented 2 years ago

hey @sjcobb2022 no worries re: work-in-progress. :)

last year when briefly looking into shaderc i recall hitting similar challenges. i didn't get as far but seem to recall one thing wanted libglslang as a static library (compiled in and no runtime dep) while the other wanted it as a shared library (soft linked with .so/.dylib/.dll runtime dep).

another reference point might be VCPKG. it's vulkan sdk port is not very useful (since it doesn't actually install anything and instead just maps existing install into vcpkg universe); however, there might be some interesting ideas/clues tucked away in vcpkg's shaderc and glslang cmake-based ports:

sjcobb2022 commented 2 years ago

Hi @humbletim. After spending a lot more time trying to fix things, I have found this. https://github.com/KhronosGroup/glslang/issues/2863 essentially stating that we can't use the combined lib directly. Consequently I don't think that my method is possible right now. This is also apparent in the shaderc vcpkg port in which they remove the compiling of the combined library.

https://github.com/microsoft/vcpkg/blob/dc9d737351f916eb349e7bc85671d3dba4043399/ports/shaderc/fix-build-type.patch#L56.

One "fix" for this would to point shaderc to glslang_SOURC_DIR and spirv-tools_SOURCE_DIR, which would take a longer time but would function. I think this is the implementaiton I will try and implement in a second.

btw sorry for the radio silence, I have been moving country for uni.

sjcobb2022 commented 2 years ago

@humbletim This is all complete and works in windows, linux and mac. I cleaned up all my debugging code so everything should be good to go.

I ended up simply directing shaderc to the location of the Glslang and SPIRV-Tools source directories. the linux vulkanSDK executable follows a similar process, however it re-clones the SPIRV-Tools and Glslang directories. Our method is a lot more storage efficient and time efficient since it does not need to re-clone the libraries.

I'll see if I have the time to build any of the other missing deps in the future, but for now I think I'll spend some more time on my own projects.

Qubaef commented 2 years ago

@humbletim are there any plans to merge this PR? I need this feature and I would like to know if it will be ready anytime soon.

humbletim commented 2 years ago

@sjcobb2022 this looks great! i've scheduled some time this week to dive deeper, test drive and hopefully merge this into a new release.

some TODOs on my side:

sjcobb2022 commented 2 years ago

@humbletim linking to shaderc is a bit annoying since it doesn't generate a FindShaderc.cmake file or anything. Here is my very messy way to link to shaderc on any platform. For implementation it should be as simple as

#include <shaderc/shaderc.hpp>