SDL2 Pthread Proxy APIs

[jakogut]

This PR implements various C APIs for SDL2 to work with PROXY_TO_PTHREAD=1 as discussed here: emscripten-ports/SDL2#77

[kripken]

Thanks @jakogut!

In general this looks great. Please also

• Document these in ./site/source/docs/api_reference/emscripten.h.rst
• Add testing. Maybe a good place is to add it in tests/test_html5.c, and it looks like that file is tested both on the main thread and when proxied, so that should be enough.

[jakogut]

@kripken I've added some documentation and tests for these APIs. These functions generally have void return types, so checking for success doesn't make a lot of sense. Let me know if these suffice, or if if there's a better way to test.

I also separated the proxied variant of the sdl2_gl_read test into a separate branch that will be submitted after the SDL2 fixes are merged.

[kripken]

Nice work!

About that CI error (https://circleci.com/gh/emscripten-core/emscripten/207438?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link), I think you just need to run that test with that env var, as it says (note that it's failing on fastcomp, so need to install latest-fastcomp). You can also apply that diff yourself manually instead.

[sbc100]

The deletion of the incoming branch triggered the automatic closing of this branch. My apologies. This was an unexpected side effect.

Re-opening and re-targeting to master.

[jakogut]

@kripken I've updated the emscripten_get_screen_size API to use a dynamically allocated buffer, and updated the documentation to reflect the change. I've also updated the tests to check the return values of the added functions, rather than just printing a message. Lastly, I've removed the emscripten_set_element_style API entirely.

[kripken]

Sorry for the delay @jakogut , looks good with just some minor comments!

[kripken]

. at end of sentence please (also in places below)

[kripken]

is it worth making these optional? we'd need to document that if we want it. but it seems simpler to assume the caller wants to receive both of them?

[jakogut]

I think I'd also tend to agree that it's simpler to assume the caller wants both the width and height. This would be used specifically in the SDL2 port, in Emscripten_VideoInit, which gets the width and height in a single call. If the caller only wants one or the other, they can simply pass NULL for either parameter, and that value will be ignored.

[kripken]

Sorry for the delay - I missed this email somehow.

Sounds like this will be used where both width and height are used, so let's not make it optional? It just increases code size, requires new docs, and overall doesn't seem worth it.

I think the PR can land, once we fix that and there's a small typo comment from earlier! Thanks for the work here.

[jakogut]

@kripken This is working in combination with modifications to SDL2 as well as the below patch, a workaround for #7684.

diff --git a/tests/sdl2_gl_frames_swap.c b/tests/sdl2_gl_frames_swap.c
index aff19e881..25e6ed4ed 100644
--- a/tests/sdl2_gl_frames_swap.c
+++ b/tests/sdl2_gl_frames_swap.c
@@ -7,6 +7,7 @@
 
 #include <SDL.h>
 #include <emscripten.h>
+#include <emscripten/html5.h>
 
 SDL_Window* window;
 SDL_Renderer* renderer;
@@ -30,6 +31,11 @@ void frame()
 
 int main()
 {
+    EmscriptenWebGLContextAttributes attr;
+    emscripten_webgl_init_context_attributes(&attr);
+    EMSCRIPTEN_WEBGL_CONTEXT_HANDLE ctx = emscripten_webgl_create_context("#canvas", &attr);
+    emscripten_webgl_make_context_current(ctx);
+
     SDL_Init(SDL_INIT_VIDEO);
     window = SDL_CreateWindow("test", 0, 0, 640, 480, SDL_WINDOW_OPENGL);
     renderer = SDL_CreateRenderer(window, 0, SDL_RENDERER_ACCELERATED);

Tested with:
emcc tests/sdl2_gl_frames_swap.c -o sdl2_gl_frames_swap.html -s USE_SDL=2 -s USE_PTHREADS=1 -s OFFSCREEN_FRAMEBUFFER=1

I also have a modified test case that I can update after this PR lands here #8282.

[kripken]

Great, thanks @jakogut !

[warvstar]

I've found that when adding -s PROXY_TO_PTHREAD=1 to the previous example, it gives the following error

Uncaught TypeError: Cannot read property 'getParameter' of undefined
    at emscriptenWebGLGet (o.js:8091)
    at _emscripten_glGetIntegerv (o.js:8292)
    at _do_call (<anonymous>:wasm-function[1628]:0x9418a)
    at emscripten_current_thread_process_queued_calls (<anonymous>:wasm-function[1627]:0x93c9a)
    at emscripten_main_thread_process_queued_calls (<anonymous>:wasm-function[1645]:0x94d7b)
    at o.js:1555
    at Worker.worker.onmessage (o.js:2037)

adding -s OFFSCREENCANVAS_SUPPORT=1 it then gives this error

Failed to execute 'getContext' on 'HTMLCanvasElement': Cannot get context from a canvas that has transferred its control to offscreen.
    at Object.createContext (http://localhost:3000/o.js:5764:20)
    at _eglCreateContext (http://localhost:3000/o.js:5947:24)
    at _emscripten_receive_on_main_thread_js (http://localhost:3000/o.js:9467:19)
    at _do_call (<anonymous>:wasm-function[1348]:0x8c66c)
    at emscripten_current_thread_process_queued_calls (<anonymous>:wasm-function[1347]:0x8bd8a)
    at emscripten_main_thread_process_queued_calls (<anonymous>:wasm-function[1365]:0x8ce5b)
    at http://localhost:3000/o.js:1555:22
    at Worker.worker.onmessage (http://localhost:3000/o.js:2045:13)

I was able to fix the second one by replacing the eglCreateContext call and making the existing context current via emscripten_webgl_make_context_current(handle); in SDL_render_gles2.c line ~1982.

However doing so gives me this error for OFFSCREEN_FRAMEBUFFER

Uncaught TypeError: Cannot set property '0' of undefined
    at Object.newRenderingFrameStarted (o.js:5714)
    at Browser_mainLoop_runner (o.js:4858)

I'd like to have support for OFFSCREEN_FRAMEBUFFER as well, anyone know the best place for me to start to make this work?

Edit: Sorry, I also have these set -s FULL_ES3 -s MIN_WEBGL_VERSION=2 -s USE_WEBGL2=1
It may actually have something to do with this.