Multithreading 34/34: OffscreenCanvas + proxied Canvas support by juj · Pull Request #6254 · emscripten-core/emscripten

juj · 2018-02-16T17:28:20Z

This PR improves supports for OffscreenCanvas, allowing pthreads to resize canvases from threads, and also creating proxied canvases on the main thread when OffscreenCanvas is not present.

Proxying GL has the benefit of enabling support for eglMakeCurrent(), for true multithreaded WebGL access, i.e. threads can acquire and release access to a single GL context.

Additionally this PR optimizes the proxied GL calls to run asynchronously whenever possible, to avoid synchronous blocking. For example UE4 runs in general without having to block to wait for sync GL calls at all.

Proxying naturally has a performance impact compared to native OffscreenCanvas. That is why proxying is optional, and codebases explicitly need to enable it via OFFSCREEN_FRAMEBUFFER flag when desired.

Codebases will want to choose whether to do true OffscreenCanvas, whether to proxy, or whether to do both, with proxying as a fallback to OffscreenCanvas, so this PR follows an "all options open" type of approach so that codebases can experiment, to find bugs and gauge performance.

This is the last PR of the Multithreading series, and also the largest, given that the WebGL proxying part touches all GL functions. This should be looked at very last after all of the other Multithreading PRs have landed.

kripken

Some initial comments and questions, but I have not read this all yet - I need feedback from you to fully understand the approach here.

Overall I think an overview of the approach should be documented somewhere - perhaps in settings.js alongside the relevant options, or in library_gl.js at the top?

kripken · 2018-10-30T23:55:40Z

