Compilation with IMGUI_DISABLE_OBSOLETE_FUNCTIONS

Legulysse commented 3 hours ago

Hi !

I am working on a project based on SFML 2.6.x, currently using ImGui 1.87 and ImGui-SFML 2.6.x. Everything compiles with IMGUI_DISABLE_OBSOLETE_FUNCTIONS.

I tried upgrading ImGui to its latest version, but there are compilation errors with imgui-sfml when using this flag, though it's mostly due to renames.

I took the time to fix the build locally and now I have a working build with the latest imgui release (v1.91.4) with IMGUI_DISABLE_OBSOLETE_FUNCTIONS flag active. The only problem with this is that depending on the ImGui version, building with those updates will break the build, and it would require to upgrade the minimum version supported (or use some version #define blocks). From my tests, it should compile when using v1.91.1 as minimum version.

Would you be interested in a PR including the code changes? If that is the case, what is the recommended course of action ? If I am not mistaken, I should fork, create a branch, push my commits, then propose a PR from my fork/fix-branch into this repo/2.6.x-branch ? I could also propose a dedicated branch if it needs some iterations/tests from others.

The problem is that I use premake instead of cmake, so I will not be able to update the cmake configuration tied to those modifications. I also dont have the ability to test that gamepad navigation is behaving correctly.

ChrisThrasher commented 2 hours ago

IMGUI_DISABLE_OBSOLETE_FUNCTIONS

What is the purpose of this option?

but there are compilation errors with imgui-sfml when using this flag, though it's mostly due to renames.

Will you please share the compilation errors?

What's with these API breaks within the same major version of ImGui? Does the library not use Semantic Versioning? We had to fix another API break just earlier this week.

Legulysse commented 2 hours ago

This option is used to disable all the deprecation workarounds provided by ImGui, when the library is making breaking API changes. The intention is to allow projects to use newer versions of ImGui without the need to immediately update their code to accommodate with those API changes, though there is no guaranty as to how long those workarounds will stay in the API.

This flag forces those workarounds away. It is recommended by the ImGui author to try compiling with this flag enforced from time to time to avoid being caught by surprise with too many breaking API changes.

I personally prefer to keep this flag active to force me to update things as soon as I upgrade my ImGui version. There is nothing mandatory with this though, and I could probably only apply this flag on my own codebase and ignore it when building imgui-sfml.

Here is a list of the things that are deprecated when using the flag :

setClipboardText/getClipboardText methods move from ImGuiIO to ImGuiPlatformIO, with a new parameter type.
IM_OFFSETOF() is removed in favor of the standard offsetof().
ImGuiKey_ModCtrl is replaced by ImGuiMod_Ctrl (alongside similar inputs).
ImGuiNavInput_Activate is replaced by ImGuiKey_GamepadFaceDown (alongside similar inputs).

Legulysse commented 2 hours ago

Something important to note : when compiling without this flag active, everything compiles normally with all versions. It is just that the compilation relies on redirections provided by ImGui. So it's mostly a matter of upgrading the code to match the latest version without relying on a deprecated API, or to ignore this and fix the API the day those redirections are removed.

EDIT: Another important thing to note is that updating the code to the latest ImGui API will reduce the range of supported ImGui versions, whereas compiling without this flag allows a broader range of API versions to coexist.

ChrisThrasher commented 2 hours ago

Has the ImGui maintainer gone on record about his attitude towards API breaks? His attitude seems to be that the version of the library is unrelated to the API and that the API is prone to break at any time. Is that accurate?

Legulysse commented 1 hour ago

My main incentive was this tweet of Omar : https://x.com/ocornut/status/1186357149598998530 There is a similar incentive in imgui.h line 3846 (v1.91.4-docking) :

// [SECTION] Obsolete functions and types
// (Will be removed! Read 'API BREAKING CHANGES' section in imgui.cpp for details)
// Please keep your copy of dear imgui up to date! Occasionally set '#define IMGUI_DISABLE_OBSOLETE_FUNCTIONS' in imconfig.h to stay ahead.

His main recommendation is "Consider enabling from time to time after updating to clean your code of obsolete function/names."

So yes, the API is mostly unrelated to the version when your are not enforcing this flag, with a few years of delay before the actual suppression of deprecated stuff, depending on what needs to be cleaned.

More importantly, there is a non-zero risk that mixing code built with and without this flag can create bugs due to memorylayouts changing : https://github.com/ocornut/imgui/issues/1695

So, either all parts of the codebase should use this flag, or none at all. Having the possibility to enforce it on imgui-sfml would be nice, but not mandatory (I could use a fork if I really want to enforce it on my projects).

Legulysse commented 1 hour ago

I saw in this thread that you were interested in enforcing this flag last year : https://github.com/SFML/imgui-sfml/issues/234 But maybe this is just for when you decide to actively upgrade the minimum imgui version.

ChrisThrasher commented 1 hour ago

If you want to open a PR for this I'll consider it. If ImGui is not API-stable then I suppose we're forced to some extent to periodically upgrade.

SFML / imgui-sfml

Compilation with IMGUI_DISABLE_OBSOLETE_FUNCTIONS #301