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

Generic storage class and Image types #402

Closed

Conversation

Hentropy
Copy link
Contributor

@Hentropy Hentropy commented Feb 1, 2021

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.

#[spirv(vertex)]
pub fn main_vs(
    in_vert: Input<Vertex>, // still has the same auto derive behaviour, uses a default type param
    out: Output<Vec2, Location<0>>,
    uni: Uniform<Mat4, DescriptorSet<0, 0>>,  // currently this is: #[spirv(set = 0, binding = 0)] uni : Uniform<Mat4>,
    uni2: UniVec3,
) {/* ... */}
// This is really the only trick that this enables without struct binding
type UniVec3 = Uniform<Vec3, DescriptorSet<0, 1>>;
#[spirv(fragment]
pub main_fs(
    uni: UniVec3, // easily reuse shared bindings with no attr tag micro-managment
) {/* ... */}

The addition that really makes this pop is binding structs of storage classes, which is a straightforward addition to entry stub generation.

// easily moveable and reusable bindings
struct MyBindings<const SET: usize, const BINDING: usize> {
    uni1: Uniform<u32, DescriptorSet<SET, BINDING>>,
    uni2: Uniform<u32, DescriptorSet<SET, BINDING + 1>>,
    ssbo1: StorageBuffer<u32, DescriptorSet<SET, BINDING + 2>>,
    // etc., could also not be generic over BINDING and instead just over SET for easily moving a block of bindings around
}

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.

Adds generic new types for Input, Output, Uniform, StorageBuffer, and Image.
@khyperia
Copy link
Contributor

khyperia commented Feb 1, 2021

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.

eddyb and others added 22 commits February 19, 2021 16:02
@Hentropy Hentropy closed this Mar 7, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants