-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[geogram] Fix imgui not downloading #32487
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| diff --git a/src/lib/geogram_gfx/CMakeLists.txt b/src/lib/geogram_gfx/CMakeLists.txt | ||
| index 195b1e2..75bc931 100644 | ||
| --- a/src/lib/geogram_gfx/CMakeLists.txt | ||
| +++ b/src/lib/geogram_gfx/CMakeLists.txt | ||
| @@ -26,8 +26,8 @@ endif() | ||
|
|
||
| add_library(geogram_gfx ${SOURCES} $<TARGET_OBJECTS:geogram_gfx_third_party>) | ||
|
|
||
| -target_include_directories(geogram_gfx PUBLIC ${PROJECT_SOURCE_DIR}/src/lib/geogram_gfx/third_party) | ||
| -target_include_directories(geogram_gfx PRIVATE ${PROJECT_SOURCE_DIR}/src/lib/geogram_gfx/third_party/imgui) | ||
| +target_include_directories(geogram_gfx PUBLIC $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src/lib/geogram_gfx/third_party>) | ||
| +target_include_directories(geogram_gfx PRIVATE $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src/lib/geogram_gfx/third_party/imgui>) | ||
|
|
||
| if(ANDROID) | ||
| target_include_directories(geogram_gfx PRIVATE | ||
| diff --git a/src/lib/geogram_gfx/third_party/CMakeLists.txt b/src/lib/geogram_gfx/third_party/CMakeLists.txt | ||
| index ef97a11..b0ab769 100644 | ||
| --- a/src/lib/geogram_gfx/third_party/CMakeLists.txt | ||
| +++ b/src/lib/geogram_gfx/third_party/CMakeLists.txt | ||
| @@ -55,6 +55,8 @@ set_target_properties( | ||
| geogram_gfx_third_party PROPERTIES | ||
| FOLDER "GEOGRAM" | ||
| ) | ||
| +find_package(glfw3 CONFIG REQUIRED) | ||
| +target_link_libraries(geogram_gfx_third_party PRIVATE glfw) | ||
|
|
||
| if(ANDROID) | ||
| target_include_directories(geogram_gfx_third_party PRIVATE | ||
| diff --git a/src/lib/geogram_gfx/third_party/ImGuiColorTextEdit/TextEditor.cpp b/src/lib/geogram_gfx/third_party/ImGuiColorTextEdit/TextEditor.cpp | ||
| index a00355b..227c084 100644 | ||
| --- a/src/lib/geogram_gfx/third_party/ImGuiColorTextEdit/TextEditor.cpp | ||
| +++ b/src/lib/geogram_gfx/third_party/ImGuiColorTextEdit/TextEditor.cpp | ||
| @@ -18,11 +18,7 @@ | ||
|
|
||
| // [Bruno Levy] includes for GLFW, needed by pixelratio() | ||
| #ifndef __ANDROID__ | ||
| -#ifdef GEO_USE_SYSTEM_GLFW3 | ||
| #include <GLFW/glfw3.h> | ||
| -#else | ||
| -#include <third_party/glfw/include/GLFW/glfw3.h> | ||
| -#endif | ||
| #endif | ||
|
|
||
| // [Bruno Levy] | ||
| @@ -482,7 +478,6 @@ void TextEditor::Render(const char* aTitle, const ImVec2& aSize, bool aBorder) | ||
| //[Bruno Levy] added 'NoNav' flag | ||
| ImGui::BeginChild(aTitle, aSize, aBorder, ImGuiWindowFlags_HorizontalScrollbar | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoNav); | ||
|
|
||
| - ImGui::PushAllowKeyboardFocus(true); | ||
|
|
||
| auto shift = io.KeyShift; | ||
| auto ctrl = io.KeyCtrl; | ||
| @@ -850,7 +845,6 @@ void TextEditor::Render(const char* aTitle, const ImVec2& aSize, bool aBorder) | ||
| mScrollToCursor = false; | ||
| } | ||
|
|
||
| - ImGui::PopAllowKeyboardFocus(); | ||
| ImGui::EndChild(); | ||
| ImGui::PopStyleVar(); | ||
| // [Bruno Levy] Commented out because I'd rather use the default style. | ||
| diff --git a/src/lib/geogram_gfx/third_party/imgui_lua_bindings/imgui_iterator.h b/src/lib/geogram_gfx/third_party/imgui_lua_bindings/imgui_iterator.h | ||
| index 8f923fd..5e3c441 100644 | ||
| --- a/src/lib/geogram_gfx/third_party/imgui_lua_bindings/imgui_iterator.h | ||
| +++ b/src/lib/geogram_gfx/third_party/imgui_lua_bindings/imgui_iterator.h | ||
| @@ -380,14 +380,14 @@ IMGUI_FUNCTION(PopTextWrapPos) | ||
| CALL_FUNCTION_NO_RET(PopTextWrapPos) | ||
| END_IMGUI_FUNC | ||
| // IMGUI_API void PushAllowKeyboardFocus(bool allow_keyboard_focus); // allow focusing using TAB/Shift-TAB, enabled by default but you can disable it for certain widgets | ||
| -IMGUI_FUNCTION(PushAllowKeyboardFocus) | ||
| +/*IMGUI_FUNCTION(PushAllowKeyboardFocus) | ||
| BOOL_ARG(allow_keyboard_focus) | ||
| CALL_FUNCTION_NO_RET(PushAllowKeyboardFocus, allow_keyboard_focus) | ||
| END_IMGUI_FUNC | ||
| // IMGUI_API void PopAllowKeyboardFocus(); | ||
| IMGUI_FUNCTION(PopAllowKeyboardFocus) | ||
| CALL_FUNCTION_NO_RET(PopAllowKeyboardFocus) | ||
| -END_IMGUI_FUNC | ||
| +END_IMGUI_FUNC*/ | ||
| // IMGUI_API void PushButtonRepeat(bool repeat); // in 'repeat' mode, Button*() functions return repeated true in a typematic manner (using io.KeyRepeatDelay/io.KeyRepeatRate setting). Note that you can call IsItemActive() after any Button() to tell if the button is held in the current frame. | ||
| IMGUI_FUNCTION(PushButtonRepeat) | ||
| BOOL_ARG(repeat) |
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imgui is a port so vendoring it like this isn't OK. Please use the vcpkg port instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it looks like this is only necessary with the graphics feature turned on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
imguiis only called when the graphics feature is compiled. Theimguiversion used for gram is version 1.89.4. Theimguiversion invcpkgis 1.89.6. Theimguiinvcpkgdoes not defineImGuiWindowFlags_NoDockingImGuiCol_DockingPreviewImGuiCol_DockingEmptyBg, but it is compiled ingeogramused in . So I am not sure whether to wait for the upstream to update to the latestimguito modify, or to modify according to the currently used version 1.89.4.I raised an issue 89 upstream to ask the author if he wants to update imgui.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for following up with upstream!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upstream will modify the differences between these different versions in the next version, so we need to wait for the upstream to adapt and update the version or use the current patch to fix the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the cmakelists.txt under the
src\lib\geogram_gfx\third_partydirectory, part of the code ofimguiwill be compiled, andimguiis a network link when thegeogramcode is downloaded, if it is not downloaded separately, it will report that there is no error in the compiled cpp file. So the download process ofimguicode is added in the file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version of
imguiused ingeogramis a specific branchdocking branch, that has additional functionalities (and the constantsImGuiWindowFlags_NoDocking,ImGuiCol_DockingPreview,ImGuiCol_DockingEmptyBg). If you update thevcpkgwrapper to downloadimgui, make sure you get thedocking branch(git clone --branch docking https://github.com/ocornut/imgui.git)If you tell me that having a dependency to a specific branch is a problem in
vcpkg, I can detect which version ofimguiis used and deactivate some features at compile time (but it is not ideal: user experience is much better with the additional functionalities given by thedockingbranch)-- Bruno (main author/maintainer of
geogram)P.S. If there is anything I can do to help packaging in the next versions of
geogram(including reorganizing a bit) do not hesitate to file issues ingeogramgithub. (I have seen what you have done forBLASandLAPACK, awesome !