f3d-web updates: parse other ui params, loading visual feedback, alert user on error, documentation

[jo-chemla]

Solves improvements of web url-param parsing listed in #1604 :

• parse more url-params: axis, grid, fxaa, tone, ssao, ambient, up
• Visual feedback for download via progress bar from bulma
• Alert user when provided model url yields error
• Document in web readme

[Meakk]

Awesome! I'll try that locally as soon as I can.

[Meakk]

Here's my feedback:
It seems to work fine when loading the URL with all options set 👍, however when options are edited several times in the URL input, it seems broken. Maybe a render() is missing? Or the browser cache is causing issues?
Also, I cannot see the progress bar.

[snoyer]

As you already figured out, the JS bindings are not complete enough yet to achieve what you're attempting here.

f3d/webassembly/F3DEmscriptenBindings.cxx

Lines 80 to 84 in f3d7627

	emscripten::class_<f3d::options>("Options")
	.function("toggle", &toggle, emscripten::return_value_policy::reference())
	.function("set_string", &set_string, emscripten::return_value_policy::reference())
	.function("set_integer", &set_integer, emscripten::return_value_policy::reference())
	.function("set_color", &set_color, emscripten::return_value_policy::reference());

You could use the value of the UI switches to guess the current values and decide whether to toggle but that'd be sketchy as there is no proper guarantee that they'll be in sync.

At the very least set_boolean would need to be added, but ideally the bindings should expose the newer setAsString method that would let you do options.set_as_string(option, parsed_param_value) directly (passing a "false"/"true" string and having the parsing done internally, as opposed to converting from string to bool in the JS and calling .set_boolean).

[jo-chemla]

The current implementation takes for granted that:

• every DOM input is checked when page loads, see this code block
• the corresponding f3d settings.options are toggled on, see this code block
• so I can only watch for when these axis, grid, fxaa, tone, ssao, ambient url-params are set to false, in which case I toggle them a second time before calling a final Module.engineInstance.getWindow().render(); just before returnging from method load_from_url()

The current implementation does work under these assumptions from my testing.
I think this PR can be merged, and the code could then be updated to account for a new set_boolean function + binding, or even higher-level set_as_string binding, once either is available.

[snoyer]

Unless I'm missing something the PR's code currently only handles toggling options off (guarded by if ( ... === 'false')) so if, for example, the grid is off and I change the hash to #grid=true it will not even attempt to turn it on.

In order to handle both #x=true and #x=false parameters values you'd need to do something like this:

            if (parsed_param_value) {
              const new_val = parsed_param_value.toLowerCase() === 'true'; //TODO actual input validation?
              const ui_switch = document.querySelector('#' + id); //TODO check the element actually exists?
              const old_val = ui_switch.checked; // let's hope this is in sync
              if(new_val != old_val) {
                options.toggle(option)
              }
              ui_switch.checked = new_val;
            }

This is still a janky workaround the limitations of the API but at least it will handle toggling things back on

[jo-chemla]

Alright, this is correct, I don't usually change hash after page was loaded but you're right. I'll include your modifications.

[jo-chemla]

Thanks for the feedback!

• Progress-bar only appears when you load a heavy model via the File Loader, on my side a 50MB local file will show progress bar for 2s. Will need to check if one can add the same progress bar for file loaded from url.
• on my side options do work even with no model url passed. Indeed sometimes I have to hard-refresh the page with SHIFT + CTRL + R for options to be taken into account - I assumed this was because we decided to pass the options via hash rather than search query.

[snoyer]

Indeed sometimes I have to hard-refresh the page with SHIFT + CTRL + R for options to be taken into account - I assumed this was because we decided to pass the options via hash rather than search query.

Changing the hash in the browser URL box indeed should not produce a full page reload, but a reload shouldn't be needed because the script is already listening for the hash change event (addEventListener("hashchange", (event) => {load_from_url()});). You may want to add a console.log inside load_from_url() to investigate whether it is triggered correctly or if something is missing with the hash handling.

edit: I guess you'd need a reload if you want to re-apply the options from the same hash without it having changed?

[jo-chemla]

Just pushed a quick PR to add a fetchWithProgress function so progress-bar can be displayed both for FileReader and model url parsed.
Regarding changing hash in the browser url bar, I did retry logging bits and it always listens to hash changes during my testing. Indeed, when hash does not change is when the reload needs to be triggered.

[jo-chemla]

Pushed 2 more commits addressing the suggested modifications, should be good to go!

[mwestphal]

FYI the wasm bindings have been improved to support set_as_string/get_as_string

[jo-chemla]

Alright just committed a minor update, hich should be mergeable - don't hesitate to edit the PR in case you'd want a better approach.

[jo-chemla]

Ready to be reviewed by f3d team from my point of view