Emscripten support take 2

[sortofsleepy]

👋

Take 2 - starting a different pull request since the comments on the other one was getting a bit long-winded (see PR #2069 for reference)

For the most part, nothing has really changed other than ensuring compatibility with 0.9.3. In the limited testing I've been doing I have not seen any issues but there is one area to pay possibly pay attention to; the main issue I ran into was to ensure GLAD was not included(Emscripten headers include the necessary functions so a loader is not needed). I am not sure if there is anything else but nothing obvious popped up during testing.

Petros's previous CMake fixes are included as well though I did a little more editing to remove some possibly un-needed things like the Vorbis files and ensuring the Audio files aren't built when building Cinder. I think there's probably other stuff that could be left out in lieu of more web native solutions like jsoncpp, ImGui but I'll leave that up to you folks to determine what's appropriate.

Other than those core changes

• Audio support has been scaled back to just including a way to easily include an <audio> tag. Currently, while it probably would not be too much work to add wrappers around the basics of the WebAudio api, to do any real processing would require substantially more work as the current recommendation is to use the AudioWorklet api as the processing is not bound to the main thread. The AudioWorklet api, in my brief experimentation a few months ago, requires actual Javascript to be utilized and does not appear to take to transliterated Javascript via Emscripten, hence the challenge as obviously, the core audience of Cinder are C++ users.
  
  See this MDN page on the ScriptProcessorNodewhich was the previous method to do audio processing. It is probably possible to still use this but it is bound to the primary thread and it is scheduled for deprecation....someday ha.

• A new header-only class, JSObject has been added to make try and help make things easier in any cases where Javascript might have to be utilized in a more C++ friendly way vs having to use EM_JS or the like. There is likely more that could be added to it but for now, it contains the foundation.

• Samples have been cleaned up a little bit. A little more cleaning to do but things work.

• Other minor updates here and there.

[sortofsleepy]

Ah, also apologies for any confusion sortofsleepy AND sortasleepy are both me. I keep forgetting which user is assigned to which computer; at some point I should fix that since I really only use one account ha.

[aboutqx]

Is this ready for use?

[vinjn]

I am feeling this PR is too huge for review, @sortofsleepy would you mind splitting it into several PRs? e.g. the core part / samples part / documentation part.

[sortofsleepy]

Is this ready for use?

Many apologies, I meant to respond sooner but this slipped my mind. For me I think it's ready, I've been using my branch for awhile now and haven't run into any issues. That said working with Cinder is not part of my day job so your mileage may vary; I would wait until this gets merged as eyes more experienced than mine will have taken a look. Also please bear in mind that some things will just simply not work due to differences between the Web and Desktop, the built-in audio classes for example.

I am feeling this PR is too huge for review, @sortofsleepy would you mind splitting it into several PRs? e.g. the core part / samples part / documentation part.

Before I try to find the time to figure out how to do that, can I ask why you feel it's too large of a PR @vinjn?

Everything Emscripten related should be in it's own section/file with the exception being changes to core files. It's been awhile since I looked but changes to core files shouldn't be greater than 10-15 lines of code(but likely far less based on what I remember). Build code was already looked over by Petros awhile back, the changes I made recently were extremely minor in my opinion(just removing some libraries if I recall correctly).

[vinjn]

Personally, I really wanna this feature to become core of Cinder.
But on the other hand, this PR contains tens of commit histories, which makes it hard for reviewers to review.
Maybe you can combine a few commits to make it easier.

[sortofsleepy]

Personally, I really wanna this feature to become core of Cinder.

Me too 🎉

But on the other hand, this PR contains tens of commit histories, which makes it hard for reviewers to review.
Maybe you can combine a few commits to make it easier.

Briefly looking over the commit history to try and refresh my memory and factoring in my current schedual(as well as how terrible I am at rebasing ha) , I wouldn't be able to sit down and fully devote enough time and energy to properly figure out how to split this PR into smaller ones. I could start over from step 1 but that also takes time as well.

I am also somewhat inclined to not do this because I guess I'm confused why a reviewer would sit down and look at each commit for something like this?

Normally I would understand someone doing that if the changes a contributor was proposing was tightly integrated into the existing codebase and I can agree that some commits are not important anymore with some of the recent changes I made; however again, most of the code is completely separate and not tightly integrated into the main Cinder core.

Yes there are some changes to core files like Platform.h, but from my brief look over of the commit history, the changes are fairly small and not complicated; they span across just 3 commits at best(whoever reviews this I can point you to those). The only tightly integrated part was the audio integration but that has already been removed with this PR. Build files were slightly tweaked recently(really just removing some libs that probably wouldn't work well on the web) but the major portions were already reviewed by a reviewer with their changes integrated into this PR.

Aside from the core integration parts I am not sure why anyone would need to look at the rest of the commits? Assuming they are not Javascript files, everything else has the words "Emscripten" somewhere as part of either the filename, folder path, or somewhere in the code, why would they not just look at those files?

In any case, I will see about trying to find some time to do this at some point but assuming I decide to split things up it may not be for awhile unfortunately, my apologies.

[richardeakin]

Here's my 2sense on finally getting emscripten into core:

From what I've seen in the past when adding new platforms has been added (android, linux, WinRT), it usually first arrives as a giant glob of getting things to work and things also still not working. Then each time I've witnessed it, there is a process of taking what is working in there and 'manually rebasing it' to clean up the commits and make sure things affecting core are isolated / easy to track down. This is time consuming, but it does improve the minds of possible future commiters months / years down the road.

It is great that people are willing to review, it's not an easy thing to find time for it. From my point of view, if things are generally working but there is still work in progress, it works well enough to get an initial merge by making a solid attempt to document what is WIP and what isn't working. We also run the risk of this list growing stale, but it does make it easier to get an initial merge.

There also has to be some working tests and samples that make it easy for reviewers to jump in and use, so I think those will have to be part of the initial PR/merge, although perhaps leaving them as a separate commit is a tidy way to do them. I think once a PR grows this complex, trying to use git's rebase is a recipe for frustration, more so than creating a new branch off of master and pulling in the mods bit by bit. It's also a good way to (re)discover what works and what's work in progress, as well as filtering out experimental code that ended up not being needed.

The emscripten platform addition has gone through at least two different solid attempts, maybe more, spanning many years now. The desire to get it in there seems worthwhile, I do think it will take a solid push to get it updated and in place for an initial merge.

[aboutqx]

@richardeakin Another possibility is Run on cloud, some thing like Unreal engine's Pixel Stream platform.It make all devices which can play web video to run very large application while actually the logics run on server and push video stream to device web browsers.Maybe the community can consider this.

[sortofsleepy]

Ok! finally have a little bit of time to think about this more.

So personally I feel that this is ready for an initial merge(after bringing it up to date with latest master commits) and has been for quite some time. The only things integrated tightly with the core now that the audio integration has been removed is just simply what's needed to get things running; everything else could be ignored or stripped and it wouldn't make a difference; things should still continue to run. Petros has already looked over the bulk of the CMake related commits ages ago and his changes were integrated, the latest changes were really just me taking out a library or two since it didn't make sense to include them.

I've been running my fork for ages now without issue, so far everything still works as expected even when I am not strictly targeting Emscripten; now that being said, I do not get to use Cinder or do any real graphics programming in my day-to-day work, so I am looking at things from the perspective of a hobbyist more or less, there may be functionality other people might use that has not been tested and may have broken. I think this is another big reason to get this out into the community so we can figure out if anything needs to be fixed or needs more work on, at the very least as a sort of "on-the-side" release.

If it would be helpful I can dig up those core commits again so someone can take a closer look and add more examples or get some current examples up and running with Emscripten. The docs were updated with this PR but I can go over those again as well.

Thinking about it more, I would definitely rather not rebase; if necessary though, now that I have more time I can consider carefully reconstructing each commit if any of the core folks think that is necessary; but to be honest I would rather not do that either cause well, things do work.

[felixguendling]

Thank you for your work on bringing Cinder to the web. This is really helpful.

I tried out your code by trying to compile one of the examples (Picking3D in my case).

Adding the following code to cinderMakeApp.cmake was useful to get a working HTML file where you can see the result (by running a local HTTP server like static from NPM) (instead of a just a JS file):

if ( CINDER_EMSCRIPTEN )
  set_target_properties( ${ARG_APP_NAME} PROPERTIES SUFFIX ".html" )
  target_link_options( ${ARG_APP_NAME} PRIVATE --bind )
  target_link_options( ${ARG_APP_NAME} PRIVATE -sERROR_ON_UNDEFINED_SYMBOLS=0 -sUSE_WEBGL2=1 -sFULL_ES3=1 -sUSE_GLFW=3 -sDISABLE_EXCEPTION_CATCHING=0 -sDEMANGLE_SUPPORT=1 )
endif()

The last part are just the CINDER_EMSCRIPTEN_LINK_FLAGS. However, CMake always added quotes so I removed the whitespace between -s and the option name. Maybe there are cleaner solutions. At least you can just run mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake .. and it works.

My Emscripten does not set the required/checked environment variable, so I had to execute export EMSCRIPTEN=1 before running CMake. I'm not sure whether makes sense to check for specific environment variables. Emscripten provides a toolchain file.

---

Overall, I would really appreciate having this on the master branch because having a potentially not perfect Emscripten support available is much better than no support at all. This way, potential shortcomings of the status quo implementation can be improved with small PRs instead of one large that amasses all changes.

---

For me, it works fine. Thank you! 🥳

[sortofsleepy]

Thank you for your work on bringing Cinder to the web. This is really helpful.

I tried out your code by trying to compile one of the examples (Picking3D in my case).

Adding the following code to cinderMakeApp.cmake was useful to get a working HTML file where you can see the result (by running a local HTTP server like static from NPM) (instead of a just a JS file):

if ( CINDER_EMSCRIPTEN )
  set_target_properties( ${ARG_APP_NAME} PROPERTIES SUFFIX ".html" )
  target_link_options( ${ARG_APP_NAME} PRIVATE --bind )
  target_link_options( ${ARG_APP_NAME} PRIVATE -sERROR_ON_UNDEFINED_SYMBOLS=0 -sUSE_WEBGL2=1 -sFULL_ES3=1 -sUSE_GLFW=3 -sDISABLE_EXCEPTION_CATCHING=0 -sDEMANGLE_SUPPORT=1 )
endif()

The last part are just the CINDER_EMSCRIPTEN_LINK_FLAGS. However, CMake always added quotes so I removed the whitespace between -s and the option name. Maybe there are cleaner solutions. At least you can just run mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake .. and it works.

My Emscripten does not set the required/checked environment variable, so I had to execute export EMSCRIPTEN=1 before running CMake. I'm not sure whether makes sense to check for specific environment variables. Emscripten provides a toolchain file.

Overall, I would really appreciate having this on the master branch because having a potentially not perfect Emscripten support available is much better than no support at all. This way, potential shortcomings of the status quo implementation can be improved with small PRs instead of one large that amasses all changes.

For me, it works fine. Thank you! 🥳

Ah! Glad you got it working, albeit in a slightly more roundabout way then I remember. I unfortunately have not looked at this in awhile due to life but good to see the code being made use of.

I unfortunately am out of town and not at my computer but I'm pretty sure I set things up so that you don't need to modify any core build files; that said if this works for you that's cool.

If you're curious, the doc source in the repo should detail the steps.

Cheers!