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

shaders cleanup #1039

Open
rukai opened this issue Sep 16, 2018 · 4 comments
Open

shaders cleanup #1039

rukai opened this issue Sep 16, 2018 · 4 comments

Comments

@rukai
Copy link
Member

rukai commented Sep 16, 2018

Once #947 is merged then, there a bunch of work I want to do on the vulkano-shaders/vulkano-shader-derive crates, in terms of updating dependencies and cleaning things up. So figured I would list my plans here:

  1. delete fn build_glsl (it was used for pre proc-macros) done
  2. deprecate glsl-to-spirv (change readme + publish) done
  3. upgrade to proc_macro2 done
  4. use proc_macro instead of proc_macro_derive done
  5. deprecate vulkano-shaders-derive (change readme + publish) done
  6. change panics to errors as per the syn lazy-static example - ensure this fixes Format SPIR-V Cross-Compilation Errors #345, its currently caused by unwrapping the Result<CompilationArtifact, String>
  7. Use spans properly to report where errors occur.
  8. Pass file names to the input_file_name argument. Filename will be either:
    • *.glsl
    • *.rs (embedded)
  9. Display shaderc-rs warnings as proc macro warnings Show warnings generated by glslang #117
  10. Change vulkano_shaders! to have the glsl specified as tokens rather then a string. Some discussion at vulkano-shaders - proc macros 2.0 #1062
  11. Map shaderc errors back to the glsl token spans to generate better errors.
  12. Replace vulkano-shaders/src/parse.rs with rspirv stuff Replace vulkano-shaders parser with rspirv #910

6,7,9,10,11 are waiting on proc macro diagnostics to stabilize rust-lang/rust#54140

@rukai
Copy link
Member Author

rukai commented Sep 30, 2018

#945 Just came across this issue, posting it here so I can find it again.

Edit: And another relevant issue - #549

@TheEdward162
Copy link

TheEdward162 commented Nov 14, 2018

I don't know if this is the right place to ask this, but is there a plan to support shaderc #include directive? According to the shaderc-rs docs, it's just a matter of supplying a loader function in compile_options and it would really be a big help when reusing shader code.

@mitchmindtree
Copy link
Contributor

@TheEdward162 ooo that would be a nice option to have. I wonder where within the vulkano API this could be exposed. I wonder - is there any drawback to just always enabling it? I'll probably look into contributing this if it doesn't happen to be supported by the time we need it. cc @JoshuaBatty relevant to our interests :)

@rukai
Copy link
Member Author

rukai commented Nov 17, 2018

Thanks for pointing that out.
I haven't seen it before and it would be nice to have.

Basic design:

  • When the shader is specified as a string in the shader! macro call we dont set the callback at all.
  • When the second arg is IncludeType::Standard the callback would return Err
  • When the second arg is IncludeType::Relative the callback would attempt to read a shader string using the path in the first arg, starting from the directory that the compiled shader is in.

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

No branches or pull requests

3 participants