EmscriptenApplication: custom canvas id, multiple applications on one page

[pezcode]

Quick draft for making the canvas CSS selector configurable.

I've tested it with the triangle example:

• compile with -s DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR. Technically default for newer Emscripten versions but required for 1.39.4 or older.
• create TriangleExample with Configuration{}.setCanvasTarget("#canvas2")
• change canvas id="canvas2" in the .html
• set Module.canvas = document.getElementById('canvas2'). Could also modify EmscriptenApplication.js directly. This variable isn't necessary with DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR anymore, so apart from setting the canvas opacity after a crash it doesn't do anything.

Few things worth noting:

• adding the string to Configuration makes it non-constexpr-constructable (at least until C++20) so I had to change the constructor
• this needs decent documentation about DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR
• does it make sense to allow changing the target selector at runtime? I don't see a use case personally but others might

If this looks OK to you I'll add missing documentation and also a few words to https://doc.magnum.graphics/magnum/platforms-html5.html

[codecov]

Codecov Report

Merging #480 (9fb8958) into master (8ec6b18) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #480   +/-   ##
=======================================
  Coverage   77.14%   77.14%           
=======================================
  Files         415      415           
  Lines       25490    25490           
=======================================
  Hits        19665    19665           
  Misses       5825     5825           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ec6b18...87fb315. Read the comment docs.

[mosra]

Thank you! :)

adding the string to Configuration makes it non-constexpr-constructable

That's fine, there wasn't any practical use case for a constexpr Configuration anyway. I'll probably experiment myself with replacing std::string with Containers::String[View] to see if that affects binary size in any way.

does it make sense to allow changing the target selector at runtime?

I'd say it gets added once someone has a use case, don't bother with that now.

I've tested it with the triangle example

There's a test for EmscriptenApplication inside the Test dir and I think it might make sense to create a dedicated variant of it for this functionality (with DISABLE_DEPRECATED_... set). Basically duplicating this into a new PlatformEmscriptenApplicationCustomCanvasTest, reusing the same cpp source with the setCanvasTarget() controlled with a define, for example. The test gets built if you enable BUILD_TESTS in CMake, the HTML runner will get copied next to the wasm so you can run it directly from the build dir.

Module.canvas = document.getElementById('canvas2')

Yeah this should go into the HTML (and keep the default in the JS), but I wonder what happens to the global Module when you have multiple canvases ... does the same instance get used for all of them? Or is new Emscripten not using any of that anymore (and so neither console log redirection nor download progress works anymore)? Or how does that work?

[pezcode]

There's a test for EmscriptenApplication inside the Test dir and I think it might make sense to create a dedicated variant of it for this functionality

👍

I wonder what happens to the global Module when you have multiple canvases ... does the same instance get used for all of them? Or is new Emscripten not using any of that anymore (and so neither console log redirection nor download progress works anymore)? Or how does that work?

For multiple canvases you'll need MODULARIZE=1 (together with EXPORT_NAME) which to me seems like it should be the default anyway. It lets you pass a local object with options (what's now the global Module) to a function that creates the application and returns a promise:

const Module = { /* same old code as in EmscriptenApplication.js */ };
createModule(Module).then(function(myModule) {
  // success, application about to run
  // here, myModule == Module
});

[pezcode]

Got this properly working as far as I can tell now.

There was a bug that returned the element ID instead of a CSS selector, as needed by Emscripten. The reason this has worked so far is that it simply found the first canvas, not the element with id="canvas". This might break some applications but it's highly unlikely that someone gave an element whose id is the html tag of what they wanted to listen to 👀

One thing I'd still like to fix to make things work with multiple EmscriptenApplications on one page is to make keyboardListeningElement not depend on Module. The only backwards-compatible way I can think of is to let those applications set Module['keyboardListeningElement'] = {} which will make the target resolve to 0 in which case the default could then be the canvas set in Configuration. Thoughts? 😬 I'm also open to breaking changes

[mosra]

Thanks, this looks great so far.

There was a bug that returned the element ID instead of a CSS selector

Wow. I had to double-check the docs, and I guess I know where my confusion came from. It's clarified in the text below, but the snippet there is misleading:

EMSCRIPTEN_RESULT emscripten_set_some_callback(
  const char *target,   // ID of the target HTML element. <---- WRONG, it's a CSS selector, not an ID
...

This looks like an obvious fix that doesn't need further discussion, so I commited it right away as c760acb.

This might break some applications but it's highly unlikely that someone gave an element whose id is the html tag of what they wanted to listen to 👀

Hahah. Yeah, this is fine :)

PatchWebApplicationCssCustomCanvas.cmake

Yeah, sorry -- at this point it probably makes sense to switch the CSS to use classes instead of IDs, so such patching isn't needed. The same for the #container, #sizer, #expander etc. I think, even though those center the element on the page and keep its aspect ratio, it shouldn't be impossible to have more than just one on the page. I'll look into that.

