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

Support for variety of data types (32/64 bits, signed/unsigned, float/double) #643

Closed
josiahmanson opened this issue May 10, 2016 · 8 comments

Comments

@josiahmanson
Copy link
Contributor

This is maybe not very common, but I want to control the value of doubles. It would be nice if there was a function to do that directly, or if SliderFloat was templated based on the variable type. Likewise, the same argument could be made for the integral controls. What if I need to set a value that is larger than 2 or 4 billion? If I were to support one integral or float type, I would support the widest native type available and let the user deal with conversion to narrow types themselves.

@ocornut
Copy link
Owner

ocornut commented May 10, 2016

It is possible but would require a bit of work/refactor.
We won't expose template to the public API but that's only a minor issue, the issue is to refactor the internals really. Similarly to how InputFloat/InputInt now go through a path that doesn't pass the value through a float.

@ocornut ocornut changed the title ImGui::SliderFloat() on doubles (or wider types in general) SliderFloat on doubles (or wider types in general) May 11, 2016
@thennequin
Copy link

I got this situation and i made a templated function, theoricly it works for all numeric type if you use the right format expression (eg: %f)
https://gist.github.com/thennequin/c75e1860b63e694ec918

@stephenberry
Copy link

I would really appreciate this kind of templated functionality. I write engineering code that requires the use of doubles and right now I’ve been casting and copying back and forth between doubles and floats, since doubles aren't supported, but this would let me avoid that. Thanks!

