Number formatting

[justinlaughlin]

Addressing #260 by adding the ability to specify number formats for the colorbar and axes.

Keybind for colorbar is Alt+c and for axes is Alt+a. Once pressed, you can specify precision, format (default, fixed, or scientific), and whether the positive sign is always shown or not.

This PR also adds prompt() in aux_vis.hpp which is a more general function for requesting user input. A default value can be specified (I think implementation could be cleaner with std::optional) as well as a validator lambda.

ex9.saved defaults

ex9.saved using (1, 'f', true) for colorbar and (0, 'f', false) for axes.

Setting colorbar number formatting...
Enter precision (4):  1
Enter format [(d)efault, (f)ixed, (s)cientific] (d):  f
Show sign? [(1)true, (0)false] (0):  1
Setting axes number formatting...
Enter precision (4):  0
Enter format [(d)efault, (f)ixed, (s)cientific] (d):  f
Show sign? [(1)true, (0)false] (0):  0

[justinlaughlin]

Suggestion from gm: change the input from the terminal to the glvis window. e.g. by typing 5f1 to set precision=5, format type=float, and showpos=true.

[najlkin]

@justinlaughlin , if you mean what I was saying, it was about streams. That could be done now 💨 . In console, I think it is ok to ask the user interactively, but you may support both 😉 . Like asking the user for the code, but when left empty (there should be a hint 😇 ), it will ask for the options interactively. In the window, it makes sense to me only when we add the popup window/overlay 🤔 .

[najlkin]

besides that, I would prefer C-style formatting flags like in printf. 👍

[v-dobrev]

Suggestion from gm: change the input from the terminal to the glvis window. e.g. by typing 5f1 to set precision=5, format type=float, and showpos=true.

I think the prompts on the terminal are much better than expecting (even experts in GLVis) to remember what keys they need to press (after the initial key, which I assume is already documented in the output from pressing h) and also do it without any visual feedback.

[justinlaughlin]

Thanks for the help explaining some of the glvis internals @v-dobrev! I added the option to change via stream and glvis script. NumberFormatter() also has an additional constructor for c-style strings @najlkin 👍. For inputs via stream/script that is the expected format, but when pressing alt+a / alt+c it will have the same behavior (prompts).

[v-dobrev]

What does Alt mean on Mac? Is it option or command -- it will be good to clarify it here.

[tzanio]

For what's it worth, we do use Alt already in https://github.com/glvis/glvis/?tab=readme-ov-file#advanced

[justinlaughlin]

I'm not able to test but I did see other places that used Alt like Tzanio mentioned. It'd probably be good to mention it somewhere at the top.

[v-dobrev]

I did not test it, but the changes look good to me.

[tzanio]

@justinlaughlin, were you able to try #288 (comment)?

@v-dobrev, can you share your alternative fix?

[justinlaughlin]

I saw some specific handling for Ctrl in lib/sdl.cpp and when I added the same for Alt thing started working for me.

Here is the patch:

@v-dobrev and @justinlaughlin -- can you confirm this works on both Mac and Linux?

If we indeed need these changes, how can we get away without them before?

@tzanio
For me on linux, this causes an additional trigger of a after terminal input is done. Meaning the axes get toggled. e.g.

Event: SDL_KEYDOWN sym=1073742050 mod=4352
Event: SDL_KEYDOWN sym=97 mod=4352
Setting axes number formatting...
Enter precision (4):  4
Enter format [(d)efault, (f)ixed, (s)cientific] (d):  f
Show sign? [(1)true, (0)false] (0):  1
Event: SDL_TEXTINPUT text[0..3]=97 0 0 0 (as codes 0-255)
Toggling axes
Event: SDL_KEYUP sym=97 mod=4352
Event: SDL_KEYUP sym=1073742050 mod=4096

It looks like SDL_TEXTINPUT is what is making the additional call (keyEvent(e.text.text[0]);) but I'm not sure yet how to fix it.

[v-dobrev]

@justinlaughlin, @tzanio, I created a PR (#294) towards this PR with my proposed changes in the key events handling. Take a look and try it.

[justinlaughlin]

@justinlaughlin, @tzanio, I created a PR (#294) towards this PR with my proposed changes in the key events handling. Take a look and try it.

Excellent! I just tried this and it is working.

[tzanio]

@justinlaughlin, @tzanio, I created a PR (#294) towards this PR with my proposed changes in the key events handling. Take a look and try it.

Thanks @v-dobrev, I can confirm #294 works on Mac too!

[tzanio]

@v-dobrev, should we mention the sld.?pp changes in CHANGELOG?

[v-dobrev]

@v-dobrev, should we mention the sld.?pp changes in CHANGELOG?

It is something internal, so I'm not sure it is important to mention it there.

[v-dobrev]

This PR need a new baseline screenshot for one test -- stream_mesh-explorer. This change will be independent of the baseline changes for #285, as prepared in GLVis/data#3. We should:

1. prepare an update for https://github.com/GLVis/data,
2. test it here (update the submodule hash),
3. when the tests are fixed, merge the update for https://github.com/GLVis/data
4. update the submodule hash with the merged hash of https://github.com/GLVis/data
5. merge this PR.

Does this sound ok?

[v-dobrev]

I did 1 and 2 from the above list. Waiting on tests to see if there are any issues.

[tzanio]

Does this sound ok?

Yes

[justinlaughlin]

Tests pass :). Can we merge? It'd probably be simpler to merge this prior to merging #285 since that PR has different baselines.

[tzanio]

Go ahead and merge @justinlaughlin.

[justinlaughlin]

Go ahead and merge @justinlaughlin.

I don't have the power to 😅

[tzanio]

Go ahead and merge @justinlaughlin.

I don't have the power to 😅

try again

[najlkin]

@justinlaughlin , the merge does not pass on master through make style check! 😱 I do not know why PR CI does not have those checks, but this must be fixed asap as it is failing in autotests!