grovesNL / glow

GL on Whatever: a set of bindings to run GL anywhere and avoid target-specific code
Apache License 2.0
1.2k stars 130 forks source link

Does glow::Context being Send+Sync mean it will be made current in the owning thread after being sent? #269

Open alexispurslane opened 11 months ago

alexispurslane commented 11 months ago

To my understanding, the only way for an OpenGL context to be sent/used on another thread than the one it was created on is for one of three platform-specific functions to be called on it once it was in the new thread to "make it current," and otherwise sending contexts between threads / using them on a different thread than the one they were created on is actually undefined behavior. So since glow marks the context as valid to send, does it do this call under the hood or anything, or at least have those platform specific calls abstracted out to a common method? I just wanted to ask because I'm considering switching to this over a raw gl-rs binding (for better cross-platform compatibility, a stripped down API, and more convenience, while maintaining almost 1:1 compatibility with OpenGL documentation and low level access) but with the gl-rs binding, if I build it myself I can make a non-send/sync gl context to prevent myself from possibly ending up in UB territory because I wouldn't be able to run OpenGL commands from another thread, because I couldn't send the context they're methods of, which is something I'd want from glow if I used it instead, or some way around the possible UB.

surban commented 11 months ago

To my knowledge: no, since glow is not concerned with OpenGL context creation and management.

All functions are marked as unsafe, thus it is the caller's responsibility to ensure that the desired OpenGL context is current on the calling thread.

grovesNL commented 11 months ago

Right, glow isn't responsible for managing the context, so the caller has to handle making the context current. I think marking the context Send + Sync is still reasonable, it's just up to the caller to switch contexts.

For what it's worth, making the context current adds non-zero overhead so I wouldn't want glow to automatically try to call this, because it can't know if you're about to make a bunch of calls on the same thread. I also wouldn't want to remove Send + Sync because multi-threading is supported in general.

If you want to disable multi-threaded GL contexts in your own application (i.e., by removing Send + Sync), you could do this by creating a new type over the context that isn't Send + Sync, and forcing all GL calls to go through that.

alexispurslane commented 11 months ago

marking the context Send + Sync is still reasonable, it's just up to the caller to switch contexts.

Oh it absolutely is, this wasn't meant to imply otherwise at all. No hate from me here 😅 I was just wondering (honestly it was a bit of a silly question now that I think about it in the morning, given what kind of wrapper glow is, sorry~)

grovesNL commented 11 months ago

Oh no worries, I didn't interpret it that way at all!

Even though glow doesn't really do much with the context today, it might still be nice to have some kind of helper functions to make it easier to switch contexts and abstract over the platform differences, but I guess that might be a better fit for glutin or something.

alexispurslane commented 11 months ago

I think just having a single make_context_current function that abstracts over the three actual functions (but isn't automatically called or anything!) would be really nice to have, and it seems like that'd be within the purview of glow, since it's supposed to prevent platform specific GL code?

grovesNL commented 11 months ago

Yeah that sounds reasonable to me. We'd probably have to update our bindings generator (reviving #241) or temporarily include handwritten bindings for now.