s_framebuffer isn't thread safe

[wtholliday]

s_framebuffer is a global variable and isn't thread safe.

Furthermore, mnvgDeleteFramebuffer leaves s_framebuffer dangling.

[olliwang]

nanovg itself is not thread safe as mentioned in https://github.com/memononen/nanovg/issues/292.

s_framebuffer is only a weak reference to the currently binded framebuffer through mnvgBindFramebuffer(). Users should call mnvgBindFramebuffer(NULL) manually afterward. The logic behind mnvgCreateFramebuffer(), mnvgDeleteFramebuffer() and mnvgBindFramebuffer() basically follow the corresponded functions defined in nanovg_gl_utils.h.

[wtholliday]

How unfortunate. Where is the global state in nanovg?

[wtholliday]

I couldn't find any global state in nanovg itself. Perhaps it is ok to use different nanovg contexts on different threads? Obviously, you can't use a single context in multiple threads.

[olliwang]

I didn't test multithreading environment. But I think it's ok to move the global s_framebuffer into MNVGcontext and renamed to bindedFramebuffer.

[wtholliday]

boundFramebuffer would be the grammatically correct name :)

[olliwang]

Thanks for correction. Do you have multithreading environment to test the result? There may need an additional mnvgUnbindFramebuffer() function to make the changes work. However, that would break the consistency of original nanovg APIs. Or, can you test the GL backend with multithreading environment? If it doesn't work anyway. Then no bother to make the changes.

[wtholliday]

I'm only rendering on a single thread at a time.

The way I did it, calling mnvgBindFramebuffer(nvg, 0) unbinds.

Here's my change: https://github.com/wtholliday/MetalNanoVG/commit/e2c9b99982c0e57a7411b9869c32ef223340328b

I think it's ok for the backend APIs to differ a little.