shame
shame copied to clipboard
Storage/Uniform Buffer Any API rework
trafficstars
Storage/Uniform Buffer Any API rework
The goal of this PR is twofold
- Improve the user facing
Anyapi for creating storage and uniform buffer bindings, mainly by making the surface of errors smaller. - Fully integrate #19 and remove any layout calculations that aren't part of
layoutable/align_size.rs.
They go hand in hand: The TypeLayout rework makes creating TypeLayouts less error prone and moves potential layout errors (mainly for uniform layout rules) outside of the Any api, so that a user of the Any api may respond to layout errors before encoding time.
The PR is based on #19, which should be merged first. Decoupling it from #19 is not possible.
Changes
The main change is splitting Any::binding into three methods Any::storage_buffer_binding, Any::uniform_buffer_binding and Any::handle_binding:
// The current `Any::binding` function
pub fn binding(
path: BindPath,
visibility: StageMask,
ty: ir::StoreType,
binding_ty: BindingType,
buffer_binding_as_ref: bool,
) -> Any {
// This function has huge error surface
// - `StoreType` and `BindingType` need to be matching variants
// - `StoreType` contains layout information (custom_min_align/size) and may
// not follow the required layout rules of `BindingType::Buffer` (storage or uniform).
// - `buffer_binding_as_ref == true` is only allowed for storage buffer bindings
}
// 1/3 new binding functions
pub fn storage_buffer_binding(
bind_path: BindPath,
visibility: StageMask,
layout: GpuTypeLayout<repr::Storage>,
access: AccessModeReadable,
buffer_binding_as_ref: bool,
has_dynamic_offset: bool,
) -> Any {
// Error surface is massively reduced:
// - `BindingType` is now created inside of the function.
// - `GpuTypeLayout<repr::Storage>` replaces `StoreType` and guarantees that it's
// useable for a storage buffer, so no layout errors can occur within the function.
}
// 2/3 new binding functions
pub fn uniform_buffer_binding(
bind_path: BindPath,
visibility: StageMask,
layout: GpuTypeLayout<repr::Uniform>,
has_dynamic_offset: bool,
) -> Any {
// Error surface similarly reduced. Note that we don't have a
// `buffer_binding_as_ref` parameter.
}
// 3/3 new binding functions
pub fn handle_binding(bind_path: BindPath, visibility: StageMask, handle_type: HandleType) -> Any {
// Speaks for itself.
}
Open questions
- I stumbled upon this function. I was assuming that
custom_min_alignandcustom_min_sizecan't be too small, because ofmin. Can this function be removed or is it supposed to exist? - Trait bounds are quite the mess now, especially for the
Arrayimplementations. I believe the entireBuffer(Ref)code could be massively cleaned up by removing the indirection throughGpuStore. I have ideas on how to do this, but I'd prefer to do it as a separate PR. Removing theGpuStoredependency ofBuffer(Ref)will allow for a lot of trait bound cleanup.
This PR is functional, but needs cleanup. Not yet ready for review!