ocornut added a commit that referenced this issue May 3, 2018
ocornut added a commit that referenced this issue May 3, 2018
…ScalarN(), removed InputFloatN(), InputInt(). Note that DragInt2/3/4 will %f format strings will currently be broken. (#320, #643, #708, #1011)
ocornut added a commit that referenced this issue May 3, 2018
ocornut added a commit that referenced this issue May 4, 2018
…h various ranges/limits. Note that Drag/Slider/Input currently fail if the format string doesn't preview the actual value. Will fix next. (#320, #643, #708, #1011)
@ocornut ocornut changed the title SliderFloat on doubles (or wider types in general) Support for double and variety of types S64 (long long), U64 (unsigned long long), etc. May 4, 2018
@ocornut
Copy link
Owner

ocornut commented May 4, 2018

Hello all,

I have pushed a data_types branch which include function to manipulate variety of data types.

image

enum ImGuiDataType_
{
    ImGuiDataType_S32,      // int
    ImGuiDataType_U32,      // unsigned int
    ImGuiDataType_S64,      // long long, __int64
    ImGuiDataType_U64,      // unsigned long long, unsigned __int64
    ImGuiDataType_Float,    // float
    ImGuiDataType_Double,   // double
    ImGuiDataType_COUNT
};

The functions are:

bool DragScalar(const char* label, ImGuiDataType data_type, void* v, float v_speed, const void* v_min, const void* v_max, const char* format = NULL, float power = 1.0f);
bool DragScalarN(const char* label, ImGuiDataType data_type, void* v, int components, float v_speed, const void* v_min, const void* v_max, const char* format = NULL, float power = 1.0f);
     
bool InputScalar(const char* label, ImGuiDataType data_type, void* v, const void* step, const void* step_fast, const char* format = NULL, ImGuiInputTextFlags extra_flags = 0);
bool InputScalarN(const char* label, ImGuiDataType data_type, void* v, int components, const void* step, const void* step_fast, const char* format = NULL, ImGuiInputTextFlags extra_flags = 0);

bool SliderScalar(const char* label, ImGuiDataType data_type, void* v, const void* v_min, const void* v_max, const char* format = NULL, float power = 1.0f);
bool SliderScalarN(const char* label, ImGuiDataType data_type, void* v, int components, const void* v_min, const void* v_max, const char* format = NULL, float power = 1.0f);
bool VSliderScalar(const char* label, const ImVec2& size, ImGuiDataType data_type, void* v, const void* v_min, const void* v_max, const char* format = NULL, float power = 1.0f);

Also recently added

bool InputDouble(const char* label, double* v, double step = 0.0f, double step_fast = 0.0f, const char* format = "%.6f", ImGuiInputTextFlags extra_flags = 0);

I've tested variety of cases including high-range, decorated format strings, power curves, etc. but there's most certainly a bug waiting to be found in there. The internal template code somehow manage to handle things like 64-bit integers and double so it's a bit messy.

One of the limitation I don't think I will try to lift is that Sliders typically support half of the maximum data type range. So e.g. a signed long long slider will work from -LLONG_MIN/2 to .. +LLONG_MAX/2 but above that depending on the exact min/max value you may run into overflows. This issue doesn't affect Drags and Inputs widgets, and Sliders are not really suited to manipulating enormous ranges anyway so I don't think it's so much a problem. (whereas fixing it would require more code specialization for each data type)

Even if you don't use those data types yet, testing the branch would be useful as the code changes are affecting all types.

(1)
Those API are taking all the values (including min/max) by pointer in order to avoid exposing the template in the public space. This make their casual usage a little more tedious, but you can create yourself a 1-line wrapper if you frequently use other types.

For example, this is SliderFloat now:

bool ImGui::SliderFloat(const char* label, float* v, float v_min, float v_max, const char* format, float power)
{
    return SliderScalar(label, ImGuiDataType_Float, v, &v_min, &v_max, format, power);
}

The wrapper turns v_min/v_max on the stack into pointers, this is why it is useful to have a 1-line wrapper here. May end up adding them for more types...

(2)
I've made variety of fixes (in the branch and in master before branching) related to handling of %g and %e formats for floating point values.

(3)
DragInt() and SliderInt() used to require default format string of "%.0f" because internally they used floating point values, which also lead to issues with large values. This is now fixed and the default format is "%d". If you used decorated format strings, the code will automatically automagically parse and patch your format string to replace %*f with %d while preserving the leading and trailing decoration (no need to tell you this is pretty ugly stuff). However if you use #define IMGUI_DISABLE_OBSOLETE_FUNCTIONS then the code will assert if you pass a %f format to DragInt() or SliderInt(), and I expect to completely obsolete this down the line.

(4)
Haven't added the demo code yet.

(5)
Question: I am currently using the XxxScalar() naming convention as it was how the internal functions were named, but I'm not wondering if we should rename them. E.g. DragAny() instead of DragScalar() ? DragData() ?

Feedback very welcome. If this work well enough, after some tweaks/polish I could package that into v1.61.

Thanks!

EDIT Various edits.

@ocornut ocornut changed the title Support for double and variety of types S64 (long long), U64 (unsigned long long), etc. Support for variety of data types (32/64 bits, signed/unsigned, float/double) May 4, 2018
ocornut added a commit that referenced this issue May 8, 2018
…to not use obsolete formats (when using IMGUI_DISABLE_OBSOLETE_FUNCTIONS) (#643)
@amc522
Copy link
Contributor

amc522 commented May 10, 2018

Just grabbed this branch and tried it out in my project. First off, this is an excellent change. I've had my own hacked together ImGui code to support un/signed integer types for a while now. It was always a pain to merge with ImGui updates. Just going to address the points you made above in order

  1. First thing I did was create wrappers for DragScalar, DragScalarN, SliderScalar, SliderScalarN which allowed me to use the type system of C++ better. I know this library is mostly trying to keep C style compatibility, but it would be nice to just use types instead of having to use an enum.
  2. Cool, fixes look good and I haven't run into any issues yet
  3. Better formatting is always better 👍
  4. If this becomes official, demo code would be nice
  5. I think XxxScalar is an appropriate name. XxxAny or XxxData would misrepresent the functions. There's only a limited number of data types that can be used right now. If the exposed functions were templated code, and you really could use any type that behaved like a scalar, that would be a different story. That's what I would really like to see, but I know it probably won't happen.

So overall, I'm a fan of this change. Allowed me to get rid of my own hacky code.

Here's the boilerplate wrappers I ended up writing. It would be nice to just have the strong typed functions already provided.

bool DragScalar(const char *label, uint32_t &value, float speed, uint32_t min, uint32_t max) {
    return DragScalar(label, ImGuiDataType_U32, &value, speed, &min, &max, "%u");
}

bool DragScalar(const char *label, int32_t &value, float speed, int32_t min, int32_t max) {
    return DragScalar(label, ImGuiDataType_S32, &value, speed, &min, &max, "%d");
}

bool DragScalar(const char *label, uint64_t &value, float speed, uint64_t min, uint64_t max) {
    return DragScalar(label, ImGuiDataType_U64, &value, speed, &min, &max, "%llu");
}

bool DragScalar(const char *label, int64_t &value, float speed, int64_t min, int64_t max) {
    return DragScalar(label, ImGuiDataType_S64, &value, speed, &min, &max, "%lld");
}

bool DragScalar(const char *label, float &value, float speed, float min, float max) {
    return DragScalar(label, ImGuiDataType_Float, &value, speed, &min, &max, "%f");
}
bool DragScalar(const char *label, double &value, double speed, double min, double max) {
    return DragScalar(label, ImGuiDataType_Double, &value, speed, &min, &max, "%f");
}

bool DragScalarN(const char *label, uint32_t *value, float speed, int components, uint32_t min, uint32_t max) {
    return DragScalarN(label, ImGuiDataType_U32, value, components, speed, &min, &max, "%u");
}

bool DragScalarN(const char *label, int32_t *value, float speed, int components, int32_t min, int32_t max) {
    return DragScalarN(label, ImGuiDataType_S32, value, components, speed, &min, &max, "%d");
}

bool DragScalarN(const char *label, uint64_t *value, float speed, int components, uint64_t min, uint64_t max) {
    return DragScalarN(label, ImGuiDataType_U64, value, components, speed, &min, &max, "%llu");
}

bool DragScalarN(const char *label, int64_t *value, float speed, int components, int64_t min, int64_t max) {
    return DragScalarN(label, ImGuiDataType_S64, value, components, speed, &min, &max, "%lld");
}

bool DragScalarN(const char *label, float *value, float speed, int components, float min, float max) {
    return DragScalarN(label, ImGuiDataType_Float, value, components, speed, &min, &max, "%f");
}

bool DragScalarN(const char *label, double *value, float speed, int components, double min, double max) {
    return DragScalarN(label, ImGuiDataType_Double, value, components, speed, &min, &max, "%f");
}

template<glm::length_t L, class T>
bool DragScalar(const char *label, glm::vec<L, T> &value, float speed, T min, T max) {
    return DragScalarN(label, glm::value_ptr(value), speed, L, min, max);
}

@ocornut
Copy link
Owner

ocornut commented May 11, 2018

Thanks for the feedback @amc522. This is indeed to be merged for 1.61, I was just waiting for more testers/feedback because I'm always worrying I may have broken things with this change.

Any reason why your DragScalar template are using references and not pointers? Pointers would be consistent with the rest of the imgui API.

The problem with current approach is that we can't add too many entry points to imgui.h, especially considering I will add S8/U8/S16/U16 to the type list. I know it is partly irrational but aside from making sure the core functionality are easy to convert to another language, I'd prefer not making imgui.h a template festival. Instead we can consider adding e.g. a misc/cpp/imgui_cpp.h helper file with C++ friendly helpers. Since the internal code can already fill a default format string for each type, presumably all you need is some template magic to turn a type into a ImGuiDataType ?

@amc522
Copy link
Contributor

amc522 commented May 11, 2018

Any reason why your DragScalar template are using references and not pointers?

Just my personal style preference. It was just wrappers in my code and I was not pushing for references in ImGui. I was just showing that I would be immediately wrapping the functions.

Instead we can consider adding e.g. a misc/cpp/imgui_cpp.h helper file with C++ friendly helpers.

That sounds great to me. My imgui_user.h/.cpp is pretty much just filled with C++ helpers and RAII wrappers.

Since the internal code can already fill a default format string for each type, presumably all you need is some template magic to turn a type into a ImGuiDataType ?

If you want, I can write up a function that does that for reference.

Thanks again for this change! Helps out a bunch.

ocornut added a commit that referenced this issue May 12, 2018
…eyboard/gamepad when speed < 1. Enforce min/max bounds when power curves are used. SliderScalar: Fixed integer/slow tweaking. (#643)
@ocornut
Copy link
Owner

ocornut commented Feb 4, 2019

Closing this (supported since 1.61).

@ocornut ocornut closed this as completed Feb 4, 2019
ocornut added a commit that referenced this issue Feb 27, 2019
…6 data types. We are reusing function instances for larger types to reduce code size. (#643, #320, #708, #1011)
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

5 participants