Number formatting: fix key events handling

[v-dobrev]

Proposed update for #288.

[justinlaughlin]

Does this fix the issue with alt in https://github.com/GLVis/glvis/tree/number-formatting for mac? Thank you!

[v-dobrev]

Does this fix the issue with alt in https://github.com/GLVis/glvis/tree/number-formatting for mac? Thank you!

For me, it does. Let's see if @tzanio can confirm it too.

[justinlaughlin]

Looks like ex11 is failing on macos.

successful test	failed test
link	link

[tzanio]

Maybe mention in CHANGELOG?

[tzanio]

I can reproduce the failure https://github.com/GLVis/glvis/pull/294#issuecomment-2243844150 with:

./glvis -saved tests/data/streams/mesh-explorer.saved

[tzanio]

The above test works for me with the following patch

diff --git a/streams/mesh-explorer.saved b/streams/mesh-explorer.saved
index ff48b09..51d8ef8 100644
--- a/streams/mesh-explorer.saved
+++ b/streams/mesh-explorer.saved
@@ -1,4 +1,4 @@
-fem3d_gf_data_keys
+solution
 MFEM mesh v1.0

 #
@@ -33141,4 +33141,5 @@ Ordering: 0
 445
 446
 447
+keys
 eppttti*************997777777777777777777777777777777777777777777777777777777777777777766666666666~3~3AoooEe

I am not sure why this was fem3d_gf_data_keys instead of keys, @v-dobrev do you remember?

[justinlaughlin]

The above test works for me with the following patch

This also works for me on linux. I get the same image with/without this branch. But the image is different from the saved file so it would fail the test.

[v-dobrev]

I am not sure why this was fem3d_gf_data_keys instead of keys, @v-dobrev do you remember?

The "input type" fem3d_gf_data_keys predates the stream commands, I think. If fem3d_gf_data_keys does not work, it must have been broken in a recent PR.

[justinlaughlin]

Actually, I tried running the keys manually and I think the image Tzanio has is what the baseline image should be. e.g. the baseline image has no rotation but all the 7s should rotate it quite a bit. I think we should update the test + the baseline image.

e.g. if you apply Tzanio's patch + remove all the rotations (99777777777766666), the image looks very much like the baseline...

diff --git a/streams/mesh-explorer.saved b/streams/mesh-explorer.saved
index ff48b09..7188ef0 100644
--- a/streams/mesh-explorer.saved
+++ b/streams/mesh-explorer.saved
@@ -1,4 +1,4 @@
-fem3d_gf_data_keys
+solution
 MFEM mesh v1.0

 #
@@ -33141,4 +33141,5 @@ Ordering: 0
 445
 446
 447
-eppttti*************997777777777777777777777777777777777777777777777777777777777777777766666666666~3~3AoooEe
+keys
+eppttti*************~3~3AoooEe

[v-dobrev]

I think the issue may be in how SdlWindow::signalKeyDown sends TextInput events -- it does not send a KeyDown event first and that is now expected in the text-input handling. I'll come up with a solution shortly.

[v-dobrev]

@justinlaughlin, I think you are right, the digit keys are ignored in the baseline for mesh-explorer.saved. With the last commit I get the picture that @tzanio posted with the modified mesh-explorer.saved.

[tzanio]

modified on unmodified?

With the last commit I get the picture that @tzanio posted with the modified mesh-explorer.saved.

You mean unmodified here, correct?

[tzanio]

I vote to merge this and update the baseline. We will likely need to change it due to https://github.com/GLVis/glvis/pull/285 anyway

[v-dobrev]

modified on unmodified?

With the last commit I get the picture that @tzanio posted with the modified mesh-explorer.saved.

You mean unmodified here, correct?

Yes, with the unmodified mesh-explorer.saved, I get the picture you got with the modified mesh-explorer.saved. Basically fixed the bug when fem3d_gf_data_keys is used.

[v-dobrev]

Was the baseline (the hash for the submodule) modified in #288?

[justinlaughlin]

Was the baseline (the hash for the submodule) modified in #288?

It should be the latest (https://github.com/GLVis/glvis/pull/288/commits/b8479b0e8dd8349ac35abc92b714bdc1f5b5d199)

[v-dobrev]

Was the baseline (the hash for the submodule) modified in #288?

It should be the latest (b8479b0)

Okay, yeah, it looks like it was not.

I agree with @tzanio -- we can merge this in #288 and go from there.