Finaling event from various widgets (sliders etc.) for Undo/Redo systems

[waidschrath]

Hi!
Thanks for the great imgui! I really made big leaps in no time with this fantastic library. I'm implementing an editor for my game but i'm failing to grab the right events for my undo/redo stack.

If i do
if(ImGui::SliderFloat())

i get every value change which makes my stack a mess. However i want only the value that was changed after the slider was released.

Is there a functionality or an event for that? I tried to hook manually by waiting for ImGui::GetIO().WantCaptureMouse to become false again but this is not set immediately but only when i move to another item (and does not resemble release states). Same goes for the TextInput, i don't want each character but only if the change was completed.

Is there a way to do that intuitively or what would be best practice?

Thanks!

TL;DR: how to grab slider input changes immediately after sliding is finished? need it for undo.

[ocornut]

This is what you could use:

#include <imgui_internal.h>

bool IsItemActiveLastFrame()
{
    ImGuiContext& g = *GImGui;
    if (g.ActiveIdPreviousFrame)
        return g.ActiveIdPreviousFrame == g.CurrentWindow->DC.LastItemId;
    return false;
}

bool IsItemJustMadeInactive()
{
    return IsItemActiveLastFrame() && !ImGui::IsItemActive();
}

(from #820)

I've been meaning to add it but couldn't think of a function name (naming is the hardest thing) :)

EDIT IsItemReleased() would be nice but it is ambiguous because we have IsItemActive() and IsItemClicked().

[ocornut]

Here's a better name, IsItemDeactivated()

bool ImGui::IsItemDeactivated()
{
    ImGuiContext& g = *GImGui;
    ImGuiWindow* window = g.CurrentWindow;
    return (g.ActiveIdPreviousFrame == window->DC.LastItemId && g.ActiveIdPreviousFrame != 0 && g.ActiveId != window->DC.LastItemId);
}

The problem with this approach is highlighted in #956, it currently fails with multi-component widgets (can be fixed) and it's particularly tricker to handle for e.g. the color edit popup. Working on finding a solution.

[ocornut]

After agonizing a over the naming convention (*) I think I've got a decent solution for this long-standing issue.

