floooh / sokol-tools

Command line tools for use with sokol headers
MIT License
219 stars 54 forks source link

Needs investigation: Matrix types as vertex attributes in shaders. #99

Open terrybrash opened 1 year ago

terrybrash commented 1 year ago

Using a mat3 as a vertex attribute causes a panic when targeting D3D11.

I repurposed the cube example in sokol-rust for a repro, check out this commit. (note: this will probably crash even if the current problem is fixed)

C:\Users\Terry\sokol-rust>cargo run --example cube
   Compiling sokol v0.1.0 (C:\Users\Terry\sokol-rust)
    Finished dev [unoptimized + debuginfo] target(s) in 0.29s
     Running `target\debug\examples\cube.exe`
[sg][error][id:142] C:\Users\Terry\sokol-rust\src\sokol\c\sokol_gfx.h(14912):
        VALIDATE_PIPELINEDESC_ATTR_SEMANTICS: D3D11 missing vertex attribute semantics in shader

[sg][error][id:142] C:\Users\Terry\sokol-rust\src\sokol\c\sokol_gfx.h(14912):
        VALIDATE_PIPELINEDESC_ATTR_SEMANTICS: D3D11 missing vertex attribute semantics in shader

[sg][panic][id:233] C:\Users\Terry\sokol-rust\src\sokol\c\sokol_gfx.h(14574):
        VALIDATION_FAILED: validation layer checks failed

ABORTING because of [panic]
error: process didn't exit successfully: `target\debug\examples\cube.exe` (exit code: 0xc0000409, STATUS_STACK_BUFFER_OVERRUN)

It looks like the validation is failing because attrs[3] and attrs[4] have empty sem_names.

I'm not sure if the problem is with sokol-shdc, sokol's validation, or a misunderstanding on my part. Fwiw, this works fine when targeting Metal.

terrybrash commented 1 year ago

I realize the workaround is to replace mat3/mat4 with multiple vec3/vec4. But mat3 is intended to work here, right?

floooh commented 1 year ago

(please ignore)

mat3 is in fact not supported atm, only mat4, but looks like sokol-shdc currently doesn't throw an error (it should though).

See here for the list of supported uniform types: https://github.com/floooh/sokol/blob/662272fca1d574aea1b2cd9e4f1171005d5834c0/sokol_gfx.h#L765-L801

I currently don't quite remember the reason why vec3 is allowed but mat3 is not, but it almost certainly had to do with alignment in arrays, and the GL backend's "uniform buffer flattening" (but I'll need to revisit that code, maybe it has been fixed with updating the Khronos dependencies).

PS: or more likely it was because a typical mat3x3 on the CPU side is usually implemented as float[3][3], while the GLSL mat3 uniform would be packed in memory as 3 vec4s (float[3][4]), so a mat3 would actually need to be a mat3x4 on the CPU side which I found a bit too confusing.

floooh commented 1 year ago

PS: please keep the ticket open, because I should at least add some sort of error message in sokol-shdc.

terrybrash commented 1 year ago

I see, I didn't realize SG_UNIFORMTYPE_* also applied to vertex attributes.

It probably should be an error because I was targeting Metal just fine and only had a problem with D3D11.

floooh commented 1 year ago

Ooops, sorry lol. Looks like I need more morning coffee...

Yeah, my bad. I actually never considered matrix types in vertices, let me think (and wake up) for a minute...

floooh commented 1 year ago

Ok, so I never actually considered this case because the underlying 3D APIs don't support matrix types as vertex attributes (which explains why sokol-shdc doesn't know how to deal with it).

Maybe shading languages usually do and expect that the vertex attributes are passed in as separate components though, I never thought about investingating this. I'll need to put aside some time for investigation.

I would definitely recommend passing in matrices as separate vertex components though, and either do the matrix multiplication "manually" in the shader via dot products, or build a matrix from the attributes (performance-wise this shouldn't make a difference).

terrybrash commented 1 year ago

I would definitely recommend passing in matrices as separate vertex components though, and either do the matrix multiplication "manually" in the shader via dot products, or build a matrix from the attributes (performance-wise this shouldn't make a difference).

No problem 👍