One thing I'd still like to fix to make things work with multiple EmscriptenApplications on one page is to make keyboardListeningElement not depend on Module.

The keyboardListeningElement was initially used by Emscripten's SDL emulation and I kept the same behavior in EmscriptenApplication for compatibility... but maybe it's time to finally break this messy compatibility chain :) In particular, I think a much simpler would be to have the keyboardListeningElement specified exclusively in the app Configuration, without having to extract anything via EM_ASM.

And then, looking into my docs, there's Module.doNotCaptureKeyboard, which also seems to be exclusive to SDL emulation and thus broken with EmscriptenApplication (because I don't query this property anywhere). Sigh.

The only backwards-compatible way I can think of is to let those applications set Module['keyboardListeningElement'] = {} which will make the target resolve to 0 in which case the default could then be the canvas set in Configuration.

I'd flip this behavior upside down -- introducing Configuration::setKeyboardListeningElement() that would control this and then having some backwards compatibility only in case Module.keyboardListeningElement is defined. Because if the canvas element is defined on the C++ side, it would seem weird to have to define keyboard listening element on the JS side.

---

Hm, now I realized there are two different ways / use cases and I'm not sure which one is better:

1. Defining the canvas and keyboard listening elements exclusively in C++. This makes the C++ code simpler as there's no need for using EM_ASM to extract the configuration and no problems with a global Module either. That's what I suggested above, but it strongly ties the compiled binary with a particular HTML markup.

2. Having an option to define the canvas / keyboard listening elements in JS (either exclusively there or as a way to override what's set in C++). This means a particular binary wouldn't be tied to a particular HTML markup, but could be used for multiple different canvases without explicitly implementing a way to override the C++ configuration from outside. I can imagine having some interactive visualization on a page and the same binary later reused in a blog post, in a different element.

   But I wonder how much is this scenario possible in practice -- in particular, is there some way to query what EXPORT_NAME got used from the C++ side so it could query ${EXPORT_NAME}.keyboardListeningElement instead? And in the extreme case, is it possible to load the same wasm binary twice and use it independently on two different canvases? (It should be also possible to have a single application control multiple canvases, that's a path to investigate when I get to implementing multi-window support in desktop apps.)

[pezcode]

Your questions just made me go and check and I realized that we DO have access to Module, even with -s MODULARIZE so really the whole Configuration change is unnecessary.

With MODULARIZE, you can do something like the following:

const Module1 = {
  canvas: ...,
  keyboardListeningElement: ...,
  ...
};
createModule(Module1).then(function(myModule) {
  // application 1
});
const Module2 = { 
  canvas: ...
};
// note: same application being instantiated, could also be another one
createModule(Module2).then(function(myModule) {
  // application 2
});

From C++ code calling JS, Module actually resolves to each application's myModule. Not exactly sure how this is wired up and I'll have to do a bit of reading to make sure this isn't a brand-new feature.

But really, the Configuration changes should be ditched again and replaced with:

_canvasTarget = EM_ASM({Module['canvas'];}); // pseudo code :v

I'll play around with PlatformMultipleEmscriptenApplicationTest and get back to you 🚀

[mosra]

Module actually resolves to each application's myModule

Oh, nice. Yeah, I gave it a bit more thought and if we can stay with Module.canvas controlling the canvas element and Module.keyboardListeningElement controlling what gets the events, that would be the best I think (thus nothing in C++ Configuration).

I'm still not exactly sure why Emscripten is gradually moving away from / deprecating these -- is there something inherently wrong with this approach that we overlooked?

[mosra]

I just opened #481 that updates the CSS to avoid copying/patching it for a different canvas ID.

It also contains various other changes that should hopefully help this PR, but there are still a few unresolved issues related to the global Module and the JS driver file. Ideally if you could base this PR off that one and further modify things if needed.

[pezcode]

Just pushed the progress so far, I'll figure out how to merge your PR and changes on master later this week.

I reverted the Configuration change so the canvas is now read from Module.canvas. Multiple applications on one page now work, see PlatformMultipleEmscriptenApplicationTest. Without CSS for now for obvious reasons 🙊

Still not sure if having access to Module in EM_ASM with MODULARIZE=1 is guaranteed behaviour, so I'd appreciate if you could test this on whatever Emscripten version you deem the minimum. It works on 2.0.8 for me. Note: it even survives --closure 1 so it looks kinda intentional.
If it fails on earlier Emscripten version, there's still this workaround:
https://github.com/emscripten-core/emscripten/blob/2a0f94ac112e4372a988fb025b9d02b40c47266f/src/library_html5.js#L300

I changed the tests to have both variants of -s DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR, not sure if that's as clean as you'd like.

[pezcode]

Alright, I'm all done here I think. Rebased on top of #481. Multiple applications on one page work properly, including per-canvas events, cursors and redraws as far as I could test.

All (🤠) that's left is some documentation.

I had to change EmscriptenApplication.js to not blindly overwrite Module since that possibly erases items Emscripten already wrote into it. Using it with --pre-js (which is necessary for -s MODULARIZE) this will be produced:

var Module=typeof createModule!=="undefined"?createModule:{};
var readyPromiseResolve,readyPromiseReject;
Module["ready"]=new Promise(function(resolve,reject){readyPromiseResolve=resolve;readyPromiseReject=reject});
null;
// EmscriptenApplication.js content
var Module={
  // ...
};

The custom canvas test is gone since that's being tested in PlatformMultipleEmscriptenApplicationTest anyway.

[pezcode]

Added a quick, uncompiled attempt at documentation, let me know if anything is unclear or missing.

[mosra]

Thanks, the docs are great.

Apart from two minor things I commented on below (which I can do post-merge myself as well, no problem), I got an idea that the EmscriptenApplication.js could be rewritten in a way that (hopefully) wouldn't require any --pre-js tricks (and thus eliminating the last remaining case where a binary is tied to a particular markup). This expands on my question (and probably answers your comment) on #481 (comment) -- instead of defining a global Module, it would define a function returning its contents and implicitly / for backwards compatibility putting that to a global Module:

function createMagnumModule(init) {
    var module = {
        /* Take the Emscripten-supplied Module object as a base */
        ...(typeof Module !== "undefined" ? Module : {}),
        /* Update it with our things */
        ...{
            preRun: [],
            postRun: [],

            arguments: [],

            // ...
        },
        /* Let the user-supplied object overwrite all the above */
        ...(typeof init !== "undefined" ? init : {})
    };

    /* Parse URL arguments */
    var args = decodeURIComponent(window.location.search.substr(1)).trim().split('&');
    // ...

    /* We can do this here because at this point `module.status` should be correct */
    module.setStatus("Downloading...");

    return module;
}

/* Default global Module object */
var Module = createMagnumModule();

(Pardon my JavaScript, I hope the spread operator thing is supported well-enough and that I'm using it as it's supposed to be used, heh. Maybe for sanity and compatibility it should be replaced with a bunch of loops iterating over the properties.)

With the above, I think for subsequent <canvas>es all you'd need to do would be something like this:

<script>
var OtherModule = createMagnumModule({
    canvas: document.getElementById('canvas1'),
    status: document.getElementById('status1'),
    statusDescription: document.getElementById('status-description1')
});
createModule(OtherModule);
</script>

Thus no --pre-js (which would basically hardcode the canvas IDs into the binary again) needed, and also this would mean the contents of EmscriptenApplication.js aren't duplicated twice, saving some precious bytes.

Thoughts? :) I hope I'm not too wrong here.

EDIT: am I correct that it should be possible to use the second compiled binary in the markup twice, for two separate canvases? Or is that not how this all works? heh

[mosra]

This is great 👍

[mosra]

The order seems more logical this way, yup, but -- did the original order cause any issue? I don't remember encountering any myself, so I'd like to know what could have happened :)

