floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
6.54k stars 469 forks source link

Don't rebind vertex buffers in Metal if only the offset index has changed #980

Closed staminajim closed 5 months ago

staminajim commented 5 months ago

In Metal, if you're reusing a vertex buffer with a different offset, you can use setVertexBufferOffset instead of setVertexBuffer: https://developer.apple.com/library/archive/documentation/3DDrawing/Conceptual/MTLBestPracticesGuide/BufferBindings.html

If you don't you get loads of redundant buffer binding warnings when running in Xcode (and presumably this is also less efficient for sending data to the GPU)

Fixes https://github.com/floooh/sokol/issues/979

I'm using this change in my game engine and seems to be working perfectly fine

floooh commented 5 months ago

The fix looks suspiciously simple ;)

TBH I'm not sure why I haven't done this in the first place when all the caching machinery is already there (I'm aware of the ability to only update the offset btw, I'm using that in the uniform data updates), I will have to dig a bit into the code and git history to make sure this doesn't revert any previous fixes.

But yes, I agree that it is at least a good thing to get rid of those Metal debugger warnings. I'll try to look into the PR during the weekend.

floooh commented 5 months ago

...it's crazy how brittle GH Actions is... (iOS build failed because it couldn't clone sokol)

floooh commented 5 months ago

I'll give this PR a whirl now.

floooh commented 5 months ago

Ah right, there is already a code path which skips the setVertexBuffer alltogether if both the buffer and offset match, just not the path for only updating the offset if the buffer matches but not the offset...

...your change has a duplicate SOKOL_ASSERT, but I'll fix that during the merge.

floooh commented 5 months ago

Ok merged. Many thanks!

I also removed a couple of unused pointers from the Metal state cache while at it (see: https://github.com/floooh/sokol/commit/41e7b04405b0423548d5f1a58cddb413ef8bae5a), and updated the changelog.

staminajim commented 5 months ago

Wonderful thanks, great job :)