Skip to content

Conversation

@TheJackiMonster
Copy link
Contributor

@TheJackiMonster TheJackiMonster commented Oct 13, 2024

I've started to implement a module for a Vulkan framework using shady for shader compilation and I needed to make a few changes to it, so I could make sure it was using external dependencies which don't interfere with other submodules.

Additionally I've added options to disable building runtime components and samples. Also I found a bug in the newer hash algorithm which I fixed and I fixed build issues with the optional Java bindings.

unsigned int i = 0;

for (i = 0; i < size; data++, i++)
for (i = 0; i < size; data_chars++, i++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was kind of quite lucky. The compilation of the framework failed for Windows because the compiler couldn't estimate the exact size of a single const void element in this line. So I was able to find this.

shd_growy_append_formatted(g, "\tnode = (Node) {\n");
shd_growy_append_formatted(g, "\t\t.arena = arena,\n");
shd_growy_append_formatted(g, "\t\t.tag = %s_TAG,\n", name);
shd_growy_append_formatted(g, "\tnode.arena = arena;\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of this change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The project I'm using it in is compiling with C++20 which throws an error if not all fields are included in such an initializer list statement. It seems to only trigger in the generated code and I figured it was easier to simply use a syntax that causes less friction between different compilers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. I've been doing a lot of deep refactoring work, and I've not looked at windows builds in a hot second. The C++ header inclusion issue is something I'll try to keep in mind!

@Hugobros3
Copy link
Collaborator

Thanks, looks like some interesting changes, I'll probably cherry-pick a few of those and we can discuss the rest here. Would love to hear more about your use-cases too, I am working on a blog post detailing the next steps for Vcc's development if you are interested, the current approach has some intrinsic issues that can be better solved inside the LLVM project (shady will still be used to legalize/transform code afterwards)

@TheJackiMonster
Copy link
Contributor Author

Thanks, looks like some interesting changes, I'll probably cherry-pick a few of those and we can discuss the rest here. Would love to hear more about your use-cases too, I am working on a blog post detailing the next steps for Vcc's development if you are interested, the current approach has some intrinsic issues that can be better solved inside the LLVM project (shady will still be used to legalize/transform code afterwards)

I was originally interested in using Vcc inside the framework to allow users of it sticking with C++ for shaders as well. The goal of the framework is to provide an API that is much easier to use than Vulkan itself by implementing a lot of boiler plate code for you but still giving you the options to configure everything regarding specific needs. It also manages memory and resources to some degree including shaders.

So while technically people could just use SPIR-V with Vulkan. It's most of the time easier to compile shaders from GLSL or some other human readable language during runtime. There's also still an issue open for implementing hot reloading of shaders in the framework which would allow making changes to shaders during runtime.

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.

2 participants