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

Advanced vertex format support #54

Closed
trevex opened this issue Jan 15, 2023 · 3 comments · Fixed by #57 · May be fixed by #56
Closed

Advanced vertex format support #54

trevex opened this issue Jan 15, 2023 · 3 comments · Fixed by #57 · May be fixed by #56
Labels
enhancement New feature or request

Comments

@trevex
Copy link

trevex commented Jan 15, 2023

This is more of a thought and feature request. Generally the inferred attribute layout is enough for my personal needs, but might be limited for more advanced use-cases other users come up with.

GLSL does not allow specifying the input precision or format precisely, so this has to be done on the Vulkan-side.
A simple example would be a user wanting to leverage R16G16B16_SNORM format to reduce the size of normals transferred or maybe requiring double precision for some input.

My proposal to solve this would be to add an additional field to GraphicPipelineInfo, that is an enum defaulting to AttributeLayout::Automatic (name tbd maybe VertexDescription is better 🤷), but this enum has a second option AttributeLayout::Explicit(...), which would allow specifying formats, input rate etc precisely (so not relying on defaults or naming conventions in the shader).

@trevex
Copy link
Author

trevex commented Jan 16, 2023

I added a draft PR to illustrate the idea, let me know if this fits the goals of Screen 13 or not.

The logic is basically inspired by similar changes and a proc-macro I contributed to Vulkano recently, relevant code:
https://github.com/vulkano-rs/vulkano/blob/master/vulkano/src/pipeline/graphics/vertex_input/vertex.rs
https://github.com/vulkano-rs/vulkano/blob/master/vulkano/src/pipeline/graphics/vertex_input/definition.rs

However I simplified the implementation and reduced the amount of types involved, I do think this would fit Screen 13 as I believe this is a nice escape hatch, when more control of the vertex input state is required.

Take the chosen type names with a grain of salt, the draft was thrown together in a couple of minutes, name proposals appreciated.

@attackgoat
Copy link
Owner

This change definitely fits the goals of the crate; I want to deliver something that allows easy use of the common Vulkan use-cases and also allows full access to all the options when needed. This is one area which falls short currently, because users cannot specify some types at all, for instance *_USCALED or *_PACK32 in addition to what you mentioned.

This area shares a similar design issue to sampler selection; I'm not really happy with the ..._sampler_nnr-type naming scheme either:

  • It puts configuration that may change (supported device/format properties) directly in the shader code
  • It feels very clunky naming things (and then using those names!)
  • It doesn't allow the other options like borderColor or unnormalizedCoordinates

This naming system came from Kajiya, and although it works I don't want to extend it to this bad API:

layout(location = 0) in vec3 position_ibind0_r16g16b16_unorm;

For both issues, there are super common values that most people will use and learning some Screen-13-specific language is not what they have in mind, so a system where sane defaults are used and then the GraphicsPipelineInfo type allows specialization which looks exactly like the native Vulkan types, but without vk::Bool32 and other icky things, is the best idea I have.

I'll add comments and suggest code changes on #56 - although it will probably over the next week. Thank you again for this!

@attackgoat attackgoat added the enhancement New feature or request label Jan 17, 2023
@trevex
Copy link
Author

trevex commented Jan 17, 2023

Perfect, that gives me some time to polish up the PR and think some more about it. As it was honestly a very rough first draft. I just wanted to get the idea out of my head.

I agree with you that samplers suffer from similar limitations right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants