-
Notifications
You must be signed in to change notification settings - Fork 244
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
Generic storage class and Image types #402
Closed
Hentropy
wants to merge
23
commits into
EmbarkStudios:main
from
Hentropy:storage-class-image-generic-types
Closed
Generic storage class and Image types #402
Hentropy
wants to merge
23
commits into
EmbarkStudios:main
from
Hentropy:storage-class-image-generic-types
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds generic new types for Input, Output, Uniform, StorageBuffer, and Image.
Just to restate my opinion from the discussion in discord, I unfortunately disagree with this direction. It's a different system than the previously designed systems we're aiming towards like #277, as well as just against what I think is good library/language design (which I explained in the discussion but I can try to rephrase if desired). I'm open to discussion, but right now I disagree with this. |
…phization-like expansion.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds generic new types for Input, Output, Uniform, StorageBuffer, and Image. I had a discussion with @eddyb and @khyperia about this last week and I wanted a concrete implementation to base discussion on.
This makes heavy use of min const generics which will land in Rust 1.51, coming to Beta on Feb 11 and Stable on March 25. Moving Ark to Nightly was mentioned as an issue in #359, but this is on a much shorter path to Stable. It may be possible to switch to assoc consts on traits to get around this in the short-term but const generics are the appropriate place for this information.
For storage classes, the implementation works similarly to before but instead of determining binding info by parsing the HIR, it looks up the consts in MIR and calls
eval_usize
. Image is similar, again using MIR instead of HIR parsing, and constructing an rspirv enum if there is one. It also makes very heavy use of pub-in-priv, and it could be changed such that when full const generics land it could be easily changed over to an enum in a non-breaking way (Mod::TypeAlias -> Enum::Variant).By itself, this change doesn't do too much. Built-ins were not changed, they fit into this model well but would require more design work. It could also easily be extended to auto-derive descriptor set bindings too.
The addition that really makes this pop is binding structs of storage classes, which is a straightforward addition to entry stub generation.
Another use case of just the outer generic wrapper is having complete control over it's interface, particularly the implementation of Deref/DerefMut and Index/IndexMut. This is even more useful for Image since some of it's variants (a "storage image") are write-only.
Additionally, the wrapper types can hold docs for tooling like RA or RLS.
This also allows deriving a shaders I/O interface from its type signature which is a key piece for seamless inter-op with rendering backends. A lot more design work is needed here, but having the necessary type info in the type system rather than locked in attr tags is an important first step and allows for experimentation.
This idea was raised in #246 and discarded because of the need to write out the whole signature for non-entry-point functions, which would be incredibly tedious. However with storage class inference this would not be necessary.