sokol_app.h html5: cleanup canvas lookup handling (see #1154) #1159
Conversation
|
This doesn't seem terribly different from my solution (switch id called "name" to selector) but you need an extra step (to setup the selector in a preRun.) Additionally, does that actually work with your |
It does, but it's only a cache to prevent calling
I think the main difference is that it automatically works with all the I also checked, and the Emscripten functions do not find a canvas that's provided via ...e.g. for clarity, this is the line of code which the ...and this is the deprecated behaviour when (note the special case for (in that case there will also be a warning on the browser console |
I'm not sure what you mean here. My functions works with selectors too.
That's true for mine too. I just used
That is exactly what I was saying. It's no longer automatic, and that is what those options are about. The old behavior was that you need For my solution, you get more options then how it currently works, and a bit more than (but including options provided by) this PR:
In each path on my solution you get the same 2 things ( I think there is a communication breakdown somewhere here, since it seems so clearly better to me to do it my way, and I see no benefit to doing it this way. Like, it's not simpler, more elegant, and it gives users less options, and is a bit more cumbersome. There is no difference with performance or memory-usage, or anything else, and It also does not work like other libs people might be used to (SDL, glfw, raylib, etc.) Fundamentally, it's your lib, and you can do whatever you like with that. It's not really usable to me, as it is, passing the id from C-side, in my current project, and I don't see the point of forcing users to set up that extra layer with |
I mean these: Lines 5834 to 5850 in 8c989e3 ...and these: Lines 5941 to 5951 in 8c989e3 ...those functions take a selector string and then call Emscripten's |
|
PS: there's also this detail in the WebGPU C API: ...e.g. the WebGPU shim is also expecting a CSS selector string to find the canvas object and this also uses the |
|
...e.g. in the end it's all about making Emscripten's |
|
This is an interesting part in your PR: if (Module.canvas) {
specialHTMLTargets["!canvas"] = Module.canvas;
stringToUTF8("!canvas", canvas_selector_cstr, 8);
return;
}E.g. don't expect if (Module['canvas']) {
const canvas_selector_str = UTF8ToString(canvas_selector_cstr);
specialHTMLTargets[canvas_selector_str] = Module['canvas'];
}I could live with that too. I'm using |
|
Ok see: (the only detail I don't quite like about that is the use of the deprecated |
Did you try it? As I said, my code sets the string selector for the emscripten functions, so in C, it works the same, and in js you can use
Again, so do I.
Are you sure it does that?
This does not do the same thing. If you look at the other code, the legacy stuff that uses id, etc, that runs before this, it overrides it with other strings. I think a few
As I said, I think you are misreading the deprecated behavior. It's not "Don't name your copy of the canvas I think the key misunderstanding about my solution is here. If you look at my code, it does this (in if (Module.canvas) {
specialHTMLTargets["!canvas"] = Module.canvas;
stringToUTF8("!canvas", canvas_selector_cstr, 8);
return;
}
// lookup and store canvas object by name
const canvas_selector = UTF8ToString(canvas_selector_cstr);
Module.canvas = document.querySelector(canvas_selector);My code supports several user-config paths:
You definitely need to set Personally, I don't like any css-selectors/id being set in C, at all. I left support for it, so you get id (like previous) and query-selector (new thing, but simple enough.) If I were using SDL/raylib/etc, and needed this, it's pretty easy without anything in the C code, as long as const instance = await setupEmscripten({
canvas: document.querySelector('main > canvas') // or whatever you want here
})To me, it feels like a deployment/implementation detail that should be set outside of sokol/C. Like when you target an opengl context on native, you don't set the video-driver that is used, or force users to have a specific kind of monitor or whatever. You just say "hey OS, give me a window that is XxY size". I think of emscritpen as just another target platform, where it should mostly do the right stuff, and let users override that, outside of the C code, if they want to. I kept support for it because I want all the old code to still work, but I disagree with that sort of usage, in the first place. I think Emscripten themselves came to the same conclusion, which is why that old behavior of automatically attaching to Again, personally, I would say just use 1 selector all the way through for |
|
Is there a way to chat on discord or something? I am I can let it rest, if we step through the code, and I feel like you understand what I was doing, and you are still like "I still don't like your way." I have a strong feeling that once you understand, you will agree with me, but I'm cool either way. In the hypothetical situation that I decide to use sokol in my thing, and you did not agree with me, I could again, just fork the 1 header & change a few lines of code, or do it the more roundabout way you are proposing (once your PR is in.) It's not a huge deal, either way. |
No, tbh I'll just merge my PR and be done with it, we already wasted too much time on a trivial feature :) One thing I'll need to fix is checking that I'm quite convinced that |
|
Oki, dropping the sokol_app.h from this PR into your multicanvas example (https://github.com/konsumer/sokol-canvas-example-1154) works without changes. I'll just keep it at that. |
Agreed. Originally, I was trying to help, but I feel like the finer points of the differences are probably not important, and this much talk about a trivial thing feels like bike-shedding.
Yeh, I think historically it could be a string, DOM-element or a function (and it would run it, if it was a function, or use it as an id, if it was a string.) As a self-running function, it could output string/DOM-element, or even return a function. I had thought you did not use
I still disagree that offering
That's great. I updated the demo. I also checked the canvases that get set to
I notice that the selector stays
|
Proposal for #1154:
sapp_desc.html5_canvas_nameto.html5_canvas_selectorfindCanvasEventTarget()to lookup the JS canvas name by canvas selector (which also considersModule['canvas']andspecialHMTLTargets[])TODO:
<canvas id="bla">=>.html5_canvas_selector="#bla"test with setting canvas in index.html viathis doesn't actually work any longerModule['canvas']specialHTMLTargets[]in index.html (see below)To register canvases that can't be looked up via
document.querySelector(), add them to thespecialHTMLTargets[]array after the Emscripten environment is setup but before the WASM is started. For instance viaModule.preRun()in index.html:...then in sokol_main() use
.html5_canvas_selector = "my_canvas"