Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed refactor of vulkano-shaders #945

Open
casey opened this issue Apr 14, 2018 · 10 comments
Open

Proposed refactor of vulkano-shaders #945

casey opened this issue Apr 14, 2018 · 10 comments

Comments

@casey
Copy link
Contributor

casey commented Apr 14, 2018

I'd like to add safe live shader reloading to vulkano. To support this, we would need to add some way to tell if the new shader can be substituted for the old shader. In particular, the shaders inputs, outputs, and other particulars should not have changed.

To add this functionality, I propose a refactor of vulkano-shaders: instead of directly generating the rust shader module code from the parsed SPIR-V instructions, we would instead create an intermediate Shader object, which contains all information needed to codegen the rust shader module code.

To live reload a shader, we would then create a Shader object representing the old shader, and one representing the new shader, and check them for compatibility. If they are compatible, the new shader bytecode can be substituted for the old shader bytecode without needing to recompile the program.

In addition to supporting shader reloading, this would have the additional benefit of making vulkano-shaders testable, since now tests can be written that assert properties of the Shader object.

I've started working on this and I'm hoping to get feedback on the approach. I've mostly finished entry points and specialization constants, descriptor sets and structs are still to go. It needs a lot of work, and I'm happy to make any changes to it, large or small.

You can find the branch here:
https://github.com/casey/vulkano/tree/refactor-vulkano-shaders

In particular, the Shader object is here:
https://github.com/casey/vulkano/blob/refactor-vulkano-shaders/vulkano-shaders/src/shader.rs

And I've started to split codegen into its own module:
https://github.com/casey/vulkano/tree/refactor-vulkano-shaders/vulkano-shaders/src/codegen

As part of the refactor, codegen could be made a separate module from vulkano-shaders.

@casey
Copy link
Contributor Author

casey commented Apr 17, 2018

I just pushed some diffs that refactored descriptor sets. The major piece of functionality that still needs to be refactored is struct handling. And of course it still needs lots of cleanup and tons of tests.

As a second step, it would be nice to refactor codegen to use a proper templating library, so that the template text is easier to read and modify.

@tomaka
Copy link
Member

tomaka commented Apr 17, 2018

I don't think we need a templating library that does text. Instead we could use the quote crate for example, and turn the Rust code into a string after generation.

@casey
Copy link
Contributor Author

casey commented Apr 17, 2018

That's an excellent idea!

@casey
Copy link
Contributor Author

casey commented May 21, 2018

I'm on OS X, and I found a bug in MoltenVK that reduced my confidence in using it, so I've stopped working on this for now :/

@MOZGIII
Copy link

MOZGIII commented Oct 21, 2018

Maybe this can be resumed after #910 is done?

@mitchmindtree
Copy link
Contributor

Just cc'ing myself as we're also currently interested in a solution for this for nannou-org/nannou#223.

@casey I'm curious if your MoltenVK bug has since been solved? We've been testing it with for nannou-org/nannou#240 and it seems to be relatively reliable.

@casey
Copy link
Contributor Author

casey commented Mar 16, 2019

@mitchmindtree Unfortunately I haven't tested it in a while, so I'm not sure if it's still broken.

@norru
Copy link

norru commented Apr 23, 2019

+1

Having to rebuild/recompile and restart the whole application just to tweak a shader is a massive productivity sink.

I'd be also happy to have this in "development mode" only and can tolerate the shader reloading fail (but not panic) with the application handling the error.

@calebwin
Copy link

Any update on this?

@casey
Copy link
Contributor Author

casey commented Nov 20, 2019

Unfortunately I stopped using Vulkan, so I'm unlikely to make further progress on this. I think it's a good approach, but my code is probably super out of date at this point, and was incomplete to begin with. If anyone feels like resurrecting and completing the branch, please feel free!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants