libf3d redesign suggestions

[snoyer]

There is currently only minimal differences between libf3d and F3D in terms of functionality and scope. While this is not a problem in itself it would be nice to have a more library-like, ie. more general-purpose, libf3d to cover more use-cases than just F3D's.

This could be done by mostly moving logic away form the library and into the application and doing some refactoring and API design along the way.

As requested through other channels, here are some [possibly rambling] points about what I think could be helpful improvements to either/both improve the lib and untangle some logic which could help solve some pending issues with the app...

File Loading

Currently the loader handles both loading models form files and managing a list of filenames to navigate between.

The library should only handle the actual loading of the models. Applications based on libf3d, starting with F3D, can handle how to go between multiple files themselves.

The loading API could be reduced to:

• loadModel(filename, options) clear scene, load a model from disk
• loadModel(buffer, options) clear scene, load a model from memory

If technically possible, expanding it to allow loading of multiple models at once would be even better:

• model_id = loadModel(filename, options) load a model from disk, returns an id
• model_id = loadModel(buffer, options) load a model from memory, returns an id
• unloadModel(model_id) unload a loaded model by id
• setModelVisibility(model_id, bool) toggle visibility of a loaded model by id (without unloading it)

Interactions

The library should provide meaningful actions to interact with the current scene independently of any UI bindings.

The actions could be identified/registered by unique key strings (based on the options naming scheme for consistency?) to facilitate later UI bindings.

Examples:

• do_action("render.effect.ssao.toggle") toggle ambient occlusion
• do_action("render.effect.ssao.on") activate ambient occlusion
• do_action("render.effect.ssao.off") deactivate ambient occlusion
• do_action("render.effect.ssao.reset") reset ambient occlusion to default
• do_action("camera.fit") fit camera to scene

Ideally we should expose an action type/object for apps to register their own (eg: std::function<void(interactor_impl::internals*)> would be in line with the current interactor impl), bonus points if plugins can register new actions (no idea if/how that is possible tho).

Calling actions by simple strings with no parameters would make it relatively easy to have configurable keyboard bindings from a configuration file on the app side, eg:

{
  "Q": "render.effect.ssao.toggle",
  "ctrl+Q": "render.effect.ssao.reset",
  "Space": "camera.fit",
}

and wherever the keyboard events are received:

std::stringstream ss;
if (rwi->GetControlKey()) ss << "ctrl+";
if (rwi->GetAltKey()) s << "alt+";
if (rwi->GetShiftKey()) ss << "shift+";
ss << keySym;
const std::string keyCombo = ss.str();

f3d_instance.do_action(keybindings[keyCombo]);

The lib could provide a simple VTK-based events implementation, apps could choose to replace it with their own if needed.

Cheatsheet

Not fully clear on this one but if we had a keybinding -> action_id mapping we should be able to reverse-lookup which key triggers which action and display the cheatsheet accordingly.

It would probably be hard to have a fully dynamically built cheatsheet but having the currently hardcoded one with just the bindings looked-up from the config should be a good start.

Camera positioning

Reworking the model loading logic would ideally cut out any camera manipulation from it.

The library should provide appropriate camera controls that would be called by the application after a file is loaded if appropriate.

Options

Options as a string -> whatever map is already nice and simple. A few minor improvements could be:

• maybe separating dynamic and load-time options?
• sorting out optional vs default-valued options
• having a schema of some sort if we need to be extra fancy?

Discussion

I definitely do not have the authority or knowledge to claim that any of the above is feasible or even objectively desirable.

Please augment or correct (or just plain trash and dismiss if warranted) any of the points or ask for clarification on anything that doesn't make sense.

[mwestphal]

Lots of good ideas here, I will have to think about it :)

[Meakk]

I'm also feeling there is no clear distinction between libf3d and F3D scope and I agree it needs to be clarified.
I like most of your ideas and it's a solid base for discussion.
We also have to think about bindings, I don't want Python users to rewrite an entire application, so if UI logic are moved to the application side, we'll have to provide write some simple out-of-the-box logic for bindings too.

