Some data conversion functions could be constexpr

[alien-brother]

Version/Branch of Dear ImGui:

Version 1.91.5, Branch: master

Back-ends:

All

Compiler, OS:

All

Full config/build information:

Not config dependant

Details:

My Issue/Question:

It could make sense for some data conversion functions to be constexpr. The one in particular I'm using is ColorConvertFloat4ToU32(), but there are a few similar ones. This would make these functions usable in C++11/17/20 code that uses constexpr or constinit. Currently, the code of these functions may need to be copy-pasted into client code.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

Since ImGui APIs use either ImVec4 or ImU32, a declaration like this makes sense:

constexpr ImVec4 ERROR_COLOR = ImVec4(/* ... */);
constexpr ImU32 ERROR_COLOR_U32 = ImGui::ColorConvertFloat4ToU32(ERROR_COLOR);

[ocornut]

Linking to #7662 I am not against making selected ones constexpr if there is a clear use/value, but see my comment there: https://github.com/ocornut/imgui/pull/7662#issuecomment-2158016989

constexpr ImVec4 ERROR_COLOR = ImVec4(/* ... */);
constexpr ImU32 ERROR_COLOR_U32 = ImGui::ColorConvertFloat4ToU32(ERROR_COLOR);

It seems a little odd to me to do this in two steps, but I guess it aligns with what's discussed in #1360 and that it may be nice to have one definitive helper.

[ocornut]

It doesn't seem to be possible to implement this in the .cpp file, for good reasons,

So in the header file it would become, e.g.

#define IM_SATURATE(_VAL)    ((_VAL < 0.0f) ? 0.0f : (_VAL > 1.0f) ? 1.0f : _VAL)     // ImSaturate() for floats, as a macro

constexpr ImVec4 ImGui::ColorConvertU32ToFloat4(ImU32 in)
{
    return ImVec4(
        ((in >> IM_COL32_R_SHIFT) & 0xFF) * (1.0f / 255.0f),
        ((in >> IM_COL32_G_SHIFT) & 0xFF) * (1.0f / 255.0f),
        ((in >> IM_COL32_B_SHIFT) & 0xFF) * (1.0f / 255.0f),
        ((in >> IM_COL32_A_SHIFT) & 0xFF) * (1.0f / 255.0f));
}

constexpr ImU32 ImGui::ColorConvertFloat4ToU32(const ImVec4& in)
{
    return
        ((ImU32)(IM_SATURATE(in.x) * 255.0f + 0.5f) << IM_COL32_R_SHIFT) |
        ((ImU32)(IM_SATURATE(in.y) * 255.0f + 0.5f) << IM_COL32_G_SHIFT) |
        ((ImU32)(IM_SATURATE(in.z) * 255.0f + 0.5f) << IM_COL32_B_SHIFT) |
        ((ImU32)(IM_SATURATE(in.w) * 255.0f + 0.5f) << IM_COL32_A_SHIFT);
}

It bothers me a little to drag that cruft for little value. What value are you getting by making that value constexpr? That sorts of conversion is already called tens of thousands times a frame.

[alien-brother]

Yeah, if you don't want to define these functions in imgui.h, the suggestion is not applicable.

The value is just being able to use the functions when declaring constexpr or constinit variables. It's mostly a matter of style or principle (like, "no dynamic initialization").

[ocornut]

It has too much maintenance or cognitive cost for us to consider for what's a matter of style/principle, sorry.

But when we eventually rework color functions we'll consider it properly.