Representation of GPUBindGroupLayoutEntry

[kainino0x]

• WGPUBindGroupLayoutEntry which in JS is not actually a union, but a struct where we validate that exactly one member is !== undefined. In C instead of a pointer we use a sentinel value inside the struct to determine which one is set. Previously discussed here https://github.com/webgpu-native/webgpu-headers/issues/242#issuecomment-1791076168
• WGPUBindGroupEntry, which in JS is an actual union Previously sorta discussed here https://github.com/webgpu-native/webgpu-headers/issues/403

[kainino0x]

Nov 25 meeting:

• CF: Would like to change BindGroupLayoutEntry to use a real union
• KN: Have talked about using a tagged union but have the tag be a bitfield just so we can still trigger the validation
• LK: Could use a chain, could put multiple entry types in the chain
  • idea about making them a tail struct for the chain so there can only be one… not practical
• CF: Problem with unions is you can't make it larger, can solve by making it a union of pointers
  • which is similar to pointer to a struct with sType
  • (later noted: actually as long as it's at least one pointer in size, you can always easily add a pointer to the union for new types)
• LK: Prefer making it a chain identified by sType
• CF: Using extension chain feels wrong for core stuff
• CF: Beginning to feel we should design for ABI
• CF: Language with fancier features like Rust can have an non-exhaustive enum …
• Can have rich info in the YAML file
• LK: Shader source also in core. Similar to this where we use it for one of two types
• CF: …
• KN: Would you want to change ShaderSource?
• CF: No. SType idea feels inconvenient in that it requires going out of your way. But don't have to force that on every user
• KN: Can make it inconvenient in C as long as other languages can be convenient?
• CF: Hm, yes, or in a more conveniently designed C header over this one. This one for ABI stability
• KN: Harder to make the struct different in bindings because then have to translate one struct to another. Dawn C++ avoids this by making the C and C++ types reinterpret_cast-able
• CF: Used to that, in wgpu we have a lot of versions of structs, like ShaderSource. Allows API to be idiomatic to its language
• LK: Still prefer using SType because it's the pattern we already use
• CF/LK: (both prefer the SType solution: change BindGroupLayoutEntry to {nextInChain, binding, visibility} and the rest goes in extension)
• KN: Maybe concerned about whether the presence of a thing in the struct chain has meaning on its own. Would need to here. Forget what we did in the other place we were considering it.
• Consider this but need more time

[kainino0x]

EDIT: I've removed the non-layout entries from this and mirrored them over in https://github.com/webgpu-native/webgpu-headers/issues/403#issuecomment-2505028735

0 - Today:

struct WGPUBindGroupLayoutEntry {
    WGPUChainedStruct const * nextInChain;
    uint32_t binding;
    WGPUShaderStage visibility;
    WGPUBufferBindingLayout buffer;
    WGPUSamplerBindingLayout sampler;
    WGPUTextureBindingLayout texture;
    WGPUStorageTextureBindingLayout storageTexture;
} 
struct WGPUBufferBindingLayout {
    WGPUChainedStruct const * nextInChain;
    WGPUBufferBindingType type; // can be BindingNotUsed
    WGPUBool hasDynamicOffset;
    uint64_t minBindingSize;
};
struct WGPUSamplerBindingLayout {
    WGPUChainedStruct const * nextInChain;
    WGPUSamplerBindingType type;
};
struct WGPUTextureBindingLayout {
    WGPUChainedStruct const * nextInChain;
    WGPUTextureSampleType sampleType;
    WGPUTextureViewDimension viewDimension;
    WGPUBool multisampled;
};
struct WGPUStorageTextureBindingLayout {
    WGPUChainedStruct const * nextInChain;
    WGPUStorageTextureAccess access;
    WGPUTextureFormat format;
    WGPUTextureViewDimension viewDimension;
};
static const WGPUSType WGPUSType_EmscriptenExternalTextureBindingLayout = 0x00040002;
struct WGPUEmscriptenExternalTextureBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_EmscriptenExternalTextureBindingLayout
};

1 - Tagged union (using bitfield (1a) or not (1b)), instead of BindingNotUsed:

enum WGPUBindingType {
    WGPUBindingType_Buffer = ...,
    WGPUBindingType_Sampler = ...,
    WGPUBindingType_Texture = ...,
    WGPUBindingType_StorageTexture = ...,
};
static const WGPUBindingType WGPUBindingType_EmscriptenExternalTexture = ...;