[pezcode]

No issues but it looked like a race condition to me. async runs the script as soon as it's downloaded; with caching or fast execution of the Emscripten code the constructor could, in theory, be run before the <script>. In general, if you want ordered script execution, you should either use the default or defer (which runs everything at the end, but in order).
https://www.growingwiththeweb.com/2014/02/async-vs-defer-attributes.html

[mosra]

Okay, I have to fix this everywhere then :)

[pezcode]

Thoughts? :) I hope I'm not too wrong here.

I went ahead and implemented that. The spread operator in object literals is experimental so I didn't use that. Object.assign seems like the best option here but I'm not exactly a Javascript expert.

What I also did is copy-pasta'd the exports section from Emscripten's .js output. This allows using EmscriptenApplication.js in some frontend frameworks via import and webpack/rollup. Note that this doesn't work with ES6 (the module system supported by browsers) as there doesn't seem to be a way to do this backward compatible, without requiring ES6 support. You could probably write out an extra EmscriptenApplication.module.js with CMake but I personally don't feel like touching that.

A note on --pre-js: You can bundle EmscriptenApplication.js with it, but the default 'global' Module that's defined by it will overwrite whatever you passed to Emscripten's createModule so in that case you have to use another --pre-js to override canvas elements, etc. Nothing worth putting in the docs, but wanted to point that out for anyone trying this.

EDIT: am I correct that it should be possible to use the second compiled binary in the markup twice, for two separate canvases? Or is that not how this all works? heh

You can reuse the same Emscripten binary multiple times on different canvases no problem, as long as you use MODULARIZE

[mosra]

Squashed the commits a bit and merged as 66c9746...cc23823, thanks a lot!

I rewrote the docs a bit and completely removed the --pre-js mention because I think it's just "extra knowledge" that's not necessarily needed to get this running.