thatcosmonaut / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
3 stars 3 forks source link

VertexElementFormat Improvements #97

Closed TheSpydog closed 2 weeks ago

TheSpydog commented 1 month ago

Currently our VertexElementFormats are named using XNA conventions. We should rename them to be more explicit about their formats to make them non-ambiguous. (For example, the name COLOR is up for interpretation.)

We can also consider adding additional vertex formats (e.g. halfvector4) if they are universally supported.

TheSpydog commented 1 month ago

More notes on this:

I would like to add that other names need to be more specific. For example I expected SDL_GPU_VERTEXELEMENTFORMAT_BYTE4 to be regular signed char but in your Vulkan implementation it is scaled unsigned char.

TheSpydog commented 2 weeks ago

Out of curiosity, I took a look at what WebGPU exposes, and their list is pretty short: https://www.w3.org/TR/webgpu/#vertex-formats

It's already pretty close to what we have, just with signed integers and a few more variants. I think we should try to match this list, minus the strange 10_10_10_2 format.

So maybe we could do this (give or take the order/naming scheme):

    /* 32-bit Unsigned Integers */
    SDL_GPU_VERTEXELEMENTFORMAT_UINT,
    SDL_GPU_VERTEXELEMENTFORMAT_UINT2,
    SDL_GPU_VERTEXELEMENTFORMAT_UINT3,
    SDL_GPU_VERTEXELEMENTFORMAT_UINT4,

    /* 32-bit Signed Integers */
    SDL_GPU_VERTEXELEMENTFORMAT_INT,
    SDL_GPU_VERTEXELEMENTFORMAT_INT2,
    SDL_GPU_VERTEXELEMENTFORMAT_INT3,
    SDL_GPU_VERTEXELEMENTFORMAT_INT4,

    /* 32-bit Floats */
    SDL_GPU_VERTEXELEMENTFORMAT_FLOAT,
    SDL_GPU_VERTEXELEMENTFORMAT_FLOAT2,
    SDL_GPU_VERTEXELEMENTFORMAT_FLOAT3,
    SDL_GPU_VERTEXELEMENTFORMAT_FLOAT4,

    /* 8-bit Unsigned Integers */
    SDL_GPU_VERTEXELEMENTFORMAT_UBYTE2,
    SDL_GPU_VERTEXELEMENTFORMAT_UBYTE4,

    /* 8-bit Signed Integers */
    SDL_GPU_VERTEXELEMENTFORMAT_BYTE2,
    SDL_GPU_VERTEXELEMENTFORMAT_BYTE4,

    /* 8-bit Unsigned Normalized */
    SDL_GPU_VERTEXELEMENTFORMAT_UBYTE2_NORM,
    SDL_GPU_VERTEXELEMENTFORMAT_UBYTE4_NORM,

    /* 8-bit Signed Normalized */
    SDL_GPU_VERTEXELEMENTFORMAT_BYTE2_NORM,
    SDL_GPU_VERTEXELEMENTFORMAT_BYTE4_NORM,

    /* 16-bit Unsigned Integers */
    SDL_GPU_VERTEXELEMENTFORMAT_USHORT2,
    SDL_GPU_VERTEXELEMENTFORMAT_USHORT4,

    /* 16-bit Signed Integers */
    SDL_GPU_VERTEXELEMENTFORMAT_SHORT2,
    SDL_GPU_VERTEXELEMENTFORMAT_SHORT4,

    /* 16-bit Unsigned Normalized */
    SDL_GPU_VERTEXELEMENTFORMAT_USHORT2_NORM,
    SDL_GPU_VERTEXELEMENTFORMAT_USHORT4_NORM,

    /* 16-bit Signed Normalized */
    SDL_GPU_VERTEXELEMENTFORMAT_SHORT2_NORM,
    SDL_GPU_VERTEXELEMENTFORMAT_SHORT4_NORM,

    /* 16-bit Floats */
    SDL_GPU_VERTEXELEMENTFORMAT_HALF2,
    SDL_GPU_VERTEXELEMENTFORMAT_HALF4
Akaricchi commented 2 weeks ago

Following WebGPU here sounds good to me. I'd be curious to know why they support 1- and 3- component vectors for int32 and float32 only; is that a limitation of one of the backends or perhaps not directly supported on some hardware? I'm ok with this naming convention, though I'd prefer explicit sizes e.g. INT32, FLOAT16.

TheSpydog commented 2 weeks ago

I'd be curious to know why they support 1- and 3- component vectors for int32 and float32 only; is that a limitation of one of the backends or perhaps not directly supported on some hardware?

DXGI doesn't support 3-component vectors for anything other than int/uint/float. Vulkan's format support is also substantially weaker for 3-component vectors. Metal has a slightly higher OS version requirement for single/three-component formats, so it appears that's why they removed the single-component versions from WebGPU.

I'm ok with this naming convention, though I'd prefer explicit sizes e.g. INT32, FLOAT16.

I experimented with that, but I think it's less readable when paired with the component count and normalization (e.g. SDL_GPU_VERTEXELEMENTFORMAT_UINT16_2_UNORM). Metal uses the naming convention I proposed, except they use char instead of byte, so there is some precedent.

Akaricchi commented 2 weeks ago

I experimented with that, but I think it's less readable when paired with the component count and normalization (e.g. SDL_GPU_VERTEXELEMENTFORMAT_UINT16_2_UNORM).

It's a little verbose but you could do SDL_GPU_VERTEXELEMENTFORMAT_UINT16_VEC2_NORM. Either way I don't feel strongly about this. Either convention is fine by me.

TheSpydog commented 2 weeks ago

Resolved by #209