gfx-rs
In struct TextureViewDescriptor optional enums are unstable
In the struct TextureViewDescriptor, the fields
pub format: Option<wgt::TextureFormat>,
pub dimension: Option<wgt::TextureViewDimension>,
appear to have an unstable representation. In the case of format, the integer that corresponds to None is 90, and for dimension it's 6. (Which appears to be the max integer of the enum + 1) This isn't documented, and if new formats or dimensions are added, the rust compiler could update the integer representing None.
Thank you for spotting! This is indeed an issue here. We need something like
struct Enum<T>(u32, PhantomData<T>);
impl Enum {
fn uwnrap(self) -> Option<T> {
if self.0 == 0 {
None
} else {
//TODO: check that we are within range
Some(mem::transmute(self.0))
}
}
}
All the webgpu-header enums start with NULL = 0 value, while Rust ones do not. They also end with MAX_VALUE=0xFFFFFFFF, while Rust ones do not.
@kvark Option<TextureFormat> might still be problematic because we don't have a guarantee that Option will use 0, max value + 1, etc. There's a lint for this which shows up when building https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b82e6a63b6fb26f4644b97b9de0a20b3, but it looks like it's hidden because we accept Option<&TextureViewDescriptor> (maybe due to https://github.com/rust-lang/rust/issues/66220?)
Ideally we could make TextureFormat non-zero somehow and have the guarantee that Option<NonZeroTextureFormat> uses 0 as None or similar, but I'm not sure there's a way to do that today, so maybe we need to duplicate the enum for wgpu-native and add a null variant?
might still be problematic because we don't have a guarantee that Option will use 0
Yes, I understand that. What I mean is that wgpu-native would need to handle 0 separately.
So, we can still make it so the variant values match between webgpu-headers and wgpu-types, we just need to start the values with 1. But we'll also need some way of forcing that 0 == None ~~without requiring a nightly compiler~~.
I.e. we can convince cbindgen to spew out all the enum variants, but have the actual struct just contain u32, and then go from this to an enum by ourselves. This also solves a nasty problem that in Rust having an unknown enum discriminant is UB, but in C++ it's not.
Ah ok, I misunderstood.
I.e. we can convince cbindgen to spew out all the enum variants, but have the actual struct just contain u32, and then go from this to an enum by ourselves.
Is there a benefit to using u32 there instead of duplicating the enum and adding the extra 0 variant (for wgpu-native/use with cbindgen) and map to the regular enum internally?
I really want us to avoid duplicating all the enums, that's all. No other benefit there.
I noticed that this is already somewhat being handled in IndexFormat. At least in webgpu-headers the enums have a combination of 0-as-undefined, and _Force32 defined, not sure if the decisions should come from that project first.
Yeah, we tentatively agreed to have 0 = undefined reserved for all enums in webgpu-headers, for consistency
So for optional arguments, 0 -> None, and for non optional arguments 0 -> error?
This was probably fixed by creating a custom mapping between webgpu.h enums & wgpu-type enums.
https://github.com/gfx-rs/wgpu-native/blob/6f95ce3fc1cd8829f23401e98967112f00f940c2/src/conv.rs#L662