• Added IsItemDeactivated() to query if the last item was active previously and isn't anymore.
• Added IsItemDeactivatedAfterChange() if the last item was active previously, isn't anymore, and during its active state modified a value. Note that you may still get false positive (e.g. drag value and while holding return on the same value).
  (How to get a slider or input field to only return results once the user stops editing? #820, Widgets returning true only when they change value? #956, Finaling event from various widgets (sliders etc.) for Undo/Redo systems #1875)

Some cases may be unsatisfactory: e.g. if you use InputFloat() and the user presses the +/- buttons several times. Depending on how your Undo system may work you may detect no-ops, or merge events in the undo stack. You might also use GetItemID() from imgui_internal.h if querying an unique ID for the item can help you merging modifications.

Let me know how it works for you.

(*) I am still not very happy with them, but considering the wide amount of widget they support I find it tricky to find a non-ambiguous name. Suggestions welcome, I can still rename the functions while they are fresh and not in a tagged release.
EDIT IsItemDeactivatedAfterChange was eventually renamed to IsItemDeactivatedAfterEdit.

This section of the Demo window can let you understand how it works:

[SuperWangKai]

We need a pair of API to record undo:
BeginChange - e.g. drag begins. In this case we can record the orignal old value.
EndChange - e.g. drag ends. As the new final value got, we can record the new and old values for the undo.

Edit: the IsItemDeactivatedAfterChange API only acts as the EndChange phase, so it's hard to track the old value, since for sliders, value is always changing.

[JSandusky]

I deal with this by just merging actions together. Whenever an action is pushed onto the undo-stack the topmost action on the stack checks if it can merge the action, if it can then it merges and discards the added action. Same field edits are on a time threshold before they are classified as new actions.

Code for one case (just to demonstrate how infantile those blocks are):

virtual bool CanMergeWith(UndoRedo* rhs) const override {
    if (auto other = dynamic_cast<MaterialUndoRedo*>(rhs))
        return other->ptr_ == ptr_ && other->offset_ == offset_;
    return false;
}

virtual void Merge(UndoRedo* rhs) override {
    if (auto other = dynamic_cast<MaterialUndoRedo*>(rhs))
    {
        newValue_ = other->newValue_;
        text_ = other->text_;
    }
}

[SuperWangKai]

@JSandusky It's a smart hack and we need something clean :)

[SuperWangKai]

Actually, we can properly handle undo recording by following code. Tested for DragFloat, DragFloat2 and DragFloat3.

// Current editing value
static float value = 0.0f;

// Old value, used for store value before editing
static float oldValue = 0.0f;

if (ImGui::DragFloat("Drag float", &value))
{
    // Here we apply change to editing target
}

if (ImGui::IsItemActive() && !IsItemActiveLastFrame())
{
    // Here we record old value when item is actived for the first time.
    oldValue = value;
}

if (ImGui::IsItemDeactivatedAfterChange())
{
    // Here we can save undo (to undo stack or something). Triggered once just after user stops editing. 
    // RecordUndo(oldValue, value);
}

[ocornut]

@SuperWangKai Interestingly, every time in the past I have implemented an Undo system it didn't need the "old value" to function but I can see why yours would need it.
I am not sure how your hypothetical BeginChange/EndChange api would work, and how it would scale to most complex widgets.
Open to more changes if needed but I'm not sure what to do at the moment.

[SuperWangKai]

@ocornut To my understanding, old value and new value are store as a pair recored for a particular undo action. When we want to undo, we apply the old value to the target; when we redo, we apply the new value back to the target.

if (ImGui::IsItemActive() && !IsItemActiveLastFrame()) acts as the BeginChange phase
while
if (ImGui::IsItemDeactivatedAfterChange()) acts as the EndChange phase. These two phases are just hypothetical as you mentioned, please don't be too serious about them.

Edit: Plus, we don't want value changes between these two phases, so we can get a clean, single piece of undo action.

Please inspire me if I'm wrong or there is something I missed.

[SuperWangKai]

For ColorEdit*, there will be two undo actions recorded and I need to study more to find the reason. However, for InputText/Drag*, AFAIK, it works great.

[JSandusky]

@SuperWangKai, not really a hack - IIRC that's what IBM laid down with CUA decades ago. Ends up being necessary for a lot of things anyways. Particularly things that have long-running consequences (ie. trigger a distance field recalc, baking, etc) or are prone to fine but meaningless editing (gizmos/manipulators), without merging actions just positioning something in a scene floods an undo-stack with garbage with all of the move -> change-view -> move -> change-view -> repeat ad-f.

Edit: but yeah, start/end would still be handy for stuff.

[ocornut]

Note that the function has been renamed to IsItemDeactivatedAfterEdit() in 1.63.
Because the function was introduced in 1.62, I have kept an inline redirection function with the old name but will completely remove it soon-ish (sooner than the typical life expectancy of obsolete redirection functions, based on the ground that the few people who used this in 1.62 will likely be reactive to the change in 1.63).

[SuperWangKai]

I'm using latest master of imgui with following code to implement my Redo/Undo system -

bool IsItemJustDeactivated()
{
    return ImGui::IsItemDeactivatedAfterEdit();
}

bool IsItemActiveLastFrame()
{
    ImGuiContext& g = *GImGui;
    if (g.ActiveIdPreviousFrame)
        return g.ActiveIdPreviousFrame == g.CurrentWindow->DC.LastItemId;

    return false;
}

bool IsItemJustActivated()
{
    return ImGui::IsItemActive() && !IsItemActiveLastFrame();
}

bool IsItemEditing()
{
    return ImGui::IsItemActive();
}

I can confirm that these functions work great for DragFloat, DragFloat2, DragFloat3, DragFloat4, InputText, DragInt, DragInt2, DragInt3, DragInt4,

However, ColorEdit4 does not work right.

If there is some way to work around, please inspire me!

Thanks!

[ocornut]

Please be more specific. You are throwing 4 functions and say "does not work right" without more details.
The official function seems to work well when interact there in the demo.
I have just added IsItemActivated() which was missing from the lot.

When starting edition IsItemActivated() returns true.
When editing and releasing IsItemDeactivatedAfterEdit() returns true.

[ocornut]

Will all those functions:

bool IsItemActive();                 // is the last item active? (e.g. button being held, text field being edited. This will continuously return true while holding mouse button on an item. Items that don't interact will always return false)
bool IsItemEdited();                 // did the last item modify its underlying value this frame? or was pressed? This is generally the same as the "bool" return value of many widgets.
bool IsItemActivated();              // was the last item just made active (item was previously inactive).
bool IsItemDeactivated();            // was the last item just made inactive (item was previously active). Useful for Undo/Redo patterns with widgets that requires continuous editing.
bool IsItemDeactivatedAfterEdit();   // was the last item just made inactive and made a value change when it was active? (e.g. Slider/Drag moved). Useful for Undo/Redo patterns with widgets that requires continuous editing. Note that you may get false positives (some widgets such as Combo()/ListBox()/Selectable() will return true even when clicking an already selected item).

My understand is that any type of undo pattern can be implemented and I could close this issue.

@waidschrath @SuperWangKai @JSandusky
Could you confirm and see if you still have issues?
@SuperWangKai can you try using the public functions (e.g. IsItemActivated() and clarify your issue if you still have one?

[JSandusky]

@ocornut I haven't had any issues with it, seems to work fine here and as expected. Though I only use it in 2 places for transform edits where the continuous edit-loop of imgui caused serious problems with matrix drift.

[SuperWangKai]

Hi @ocornut, it took me some time to figure out the issue I met.

I may be very wrong, but I expected that for the ColorEdit4, IsItemActivated happens when color picker appears (so I record the old value) and IsItemDeactivatedAfterEdit happens when the color picker is closed (so I get the new value, then old-new value pair can be used as an undo record if the two values are different).

For current implementation, once user click on the opened color picker, there will be IsItemActivated/IsItemDeactivatedAfterEdit pair generated, which means multiple undo actions will be recorded when the user is trying different colors, even though the user only choose one color after some tries.

Again, there is may be much better solusions for me to implement undo. Please correct and inspire me. @ocornut @JSandusky

Edit: my code of color editing with a little simplification.

/// This function handles undo recording of an imgui widget with continuous value changing
void CheckContinuousValueUndo(Variant& oldVal, const Variant& newVal)
{
    if (ImGui::IsItemActivated())
    {
        // here we record the old value for later undo record.
        oldVal = newVal;
    }

    if (ImGui::IsItemDeactivatedAfterEdit() && oldVal != newVal)
    {
        // here we record a new undo record
        RecordUndo(oldVal, newVal);
    }
}

//========================================
// the following code is in a draw property panel function
Color val = value_.GetColor; // the color value we are drawing
static Variant oldVal; // old color value used for undo record

// draw the color editor
if (ImGui::ColorEdit4(hiddenLabel, &val.r_, ImGuiColorEditFlags_AlphaBar | ImGuiColorEditFlags_AlphaPreview | ImGuiColorEditFlags_RGB))
    ApplyChangeAttribute(val); // apply the color change in real-time

// check undo for the continuous value change
CheckContinuousValueUndo(oldVal, val);

[eliasdaler]

@Mooseart I've opened the issue to investigate this: #2550

[ocornut]

Thank you @Mooseart and @eliasdaler for the report, I'm looking for a solution at the moment!

[ocornut]

Using v1.69, IsItemDeactivatedAfterEdit() doesn't seem to fully work with InputScalarN() widget

FYI for other readers, this is now fixed by #2550

As for @SuperWangKai questions on behavior with ColorEdit and its picker:

I may be very wrong, but I expected that for the ColorEdit4, IsItemActivated happens when color picker appears (so I record the old value) and IsItemDeactivatedAfterEdit happens when the color picker is closed

We would potentially aim to implement a flag that does like you suggest.

However, note that you would get the same situation when tweaking variables any other editor e.g. dragging a SliderFloat multiple time, or even when using ColorPicker directly. I feel like a ColorEdit-popup-only solution wouldn't be particularly appropriate. Your underlying undo system may still want to merge changes somehow?

[SuperWangKai]

As for @SuperWangKai questions on behavior with ColorEdit and its picker:

I may be very wrong, but I expected that for the ColorEdit4, IsItemActivated happens when color picker appears (so I record the old value) and IsItemDeactivatedAfterEdit happens when the color picker is closed

We would potentially aim to implement a flag that does like you suggest.

However, note that you would get the same situation when tweaking variables any other editor e.g. dragging a SliderFloat multiple time, or even when using ColorPicker directly. I feel like a ColorEdit-popup-only solution wouldn't be particularly appropriate. Your underlying undo system may still want to merge changes somehow?

Is there anyway that I can get the open and close events of the picker? Otherwise I don't know which undo records should be merged or discarded.

[ocornut]

Is there anyway that I can get the open and close events of the picker? Otherwise I don't know which undo records should be merged or discarded.

The whole point is it shouldn't matter and this is not a ColorEdit/Picker specific issue. Try to find out how you would handle it for a simple SliderFloat() float.

[SuperWangKai]

The following code works for me when I handle SliderFloat() and other simple widgets.

/// This function handles undo recording of an imgui widget with continuous value changing
void CheckContinuousValueUndo(Variant& oldVal, const Variant& newVal)
{
    if (ImGui::IsItemActivated()) // THIS WORKS!
    {
        // here we record the old value for later undo record.
        oldVal = newVal;
    }

    if (ImGui::IsItemDeactivatedAfterEdit() && oldVal != newVal) // THIS WORKS!
    {
        // here we record a new undo
        RecordUndo(oldVal, newVal);
    }
}

Please kindly suggest any other possible solutions for an undo system.

Edit: I define an undo record like a kind of transaction. e.g. Any operations between "begin dragging" and "end dragging" operations on float slider should be ignored. However, the value when drag begins and the value when drag ends are the pair for one undo record.

[ocornut]

OK but the reality is that the user may click multiple times on the Slider or other widgets. You may want to merge those events. You also need to handle situation where the value returns to the same value and ignore the event.

[eliasdaler]

@SuperWangKai
I use similar code. What don't you like about it? It works as expected, as far as I know.

No merging is needed, as start of dragging (checked by IsItemActivated) is beginning of operation and end of it is releasing the slider (checked by IsItemDeactivatedAfterEdit).

[SuperWangKai]

If the user clicks for multiple times, they should be regarded as multiple intended adjustments. And this case is different from dragging.

[ocornut]

So, you are happy with this behavior, but it is the same behavior you are trying to remove from the ColorEdit>Picker interactions? In both cases there will be multiple sequences of edition.

[eliasdaler]

If the user clicks for multiple times, they should be regarded as multiple intended adjustments

Are you talking about double-clicking on a slider and then editing the value via keyboard input?

[SuperWangKai]

@ocornut

I hope I could express better...

Yes, for DragFloat, DragFloat2, DragFloat3, DragFloat4, InputText, DragInt, DragInt2, DragInt3, DragInt4 and also widgets of the ColorEdit except the color wheel, I'm happy with current behavior. But I think it is more natural for ColorEdit to merge users mouse moves on the wheel and store one undo record after user closed the picker. (Edit: To me, for the mouse moves on the color wheel, they are more like adjustments before user finally submitting the update. So, I prefer to store only one undo record for this color editing.)

@eliasdaler

I can confirm that these functions work great for DragFloat, DragFloat2, DragFloat3, DragFloat4, > InputText, DragInt, DragInt2, DragInt3, DragInt4,

I just met some issue with ColorEdit4, and should with as well as ColorEdit3

[SuperWangKai]

To make my expression better... Just imagine the following bad scenario -

You suddenly want to adjust some color in your cool editor. So you open this color picker, and then you click and move around for some times on the color wheel, then you are happy with the result and close the color picker.

Then you regret, so you press Ctrl+Z, but you are so confused, since there are many color editing records. You just don't know which one is the original color...

[ocornut]

I understand what you mean but wouldn't you have the exact same multiple CTRL+Z issue by editing a standard float multiple time?

(I however acknowledge your request, I just don't know what's the best code and design to achieve it nor when I will be able to look at it.)

[SuperWangKai]

For a standard float widget, when the users release they mouse after dragging or finished entering some number by return key, we should regard the operation as an intended editing. It seems to be the best we can do.

For a color picker, if we regard every movement on the open picker as a valid editing, it will be a mess. But if we choose user's closing picker as a confirmation, we get the same clean result as a float widget.

Additionally, we have to select multiple times to get a valid color on the picker, e.g. first hue, then alpha... and you get a color you want.

[eliasdaler]

Yeah, I understand now. It looks like ideally IsItemActivated should happen when you click on color and open picking widget, and IsItemDeactivatedAfterEdit IsItemDeactivated should happen when you click on some other widget (which closes the picking widget).

What is the current behaviour?

[SuperWangKai]

@eliasdaler

I don't have my testing code now, however, generally, current behavior is that IsItemDeactivatedAfterEdit happens after you clicked on the widgets of ColorEdit or moved your mouse on the color wheel.

[kudaba]

I noticed another small issue with IsItemDeactivatedAfterEdit. If you have a single item (i.e. InputInt) in a window and try to tab out, since the active control doesn't change, it won't trigger a deactivate.

[JakubLukas]

I also noticed small problem with IsItemDeactivatedAfterEdit when used after slider widget.
IsItemDeactivatedAfterEdit returns true in frame when there is mouse up (so io.MouseDown[0] is false) but in that frame slider do not calculate value, so out parameter (e.g. float* v of SliderFloat) is default value.
That means that if I want to apply value on IsItemDeactivatedAfterEdit, nothing changes.
Possible fix would be removing pointed line in SliderBehaviorT

if (!g.IO.MouseDown[0])
            {
                ClearActiveID();
            }
            else // <--- removing this line /////////////////////////////////////////
            {
                const float mouse_abs_pos = g.IO.MousePos[axis];
                clicked_t = (slider_usable_sz > 0.0f) ? ImClamp((mouse_abs_pos - slider_usable_pos_min) / slider_usable_sz, 0.0f, 1.0f) : 0.0f;
                if (axis == ImGuiAxis_Y)
                    clicked_t = 1.0f - clicked_t;
                set_new_value = true;
            }

[ocornut]

When investigating and working on a solution for #4714 I came up with an issue:

{
    static ImVec4 color0(1.0f, 0.0f, 0.0f, 1.0f);
    static ImVec4 color1(0.0f, 1.0f, 0.0f, 1.0f);
    static int edited_ret_frame = -1;
    static int edited_ret_field = 0;
    static int edited_query_frame = -1;
    static int edited_query_field = 0;
    static int deactivated_frame = -1;
    static int deactivated_field = 0;

    if (ImGui::ColorEdit4("color0", &color0.x)) { edited_ret_frame = ImGui::GetFrameCount(); edited_ret_field = 0; }
    if (ImGui::IsItemEdited()) { edited_query_frame = ImGui::GetFrameCount(); edited_query_field = 0; }
    if (ImGui::IsItemDeactivatedAfterEdit()) { deactivated_frame = ImGui::GetFrameCount(); deactivated_field = 0; }

    if (ImGui::ColorEdit4("color1", &color1.x)) { edited_ret_frame = ImGui::GetFrameCount(); edited_ret_field = 1; }
    if (ImGui::IsItemEdited()) { edited_query_frame = ImGui::GetFrameCount(); edited_query_field = 1; }
    if (ImGui::IsItemDeactivatedAfterEdit()) { deactivated_frame = ImGui::GetFrameCount(); deactivated_field = 1; }

    ImGui::Text("Edited (ret) frame %d, field %d", edited_ret_frame, edited_ret_field);
    ImGui::Text("Edited (query) frame %d, field %d", edited_query_frame, edited_query_field);
    ImGui::Text("Deactivated frame %d, field %d", deactivated_frame, deactivated_field);
}

When you drag a color into another it writes into a different item without that written item being ever activated.
As a result IsItemDeactivatedAfterEdit() never returns true there:

While #4714 (after upcoming fix) would not suffer from this, a drag and drop target would.