Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build gl separately for OFFSCREEN_FRAMEBUFFER #8530

Merged
merged 4 commits into from
May 3, 2019
Merged

Build gl separately for OFFSCREEN_FRAMEBUFFER #8530

merged 4 commits into from
May 3, 2019

Conversation

kripken
Copy link
Member

@kripken kripken commented May 1, 2019

This lets a program use pthreads, but not OFFSCREEN_FRAMEBUFFER, and still work. Otherwise, the proxying code would have been reached anyhow, in the case that the context was created from JS. With this PR, it's not even linked in.

Note that I had to change some tests for this - do those look right?

@kripken kripken requested a review from juj May 1, 2019 16:00
@kripken
Copy link
Member Author

kripken commented May 1, 2019

Fixes a regression from #8059

@juj
Copy link
Collaborator

juj commented May 3, 2019

Perfect! Yeah reading through the changes I see that I had been working from the mindset that in multithreaded builds one would always be using either OffscreenCanvas or OffscreenFramebuffer, but naturally one can do without either, as long as one restricts GL to the main thread.

However I think before this PR things had worked well with GL-just-on-main-thread-with-use-pthreads case, due to the cases that had been tested had been using emscripten_webgl_create_context() API to create the context, with OffscreenFramebuffer being disabled - so the code would be getting the proxying machinery in, but always just not use proxying, since the proxying code is aware of the fact that proxying will not be needed if the GL context does actually live in the calling thread. In your case, I understand it was initializing the GL context in a way that avoided the proxying init somehow(?)

@kripken
Copy link
Member Author

kripken commented May 3, 2019

Yes, the code was creating the proxy in JS (using Browser.createContext etc.). It might be interesting to unify all the code paths for context creation, but it might not be good for code size, I'm not sure. But what this PR does seems optimal for that (no GL proxying code unless the user asks for it).

@kripken kripken merged commit debac23 into incoming May 3, 2019
@kripken kripken deleted the ofb branch May 3, 2019 17:33
VirtualTim pushed a commit to VirtualTim/emscripten that referenced this pull request May 21, 2019
This lets a program use pthreads, but not OFFSCREEN_FRAMEBUFFER, and still work. Otherwise, the proxying code would have been reached anyhow, in the case that the context was created from JS. With this PR, it's not even linked in.
VirtualTim added a commit to VirtualTim/emscripten that referenced this pull request May 23, 2019
VirtualTim added a commit to VirtualTim/emscripten that referenced this pull request May 23, 2019
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
This lets a program use pthreads, but not OFFSCREEN_FRAMEBUFFER, and still work. Otherwise, the proxying code would have been reached anyhow, in the case that the context was created from JS. With this PR, it's not even linked in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants