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

New feature: default number for empty float field. #7305

Closed
supermerill opened this issue Feb 12, 2024 · 8 comments
Closed

New feature: default number for empty float field. #7305

supermerill opened this issue Feb 12, 2024 · 8 comments

Comments

@supermerill
Copy link

Version/Branch of Dear ImGui:

1.83

Back-ends:

don't know, the cpp library

Compiler, OS:

windows (visual), linux, macos

Full config/build information:

No response

Details:

Hi
I wanted to change the behavior, so when the user erase the content of an input the value return to a default value instead of not changing.
And also to not draw anything when the value is the default.

Here is apatch for the feature (for float field only):
diff --git a/src/imgui/imgui.h b/src/imgui/imgui.h
index 06a610d408..f6646cfd7e 100644
--- a/src/imgui/imgui.h
+++ b/src/imgui/imgui.h
@@ -552,7 +552,7 @@ namespace ImGui
     IMGUI_API bool          InputText(const char* label, char* buf, size_t buf_size, ImGuiInputTextFlags flags = 0, ImGuiInputTextCallback callback = NULL, void* user_data = NULL);
     IMGUI_API bool          InputTextMultiline(const char* label, char* buf, size_t buf_size, const ImVec2& size = ImVec2(0, 0), ImGuiInputTextFlags flags = 0, ImGuiInputTextCallback callback = NULL, void* user_data = NULL);
     IMGUI_API bool          InputTextWithHint(const char* label, const char* hint, char* buf, size_t buf_size, ImGuiInputTextFlags flags = 0, ImGuiInputTextCallback callback = NULL, void* user_data = NULL);
-    IMGUI_API bool          InputFloat(const char* label, float* v, float step = 0.0f, float step_fast = 0.0f, const char* format = "%.3f", ImGuiInputTextFlags flags = 0);
+    IMGUI_API bool          InputFloat(const char* label, float* v, float step = 0.0f, float step_fast = 0.0f, const char* format = "%.3f", ImGuiInputTextFlags flags = 0, float p_data_void = FLT_MAX);
     IMGUI_API bool          InputFloat2(const char* label, float v[2], const char* format = "%.3f", ImGuiInputTextFlags flags = 0);
     IMGUI_API bool          InputFloat3(const char* label, float v[3], const char* format = "%.3f", ImGuiInputTextFlags flags = 0);
     IMGUI_API bool          InputFloat4(const char* label, float v[4], const char* format = "%.3f", ImGuiInputTextFlags flags = 0);
@@ -561,7 +561,7 @@ namespace ImGui
     IMGUI_API bool          InputInt3(const char* label, int v[3], ImGuiInputTextFlags flags = 0);
     IMGUI_API bool          InputInt4(const char* label, int v[4], ImGuiInputTextFlags flags = 0);
     IMGUI_API bool          InputDouble(const char* label, double* v, double step = 0.0, double step_fast = 0.0, const char* format = "%.6f", ImGuiInputTextFlags flags = 0);
-    IMGUI_API bool          InputScalar(const char* label, ImGuiDataType data_type, void* p_data, const void* p_step = NULL, const void* p_step_fast = NULL, const char* format = NULL, ImGuiInputTextFlags flags = 0);
+    IMGUI_API bool          InputScalar(const char* label, ImGuiDataType data_type, void* p_data, const void* p_step = NULL, const void* p_step_fast = NULL, const char* format = NULL, ImGuiInputTextFlags flags = 0, void* p_data_void = nullptr);
     IMGUI_API bool          InputScalarN(const char* label, ImGuiDataType data_type, void* p_data, int components, const void* p_step = NULL, const void* p_step_fast = NULL, const char* format = NULL, ImGuiInputTextFlags flags = 0);
 
     // Widgets: Color Editor/Picker (tip: the ColorEdit* functions have a little color square that can be left-clicked to open a picker, and right-clicked to open an option menu.)
diff --git a/src/imgui/imgui_internal.h b/src/imgui/imgui_internal.h
index 8006246b6d..98469f9c02 100644
--- a/src/imgui/imgui_internal.h
+++ b/src/imgui/imgui_internal.h
@@ -2580,7 +2580,7 @@ namespace ImGui
     IMGUI_API const ImGuiDataTypeInfo*  DataTypeGetInfo(ImGuiDataType data_type);
     IMGUI_API int           DataTypeFormatString(char* buf, int buf_size, ImGuiDataType data_type, const void* p_data, const char* format);
     IMGUI_API void          DataTypeApplyOp(ImGuiDataType data_type, int op, void* output, const void* arg_1, const void* arg_2);
