IMGUI cannot compile with _vectorcall, qsort requires functions pointers be marked _cdecl

[TrianglesPCT]

With MSVC If you change the default calling convention to _vectorcall IMGUI does not compile.

This is because it uses qsort which expects _cdecl.

To fix this, all the function pointers that get passed to qsort need to be explicitly marked as _cdecl.

You can set calling convention in C++->Advanced->calling converion to /Gv

[ocornut]

Hello,
Even if I fix it I see similar issues with mallloc, free, and inside stb_rectpack.h. Did you fix those? I don't know how malloc/free can be fixed.
Do you know how to define a portable CDECL macro?
Why not using standard calling convention for the imgui code? (curious)
A pull-request would be appreciated.

// Force declare a function to use C calling-convention, in order to fix functions types passed to stdlib (e.g. qsort) when default compiler calling-convention is modified
#ifdef _MSC_VER
#define IM_CDECL __cdecl
#else
#define IM_CDECL
#endif

static int IM_CDECL ChildWindowComparer(const void* lhs, const void* rhs)

[TrianglesPCT]

hi ocornut, I'll try and put together a pull request in the next few days.

Yes I also fixed the lack of _cdecl in stb_rect_pack, since it was also calling qsort.

I switched all of my code to _vectorcall on MSVC, I'd prefer to have one consistent calling convention--well except in cases like qsort where I have no choice but cdecl-- _vectorcall is newer and uses SIMD registers etc.

If I make the imgui project cdecl, any projects that are _vectorcall fail to link with it, because they are looking for _vectorcall not cdecl.

Your IM_CDECL macro should work I think, I'll use that when I put together the pull request

[ocornut]

@TrianglesPCT Any news on this?

• I don't know how you fixed malloc/free. I think I will however change the API and just rely on a macro to redefine them, because storing the function pointers inside the context is causing other problems.

• How about the non _MSC_VER path do we have a repro/fix for other compilers/std librairies or shall we just leave it for MSVC at the moment?

• For the stb_xxx part I can also merge it but please also submit a PR to nothings/ repo so it doesn't get lost.

[RandyGaul]

Just popping in. I only had a couple spots in stb_rect_pack and imgui.cpp. From rect pack, lines 512 and 523 need a __cdecl. From imgui.cpp these lines needed __cdecl as well:

static int ChildWindowComparer(const void* lhs, const void* rhs)

and

static int PairCompareByID(const void* lhs, const void* rhs) 

I was compiling with __vectorcall. malloc and free do not need any special handling. Used MSCV 2017. Worked like a charm after fixing up the qsort predicates with __cdecl.

[ocornut]

Hello @RandyGaul would you mind making a PR or pointing to a commit that has those changes if you already made it, so I don’t miss any?

[RandyGaul]

@ocornut OK give me a few minutes, I'll make an attempt at PR... I'm new to git still, so if I fail I'll just come back here and let you know :)

[ocornut]

PR is basically a pointer to a branch, so you can fork/branch and cherry-pick this commit into it then use github interface to make that branch a PR. If you have the commit in any public branch, PR or not I can cherry-pick the commit myself.

[ocornut]

Merged with minor tweaks (comments, removed tab), thanks! The reason I was sitting on this was because I wasn't sure what to do with stb_rectpack, if STBRP_SORT is redefined in theory our change isn't as required or may be problematic. In practice we are the problematic the only user of the stb_rectpack.h in imgui/ folder and occasional external users won't have this issue.

[ocornut]

For the record, I discovered that non-capturing lambdas are automatically promoted to the right calling convention (at least in visual studio).
From https://devblogs.microsoft.com/oldnewthing/20150220-00/?p=44623

So whereas this (commenting IMGUI_CDECL) does NOT work with /Gv:

static int /*IMGUI_CDECL*/ PairComparerByID(const void* lhs, const void* rhs)
{
    // We can't just do a subtraction because qsort uses signed integers and subtracting our ID doesn't play well with that.
    ImGuiID lhs_v = ((const ImGuiStoragePair*)lhs)->key;
    ImGuiID rhs_v = ((const ImGuiStoragePair*)rhs)->key;
    return (lhs_v > rhs_v ? +1 : lhs_v < rhs_v ? -1 : 0);
}

// For quicker full rebuild of a storage (instead of an incremental one), you may add all your contents and then sort once.
void ImGuiStorage::BuildSortByKey()
{
    ImQsort(Data.Data, (size_t)Data.Size, sizeof(ImGuiStoragePair), PairComparerByID);
}

This does work:

static auto PairComparerByID = [](const void* lhs, const void* rhs)
{
    // We can't just do a subtraction because qsort uses signed integers and subtracting our ID doesn't play well with that.
    ImGuiID lhs_v = ((const ImGuiStoragePair*)lhs)->key;
    ImGuiID rhs_v = ((const ImGuiStoragePair*)rhs)->key;
    return (lhs_v > rhs_v ? +1 : lhs_v < rhs_v ? -1 : 0);
};

// For quicker full rebuild of a storage (instead of an incremental one), you may add all your contents and then sort once.
void ImGuiStorage::BuildSortByKey()
{
    ImQsort(Data.Data, (size_t)Data.Size, sizeof(ImGuiStoragePair), PairComparerByID);
}

Also linking to #904.

I considered using this property to tidy up some code but it didn't seem it was worth it. But I though I would post the information!