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.

Sign up for GitHub

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

Jump to bottom

Rewrite shader and specialization handling in pipelines #2181

Merged
merged 6 commits into from
Apr 18, 2023
Merged

Rewrite shader and specialization handling in pipelines #2181

merged 6 commits into from
Apr 18, 2023

Conversation

Rua
Copy link
Contributor

@Rua Rua commented Apr 17, 2023 •
edited
Loading

Changelog:

### Breaking changes
Changes to shaders and pipelines:
- `EntryPoint` now owns instead of borrows a reference to the `ShaderModule`.
- Instead of an entry point and specialization constants, pipeline construction now takes a `PipelineShaderStageCreateInfo` structure.
- Specialization constants are now provided with a `HashMap` containing `SpecializationConstant` enum values. The `SpecializationConstants` trait is removed, and `vulkano_shaders` no longer generates structs for specialization constants.
- `GraphicsPipelineBuilder` now has a `stages` method that takes an iterator with all shader stages at once, instead of separate methods for each shader type.
- `GraphicsPipelineBuilder` now requires you to provide `input_assembly_state`, `rasterization_state`, `multisample_state` and `color_blend_state` instead of them having default values. You can still call `default()` to generate default values for each of them.

This makes specialization constants a bit more flexible: it's now possible to provide only some of the constants, and leave the rest at their default values. You no longer need to mess with the SpecializationConstants trait if you're not using Vulkano-shaders, you can just provide the constants directly by id without much issue. GraphicsPipelineBuilder also loses most of its type and lifetime parameters, which can be helpful for future changes.

The tessellation and dynamic-local-size examples show off the differences well.

@marc0246
Copy link
Contributor

The deferred example isn't working:

     Running `/home/marc/g/vulkano-upstream/target/debug/deferred`
Using device: NVIDIA GeForce GTX 1050 (type: DiscreteGpu)
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: MismatchBlendingAttachmentsCount', examples/src/bin/deferred/triangle_draw_system.rs:100:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Everything else works for me.

@Rua
Copy link
Contributor Author

Rua commented Apr 18, 2023

Is it ok now?

@marc0246
Copy link
Contributor

The example works now, yes.

marc0246
marc0246 reviewed Apr 18, 2023
View reviewed changes
vulkano/src/shader/mod.rs Outdated Show resolved Hide resolved
@Rua @marc0246
Use mem::discriminant
0d1f055 
Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com>
@marc0246
Copy link
Contributor

Amazing work, thank you so much! I love the fact that the shader! macro no longer needs to generate a struct for constants. This is going to make my life simpler when implementing glam support. Also GraphicsPipelineBuilder is beautiful now.

@marc0246 marc0246 merged commit b7679f8 into vulkano-rs:master Apr 18, 2023
marc0246 added a commit that referenced this pull request Apr 18, 2023
@marc0246
#2181 changelog
783b1ba
@Rua Rua deleted the shader-spec branch October 25, 2023 14:25
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
@Rua @marc0246 @hakolao
Rewrite shader and specialization handling in pipelines (vulkano-rs#2181
e2aaba7 
)

* Rewrite shader and specialization handling in pipelines

* Make the shader loading in examples a bit cleaner

* Forgot some

* Fix incorrect color blend states in examples

* Nicer fix

* Use mem::discriminant

Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com>

---------

Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com>
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
@marc0246 @hakolao
vulkano-rs#2181 changelog
74414db
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc0246 marc0246 marc0246 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Rua @marc0246