Strange behavior with InputXXX widgets editing stack variables

[BobbyAnguelov]

Version/Branch of Dear ImGui:

Version: 1.85
Branch: docking

My Issue/Question:

I noticed some strange behavior with imgui when editing stack vars.

In my case, I have a stack var that is set to a copy of a specific variable each frame, I then edit that stack copy with a basic imgui widget. I wish to only change the original value if the widget is deactivated, so I use the IsItemDeactivatedAfterEdit function to detect that and them update the original value.

This doesnt work reliably, if I switch focus to a widget that was declared/submitted BEFORE the active widget, I get the signal the value changed but it actually didn't change the stack var. Now if I change focus to a widget declared after the currently active one everything works as expected. This doesnt seem correct...

I am not hitting return/enter at all, just trying to commit the value via the focus switch/deactivation of the widget.

Screenshots/Video

bandicam.2021-11-11.16-46-56-267.mp4

(sorry about the black frames, video capture on W11 has some issues it seems)

Standalone, minimal, complete and verifiable example:

            ImVec4 v = temp;
            ImGui::Begin( "Example" );

            ImGui::InputFloat( "##f4x", &v.x, 0, 0, "%.3f", 0 );
            if ( ImGui::IsItemDeactivatedAfterEdit() )
            {
                temp = v;
            }

            ImGui::InputFloat( "##f4y", &v.y, 0, 0, "%.3f", 0 );
            if ( ImGui::IsItemDeactivatedAfterEdit() )
            {
                temp = v;
            }

            ImGui::InputFloat( "##f4z", &v.z, 0, 0, "%.3f", 0 );
            if ( ImGui::IsItemDeactivatedAfterEdit() )
            {
                temp = v;
            }

            ImGui::InputFloat( "##f4w", &v.w, 0, 0, "%.3f", 0 );
            if ( ImGui::IsItemDeactivatedAfterEdit() )
            {
                temp = v;
            }
            ImGui::End();

[PathogenDavid]

With Dear ImGui you're responsible for maintaining the backing store for values being edited. In your code you're constantly overwriting v with temp.

You need to be more picky about when you set v to temp:

If temp is never written externally, the easiest solution is to make v a static variable (or something else that persists between calls.)

If temp can change externally you need to be careful about when you copy it to v.

---

The reason it seems to work at all is InputText (which is used to implement InputFloat) internally creates a private copy of the string buffer when the text is being edited.

^{(The reason for why it seems to work sometimes and not others is more complicated. I started to explain it but it was turning into a wall of text. The short version is IsItemDeactivatedAfterEdit always has a frame of latency and updating the value inside InputText has 0 or 1 frames of latency depending on the situation. When it ends up being 0 the value of v gets overwritten before you apply it back to temp.)}

[BobbyAnguelov]

So I guess I have to duplicate every single edited value. My use case is a property grid. Where I need to only change the actual value once the editing operation is complete or else my undo/redo stack explodes.

Just so I understand, I basically need to build a retained mode wrapper on top of imgui to solve this issue? Cache a value per property edited, have the imgui widgets modify that, then reflect that back to actual edited type?

[PathogenDavid]

Yeah that sounds like a solid plan.

Another alternative (if the undo/redo stack issue is your main concern) is to have your undo/redo stack only actually commit on IsItemDeactivatedAfterEdit or something like that.

[rokups]

This looks like a bug with InputText(). This behavior stems from the fact that all instances of InputText() use a single data structure to store data while they are active. So, for example when we have InputText(&y) active, but changes are not yet committed and we activate InputText(&x), this newly activated widget discards state of InputText(&y) and overwrites with it's own. When time comes to render InputText(&y) it sees other widget being active and simply continues rendering text formatted from parameters passed to the widget instead of it's internal state that is no more.

Solving it is not exactly straightforward. If we wanted to patch code as it is now, we would have to introduce a 1 frame lag, because when previous widget gets activated it should realize that another InputText() was active and it did not commit changes and then wait for that to happen.

Not all is lost though. There is a rework of InputText() going and we have been considering introduction of multiple internal states of InputText(). Not only it would immediately fix this particular issue, but it would also give us nice things like displaying cursor/selections/scrollbars in deactivated InputText().

[ocornut]

Hello,

Just so I understand, I basically need to build a retained mode wrapper on top of imgui to solve this issue?

I heard you say variation of that half a dozen time already in other places, it might generally be more constructive to think in term of fixing bugs or implement features on library side, given that the library purpose is to minimize work for the user. It might also be misleading to use blanket term "retained mode wrapper" as it can mean so many things. e.g. There are many situation where you only need storage for 1 element (e.g. there can be only one active item), which tend to be very simple to implement.

There's mixture of things here.

(1) We definitively want the equivalent of _NoLiveEdit flag for inputs (whatever the final name is). It may generally a good default for numerical value (#701). The friction against implementing the general purpose version (which include InputText()) is probably going to be lower if we do it for scalar, might work before we merge the upcoming InputText() refactor.

The short version is IsItemDeactivatedAfterEdit() always has a frame of latency

I don't think there is, it can be confirmed with a test like:

static ImVec4 w;
static int activated_frame = -1;
static int activated_field = 0;
static int deactivated_frame = -1;
static int deactivated_field = 0;

ImGui::InputFloat("w.x", &w.x, 0, 0, "%.3f", 0);
if (ImGui::IsItemActivated()) { activated_frame = ImGui::GetFrameCount(); activated_field = 0; }
if (ImGui::IsItemDeactivatedAfterEdit()) { deactivated_frame = ImGui::GetFrameCount(); deactivated_field = 0; }

ImGui::InputFloat("w.y", &w.y, 0, 0, "%.3f", 0);
if (ImGui::IsItemActivated()) { activated_frame = ImGui::GetFrameCount(); activated_field = 1; }
if (ImGui::IsItemDeactivatedAfterEdit()) { deactivated_frame = ImGui::GetFrameCount(); deactivated_field = 1; }

ImGui::InputFloat("w.z", &w.z, 0, 0, "%.3f", 0);
if (ImGui::IsItemActivated()) { activated_frame = ImGui::GetFrameCount(); activated_field = 2; }
if (ImGui::IsItemDeactivatedAfterEdit()) { deactivated_frame = ImGui::GetFrameCount(); deactivated_field = 2; }

ImGui::InputFloat("w.w", &w.w, 0, 0, "%.3f", 0);
if (ImGui::IsItemActivated()) { activated_frame = ImGui::GetFrameCount(); activated_field = 3; }
if (ImGui::IsItemDeactivatedAfterEdit()) { deactivated_frame = ImGui::GetFrameCount(); deactivated_field = 3; }

ImGui::Text("w %.3f %.3f %.3f %.3f", w.x, w.y, w.z, w.w);
ImGui::Text("Activated frame %d, field %d", activated_frame, activated_field);
ImGui::Text("Deactivated frame %d, field %d", deactivated_frame, deactivated_field);

The problem is indeed that the behavior of that backing copy of the data in InputText() described in @PathogenDavid is a quite ill-defined. People have been relying on it for some idioms but it has issues and here you are stumbling on one.
This is sort of an abuse of undefined behavior here, but in the absence of a _NoLiveEdit flag its very understandable that people are trying to do it and I myself often suggested code that made use of that behavior (in fact we have some regression tests designed to validate some aspects of that behavior).

I get the signal the value changed but it actually didn't change the stack var.

Here IsItemDeactivatedAfterEdit() is reported on the right frame, but the associated widget already lost its backing data and doesn't write to the stack variable. It's not technically incorrect by the definition of IsItemDeactivatedAfterEdit() as the edit could have happened many frames/seconds before, but combined with the aforementioned reliance on that backing copy we have an issue. The inconsistency depending on whether an item before or after gets focused makes a definitive bug.

I think we should fix this on both side:

• Aim toward honoring a last write matching the Deactivated frame. That would technically also allow an hypothetical "RevertOnUnfocus" flag which would absolutely requires that last write.
• Aim toward having a _NoLiveEdit flag (InputText, InputInt, InputFloat: add ImGuiInputTextFlags_NoLiveEdit flag #701)

Either would solve the problem but I think both are ultimately desirable.

Another alternative (if the undo/redo stack issue is your main concern) is to have your undo/redo stack only actually commit on IsItemDeactivatedAfterEdit or something like that.

On my last game the undo stack was stored in term of only storing the new value.
So instead of storing

• "Event: Change X from 0.0f to 42.0f"
• "Event: Change X from 42.0f to 100.0f"

It would store:

• "Initial State of X is 0.0f"
• "Event: Change X to 42.0f"
• "Event: Change X to 100.0f"

Which isn't as nice** but I guess that's one of the reason I side-stepped the _NoLiveEdit thing for a while. My bad.
(** for various reasons: it requires either a cache of states (which doesn't scale) or a seek during undo (which does scale given it happens on an interactive event, but it is a little more fragile to side-effects or other code altering that same field)

[ocornut]

Solving it is not exactly straightforward. If we wanted to patch code as it is now, we would have to introduce a 1 frame lag, because when previous widget gets activated it should realize that another InputText() was active and it did not commit changes and then wait for that to happen.

We would absolutely avoid a frame of lag here primarily because it would likely have side-effect on other logic.

Not all is lost though. There is a rework of InputText() going and we have been considering introduction of multiple internal states of InputText(). Not only it would immediately fix this particular issue, but it would also give us nice things like displaying cursor/selections/scrollbars in deactivated InputText().

We can possibly implement this even before the larger merge (I guess I am going to try and if it's too messy I'll postpone to when we merge the InputText change).

[ocornut]

• Aim toward honoring a last write matching the Deactivated frame.
• Aim toward having a _NoLiveEdit flag (
  Either would solve the problem but I think both are ultimately desirable.

Brain fart here: the _NoLiveEdit would facilitate use without a stack/temp variable but would rely on us performing a last write on the Deactivated frame. So first item is 100% required first.

[BobbyAnguelov]

I heard you say variation of that half a dozen time already in other places, it might generally be more constructive to think in term of fixing bugs or implement features on library side, given that the library purpose is to minimize work for the user. It might also be misleading to use blanket term "retained mode wrapper" as it can mean so many things. e.g. There are many situation where you only need storage for 1 element (e.g. there can be only one active item), which tend to be very simple to implement.

My bad, I assumed from the first reply that the issue was on my side. So I started to work around it by providing a fixed value for imgui to work on and copying it back to the actual type when I received the signal the edit operation was complete. Ideally if I didn't have to do any of that it would be amazing and remove a ton of code on my end (I needed it working fast, so I had no choice but to work around it). Ideally if this can be resolved on the library side this is obviously way better. I think if I understand correctly what the _noliveedit flag would do, then it would solve 99% of my issues.

[ocornut]

Apologies for lack of reaction on this, I intended to get the fix for 1.86 but didn't, I hope it'll come soon.

I basically have a 99% fix for this nearly ready but there are subtle edge cases that are problematic (e.g. programmatic change of focus which would be equivalent of clicking on an earlier widget, making the previously active widget clipped due to instant scrolling, making it unable to "commit" its data). The work on clipper changes for 1.86 should allow that stuff to be fixed, marking specific ID or zone as non-clippable for e.g an extra frame.