struct WGPUBindGroupLayoutEntry {
    // May not be needed; or alternately could move the nextInChain out of the WGPU*BindingLayout
    // structs and allow different extensions here depending on the WGPUBindingType.
    WGPUChainedStruct const * nextInChain;
    uint32_t binding;
    // If WGPUBindingType is a bitfield, then we validate this has exactly one bit set.
    // If not, JS->C (e.g. Blink) bindings must inject an error when needed.
    WGPUBindingType type;
    union {
        // Structs same as above, except BindingNotUsed doesn't exist anymore.
        // Alternative: all of these could be behind pointers, instead of inline
        WGPUBufferBindingLayout buffer;
        WGPUSamplerBindingLayout sampler;
        WGPUTextureBindingLayout texture;
        WGPUStorageTextureBindingLayout storageTexture;
        // Pointer to a new thing (though in the case of ExternalTexture, there is none, just null)
        WGPU_NULLABLE void * other;
    };
};

2 - Use struct chaining instead of tags:

enum WGPUSType {
    // ...
    WGPUSType_BufferBindingLayout = ...,
    WGPUSType_SamplerBindingLayout = ...,
    WGPUSType_TextureBindingLayout = ...,
    WGPUSType_StorageTextureBindingLayout = ...,
};

struct WGPUBindGroupLayoutEntry {
    WGPUChainedStruct const * nextInChain; // Everything goes in here now.
    uint32_t binding;
};
// Each of these is chained in WGPUBindGroupLayoutEntry. The presence of one in the
// chain indicates the binding type, so there must be exactly one in the chain.
struct WGPUBufferBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_BufferBindingLayout
    WGPUBufferBindingType type;
    WGPUBool hasDynamicOffset;
    uint64_t minBindingSize;
};
struct WGPUSamplerBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_SamplerBindingLayout
    WGPUSamplerBindingType type;
};
struct WGPUTextureBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_TextureBindingLayout
    WGPUTextureSampleType sampleType;
    WGPUTextureViewDimension viewDimension;
    WGPUBool multisampled;
};
struct WGPUStorageTextureBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_StorageTextureBindingLayout
    WGPUStorageTextureAccess access;
    WGPUTextureFormat format;
    WGPUTextureViewDimension viewDimension;
};
static const WGPUSType WGPUSType_EmscriptenExternalTextureBindingLayout = 0x00040002;
struct WGPUEmscriptenExternalTextureBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_EmscriptenExternalTextureBindingLayout
};

3 - Using STypes but with a union of pointers:

// Same WGPUSType

struct WGPUBindGroupLayoutEntry {
    WGPUChainedStruct const * nextInChain; // May not be needed
    uint32_t binding;
    union {
        // Using the WGPU*BindingLayout definitions from 2. Tagged by SType,
        // instead of by an outside tag. It is like a normal chained struct,
        // except the first one in the chain must be one of the layout types.
        // JS->C implementations must inject an error when needed.
        WGPU_NONNULL WGPUBufferBindingLayout * buffer;
        WGPU_NONNULL WGPUSamplerBindingLayout * sampler;
        WGPU_NONNULL WGPUTextureBindingLayout * texture;
        WGPU_NONNULL WGPUStorageTextureBindingLayout * storageTexture;
        // A new thing, such as a WGPUEmscriptenExternalTextureBindingLayout
        WGPU_NONNULL WGPUChainedStruct * other;

        // Alternative: make the core ones all non-pointers; I couldn't figure
        // out a non-messy way of making it extensible, though.
    };
};
struct WGPUEmscriptenExternalTextureBindingLayout {
    WGPUChainedStruct chain; // WGPUSType_EmscriptenExternalTextureBindingLayout
};

[Kangz]

BGL creation is something that happens relatively often. I have quite some concerns about reworking this:

• like OMG the amount of updating of user code that will need to happen is next level, and not just search-replace
• the ergonomy of the C API is still important, and solution 2 and 3 are big regressions in how much pointer chaining needs to happen.
• solution 1 with C union is likely to cause UB in user code.

Given that BGL creation is cold, and the ergonomy regressions of other solutions, I'd suggest sticking to the current way to create BGLs.

[kainino0x]

cc @cwfitzgerald

[kainino0x]

There is one problem with WGPUBindGroupEntry (not Layout) which is that it is not currently convertible from C to JS, because what do you do if more than one of the binding types is set? This can only be solved by injecting an error (or crashing), but we already said we would do this, so it's fine.

[eliemichel]

My two cents FWIW: solutions 0,1,2 seem rather ok to me in terms of ergonomics but solution 3 feels weird, would introduce a new usage idiom (1 as well, but in a more straightforward way).

[kainino0x]

I've split out the discussion of the non-layout entries back out to #403 because I decided they're not that similar (since JS uses different patterns) and the two don't necessarily need to match.