[mwestphal]

Hi @snoyer

A few questions about your suggestions.

loadModel(buffer, options) clear scene, load a model from memory

What kind of buffer could that be ? A binary buffer of the file from disk ? Just trying to follow what you have in mind.

setModelVisibility(model_id, bool) toggle visibility of a loaded model by id (without unloading it)

So you would like to be able to show multiple models at the same time, is that correct ?

do_action

Interesting API. However it sometimes is much more complex than that and I'm afraid it will be much more complex.
Indeed, it would be a interesting way to be able to specify interaction from the Application using a config file.

Camera positioning

This one is already covered I think, unless I'm missing something obvious.

maybe separating dynamic and load-time options?

We tried that initially but it proved to be challenging, also we want to put more options in the dynamic section as time progress.

sorting out optional vs default-valued options

Not sure what you mean by that. All values have default.

having a schema of some sort if we need to be extra fancy?

There is already a sort of schema ? Unless I'm missing something.

[snoyer]

Hi @mwestphal , happy new year!

loadModel(buffer, options) clear scene, load a model from memory

What kind of buffer could that be ? A binary buffer of the file from disk ? Just trying to follow what you have in mind.

Yes, same bytes as on disk but from memory. May need an extra parameter to pass the extension or mimetype if needed to pick the appropriate loader.

Use-case would be using libf3d from a program that generates/modify models before viewing them, currently you'd need to write to a named temporary file on disk for the lib to read from. Loading directly from more efficient model structures would be out of scope for f3d but skipping the disk round-trip to read file-like data seems reasonable.

setModelVisibility(model_id, bool) toggle visibility of a loaded model by id (without unloading it)

So you would like to be able to show multiple models at the same time, is that correct ?

Correct, for example you could have an object that is 3d printed as multiple parts and you'd want to render all the .stl files together to show the finished product or check that everything fits.

do_action

Interesting API. However it sometimes is much more complex than that and I'm afraid it will be much more complex. Indeed, it would be a interesting way to be able to specify interaction from the Application using a config file.

For what it's worth my reasoning was that the big keyboard interaction switch is awkward and ugly and could probably do with having every case refactored as it's own function, and one you've gone from:

void handle_kb(app){
    switch(key):
        case key1: ... app.foo() ...
        case key2: ... app.bar() ...
        case key3: ... app.whatever() ...
        ...
}

...to:

void handle_kb(app){
    switch(key):
        case key1: action_a(app)
        case key2: action_b(app)
        case key3: action_c(app)
        ...
}

...you're only a couple of indirections away from:

map<name, void(app)> actions = {"a": action_a, "b": action_b, "c": action_c, ...};
map<key, name> kb_mapping = {key1: "a", key2: "b",  key3: "c", ...};
void handle_kb(app){
    actions[kb_mapping[key]](app)
}

...which would let you bind the actions in a bunch of other ways (--action=... CLI flag (see Inkscape), command palette (see vscode, sublime text), overwrite the bindings map from user config, ...) basically for free.

Now I do know enough not to expect it to be that simple, but I don't know enough see the critical flaw, so I'll leave it with you.

Camera positioning

This one is already covered I think, unless I'm missing something obvious.

The feature that was raised earlier was being able to reload without resetting the camera. That could be done now by adding some code to save the camera state to be able to restore to undo whatever camera positioning is done in the loading code. If the loading code is going to be refactored anyway it would be a cleaner solution to just not touch the camera in there but rather do it afterwards (or not, accordingly).

maybe separating dynamic and load-time options?

We tried that initially but it proved to be challenging, also we want to put more options in the dynamic section as time progress.

Fair enough. For reference, the vague idea I had with that was that only a subset of the option defines the dynamically set "state" of the rendering and it could be useful to know what it is. The ? action could dump that state in a config or command line format instead of the current human-readable one. Use-case: play with the options interactively in a window on a sample model, dump current options, reload options later on different model or to render offscreen.

