gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.44k stars 909 forks source link

Use `enumset` instead of `u128` for feature flags #5247

Open Wumpf opened 8 months ago

Wumpf commented 8 months ago

We just ran out of bits in the u64 feature flags in

Using 128bit integers might be problematic on the web and is generally not scalable going forward. We should instead try to use enumset

See also discussion in Matrix

nical commented 8 months ago

From a glance at the code it looks like enumset uses u128 if you need more than 64 bits, and then arrays of u64 if you need more than 128 bits. So we'd probably have the same ffi issues with u128 we are currently running into in gecko.

nical commented 8 months ago

Due to the issue with u128 and ffi my suggestion is to split into the standard flags and native-only ones into two types. We are pretty safe with 64 bits for the standard flags for a long while I think and they are the ones Gecko relies on in its FFI. If it doesn't break wgpu-native we can consider non-ffi-safe solutons for native-only flags in the future but splitting them off already gives us 30 or so bits to spare so we are good for a while too.

Wumpf commented 8 months ago

sounds good! better to not take another dependency

cwfitzgerald commented 8 months ago

I would still prefer to use enumset if we can - more specifically I really don't want to split the features, this is making the users bare the brunt of a minor internal issue. Many people's renderers have a single slot for features, one for limits, and this would force them to take on another slot for no gain on their end.

grovesNL commented 8 months ago

I think it's likely that we'll eventually run out of space for u128 too. Maybe we could consider moving these into a big feature flags struct (could be directly exposed to FFI)? We could still have convenience methods to get roughly the same ergonomics as the current setup. Or we could expose a struct publicly and internally use something else if we want to keep the representation compact.

nical commented 8 months ago

I just tried to see if I could alter enumset to produce something that would work over ffi ( I don't think getting it to use arrays of u64 would be difficult), unfortunately just depending on enumset causes cbindgen to crash. Here is the quick and dirty experiment if anyone wants to play around: https://github.com/nical/ffi_flags_test

There's a variety of things we can do long term. Splitting the flags probably being the simplest but I'm not particularly attached to it. I would prefer that the solution works well over ffi (without us having to do manually rewrite the flags). If it has to be rewritten on our side, then so be it, but it has to work with cbindgen.

We still have 18 bits available, that gives some time for motivated individuals to fix enumset or try out other solutions.