src/library_gl.js


 var LibraryGL = {
+#if USE_PTHREADS
+  // If GLctxIsOnParentThread is true, then


how about GLctxIsOnParentThread => proxiedGLctx? That seems more self-explanatory I think.

GLctxIsOnParentThread is more self-explanatory to suggest the type is a boolean, whereas proxiedGLctx could be mistaken with GLctx to be some kind of object, but just a proxied flavor, instead of the real thing. I'll change to GLctxIsProxied to hint at the boolean nature?

Sounds good.

kripken · 2018-10-30T23:56:45Z

src/library_gl.js

+      var handle = _malloc(8); // Make space on the heap to store GL context attributes that need to be accessible as shared between threads.
+      {{{ makeSetValue('handle', 0, 'webGLContextAttributes["explicitSwapControl"]', 'i32')}}}; // explicitSwapControl
+#if USE_PTHREADS
+      {{{ makeSetValue('handle', 4, '_pthread_self()', 'i32')}}}; // the thread pointer of the thread that owns the control of the context


what does "own control of the context" mean?

A place with a general overview of how the proxying works would be good I think.

"own control of the context" means that the JavaScript WebGLRenderingContext resides in that thread. Adjusted the comment.

kripken · 2018-10-31T00:01:09Z

src/library_html5.js

+  $emscripten_get_canvas_element_size_js: function(target) {
+    var stackTop = Runtime.stackSave();
+    var w = Runtime.stackAlloc(8);
+    var h = w + 4;


this will not work on the LLVM wasm backend, the stack grows down there.

h = (w + stackTop) / 2 should work ;)

Surely stackAlloc() will still return a pointer to the beginning of the allocated memory address on the stack? So if one calls var p = stackAlloc(8), then HEAPU8[p] through HEAPU8[p+7] are the bytes that were allocated?

Oh right, you add to the beginning of the allocation. That seems fine then.

kripken · 2018-10-31T00:03:20Z

src/library_html5.js

+    var w = Runtime.stackAlloc(8);
+    var h = w + 4;
+
+    if (typeof target === 'string') {


is this path necessary? if called from JS, the caller can just convert the string to an int..?

We would not want to give the caller the burden to have to allocate and freestrings on the heap - it is smaller code wise to convert here inside (1 call site), rather than requiring all callers to convert (N call sites). User might also accidentally malloc, running into perf issues compared to stackAlloc.

kripken · 2018-10-31T00:11:12Z

src/library_html5.js

+        var newWidth = (cssWidth * dpiScale)|0;
+        var newHeight = (cssHeight * dpiScale)|0;
+
+        if (!target.controlTransferredOffscreen) {


this pattern (if not control transferred, apply, othrewise do the call) appears 3 times. can the logic be moved into the call, that is, emscripten_set_canvas_element_size_js would check if the target has controlTransferredOffscreen and handle that?

Really good point. Changed this, and while doing that, I realize that I am not actually ready to expose this function as a "public" function to JavaScript side, because that is something that will need a closer thought as part of the big elephant issue we have with the JSEvents object being a non-DCEable bloat (#2999, #3298). I think we need to remove that whole object and make all functions individually DCE well (even when Closure is not used), and that will require changes to how users expose the functions to JS code, if they do not call them from C++.

So for now changed the canvas set and get functions to be underscored internal functions - the above refactor would publicize them in a proper manner, while doing the same for other functions.

kripken · 2018-10-31T00:17:27Z

system/lib/gl/gl.c

@@ -1689,7 +1689,6 @@ void* emscripten_GetProcAddress(const char *name_) {
  if (!strcmp(name, "glNormalPointer")) return emscripten_glNormalPointer;


i have some worry about how this stuff will work with the GetProcAddress logic - it would be good to make sure all our GPA tests also run in this proxied mode.

This PR only moves this file around for organizational purposes. The intent of obtaining function pointers to JS functions remains same - pointer to JS function is returned on the calling thread, and the called GL function then proxies if needed.

kripken · 2018-10-31T22:58:38Z

Noticed while looking at the code with @kainino0x - some of the new tests have just cpp files, but no python setup to run them, specifically

gl_in_two_pthreads.cpp
resize_offscreencanvas_from_main_thread.cpp

I verified it's just those two.

kainino0x · 2018-11-01T01:04:16Z

Having issues with a basic SDL test program against juj:multithreading_october_2018_old_sockets, which contains this patch. It's also based on 1.38.11, so could be out-of-date.

screen.width/screen.height fails, and if you work around that:
setWindowTitle fails, and if you work around that:
SDL_GL_CreateContext calls eglCreateContext, which calls glutCreateWindow, which calls createContext with Module['canvas'], which is undefined.
also, registerFocusEventCallback tries to run on the proxied main and fails

// WORKS:  emcc -s WASM=1 -s USE_SDL=2
// BROKEN: emcc -s WASM=1 -s USE_SDL=2 -s USE_PTHREADS=1 -s PROXY_TO_PTHREAD=1

#include <assert.h>
#include <stdio.h>

#include <SDL.h>
#include <SDL_opengl.h>

int main() {
  printf("main\n");

  assert(0 == SDL_Init(SDL_INIT_VIDEO|SDL_INIT_EVENTS));
  SDL_Window *window = SDL_CreateWindow("title", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 512, 512, SDL_WINDOW_OPENGL);
  assert(window);
  SDL_GLContext gl_context = SDL_GL_CreateContext(window);
  assert(gl_context);

  assert(0 == SDL_GL_MakeCurrent(window, gl_context));

  printf("main VENDOR: %s\n", glGetString(GL_VENDOR));
  glClearColor(0, 1, 0, 1);
  glClear(GL_COLOR_BUFFER_BIT);
  SDL_GL_SwapWindow(window);

  printf("main exit\n");
}

juj · 2018-11-01T15:05:05Z

I never had a chance to look to bring SDL2 library over to multithreading world, and reading the description above, the bit SDL_GL_CreateContext calls eglCreateContext reads like something that will cause issues, because current JS-implemented EGL (library_egl.js) is not multithreading aware, and probably not even proxying aware. PR #5580 looks to make EGL fully multithreading aware, but that one is not ready yet. I would expect SDL2 will need some amount of adjustment after that PR to make things flow smooth. If you did have a path that can avoid SDL2, that will likely be of less resistance (or another approach may be to work towards making SDL2+EGL multithreading-enabled first)

juj · 2018-11-09T14:39:44Z

Some initial comments and questions, but I have not read this all yet - I need feedback from you to fully understand the approach here.

Overall I think an overview of the approach should be documented somewhere - perhaps in settings.js alongside the relevant options, or in library_gl.js at the top?

Added documentation to https://github.com/kripken/emscripten/pull/6254/files#diff-16eca2b70acff1fd624285ab9390a9c7

juj · 2018-11-09T14:56:26Z

Noticed while looking at the code with @kainino0x - some of the new tests have just cpp files, but no python setup to run them, specifically
* `gl_in_two_pthreads.cpp`

* `resize_offscreencanvas_from_main_thread.cpp`
I verified it's just those two.

Good catch, this was result from first having authored all the multithreading work in one big branch, and then later splitting it up in different PRs, so test code for those two were lost in the change. Added tests for these now.

Rebased against latest incoming, this should now be good.

kripken · 2018-11-12T20:57:43Z

site/source/docs/api_reference/html5.h.rst

+
+    If a WebGL rendering context is created as an OffscreenCanvas-based context, it will have the limitation that only the pthread that created the context can enable access to it (via ``emscripten_webgl_make_context_current()`` function). Other threads will not be able to activate rendering to the context, i.e. OffscreenCanvas-based contexts are essentially "pinned" to the pthread that created them.
+
+    If the current browser does not support OffscreenCanvas, you can specify the ``EMSCRIPTEN_WEBGL_CONTEXT_PROXY_ALWAYS`` WebGL context creation flag. If this flag is passed, and code was compiled with ``-s OFFSCREEN_FRAMEBUFFER=1`` enabled, the WebGL context will be created as a "proxied context". In this context mode, the WebGLRenderingContext object will actually be created on the main browser thread, and all WebGL API calls will be proxied as asynchronous messages from the pthread into the main thread. This will have a performance and latency impact in comparison to OffscreenCanvas contexts, however unlike OffscreenCanvas-based contexts, proxied contexts can be shared across any number of pthreads: you can use the ``emscripten_webgl_make_context_current()`` function in any pthread to activate and deactivate access to the WebGL context: for example, you could  have one WebGL loading thread, and another WebGL rendering thread that coordinate shared access to the WebGL rendering context by cooperatively acquiring and releasing access to the WebGL rendering context via the ``emscripten_webgl_make_context_current()`` function. Proxied contexts do not require any special support from the browser, but any WebGL capable browser can create a proxied WebGL context.


The final "but" (in "but any WebGL capable browser..") seems like it should be "so"? Or did I misunderstand the intention?

Oh yeah, that's much better. The second statement in the sentence is a continuation to the first, not a contrary.

kripken · 2018-11-12T20:58:58Z

src/library_gl.js

+  // If GLctxIsProxied is true, then
+  //   a) the running thread is not the main browser thread, and
+  //   b) the currently active GL context is running on the main browser thread, so all GL calls need to be proxied to it.
+  $GL__postset: 'var GLctx; var GLctxIsProxied = false; GL.init()',


why do we have GLctxIsProxied instead of GL.ctxIsProxied (i.e. as a property on GL)?

That is good point - at some point I had part of the proxying extend to src/pthread-main.js, which is currently outside minification, so it did not see the GL variable to access, but looking at it now, I no longer have that, so this no longer needs to be global. Given that we have GL.currentContext, I also renamed this once more to GL.currentContextIsProxied to make it look similar.

tools/system_libs.py

kripken · 2018-11-12T21:03:54Z

Thanks, great updates!

I had a few minor questions and comments now, but aside from those I think this is good to land.

This PR improves supports for OffscreenCanvas, allowing pthreads to resize canvases from threads, and also creating proxied canvases on the main thread when OffscreenCanvas is not present. Bump version to 1.38.19.

kripken

Great, thanks!

dschuff · 2018-11-14T22:02:30Z

Since this includes a version bump, I'm going to go ahead and merge it so we can update the other tags and get CircleCI and the integration waterfall green again.

This PR improves supports for OffscreenCanvas, allowing pthreads to resize canvases from threads, and also creating proxied canvases on the main thread when OffscreenCanvas is not present. Proxying GL has the benefit of enabling support for eglMakeCurrent(), for true multithreaded WebGL access, i.e. threads can acquire and release access to a single GL context. Additionally this PR optimizes the proxied GL calls to run asynchronously whenever possible, to avoid synchronous blocking. For example UE4 runs in general without having to block to wait for sync GL calls at all. Proxying naturally has a performance impact compared to native OffscreenCanvas. That is why proxying is optional, and codebases explicitly need to enable it via OFFSCREEN_FRAMEBUFFER flag when desired. Codebases will want to choose whether to do true OffscreenCanvas, whether to proxy, or whether to do both, with proxying as a fallback to OffscreenCanvas, so this PR follows an "all options open" type of approach so that codebases can experiment, to find bugs and gauge performance. This is the last PR of the Multithreading series, and also the largest, given that the WebGL proxying part touches all GL functions. This should be looked at very last after all of the other Multithreading PRs have landed.

The logic here for proxying this call seems to exist in both the JS code and in the native code. It seems to have been that way since it was added in emscripten-core#6254.

juj added performance GL multithreading labels Feb 16, 2018

nickshinpho mentioned this pull request Apr 24, 2018

Multithreading 29/N: Backproxying #6201

Merged

nickshinpho mentioned this pull request May 3, 2018

Multithreading 30/N: HTML5 Event Backproxying #6202

Merged

juj force-pushed the incoming branch from e1622af to d54eb4e Compare May 21, 2018 13:46

juj force-pushed the offscreencanvas_and_proxied_canvas branch from f6707dd to d85e344 Compare July 21, 2018 17:48

kripken reviewed Oct 31, 2018

View reviewed changes

juj mentioned this pull request Nov 3, 2018

Update EM_QUEUED_CALL_MAX_ARGS #7445

Closed

juj force-pushed the offscreencanvas_and_proxied_canvas branch 6 times, most recently from 9dda33c to 0bdba72 Compare November 9, 2018 14:38

juj force-pushed the offscreencanvas_and_proxied_canvas branch 2 times, most recently from fd51a60 to 2908e26 Compare November 9, 2018 14:43

juj force-pushed the offscreencanvas_and_proxied_canvas branch 3 times, most recently from 2d1e9fe to 9663ba2 Compare November 12, 2018 15:45

kripken reviewed Nov 12, 2018

View reviewed changes

tools/system_libs.py Outdated Show resolved Hide resolved

juj force-pushed the offscreencanvas_and_proxied_canvas branch from 9663ba2 to 174cc88 Compare November 14, 2018 09:30

OffscreenCanvas + proxied Canvas support

ee5f632

This PR improves supports for OffscreenCanvas, allowing pthreads to resize canvases from threads, and also creating proxied canvases on the main thread when OffscreenCanvas is not present. Bump version to 1.38.19.

juj force-pushed the offscreencanvas_and_proxied_canvas branch from 174cc88 to ee5f632 Compare November 14, 2018 13:07

kripken approved these changes Nov 14, 2018

View reviewed changes

dschuff merged commit 28a144c into emscripten-core:incoming Nov 14, 2018

sbc100 mentioned this pull request Jan 29, 2024

Remove duplicate proxying for emscripten_webgl_create_context #21200

Open

		@@ -1689,7 +1689,6 @@ void* emscripten_GetProcAddress(const char *name_) {
		if (!strcmp(name, "glNormalPointer")) return emscripten_glNormalPointer;


		If a WebGL rendering context is created as an OffscreenCanvas-based context, it will have the limitation that only the pthread that created the context can enable access to it (via ``emscripten_webgl_make_context_current()`` function). Other threads will not be able to activate rendering to the context, i.e. OffscreenCanvas-based contexts are essentially "pinned" to the pthread that created them.

		If the current browser does not support OffscreenCanvas, you can specify the ``EMSCRIPTEN_WEBGL_CONTEXT_PROXY_ALWAYS`` WebGL context creation flag. If this flag is passed, and code was compiled with ``-s OFFSCREEN_FRAMEBUFFER=1`` enabled, the WebGL context will be created as a "proxied context". In this context mode, the WebGLRenderingContext object will actually be created on the main browser thread, and all WebGL API calls will be proxied as asynchronous messages from the pthread into the main thread. This will have a performance and latency impact in comparison to OffscreenCanvas contexts, however unlike OffscreenCanvas-based contexts, proxied contexts can be shared across any number of pthreads: you can use the ``emscripten_webgl_make_context_current()`` function in any pthread to activate and deactivate access to the WebGL context: for example, you could have one WebGL loading thread, and another WebGL rendering thread that coordinate shared access to the WebGL rendering context by cooperatively acquiring and releasing access to the WebGL rendering context via the ``emscripten_webgl_make_context_current()`` function. Proxied contexts do not require any special support from the browser, but any WebGL capable browser can create a proxied WebGL context.

Conversation

juj commented Feb 16, 2018

Uh oh!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Oct 31, 2018

Uh oh!

kainino0x commented Nov 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juj commented Nov 1, 2018

Uh oh!

juj commented Nov 9, 2018

Uh oh!

juj commented Nov 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kripken commented Nov 12, 2018

Uh oh!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

dschuff commented Nov 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kainino0x commented Nov 1, 2018 •

edited

Loading