vulkano-rs / vulkano

Safe and rich Rust wrapper around the Vulkan API
Apache License 2.0
4.54k stars 438 forks source link

Fix SPIR-V validation of image ops with offsets #2575

Closed LDeakin closed 1 month ago

LDeakin commented 1 month ago

Shader functions like textureLodOffset would incorrectly fail validation.

Checks like this would always evaluate to true (and fail validation) because the property wraps: https://github.com/vulkano-rs/vulkano/blob/a2704689603bd614186edd17c73779fd1f401548/vulkano/src/pipeline/shader/validate_runtime.rs#L2540

Changelog:

### Bugs fixed
- Fix SPIR-V validation of image ops with offsets
Rua commented 1 month ago

Thanks! One remark: The get_constant_* functions are supposed to return the constants as unsigned integers. If they should return signed integers, they should be renamed to get_constant_signed_* so that they match get_constant_float_*

LDeakin commented 1 month ago

I've renamed the offending functions. Note that now there are no unsigned variants of get_constant_composite_composite and get_constant_maybe_composite, but they would be dead code if I brought them back. Thoughts?

marc0246 commented 1 month ago

I'm pretty sure this is what Rua meant. We will add those functions back if/when they are needed. Thank you for this :heart:

marc0246 commented 1 month ago

A sidenote: this validation was never released. There isn't going to be a changelog entry.