-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Emscripten 3.1.19 #7056
Emscripten 3.1.19 #7056
Conversation
I guess, the checks are not successful because I added |
@Jonathhhan does it mean we need to change the emscripten version used by the CI here? The rest of the PR looks good to me. |
@ofTheo not sure, because I build the libs directly, without docker. |
@Jonathhhan I guess I am wondering, what the Or is it just about passing the flag to emscripten ( right now the flag is not recognized ). Could you push to your branch with a change to this line: from:
to
Then the OF CI will be running the same version as you 🙂 |
@ofTheo I made the change in https://github.com/openframeworks/openFrameworks/blob/master/.github/workflows/build-emscripten.yml#L24 I actually compiled the libs with that flag, but it does not seem to make a difference (the mentioned changes are still necessary). And maybe I did not use that file at all. I compiled with something like:
|
Thanks @Jonathhhan I think maybe I'll need to build the apothecary stuff against |
Not sure if it changes anything but I've noticed 3.1.20 is out |
@dimitre yes, it makes sense to compile with the latest version. |
I replaced |
I made a first attempt to implement the new audio worklet branch (see the end of the Emscripten link in the first post of this thread). Not working yet, but should be doable. Maybe someone more experienced can have a look. It is not part of this pull request. |
I made a first working example with the current audioWorklet branch. It is very hacky and I needed to edit some of the Emscripten files. It may be also possible to implement it much better and without editing the emscripten files. But maybe it is helpful as a first step. https://github.com/Jonathhhan/openFrameworks/tree/emscripten_3.1.9_audioWorklet/addons/ofxEmscripten |
EGLContext ofxAppEmscriptenWindow::getEGLContext(){ | ||
return context; | ||
shared_ptr<ofBaseRenderer> & ofxAppEmscriptenWindow::renderer(){ | ||
return _renderer; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious if all this stuff needs to be removed?
Do we not have a context anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most of the changes in this file are from @roymacdonald: #6781 (comment)
as far as I can tell it is a web_gl context now:
context = emscripten_webgl_create_context("#canvas", &attrs);
Re-running the emscripten job now that Freeimage is updated in apothecary. @Jonathhhan @themancalledjakob - if this passes let me know if you think its good to merge or if there is any small fixes cleanup that needs to happen. |
Currently erroring with a linking error:
This seems to suggest that maybe apothecary might need to be building libs with the newer SDK? |
@ofTheo thats probably the case, maybe because they updated llvm in 3.1.25: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md |
attrs.antialias = 1; | ||
attrs.majorVersion = 2; | ||
attrs.minorVersion = 0; | ||
attrs.alpha = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we be passing in any other settings from the settings argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if the other settings are needed in some cases...
Okay now with libs in apothecary being built with 3.1.21 this looks like it all works. Maybe I'll merge this in for now and we can do some smaller PRs for other things. @Jonathhhan does this fix the fullscreen issues? |
Thanks @Jonathhhan !! |
@ofTheo maybe just update it for 3.1.26 (if it is not difficult - but I guess that only the apothecary libs need to be recompiled. I use 3.1.25 at the moment)? |
Out of curiosity I just tried compiling all apothecary libs and this branch with emsdk 1.40.0, and except using emrun to launch a dev server everything seems to just work. running something like of course otherwise it's better to just compile with 3.1.25 or 3.1.26 (it seems as if there are is a new version almost every other week atm) |
to get a better overview over what works and what doesn't, I thought it would be good to have a gallery of all examples. I made a little sketch of how this may work, it's super rough. but it might help with getting an overview. if you want to try it yourself you need to download this: https://pointer.click/files/testAllExamples.zip and put it in your note: some examples also have runtime errors when loading them. these errors are not red, you'd have to open the example to catch them. and of course, some examples cannot work, but some could possibly be fixed. also I noticed there are some glitches and the whole thing could be more elegant, but what do you think, can this be useful? |
@themancalledjakob thanks. I think this is really useful for finding out what is working (I already recognized some examples that were working before, like the 3DPrimitivesExample). |
The 3DPrimitivesExample works if I comment out this line:
Otherwise this is the error message:
|
wow - this is super @themancalledjakob and something that could be great to have hosted on the site ( once the kinks are worked out ). makes seeing potential issues easier to see. |
cool :) i'm happy this is useful!
which main console do you mean? |
oh, cool! good you found that, there is a similar error with quite some other examples |
The sort of fake terminal that OF outputs messages to below the renderer. |
hmmm not having tested it, but it should be possible to override console.errorOriginal = console.error;
console.error = (...messages) => {
console.errorOriginal(...messages);
// add messages to oF terminal
} could that work? |
This is a bit(e) size pull request for Emscripten 3.1.19.
Here are some additional details: https://forum.openframeworks.cc/t/emscripten-3-1-19/40294
Most of the changes in ofxAppEmscriptenWindow.cpp are from @roymacdonald
I left out all the Audioworklet changes, but as soon as they are part of the main Emscripten branch, it would be nice to implement them: emscripten-core/emscripten#16449