sorting out optional vs default-valued options

Not sure what you mean by that. All values have default.

It's a bit of a technical nitpick but values that are computed if the user does not provide them (rather than known in advance) should not have default values.
For example, the camera position should be optional and not have a default value because the "default value" needs to be computed dynamically down the line. Currently (maybe due to a limitation of the parser) this option has a sketchy (empty vector? 0? can't remember) default value instead.
Having a quick look at the --help text, --max-size should also be optional and not have a default value, the text actually says -1 means unlimited (default: -1) so I guess you guys are already aware.

having a schema of some sort if we need to be extra fancy?

There is already a sort of schema ? Unless I'm missing something.

I was thinking of something machine readable that could be used to generate command-line parsers or some sort of UI but that's as far as that idea goes so no need to take it too seriously.

I might have been in rambling mode by the end of the list :)

[mwestphal]

Thanks a lot for the details, I will think about it and try to create dedicated issues and see where we con go from there.

[mwestphal]

loadModel(buffer, options) clear scene, load a model from memory

What kind of buffer could that be ? A binary buffer of the file from disk ? Just trying to follow what you have in mind.

Yes, same bytes as on disk but from memory. May need an extra parameter to pass the extension or mimetype if needed to pick the appropriate loader.

This will definitely be adressed in the future. There is an issue tracking that here: #534

So you would like to be able to show multiple models at the same time, is that correct ?

Correct, for example you could have an object that is 3d printed as multiple parts and you'd want to render all the .stl files together to show the finished product or check that everything fits.

There is an issue tracking that: #127

I hope we can do it but it is not as trivial as it may look and this mode of operation can not be considered the "default" behavior. See my last comment on the issue for a possible approach.

FileLoading API to load multiple models

See above. Loading a model means loading a whole scene unless in specific cases, so it can't be the default. That being said it could be considered that the multi-files part should be application side. I've opened an issue about it:

#597

do_action

Thinking about it, I like it. This is something we have in mind for a long time as we plan to provide a vim-like command where one could do this:

:render.effect.ssao on

I've created an issue to track this: #595

Camera positioning

This one is already covered I think, unless I'm missing something obvious.

The feature that was raised earlier was being able to reload without resetting the camera. That could be done now by
adding some code to save the camera state to be able to restore to undo whatever camera positioning is done in the
loading code. If the loading code is going to be refactored anyway it would be a cleaner solution to just not touch the
camera in there but rather do it afterwards (or not, accordingly).

"Reloading" a file will always means recreating everything IMO, even if we give a way to restore a state.

The ? action could dump that state in a config or command line format instead of the current human-readable one. Use-case: play with the options interactively in a window on a sample model, dump current options, reload options later on different model or to render offscreen.

That is a good idea !

Added here: #596

It's a bit of a technical nitpick but values that are computed if the user does not provide them (rather than known in advance) should not have default values.

Got it. This is indeed a limitation of cxxopts which ended up being mirrored in the libf3d. I think this requires moving away from cxxopts, see this issue: #434

I was thinking of something machine readable that could be used to generate command-line parsers or some sort of UI but that's as far as that idea goes so no need to take it too seriously.

Got it, seems related to #595 then :)

Not fully clear on this one but if we had a keybinding -> action_id mapping we should be able to reverse-lookup which key triggers which action and display the cheatsheet accordingly.
It would probably be hard to have a fully dynamically built cheatsheet but having the currently hardcoded one with just the bindings looked-up from the config should be a good start.

Got it, but I suppose #595 is needed to move forward with this.

Let's try to discuss implementation specific in the dedicated issues and identify what is critical so that it can be tackled as soon as possible, for the 2.0 release.

[mwestphal]

I think I've separated all your suggestions into their own issues, so I will go ahead and close this, but feel free to comment on general topic / other ideas here @snoyer :)