IMGUI_DISABLE_OBSOLETE_FUNCTIONS does more than hiding obsoleted functions

rokups commented 1 month ago

Version/Branch of Dear ImGui:

Version 1.91, Branch: docking

Back-ends:

Not applicable

Compiler, OS:

Not applicable

Full config/build information:

No response

Details:

tl;dr; I would like to build Dear ImGui without IMGUI_DISABLE_OBSOLETE_FUNCTIONS and then build my project with IMGUI_DISABLE_OBSOLETE_FUNCTIONS safely, so dependencies i use can use obsoleted functions, while my own code should be upgraded to not use obsoleted functions.

My project has several dependencies that implement features using Dear ImGui. These projects have varying activity and thus most of the time they depend on obsolete functions, therefore i can not build Dear ImGui with IMGUI_DISABLE_OBSOLETE_FUNCTIONS defined. I still would like to avoid using obsolete functions, but i can not define IMGUI_DISABLE_OBSOLETE_FUNCTIONS in projects that link to Dear ImGui built without this flag, because this define does a little more than affecting function availability. For example, there is this bit of code:

#if defined(IMGUI_DISABLE_OBSOLETE_FUNCTIONS) && !defined(IMGUI_DISABLE_OBSOLETE_KEYIO)
#define IMGUI_DISABLE_OBSOLETE_KEYIO
#endif

If i defined IMGUI_DISABLE_OBSOLETE_FUNCTIONS without IMGUI_DISABLE_OBSOLETE_KEYIO then it would affect ImGuiIO structure size, and version check function now asserts. I could define IMGUI_DISABLE_OBSOLETE_KEYIO for Dear ImGui and avoid this specific issue, but this is a mine i stepped on, which took a while to debug and figure out.

More important detail is that IMGUI_DISABLE_OBSOLETE_FUNCTIONS affects code behavior as well. There are multiple places in code itself, which customize behavior depending on whether IMGUI_DISABLE_OBSOLETE_FUNCTIONS is defined or not. I do not think i am impacted by those places, however this seems suspicious.

Please consider whether it is possible to enable use of IMGUI_DISABLE_OBSOLETE_FUNCTIONS the way i described. Thanks 🫡

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

ocornut commented 1 month ago

It’s been always the case that this flag can alter data layout, and the descriptions refers to “obsolete functions/enums/behaviors”. There were times when data layout wasn’t affected, times where it was.

Basically all the define needs to be consistent across compilation unit which is way they are usually in your imconfig file.

I don’t have a better solution unfortunately. I would suggest to update/fix dependencies to not use obsolete functions.

We however made a conscious decision to intentionally move the fields affected by data layout changes to end of the structure so they shouldn’t have any effects on code with mismatching defines, apart from the version check macro. So maybe disable the version check macros in dependencies is a solution.

ocornut commented 1 month ago

I'd also be interested in your point/link to dependencies which may rely on obsolete function/behaviors, as I sometime aim to submit fixes for some of them.

rokups commented 1 month ago

Those dependencies are:

ocornut commented 1 month ago

Shouldn't Tracy (assuming you are compiling the client) embed its own version of imgui ? It doesn't seem to make sense to compile such a big app with your own version of the lib.

I will investigate ImGuizmo, can you confirm it is specifically Imguizmo? as that repo has many sub-projects.

rokups commented 1 month ago

Tracy does indeed bundle its version of imgui, however we avoid dependency duplication in our project (all dependencies are in the main repository) and build is modified to use our own version of imgui. As a bonus, we have a reasonable appearance on HDPI monitors due to our custom patches. Tracy indeed has many projects, but relevant stuff is in tracy/profiler subdirectory and that part can be built in isolation, ignoring all the other stuff.

ocornut commented 4 weeks ago

I have submitted a PR for the fixes for ImGuizmo libraries here: https://github.com/CedricGuillemet/ImGuizmo/pull/334

Tracy does indeed bundle its version of imgui, however we avoid dependency duplication in our project (all dependencies are in the main repository) and build is modified to use our own version of imgui. As a bonus, we have a reasonable appearance on HDPI monitors due to our custom patches. Tracy indeed has many projects, but relevant stuff is in tracy/profiler subdirectory and that part can be built in isolation, ignoring all the other stuff.

I am sorry but I cannot support this on my end if that's your reason. I believe you are manufacturing your own problem by compiling Tracy not with the version it is shipping with - and for little value. What's to say that Tracy itself doesn't occasionally use custom patches? What's to say it's not using obsolete functions that would be completely removed from your copy of dear imgui? I don't know. If you need your own custom patches for the Tracy client consider applying them over Tracy.

As mentioned above, the struct layouts are designed to have the fields under ifdefs at the end, so if you have IMGUI_CHECKVERSION() macros called from compilation units with different imgui settings you may remove that check.

Let me know if there's anything else!

ocornut / imgui