Unexpected behavior of ImGui::IsItemDeactivatedXXX with multi-components widgets

[eliasdaler]

Example code:

ImGui::InputFloat2("Test", test);

if (ImGui::IsItemActivated()) {
    std::cout << "A\n";
}

if (ImGui::IsItemDeactivatedAfterEdit()) {
    std::cout << "B\n";
};

When I start editing one of the components and then switch to another (via "Tab" or by clicking), I get the following output:

A
A
B

Is this expected? I would expect that "Item" in IsItemActivated would refer to the InputFloat2 widget as a whole, not just individual text boxes.

This is important for undo functionality in InputFloat2 and similar widgets. Imagine having InputFloat2 representing position. When IsItemActivated is "true", we save position before editing. When IsItemDeactivatedAfterEdit returns true, we add "undo" command to undo stack. The current behaviour makes this impossible.

I'll take a look at the problem myself, but would be great to hear your thoughts about this. Should the output become just:

A
B

? This would be what I'd expect from this widget. Another possibility:

A
A
B
B

(not perfect, but better than what we have now)

[eliasdaler]

It's easy to understand why IsItemActivated returns true twice - under the hood two widgets in InputFloat2 are almost unrelated (except for Begin/EndGroup, I guess?), so changing focus from one input to another should trigger.

But then, one might expect IsItemDeactivatedAfterEdit returning true twice as well, but it only returns it once, because in the frame when switching from one component to another, we get the following:

IsItemDeactivatedAfterEdit is false, because IsItemDeactivated is false, because g.ActiveIdPreviousFrame != window->DC.LastItemId (one of the checks in IsItemDeactivated)

---

So yeah, quite a conflicting situation here... Looks like "Item" in both cases refers to the last component of multi-input widget?

[eliasdaler]

Also maybe these variables might help when looking at logic.

When switching from first component to second:

(gdb) p window->DC.LastItemId
$12 = 1973097345
(gdb) p g.ActiveIdPreviousFrame
$13 = 3107001119
(gdb) p g.ActiveId
$14 = 1973097345

When switching from second to first:

(gdb) p window->DC.LastItemId
$15 = 3107001119
(gdb) p g.ActiveIdPreviousFrame
$16 = 1973097345
(gdb) p g.ActiveId
$17 = 3107001119

When switching from InputFloat2 to other widget:

(gdb) p window->DC.LastItemId
$21 = 3107001119
(gdb) p g.ActiveIdPreviousFrame
$22 = 3107001119
(gdb) p g.ActiveId
$23 = 0

[ocornut]

I’ll look at this when I have time.

we save position before editing. When IsItemDeactivatedAfterEdit returns true, we add "undo" command to undo stack. The current behaviour makes this impossible.

I am not sure why that is impossible tho? The same way you will need to handle false positives with IsItemDeactivatedAfterEdit() you should be able to trivially handle duplicate of IsItemActivated() ?

[eliasdaler]

I figured out a temporary workaround - I just store bool for each widget which checks if item was already activated or not.
If it's "true" when IsItemActivated returns true, I can say that it's caused by switching to other component.

Feels a bit tricky and unstable, though.

What behaviour in my example do you think is the most logical?

[ocornut]

For a start I cannot even repro the first assessment of your first post.

When I start editing one of the components and then switch to another (via "Tab" or by clicking), I get the following output: A A B

Doing what you suggest (e.g. start editing first value within InputFloat2, then press tab, then tab or enter to leave the second component) I get A A and never a B (IsItemDeactivatedAfterEdit()) which is even more problematic. So while I acknowledge there is at least definitively one issue in there, your report seems incorrect or partially missing information. I'll dig in further.

[ocornut]

I believe the correct behavior for:

click --> edit --> tab --> enter

Should either be:

Activated -> Edited --> DeactivatedAfterEdit + Activated --> Deactivated

Or

Activated -> Edited --> ... --> DeactivatedAfterEdit

Either would work, the second one would be better.
(I'm currently writing a bunch of tests to get those into the testing system first.)

[eliasdaler]

Agreed on second being much better.

And it's strange that you don't get DeactivatedAfterEdit, I've used latest 'master' and it was just minimal working code + this. Do you get Deactivated at least when you exit the editing?

[Mooseart]

Right, so just to summarize my thoughts a little. I think the problem is two fold. The main issue is what you describe above ocurnut, that I don't get the IsItemDeactivatedAfterEdit at all. This is perhaps even more apparent if you make an InputFloat3 with three fields instead. Editing the middle value and tabbing back and forth just doesn't trigger any IsItemDeactivatedAfterEdit, even if you tab out of the group. That event does not get propagated to the group as a whole, it's just lost. (this was with v1.69, will upgrade)

The other discussion revolves around what expected behavior is here. I suppose it boils down to what constitutes an undo history action. If you want to implement undo for editing each single field or only after leaving the group. I don't have a strong opinion on that but I can imagine that single field editing triggers undo in most applications with such controls? It would also be consistent with how IsItemActivated behaves now, triggering on each single field. So tabbing and editing straight through an InputFloat3 we could expect something like ABABAB.

[ocornut]

this was with v1.69, will upgrade)

Issue is still there. The problem is due to how the IsItemXXX functions are written and how EndGroup tries to masquerade as whichever item was active within its bound to "forward" the information. Those problems are mostly caused by how this is all designed to be as efficient as possible.

[ocornut]

I have pushed a fix now.

Also found and fixed another bug where IsItemEdited() would return true multiple times when involving text edition of InputFloat, and added a bunch of tests in my testing back-end.

(Note that they were multiple commits leading to this change, so you can't just merge the two tagged commits but you can try updating to latest master.)

(Many of the code there is a little weird because it wants to be highly optimized.)

[eliasdaler]

Can confirm this works. My scenario now outputs:

A
A* 
B*
B

* - these two happen at the same frame as expected.

[Mooseart]

Tested and verified, this totally solves it on my end and my undo/redo functionality now works like a charm. Great stuff. Thank you.

[ocornut]

Good to hear! Apologies I posted hastily yesterday and forgot to clarify which route I went for first.

I went this route because it is simpler to implement (and even that took me several hours to figure out the iterate) but I will try to aim for the other version later. Receiving both activate/deactivate on same frame makes user code a little less obvious.

It could be a flag of the group system to decide how to treat those events as well.

[ocornut]

I'm closing this as solved for now, and added myself a task to try making it possible at least of multi-component widgets to enable a mode when change of components don't trigger activated/deactivated. I'll post again if there's a change being made to this.