Allow packaged imgui to be used (and allow newer version of this library #657
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
coldtobi
changed the title
coldtobi
changed the title
…ary.) The changes in CMakeList.txt allows to avoid the bundled imgui, as for Debian this is a policy*. This is via the USE_SYSTEM_IMGUI option, defaulting to OFF. The only change I see to imgui.cpp is that there is an assertion commented out, in ImGui::End() -- this triggers if there is a mismatched number of counts to ImGui::Begin() and End(), howeever looking over the code, End() is only called in one location (and another in the Editor) and it matches a Begin() call, so it should not trigger. (Tested also with com_showFPS = 2) The change requires some include directives changes from ''libs/imgui/imgui.h'' to ''imgui/imgui.h'', but this is ensured in CMakeList.txt that it will be transparent whether its the system one or bundled one. The change in BFGimguiImpl.cpp is recommended with any imgui > 1.60, required with > 1.80: ''io.RenderDrawListsFn'' has been deprecated in 1.60 and subsequently removed in 1.80: Code should call their backend render function directly after ImGui::Render() themselves. Strictly, the version guard is not required, as the bundled version is newer than 1.60; I've just added it for clarity; let me know if you prefer me to remove the guard. Upstream details on this topic: ocornut/imgui#1599 and the Changelog entry: https://github.com/ocornut/imgui/blob/dfbe938e54858b8b443ca2981d037cc2f35cc931/docs/CHANGELOG.txt#L821 Cheers, tobi
|
@coldtobi can't we just bump the bundled version to latest instead? |
@runlevel5 Well, Debian has policies against embedded code copies. So basically, I need this patch applied.. Either by carrying it in the Debian packaging repository or (my preferred way) by sending it here.... This is also true for other distribution; e.g Fedora (also in the packaging guidelines ) and Gentoo have similar policies... Good essay on the topic why this is not only of benefits for pacakgers.., |
|
Thanks for the explanation. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes in CMakeList.txt allows to avoid the bundled imgui, as for Debian
this is a policy*. This is via the USE_SYSTEM_IMGUI option, defaulting to OFF.
The only change I see to imgui.cpp is that there is an assertion commented out,
in ImGui::End() -- this triggers if there is a mismatched number of counts to
ImGui::Begin() and End(), howeever looking over the code, End() is only called
in one location (and another in the Editor) and it matches a Begin() call, so it
should not trigger. (Tested also with com_showFPS = 2)
The change requires some include directives changes from ''libs/imgui/imgui.h''
to ''imgui/imgui.h'', but this is ensured in CMakeList.txt that it will be
transparent whether its the system one or bundled one.
The change in BFGimguiImpl.cpp is recommended with any imgui > 1.60, required with > 1.80:
''io.RenderDrawListsFn'' has been deprecated in 1.60 and subsequently removed
in 1.80: Code should call their backend render function directly after ImGui::Render()
themselves.
Strictly, the version guard is not required, as the bundled version
is newer than 1.60; I've just added it for clarity; let me know if you prefer
me to remove the guard. Upstream details on this topic:
ocornut/imgui#1599 and the Changelog entry:
https://github.com/ocornut/imgui/blob/dfbe938e54858b8b443ca2981d037cc2f35cc931/docs/CHANGELOG.txt#L821
Cheers,
tobi