You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Migrated] spirv-std/Image! should expose AccessQualifier. #69
Issue automatically imported from old repo: EmbarkStudios/rust-gpu#1097 Old labels: t: enhancement Originally creatd by FishArmy100 on 2023-11-11T19:28:09Z 👍: 1
I think it would be beneficial to expose the AccessQualifier in the Image! macro, as at least for wgpu, ReadWrite and ReadOnly storage textures are only supported on native platforms. So, it would be benificial for Images allow for WriteOnly.
Comment from eddyb (CONTRIBUTOR) on 2024-01-31T13:58:30Z
Oh I'm really sorry I haven't checked what SPIR-V's "Access Qualifier"is.
The reason it's optional and not included in spirv_std::Image seems to be that it's Kernel-only.
That is, this is an OpenCL feature, not a Vulkan one, and wgpu couldn't possibly make use of it (unless they misunderstood the specification, too?).
The Vulkan feature is NonWritable/NonReadable on the OpVariable, seems like, just like StorageBuffers.
It does seem weird to put it in the attribute in the #[spirv(...)] img: Image!(...) declaration, as the Image<...> type could really use the knowledge, and a writable &Image behaves more like a &[Cell<u32>]/&[AtomicU32] (relying on interior mutability), if that makes sense.
It may be possible to pretend that it is a parameter on image types, or even reuse the AccessQualifier (and rewrite it away to the Vulkan equivalent), but that seems a bit sketchy, not sure what to do about it for now.
(cc @Algorhythm-sxv - sorry you ended up implementing a different feature that the needed one)
Comment from Algorhythm-sxv on 2024-01-31T15:21:47Z
@eddyb is there a workaround to allow use of a wgpu StorageTexture using the attribute syntax then? I see the AccessQualifier in spirv-headers but no documentation on how to use it, or if it's possible to use it.
Comment from eddyb (CONTRIBUTOR) on 2024-01-31T17:06:55Z
This is the relevant part of my message:
The Vulkan feature is NonWritable/NonReadable on the OpVariable, seems like, just like StorageBuffers.
We currently support toggling NonWritable for buffers based on whether you use &T vs &mut T with #[spirv(storage_buffer, ...)] entry-point parameter declarations, since:
#1011
In theory, we could do it for images based on &Image vs &mut Image, but like I was saying earlier that doesn't fit very well IMO, since images already behaves as if they wrap interior mutability anyway, and also that doesn't cover NonReadable (because Rust has nothing like a "write-only reference"), which I believe you need?
There's two possible paths, as I see it:
#[spirv(descriptor_set = ..., binding = ..., non_readable)] img: &Image!(...) in the entry-point params
this has been proposed before at least for buffers: #692
this seems potentially problematic in terms of being able to turn off methods on the actual types, as the Image<...> would have no information about how it's used
Image!(..., non_readable) - in the type, even if Vulkan SPIR-V does not store that information there
ideally mapped to e.g. struct Image<..., const READABLE: bool, const WRITABLE: bool> to allow impl blocks to select whether they need one or the other (or work regardless), tho see also the trait HasImageFoo pattern
this might cause problems depending on the exact details, because the SPIR-V spec states:
It is invalid to declare multiple non-aggregate, non-pointer type <id>s having the same opcode and operands.
that is, you would want tests which use the same identical Image<...> type with only these new differences being toggled between them, to show that they don't lead to duplicated definitions or ID conflicts
in theory this could even be done with the same syntax and names as for the OpenCL feature (i.e. like in your PR), but at some point those would have to be replaced with the Vulkan equivalent (entry.rs can generate the Vulkan annotations at the same time, and then you have to remove AccessQualifier from the type, replacing all uses of the type, or maybe just never generate the AccessQualifier in the first place, getting you back to what I was saying earlier, but with the downside of seemingly-OpenCL concepts used with Vulkan shaders)
In theory, we could do it for images based on &Image vs &mut Image, but like I was saying earlier that doesn't fit very well IMO, since images already behaves as if they wrap interior mutability anyway, and also that doesn't cover NonReadable (because Rust has nothing like a "write-only reference"), which I believe you need?
Annotation on argument could be a solution to this and I'd actually like to be able to annotate something as NonReadable to hopefully help GPU driver make better decisions about storage buffer caching (I don't work with images) when I know read-only data set will fit into L2 cache, but caching writes will mess with that and I'd really like to avoid it if possible, but there isn't an explicit API for it.
UPD: Might make sense to require &mut [MaybeUninit<T>] in that case, so it is not possible to read it by accident in safe Rust.
Activity
rust-gpu-bot commentedon Nov 13, 2024
Comment from eddyb (CONTRIBUTOR) on 2024-01-31T13:58:30Z
Oh I'm really sorry I haven't checked what SPIR-V's "Access Qualifier" is.
The reason it's optional and not included in
spirv_std::Imageseems to be that it'sKernel-only.That is, this is an OpenCL feature, not a Vulkan one, and
wgpucouldn't possibly make use of it (unless they misunderstood the specification, too?).The Vulkan feature is
NonWritable/NonReadableon theOpVariable, seems like, just likeStorageBuffers.It does seem weird to put it in the attribute in the
#[spirv(...)] img: Image!(...)declaration, as theImage<...>type could really use the knowledge, and a writable&Imagebehaves more like a&[Cell<u32>]/&[AtomicU32](relying on interior mutability), if that makes sense.It may be possible to pretend that it is a parameter on image types, or even reuse the
AccessQualifier(and rewrite it away to the Vulkan equivalent), but that seems a bit sketchy, not sure what to do about it for now.(cc @Algorhythm-sxv - sorry you ended up implementing a different feature that the needed one)
rust-gpu-bot commentedon Nov 13, 2024
Comment from Algorhythm-sxv on 2024-01-31T15:21:47Z
@eddyb is there a workaround to allow use of a wgpu
StorageTextureusing the attribute syntax then? I see theAccessQualifierinspirv-headersbut no documentation on how to use it, or if it's possible to use it.rust-gpu-bot commentedon Nov 13, 2024
Comment from eddyb (CONTRIBUTOR) on 2024-01-31T17:06:55Z
This is the relevant part of my message:
We currently support toggling
NonWritablefor buffers based on whether you use&Tvs&mut Twith#[spirv(storage_buffer, ...)]entry-point parameter declarations, since:In theory, we could do it for images based on
&Imagevs&mut Image, but like I was saying earlier that doesn't fit very well IMO, since images already behaves as if they wrap interior mutability anyway, and also that doesn't coverNonReadable(because Rust has nothing like a "write-only reference"), which I believe you need?There's two possible paths, as I see it:
#[spirv(descriptor_set = ..., binding = ..., non_readable)] img: &Image!(...)in the entry-point paramsImage<...>would have no information about how it's usedImage!(..., non_readable)- in the type, even if Vulkan SPIR-V does not store that information thereideally mapped to e.g.
struct Image<..., const READABLE: bool, const WRITABLE: bool>to allowimplblocks to select whether they need one or the other (or work regardless), tho see also thetrait HasImageFoopatternthis might cause problems depending on the exact details, because the SPIR-V spec states:
Image<...>type with only these new differences being toggled between them, to show that they don't lead to duplicated definitions or ID conflictsin theory this could even be done with the same syntax and names as for the OpenCL feature (i.e. like in your PR), but at some point those would have to be replaced with the Vulkan equivalent (
entry.rscan generate the Vulkan annotations at the same time, and then you have to removeAccessQualifierfrom the type, replacing all uses of the type, or maybe just never generate theAccessQualifierin the first place, getting you back to what I was saying earlier, but with the downside of seemingly-OpenCL concepts used with Vulkan shaders)fixed `impl_glam|mint|vek` feature flags being an illusion of choice (R…
nazar-pc commentedon Jul 6, 2025
Annotation on argument could be a solution to this and I'd actually like to be able to annotate something as
NonReadableto hopefully help GPU driver make better decisions about storage buffer caching (I don't work with images) when I know read-only data set will fit into L2 cache, but caching writes will mess with that and I'd really like to avoid it if possible, but there isn't an explicit API for it.UPD: Might make sense to require
&mut [MaybeUninit<T>]in that case, so it is not possible to read it by accident in safe Rust.refactor: use SpirvBuilder directly, just manage `rustc_backend_spirv…