InputText IsItemDeactivated() doesn't trigger after right-click on Button

[Web-eWorks]

Dear ImGui Version

This is running on Pioneer's version of ImGui 1.89.8-docking, which contains some minor patches applied for on-demand font glyph loading, a custom cursor, and hardware depth sort of ImDrawCmds.

Dear ImGui 1.89.8 (18980)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201703
define: __linux__
define: __GNUC__=4
define: __clang_version__=15.0.7 
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_sdl2
io.BackendRendererName: Pioneer Renderer
io.ConfigFlags: 0x00000040
 DockingEnable
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00000C0E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasVtxOffset
--------------------------------
io.Fonts: 5 fonts, Flags: 0x00000000, TexSize: 512,512
io.DisplaySize: 1600.00,900.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 6.00,5.00
style.FrameRounding: 2.00
style.FrameBorderSize: 1.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

My Issue/Question:

I'm using ImGui::IsItemActivated() / ImGui::IsItemDeactivated() to implement an undo system for an editor. In brief, the undo system relies on pushing the current value when the user begins editing a field and allows ImGui to update the field to the "new value" without needing additional handling code. Individual value changes are grouped into "UndoFrames" i.e. state packets associated with a single user undo/redo operation.

This works fine for most widget types, however there's an issue where IsItemDeactivated() does not return true when the user navigates away from an InputText by right/middle clicking on a Button/ButtonBehavior which activates on right/middle mouse button. If the ButtonBehavior appears later in execution order than the InputText field, IsItemDeactivated() will never return true even though the widget is deactivated at the end of the frame. InputText correctly signals deactivation if the user uses the left mouse button to click away however.

I'm working around this in my editor code currently by rendering the offending ButtonBehavior at the start of the frame, before any user-editable inputs are submitted. I require the use of the ButtonBehavior to determine when to forward input to the viewport's input subsystem while still rendering interactable ImGui interface elements over the viewport.

I understand this to be a subset of a larger issue with IsItemDeactivated() and previous-frame-ID tracking, but the scope of this issue is specifically aimed at InputText and its behavior of clearing the active ID when the user left-clicks outside of it. The issues I saw tangentially related to the problem addressed window focus issues and/or using ImGui for all viewport input handling, neither of which are relevant to this case. Additionally, I consider opening this issue report a positive impetus towards a solution for the larger issue at hand.

Screenshots/Video

2023-08-30.04-42-35.mp4

Standalone, minimal, complete and verifiable example:

// Here's some code anyone can copy and paste to reproduce your issue
ImGui::Begin("Bug Repro");

std::string text = "test value";
ImGui::InputText("Edit Value", &text);

if (ImGui::IsItemDeactivated())
    printf("End UndoFrame\n");
if (ImGui::IsItemActivated())
    printf("Begin UndoFrame\n");

ImGui::ButtonEx("Break UndoSystem!", ImVec2(0, 0), ImGuiButtonFlags_MouseButtonRight);
ImGui::End();

Click into the InputText, click away, click back, then right-click on "Break UndoSystem". You'll note that ImGui::IsItemDeactivated() did not return true after the last click even though the text input was deactivated.

[GamingMinds-DanielC]

I had a related issue in #5904. The workaround for my specific problem won't help in this case though, same reason it won't work for #5184.

My proposal towards a solution there was:
"For that, a proper solution would most likely have to memorize all items that were active during a frame, not just the last one active at the end of it. And then on the new frame mark all of them as having been deactivated if they are no longer active."

Expanding on that, it should be enough to memorize at most two active items, since only one item can be activated per frame (normally, user code can activate any amount during the same frame but I would count that as a user code problem). Therefore, ImGui::SetActiveID() should make a backup of ActiveIdIsAlive (from a previous call to ImGui::KeepAliveID()), so that then ActiveIdPreviousFrame will get a second value. For a clean solution, ActiveIdPreviousFrameHasBeenEditedBefore would also have to hold two values and ImGui::IsItemDeactivatedAfterEdit() needs to check the correct one. That should do the trick.

[ocornut]

We have a similar mechanism in place (see InputTextDeactivateHook(), #4714 which also store/recovers the last value as InputText() carries one (vs most other widgets wouldn't), I guess it is a matter of extending this mechanism and make it a little more first-class citizen and it would cover #6766 #5904 #5184.

[ocornut]

I investigated this a little and I think we could get this to work by reformulating all g.ActiveIdPreviousFrameXXX fields as g.DeactivatedIdXXXX fields, where the assignment in NewFrame() and SetActiveID() handle the fact that an g.ActiveId that has been alive before the current frame will have the priority setting up this, over an the ActiveId that has just started becoming active.

Requires some careful/minutia work to get right, any non-obvious situation should have a test added in the Dear ImGui Test Suite but I think it can be elegantly solved.

Then, the InputText() specific solution of #4714 can be reworked to sit over that more general logic.

[Web-eWorks]

That was the general solution I was thinking of as well. The only problem I can think of here is that this would create an activated->activated->deactivated->deactivated order of events when clicking into another text field or using a button with PressedOnClick (where desired is A->D->A->D). This may already be solved with a 1-frame delay for IsItemActivated() inside InputText however, as it works properly when activating an InputText earlier in submission order than the currently activated InputText.