Use Update After Bind Descriptors for Bind Groups With Binding Arrays#6815
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
88b1ce1 to
78549c4
Compare
Update After Bind x
78549c4 to
56d33e8
Compare
|
This PR is a bit of a mess, but I think it's broadly correct and the following PRs will further refine feature specifics. Specifically, we're moving towards two features for bindless, not many. One for "Baseline Bindless" which gets you non-uniform indexing of Storage Textures, Storage Buffers, and Sampled Textures. Then one feature for non-uniform indexing of uniform buffers, if people end up wanting that. |
Wumpf
left a comment
There was a problem hiding this comment.
I'm a bit confused: The new split out UNIFORM_BUFFER_INDEXING feature is never set, thus we never pass the new capability to naga, therefore it should now always reject uniform buffer indexing as its analyzer already correctly distinguishes that now to emit the (new) naga capability as required.
| if let Some(ref descriptor_indexing) = self.descriptor_indexing { | ||
| const STORAGE: F = F::STORAGE_RESOURCE_BINDING_ARRAY; | ||
| features.set( | ||
| F::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING, | ||
| (features.contains(F::TEXTURE_BINDING_ARRAY) | ||
| && descriptor_indexing.shader_sampled_image_array_non_uniform_indexing != 0) | ||
| && (features.contains(F::BUFFER_BINDING_ARRAY | STORAGE) | ||
| && descriptor_indexing.shader_storage_buffer_array_non_uniform_indexing | ||
| != 0), | ||
| ); | ||
| features.set( | ||
| F::UNIFORM_BUFFER_AND_STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING, | ||
| (features.contains(F::BUFFER_BINDING_ARRAY) | ||
| && descriptor_indexing.shader_uniform_buffer_array_non_uniform_indexing != 0) | ||
| && (features.contains(F::TEXTURE_BINDING_ARRAY | STORAGE) | ||
| && descriptor_indexing.shader_storage_image_array_non_uniform_indexing | ||
| != 0), | ||
| ); | ||
| if descriptor_indexing.descriptor_binding_partially_bound != 0 && !intel_windows { | ||
| features |= F::PARTIALLY_BOUND_BINDING_ARRAY; | ||
| } | ||
| // We use update-after-bind descriptors for all bind groups containing binding arrays. | ||
| // | ||
| // In those bind groups, we allow all binding types except uniform buffers to be present. | ||
| // | ||
| // As we can only switch between update-after-bind and not on a per bind group basis, | ||
| // all supported binding types need to be able to be marked update after bind. | ||
| // | ||
| // As such, we enable all features as a whole, rather individually. | ||
| let supports_descriptor_indexing = | ||
| // Sampled Images | ||
| descriptor_indexing.shader_sampled_image_array_non_uniform_indexing != 0 | ||
| && descriptor_indexing.descriptor_binding_sampled_image_update_after_bind != 0 | ||
| // Storage Images | ||
| && descriptor_indexing.shader_storage_image_array_non_uniform_indexing != 0 | ||
| && descriptor_indexing.descriptor_binding_storage_image_update_after_bind != 0 | ||
| // Storage Buffers | ||
| && descriptor_indexing.shader_storage_buffer_array_non_uniform_indexing != 0 | ||
| && descriptor_indexing.descriptor_binding_storage_buffer_update_after_bind != 0; | ||
|
|
||
| let descriptor_indexing_features = F::BUFFER_BINDING_ARRAY | ||
| | F::TEXTURE_BINDING_ARRAY | ||
| | F::STORAGE_RESOURCE_BINDING_ARRAY | ||
| | F::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING | ||
| | F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING; | ||
|
|
||
| features.set(descriptor_indexing_features, supports_descriptor_indexing); | ||
|
|
||
| let supports_partially_bound = | ||
| descriptor_indexing.descriptor_binding_partially_bound != 0; | ||
|
|
||
| features.set(F::PARTIALLY_BOUND_BINDING_ARRAY, supports_partially_bound); | ||
| } |
There was a problem hiding this comment.
I might be missing something, but I can't see anywhere where this would set F::UNIFORM_BUFFER_INDEXING. According to the comment block that would be a separate feature that needs to be checked
There was a problem hiding this comment.
Yeah this is a thing I just forgot to mention entirely. Currently it's impossible to use this new feature. The validation added in #6811 mean that such a bind group will always get rejected. I'm going to update the feature description and make an issue to properly implement it.
…gfx-rs#6815) * Use Update After Bind Descriptors for Bind Groups With Binding Arrays Update After Bind x * Comments * Fix URL
…gfx-rs#6815) * Use Update After Bind Descriptors for Bind Groups With Binding Arrays Update After Bind x * Comments * Fix URL
Closes #6737
Closes #6606