Open FishArmy100 opened 1 year ago
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 StorageBuffer
s.
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)
@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.
This is the relevant part of my message:
The Vulkan feature is
NonWritable
/NonReadable
on theOpVariable
, seems like, just likeStorageBuffer
s.
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:
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
Image<...>
would have no information about how it's usedImage!(..., 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)
I think it would be beneficial to expose the
AccessQualifier
in theImage!
macro, as at least forwgpu
,ReadWrite
andReadOnly
storage textures are only supported on native platforms. So, it would be benificial forImage
s allow forWriteOnly
.