search

floooh / sokol

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

Fail buffer creation if metal buffer couldn't be allocated #883

Closed danielchasehooper closed 1 year ago

danielchasehooper commented 1 year ago

Before this change, the underlying resource could fail to be created and you wouldn't ever know. This same issue exists in other backends (buffers in GL) and for other resource types (images in metal). Let me know if you'd prefer those other fixes to be put into this PR. I didn't go ahead with all that work because I wasn't sure if this silent failure was by design (though I can't think of why it would be?)

Background: I have some code that destroys, then doubles the size of a buffer every time appending would've caused an overflow. I needed to know when doubling it failed so I could back off a bit. Let me know if theres a better way to go about that.

floooh commented 1 year ago

Oops, that's actually a bug in the Metal and GL backends, the D3D11 backend already handles this case:

https://github.com/floooh/sokol/blob/d4ac122f36d7659a18b312fd4fa2317fb9e06a63/sokol_gfx.h#L9152-L9156

Thanks for catching that.

But I don't think it's a good idea to depend on buffer allocation to fail as an indicator to stop growing the buffer, instead you should have an upper limit in mind that makes sense for the application and stop reallocating when that limit is reached, or allocating the maximum size right from the start (that's what I usually prefer, at least for "game-ey stuff".

I will need to go through all backends and resource types and fix this properly, sorry for closing the PR (I wrote a ticket instead: https://github.com/floooh/sokol/issues/884)

danielchasehooper commented 1 year ago

I’m interested in using Sokol for www.principle.app - and due to how it has to render user-created content, there isn’t a way to know at compile time what reasonable buffer sizes are, and it has to run on everything from low end android phones up to mac studios. So our stance is “allow the code to run as large a document as each user’s hardware will allow”, rather sandbagging the highest end machines to cater to the lowest common denominator.

floooh commented 1 year ago

I'm looking into this now (via ticket #884)

floooh commented 1 year ago

Btw, in GL this isn't really fixable in a good way, because glGetError() involves a full pipeline flush in WebGL, so calling glGetError() in release mode generally isn't a great idea. I'll implement the fix only for Metal for now, but not for GL (GL will continue to fail hard in debug mode, and ignore the error in release mode).

Even in WebGPU it's not possible to report such out-of-memory errors without a full roundtrip to the renderer process, so a buffer or texture object will appear to be created without errors when the creation function returns, and only go into a "failed state" in the next frame, e.g. you can't use the API error reporting facilities to decide whether to stop re-allocating resources (because that information is only available in the next frame).