Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression? IsItemDeactivatedAfterEdit on InputScalar step buttons doesn't happen on frame writing to backing data #8149

Open
olivier-gerard opened this issue Nov 13, 2024 · 9 comments

Comments

@olivier-gerard
Copy link

Version/Branch of Dear ImGui:

Version 1.95.5

Back-ends:

imgui_impl_Vulkan.cpp + imgui_impl_GLFW.cpp

Compiler, OS:

Windows 11, Visual Studio 2022

Full config/build information:

No response

Details:

My Issue/Question:

Hi,
I noted a regression from 1.91.4 using IsItemDeactivatedAfterEdit() on 'temporary' (I mean not static nor saved in global objects) values.
With 1.91.4 I used to get the new value after calling IsItemDeactivatedAfterEdit(), now I always get the value before user input.

I didn't find anything on the changelog about it, and believe I have no particular configuration regarding this.
Anyway just changing the version of DearImgui changes the behavior.

I provide a small example below, better than words.
(I use this trick with temporary objects to resize vectors.)

Hope this is clear enough.
Regards

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

 static int bak = 10;
 int tmp = bak;
 ImGui::InputInt("tmp", &tmp);
 if (ImGui::IsItemDeactivatedAfterEdit()) {
     int newval = tmp; //now always 10, was 9/11/user input in 1.91.4
 }
@ocornut
Copy link
Owner

ocornut commented Nov 13, 2024

Thank you for reporting.
This came as an accidental side-effect to 9a0dff1 which altered the timing of reacting to the +/- buttons.

I actually introduced that change exactly for those +/- buttons, because it felt not right that a single click would react on mouse-up while a held button would repeat. For a repeating button of that kind it feels right than the initial mouse-down alter the value and then holding the button for a longer time alters it further.

How it affected the timing if setting ActiveId and breaking this I will investigate. Note that I'm frankly not even sure this use case for non-string value has ever been officially supported.

@ocornut
Copy link
Owner

ocornut commented Nov 13, 2024

It actually never correctly worked before:

int tmp = 10;
if (ImGui::InputInt("tmp", &tmp))
    IMGUI_DEBUG_LOG("%d - Edited\n", tmp);
if (ImGui::IsItemDeactivatedAfterEdit())
    IMGUI_DEBUG_LOG("%d - DeactivatedAfterEdit\n", tmp);

A single/fast click would work because initial edit was on deactivated frame:

[00253] 11 - Edited
[00253] 11 - DeactivatedAfterEdit

But a hold-to-repeat click could release on any frame:

[01328] 11 - Edited
[01331] 11 - Edited
[01334] 11 - Edited
[01337] 11 - Edited
[01338] 10 - DeactivatedAfterEdit

The new behavior is actually more consistent.

As per the API definition the current behavior is not incorrect.
You may use a variable to store last value after InputInt() and use that in the IsItemDeactivatedAfterEdit() path.

(we partially/intently supported this idiom for direct InputText() calls but partly because InputText() works in a way not super consistent with other widgets, which we aim to fix. See e.g. #8004 (comment), and #4714 (comment))

@ocornut
Copy link
Owner

ocornut commented Nov 13, 2024

(I use this trick with temporary objects to resize vectors.)

I'd be interested if you can elaborate on that, while I investigate if it seems sane to add a workaround to support this.

@olivier-gerard
Copy link
Author

Thanks for your quick feedback.
Yes I was only using it on single clicks.
There is no problem to find an alternative by storing values if you don't plan to manage this behavior, I just wanted to highlight what I found as a regression.

A basic use case is a spline curve controlled by some drag points. Standard is having 3 control points, but you can control this number. If you change it then I have to add an item in the std::vector holding the points.

lets say:

 int vecSz = myVec.size();
 ImGui::InputInt("Items", &vecSz);
 if (ImGui::IsItemDeactivatedAfterEdit()) {
     myVec.resize(vecSz);
     ...
 }

ocornut added a commit that referenced this issue Nov 13, 2024
…putInt/InputFloat step buttons affected some mis-uses (#8149)
@ocornut
Copy link
Owner

ocornut commented Nov 13, 2024

I poked a bit. I don't think there is a sane way to workaround this from the point of view of the library (other than relying on the previous repeat behavior). The previous behavior was accidental and as mentioned didn't work if you held the step buttons. The general paradigm of dear imgui is that you should carry the only source of truth.

I have amend the Changelog 8be0723 to resurface this better.

Note that you are free to use Button() and react on them. Here specifically using IsItemDeactivatedAfterEdit() would suggest you want to handle batches edits at once.

But if you change your code to:

 int vecSz = myVec.size();
 if (ImGui::InputInt("Items", &vecSz))
     myVec.resize(vecSz);
     ...
 }

It'll work just as well.

But I agree it's not ideal because you would ideally want immediate feedback on pressing +/- but delayed feedback when inputting a number...

@ocornut
Copy link
Owner

ocornut commented Nov 13, 2024

The hypothetical ImGuiInputTextFlags_NoLiveEdit feature from #701 would solve since, then you can use the return value from InputInt().

We did some simplification work on InputText() recently so I am going to reevaluate if _NoLiveEdit is easier to implement now.

@olivier-gerard
Copy link
Author

Yes, sure that i should hold the data, but in this case it would mean adding a variable per vector, which I find not very nice.
Directly picking the return of InputInt works, but in other cases than the spline the vector represents configuration which is sent to a hardware, so I prefer limiting the updates and avoiding setting '1' before '10' when the user is typing.
So the flag ImGuiInputTextFlags_NoLiveEdit looks the best option, but I understand it means some work to be easy to integrate.

For the moment I will patch my code with 1st or 2nd solution depending on the presence of hardware behind, and if some day you manage the flag it will be great!

@ocornut
Copy link
Owner

ocornut commented Nov 13, 2024

I think the best workaround may be to use InputInt() with step == 0 to disable those buttons, and then IsItemDeactivatedAfterEdit() should work fine for you.

@ocornut ocornut changed the title Regression - IsItemDeactivatedAfterEdit inserts a delay fatal for temporary values Regression? IsItemDeactivatedAfterEdit on InputScalar step buttons don't happen on frame writing to backing data Nov 13, 2024
@ocornut ocornut changed the title Regression? IsItemDeactivatedAfterEdit on InputScalar step buttons don't happen on frame writing to backing data Regression? IsItemDeactivatedAfterEdit on InputScalar step buttons doesn't happen on frame writing to backing data Nov 13, 2024
@olivier-gerard
Copy link
Author

Thanks, I patched my stuff, mainly using a step to Null (not equal to 0) to hide +/- buttons, and manually added them right after. It does not handle repeat, but this is ok for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants