sokol_gfx.h: remove internal static variables

[mysterycoconut]

There are a number of static globals in sokol_gfx that make it difficult to use with reentrant functions, and thread safe code. Could these be supplied to the library as parameters instead?

A particular case where this is a problem for me is when using the library as part of a dll that is reloaded at runtime (like this: https://nullprogram.com/blog/2014/12/23/).

[mysterycoconut]

I understand most of these are private types that you may not want to expose. Having functions that return their size could be enough to be able to allocate them outside of the library without exposing their internal structure.

[floooh]

Hmm, that point came up several times (essentially having an API where each function takes a context pointer, or maybe have a single setcontextptr function similar to how Dear ImGui is doing it since recently) - for the sokol headers it would probably make sense to expose a slightly different 'context aware' API via a #define.

For threading this usually isn't a problem, since sokol_gfx functions must be called from the same thread anyway (it doesn't have to be the main thread, just the same thread).

I understand that it's a problem with hot-code-reloading though.

I don't care about public vs private data that much, but it's important whether the data is visible to the declaration part or only the implementation because the backend may be implemented in a different language (Obj-C) that must not leak into the declaration (which must remain C).

The Objective-C code in the Metal backend has restrictions because of ARC. Obj-C id's can not be stored in structs, because the compiler can't track the lifetime of an id when it is part of a struct (that's only possible in Obj-C++). That's the reason why there are dozens of unique static globals in the Metal backend instead of packing everything into a struct.

There may be workarounds with explicit lifetime control, but this turns the problem into a pretty big rewrite.

I'll keep the ticket open, I think it's a worthy goal, but don't expect quick progress ;)

[alexkarpenko]

I think a simple solution that would avoid a rewrite is to use thread local static state variable. That would make code reentrant as long as the context is made active on the current thread (which one has to do when working with gl contexts anyway).

So instead of:

static _sg_state_t _sg;

You would define:

static __thread _sg_state_t *_sg = NULL;

_sg_state_t *sg_state_create() {
  // malloc & init a new _sg_state_t
}

void sg_state_destroy(_sg_state_t *sg) {
  // destroy and free `sg`
}

void sg_state_make_active(_sg_state_t *sg) {
  _sg = sg; // bind the `sg` state to this thread
}

Then the remaining functions would remain the same. Except to convert _sg.blah to _sg->blah.

This API could also replace the current handling of multiple contexts, which currently still limits all calls to a single threaded use case.

[alexkarpenko]

To get around the ObjC ARC issue, you could also require that this header's implementation be included into an m file when compiling for the Metal target. Then you can wrap all the ObjC objects inside a @interface SGState that gets created and __bridge_retained in sg_state_create & then released in sg_state_destroy.

[floooh]

...hmm, at least in the sokol_gfx.h API, all functions must be called from the same thread anyway, it doesn't make sense to call into sokol-gfx from different threads. It's safe to run sokol-gfx on a different thread as long as all calls happen from that thread, and it's the same thread where the backend 3D context/device lives.

I've done some work in that general direction though, with the exception of some Metal structures, all data is now in a single struct:

https://github.com/floooh/sokol/blob/82346ae0365a3b1157e2ddce591a2d01087316a1/sokol_gfx.h#L2800-L2825

And this is the block of Metal data which currently must live outside of structs:

https://github.com/floooh/sokol/blob/82346ae0365a3b1157e2ddce591a2d01087316a1/sokol_gfx.h#L2640-L2646

So with the exception of this Metal block, at least the data isn't spread over dozens of static variables. And there might be solutions for integrating the Metal stuff into the single "context data" struct as well. After this, there could be some additional functions to "delegate" allocation of this data chunk to the outside world (Dear ImGui has gone through similar changes a little while ago, might be worth looking at that).

[floooh]

PS: Basically I want to specifically avoid to having to carry around a context-pointer on the user side, and provide that as argument for every function call. Even when the data doesn't live in a static global anymore, the pointer to this data should only be provided once (so there still would need to be at least a global static pointer somewhere).

(so basically exactly the same as you describe in your first comment @alexkarpenko, but with the context-pointer not being thread-local).

[alexkarpenko]

Hey @floooh yeah I've looked at the master branch. The issue with your proposed approach is that you can't have two GL contexts, each running on a separate thread. The reason you often want this is to do background (off-screen) rendering. Here's some pseudo code to explain this use case (going to use cpp for simplicity):

auto context1 = sg_state_create(); // this would really be created somewhere else...
auto context2 = sg_state_create(); // putting this here just for illustration
_processingLoop = std::thread([=]{
   sg_state_activate(context2);
   sg_pipeline pip = sg_make_pipeline(&(sg_pipeline_desc){ ... });
   ...
   while (workToDo) {
      // do some gl rendering here to some frame buffer
      // e.g.: render, read pixels, save to an image file
      // note that context2 is only accessed on this thread
      // no-one else is accessing context2 at the same time
      ...
      sg_apply_pipeline(pip);  // apply some pipeline on this context, on this thread
      ...
   }
});

// meanwhile back on the main thread...
sg_state_activate(context1);
sg_pipeline pip = sg_make_pipeline(&(sg_pipeline_desc){ ... });
...
while (appIsActive) {
   // do some rendering with context1 on the main loop
   // notice that context1 is *not* accessed concurrently
   // however _processingLoop *is* accessing context2
   // at the same time that context1 is being accessed here
   // so context1 & context2 *cannot* use the same static _sg
   // struct (unless it is thread-local)
   ...
   sg_apply_pipeline(pip);  // apply some pipeline on this context, on this thread
   ...
}

// join the processing loop thread when exiting the app
_processingLoop.join();

Let me know if that makes sense. By making _sg a thread local variable, the above issue goes away. In addition, your API won't change. You won't need to pass around a context pointer.

[floooh]

...yes for GL that might work, probably not for the D3D11 and Metal backends. It would probably make sense to add this sort of threading support for GL only behind a config define, but I'm hesitant to add features which only work with one backend (even the "separate GL contexts" thing that's currently in for multi-window support is something that I'd like to move out of "core sokol-gfx" and into an extension header one day)...

[alexkarpenko]

Hm, this would work the same for Metal and DirectX. You can create MTLCommandBuffers, MTLBuffers etc on different threads and use those objects on their respective threads, so long as you don't access the same MTLCommandBuffer or MTLBuffer concurrently from two different threads.

To be more precise: multi-threading is supported by GL, Metal, Vulkan, and DX. However, the current design in sokol prevents multi-threaded use cases because a single state struct is used for multiple contexts.

[alexkarpenko]

This figure below shows how it works for Metal. I believe it's very similar for Vulkan. Don't have much experience with DX, but I would expect it to support similar use cases.

From: https://developer.apple.com/library/archive/documentation/Miscellaneous/Conceptual/MetalProgrammingGuide/Cmd-Submiss/Cmd-Submiss.html#//apple_ref/doc/uid/TP40014221-CH3-SW6

[floooh]

Yes I'm familiar with that, but note how there's only a single command queue (and also MTLDevice) which lives on one thread, and the other threads only encode commands into command buffers, which then need to be enqueued on the device thread for execution.

Vulkan works similar, D3D11 works different (the multithreading-design with "deferred device contexts" didn't really work out though).

My plan is to expose Metal/Vulkan/WebGPU style command lists in a separate API at a later point, when WebGPU is being released. It doesn't make much sense to "emulate" such a design in APIs like WebGL or GL (recording a command list, and enqueuing it onto the main thread). Such a "sokol-gfx 2.0" would still only have a single sokol-gfx "context/state" object on one thread, and some API calls would still need to happen on the "main thread" (resource creation, and enqueuing command buffers for execution), other API calls (the rendering functions which encode new draw commands into the command lists) could ran either on a thread, or on the main thread.

That's a very different design from generally having independent "sokol instances" on different threads, and better suited to Metal/Vulkan/D3D12/WebGPU (but not D3D11 and GL).

[alexkarpenko]

Command queues are thread safe for that reason (see first paragraph in the link below). And MTLDevice allows resource allocation from any thread.
https://developer.apple.com/documentation/metal/mtlcommandqueue?language=objc

Anyway, just my 2c on this.

[ElArtista]

Would love to see this happen too!