Compilation with IMGUI_DISABLE_OBSOLETE_FUNCTIONS
Closed this issue · 12 comments
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.
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.
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).
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.
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?
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 : ocornut/imgui#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).
I saw in this thread that you were interested in enforcing this flag last year : #234
But maybe this is just for when you decide to actively upgrade the minimum imgui version.
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.
I just pushed a PR with those modifications : #302
That way you can have a clear view on the impacted API.
Feel free to ask if I can help with something !
The PR as-is includes the fixes for several imgui versions (up to v1.91.1), but if you prefer I can provide a dedicated PR for the current version officially supported by imgui-sfml (v1.89), and eventually provide a dedicated PR for the other versions.
(v1.90 is one year old, but 1.91.1 only one month old, so forcing the upgrade might be too soon).
I would personally recommend to at least integrate the fixes for v1.89, in order to have imgui-sfml buildable with and without the IMGUI_DISABLE_OBSOLETE_FUNCTIONS flag (maybe this could be enforced on the CI checks when upgrading the minimum supported version ?).
but if you prefer I can provide a dedicated PR for the current version officially supported by imgui-sfml (v1.89)
If we're already using functions deprecated in 1.89 then let's definitely remove those. That's an easy PR for me to approve.
(maybe this could be enforced on the CI checks when upgrading the minimum supported version ?).
Agreed. No need to use functions already made obsolete in the minimum version of ImGui we're requiring.
I just added the dedicated PR with the fixes for imgui 1.89.
I will not be able to provide/test a PR that would include the deprecation flag to the CI, but if you can provide me with the required changes I can add it to my PR.
If you are willing to upgrade the minimum imgui version, I can provide the PR for imgui 1.90, it should be compatible for all versions ranging from 1.90 to 1.91.0.
Starting 1.91.1 it will need the last part of my initial PR.
(for reference, 1.89 is 2y old, 1.90 is 1y old, and 1.91.1 is 2 month old)
Now that CI builds with this flag then I think we can close this issue. Feel free to propose a higher ImGui requirement for ImGui-SFML. I'm open to that.