-    IMGUI_API bool          DataTypeApplyOpFromText(const char* buf, const char* initial_value_buf, ImGuiDataType data_type, void* p_data, const char* format);
+    IMGUI_API bool          DataTypeApplyOpFromText(const char* buf, const char* initial_value_buf, ImGuiDataType data_type, void* p_data, const char* format, void* p_default_data = nullptr);
     IMGUI_API int           DataTypeCompare(ImGuiDataType data_type, const void* arg_1, const void* arg_2);
     IMGUI_API bool          DataTypeClamp(ImGuiDataType data_type, void* p_data, const void* p_min, const void* p_max);
 
diff --git a/src/imgui/imgui_widgets.cpp b/src/imgui/imgui_widgets.cpp
index faf616307e..65981d9db4 100644
--- a/src/imgui/imgui_widgets.cpp
+++ b/src/imgui/imgui_widgets.cpp
@@ -1909,7 +1909,7 @@ void ImGui::DataTypeApplyOp(ImGuiDataType data_type, int op, void* output, const
 
 // User can input math operators (e.g. +100) to edit a numerical values.
 // NB: This is _not_ a full expression evaluator. We should probably add one and replace this dumb mess..
-bool ImGui::DataTypeApplyOpFromText(const char* buf, const char* initial_value_buf, ImGuiDataType data_type, void* p_data, const char* format)
+bool ImGui::DataTypeApplyOpFromText(const char* buf, const char* initial_value_buf, ImGuiDataType data_type, void* p_data, const char* format, void* p_default_data)
 {
     while (ImCharIsBlankA(*buf))
         buf++;
@@ -1927,8 +1927,6 @@ bool ImGui::DataTypeApplyOpFromText(const char* buf, const char* initial_value_b
     {
         op = 0;
     }
-    if (!buf[0])
-        return false;
 
     // Copy the value in an opaque buffer so we can compare at the end of the function if it changed at all.
     const ImGuiDataTypeInfo* type_info = DataTypeGetInfo(data_type);
@@ -1937,6 +1935,15 @@ bool ImGui::DataTypeApplyOpFromText(const char* buf, const char* initial_value_b
 
     if (format == NULL)
         format = type_info->ScanFmt;
+    
+    if (!buf[0])
+        if (p_default_data && data_type == ImGuiDataType_Float) {
+            float* v = (float*)p_data;
+            *v = (*(const float*)p_default_data);
+            return memcmp(&data_backup, p_data, type_info->Size) != 0;
+        } else {
+            return false;
+        }
 
     // FIXME-LEGACY: The aim is to remove those operators and write a proper expression evaluator at some point..
     int arg1i = 0;
@@ -3339,7 +3346,7 @@ bool ImGui::TempInputScalar(const ImRect& bb, ImGuiID id, const char* label, ImG
 
 // Note: p_data, p_step, p_step_fast are _pointers_ to a memory address holding the data. For an Input widget, p_step and p_step_fast are optional.
 // Read code of e.g. InputFloat(), InputInt() etc. or examples in 'Demo->Widgets->Data Types' to understand how to use this function directly.
-bool ImGui::InputScalar(const char* label, ImGuiDataType data_type, void* p_data, const void* p_step, const void* p_step_fast, const char* format, ImGuiInputTextFlags flags)
+bool ImGui::InputScalar(const char* label, ImGuiDataType data_type, void* p_data, const void* p_step, const void* p_step_fast, const char* format, ImGuiInputTextFlags flags, void* p_data_void)
 {
     ImGuiWindow* window = GetCurrentWindow();
     if (window->SkipItems)
@@ -3353,6 +3360,8 @@ bool ImGui::InputScalar(const char* label, ImGuiDataType data_type, void* p_data
 
     char buf[64];
     DataTypeFormatString(buf, IM_ARRAYSIZE(buf), data_type, p_data, format);
+    if (p_data_void && data_type == ImGuiDataType_Float && (*(const float*)p_data) == (*(const float*)p_data_void))
+        buf[0] = 0;
 
     bool value_changed = false;
     if ((flags & (ImGuiInputTextFlags_CharsHexadecimal | ImGuiInputTextFlags_CharsScientific)) == 0)
@@ -3368,7 +3377,7 @@ bool ImGui::InputScalar(const char* label, ImGuiDataType data_type, void* p_data
         PushID(label);
         SetNextItemWidth(ImMax(1.0f, CalcItemWidth() - (button_size + style.ItemInnerSpacing.x) * 2));
         if (InputText("", buf, IM_ARRAYSIZE(buf), flags)) // PushId(label) + "" gives us the expected ID from outside point of view
-            value_changed = DataTypeApplyOpFromText(buf, g.InputTextState.InitialTextA.Data, data_type, p_data, format);
+            value_changed = DataTypeApplyOpFromText(buf, g.InputTextState.InitialTextA.Data, data_type, p_data, format, p_data_void);
 
         // Step buttons
         const ImVec2 backup_frame_padding = style.FramePadding;
@@ -3402,8 +3411,8 @@ bool ImGui::InputScalar(const char* label, ImGuiDataType data_type, void* p_data
     }
     else
     {
-        if (InputText(label, buf, IM_ARRAYSIZE(buf), flags))
-            value_changed = DataTypeApplyOpFromText(buf, g.InputTextState.InitialTextA.Data, data_type, p_data, format);
+        if (InputText(label, buf, flags))
+            value_changed = DataTypeApplyOpFromText(buf, g.InputTextState.InitialTextA.Data, data_type, p_data, format, p_data_void);
     }
     if (value_changed)
         MarkItemEdited(window->DC.LastItemId);
@@ -3446,10 +3455,10 @@ bool ImGui::InputScalarN(const char* label, ImGuiDataType data_type, void* p_dat
     return value_changed;
 }
 
-bool ImGui::InputFloat(const char* label, float* v, float step, float step_fast, const char* format, ImGuiInputTextFlags flags)
+bool ImGui::InputFloat(const char* label, float* v, float step, float step_fast, const char* format, ImGuiInputTextFlags flags, float p_data_void)
 {
     flags |= ImGuiInputTextFlags_CharsScientific;
-    return InputScalar(label, ImGuiDataType_Float, (void*)v, (void*)(step > 0.0f ? &step : NULL), (void*)(step_fast > 0.0f ? &step_fast : NULL), format, flags);
+    return InputScalar(label, ImGuiDataType_Float, (void*)v, (void*)(step > 0.0f ? &step : NULL), (void*)(step_fast > 0.0f ? &step_fast : NULL), format, flags, p_data_void == FLT_MAX ? (void*)nullptr : (void*)&p_data_void);
 }
 
 bool ImGui::InputFloat2(const char* label, float v[2], const char* format, ImGuiInputTextFlags flags)

If it's something you want, I can devote a little time to create a pr to have the default for all types, and maybe a way to activate the value hiding separately.

An exemple of the end result (with a min set and a max at default value, ie not set by the user):
image

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

// Here's some code anyone can copy and paste to reproduce your issue
        ImGui::PushItemWidth(imgui.get_style_scaling() * 100.f);
        ImGui::InputFloat("layer", &mod_layer, 0.0f, 0.0f, format.c_str(), ImGuiInputTextFlags_CharsDecimal);
        ImGui::PushItemWidth(imgui.get_style_scaling() * 36.f);
        ImGui::InputFloat("min", &mod_min, 0.0f, 0.0f, format.c_str(), ImGuiInputTextFlags_CharsDecimal, 0.f);
        ImGui::PushItemWidth(imgui.get_style_scaling() * 36.f);
        ImGui::InputFloat("max", &mod_max, 0.0f, 0.0f, format.c_str(), ImGuiInputTextFlags_CharsDecimal, 0.f);
@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

I don't think realistically we would accept a PR adding an extra parameter to those frequently-called-and-already-long-signatures functions. However:

  • If that value is going to be a 0.0f, then we could consider a ImGuiInputTextFlags flag value to express it in the API.
  • Another option is to copy InputScalar(), remove the +/- handling code, it becomes 25 lines of code, and then you can add this behavior manually by querying the string and not calling DataTypeApplyFromText(). It may be a more reasonable workaround.

(PS: 1.83 will soon be 3 years ago, and is 2000+ lines down in the changelog, I would suggest updating :)

@supermerill
Copy link
Author

supermerill commented Feb 12, 2024

something like

ImGuiInputTextFlags_DefaultZero        = 1 << 21  // Set the field value to 0 when the the field is empty.
ImGuiInputTextFlags_EmptyZero       = 1 << 21  // Show the field empty if the value is 0.
ImGui::PushItemWidth(imgui.get_style_scaling() * 36.f);
ImGui::InputFloat("max", &mod_max, 0.0f, 0.0f, format.c_str(), 
    ImGuiInputTextFlags_CharsDecimal | ImGuiInputTextFlags_DefaultZero | ImGuiInputTextFlags_EmptyZero);

?
DataTypeApplyOpFromText is only used by InputScalar? It seems so...
But even if it's only 25 lines, it need to be its own function, as it's called from two location in InputScalar.
Maybe passing the ImGuiInputTextFlags to DataTypeApplyOpFromText is good enough?

(PS: 1.83 will soon be 3 years ago, and is 2000+ lines down in the changelog, I would suggest updating :)

I'm forking an existing project, so I tend to not update the library before the main branch do it.

Edit: also, is it possible to pass the default value (here 0) via ImGuiNextItemData ?

@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

something like

I don't fancy this a lot and the names would need to be more self-explanatory, but at least if you go this way your patch will touch much less code and will be easier to merge.

DataTypeApplyOpFromText is only used by InputScalar? It seems so...
But even if it's only 25 lines, it need to be its own function, as it's called from two location in InputScalar.

You don't need to call it if the buffer is empty, it makes sense to apply your logic in the caller code.
(btw it is called in DataTypeApplyFromText() in current version)
And you likely don't need the +/- handling of InputScalar() anyhow, that's why your custom version of InputScalar() would become 25 lines.

Edit: also, is it possible to pass the default value (here 0) via ImGuiNextItemData ?

In theory yes by storing it into an opaque ImGuiDataType + ImGuiDataTypeTempStorage and adding the corresponding _HasFlag, but I need you can solve your need with less changes by creating your copy of InputScalar().

supermerill added a commit to supermerill/imgui that referenced this issue Feb 12, 2024
supermerill added a commit to supermerill/imgui that referenced this issue Feb 12, 2024
@supermerill
Copy link
Author

something like that so?
master...supermerill:imgui:patch-1

I'm not really sure of what I'll be using. maybe I will need another default value than 0 (like FLOAT_MAX or -1, and i set any invalid value to that). So I'll wait to let it settle.

@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

something like that so?

Yes, but I think your IsZero() and SetZero() functions are overkill, you may use a memcmp/memset based on DataTypeGetInfo(type)->Size.

Now your patch becomes short and easy to maintain.

@ocornut
Copy link
Owner

ocornut commented Jun 6, 2024

I am going to try to work on finishing that.
Thanks for posting the patch, I believe I can simplify the code.

I believe two separate flags are preferable, as in some instances user may want to parse empty as zero/default but still display the editable value.

IMHO the issue with your previous version was the names were a little ambiguous. My answer was also ambiguous ("I don't fancy this a lot and the names would...") I believe I may have intended to suggest that only the names was as a problem (maybe I mistyped "I don't fancy this a lot AS the names would" ?), because presently I don't see an issue with the idea itself.

First suggestion:

ImGuiInputTextFlags_ParseEmptyAsZero
ImGuiInputTextFlags_DisplayZeroAsEmpty

or preferably

ImGuiInputTextFlags_ParseEmptyAsDefault
ImGuiInputTextFlags_DisplayDefaultAsEmpty

I also considered:

ImGuiInputTextFlags_ParseEmptyAsRefValue
ImGuiInputTextFlags_DisplayRefValueAsEmpty

But somehow "Default" seems simpler.

It won't be costly to add a new field in NextItemData, so even if we don't expose it publicly you may use it. This kind of advanced use is usually used in high-level functions and easy to wrap anyhow. Therefore it seems sane to use the 'Default' terminology rather than 'Zero'.

Note that coincidentally as your last patch uses 'Default' as a verb (ImGuiInputTextFlags_EmptyDefaultToZero), writing the name down next to the above suggestions is unusually confusing :)

@ocornut
Copy link
Owner

ocornut commented Jun 6, 2024

I realize one ambiguity for the user would be to parse an empty field as 0.0/default, but not update underlying value on "invalid" input. DataTypeApplyFromText() has three outputs: empty or all blanks, parse success, parse failure.
I am not yet sure what is the best design here.

ocornut added a commit that referenced this issue Jun 6, 2024
…tyRefVal, ImGuiInputTextFlags_DisplayEmptyRefVal. (#7305)
@ocornut
Copy link
Owner

ocornut commented Jun 6, 2024

I went back and forth and ended up calling them: ImGuiInputTextFlags_ParseEmptyRefVal and ImGuiInputTextFlags_DisplayEmptyRefVal.

I also added the necessary data in NextItemData, but there's no public static-typed accessor for it.
You can do:

float my_float = 42.0f;
ImGui::SetNextItemRefVal(ImGuiDataType_Float, &my_float);

@ocornut ocornut closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants