Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major version bump #51

Open
3 tasks
danlehmann opened this issue Nov 7, 2023 · 2 comments
Open
3 tasks

Major version bump #51

danlehmann opened this issue Nov 7, 2023 · 2 comments
Assignees

Comments

@danlehmann
Copy link
Owner

danlehmann commented Nov 7, 2023

There are a couple of things that would be good to do in a major release so that the syntax can be tidied up:

@nicopap
Copy link
Contributor

nicopap commented Nov 8, 2023

I have two extremely-breaking-change ideas:

  1. A Bitsized trait with a const associated constant saying the size in bits of the struct
  2. The ability to "nest" Bitsized stuff. Be it an enum or another Bitsized struct. As far as I know, it is currently impossible using bitbybit.

Using bitbybit in bevy is still contentious, but if we were to use it, here is how it would work:

In my fork of bevy, I use bitbybit to define "shader keys". Shaders are pieces of code that run on the GPU, in bevy, they are written in the wgsl language. We have a special syntax in wgsl, using #ifdef DEFINE to conditionally compile code (à la C preprocessor). The set of DEFINEs varies between entities, generating different shaders per entity. We cache shader, because they are expensive to compile, and we use a "key" to identify cached shader. The key is every enabled DEFINEs used in the shader. When hashing a struct for caching, we want that struct to be as small as possible. So, instead of having a struct with many bool field, we currently use bitflags! to have a single u32 with the 23 different DEFINEs in our shader. Using bitbybit, we would have a struct with many bools, but it would still be a single u32.

But here is the thing. We are making the shading in bevy extensible. This means we need to be able to embed a shader key into another one. I get it is currently impossible to do that with bitbybit, but if we had the ability to embed bit-sized structs into other structs, we could define a shader extension as follow:

// Bevy definition
#[rustfmt::skip]
#[bitfield(u32, default: 0)]
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct MeshPipelineKey {
    #[bit(0, rw)] pub hdr: bool,
    // ...
    #[bit(11, rw)] pub taa: bool,
    #[bit(12, rw)] pub morph_targets: bool,
    #[bits(13..=14, rw)] pub blend: BlendMode,
    // ...
    #[bits(24..=25, rw)] pub shadow_filter_method: Option<ShadowFilteringMethod>,
    #[bits(26..=27, rw)] pub view_projection: ViewProjection,
}

// User extension
#[bitfield(u32, default: 0)]
pub struct CustomMeshExtensionKey {
    #[bits(0..=27, rw)] pub mesh_key: MeshPipelineKey,
    #[bit(28, rw)] pub custom_define1: bool,
    #[bit(29, rw)] pub custom_define2: bool,
    // ...
}

@vortexofdoom
Copy link
Contributor

vortexofdoom commented Jan 29, 2024

@nicopap You should be able to use bitfields inside other bitfields, the "primitive" in the bitfield attribute macro just has to match the number of bits you set aside for it, whether it's one of the arbitrary-int types or a Rust primitive. The problem with your code as written is that as far as the type system knows, MeshPipelineKey uses all 32 bits, or rather, it's important that they're there. So you should only need to change to #[bitfield(u28, default: 0)] and it should just work (after you include the u28 type alias).

Making the associated constant accessible might be a good idea in any case. It should be pretty trivial since you can use the BITS constant of the underlying type. Would probably still be a breaking change if users defined their own constants with whatever name we would choose, although if we auto-implemented a trait with the macro that included the associated constant, it might be achievable without breaking compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants