Upgrade to 1.92.1 #51
Conversation
- Add TextureRef and update related APIs - Set IMGUI_DISABLE_OBSOLETE_FUNCTIONS and remove usage of deprecated functions - Fixup deprecated API usages (callconv, alignment)
|
This is ready for review, but currently the build.zig.zon points to my forks for these changes: zig-gamedev/zsdl#24 and zig-gamedev/zgpu#18. I'll update the build.zig.zon once those are merged. |
|
I merged some of this into my own zgui fork so that I could test it against the zgpu changes I've been making. There are a few things that might have to be addressed later, but there was one thing I noticed which surely can be fixed right away - in zgui.cpp, there are some lingering usages of ImTextureID which I think should have been changed to ImTextureRef, but were maybe overlooked. I'll just link to the commit where I fixed them on my own fork to show you what I'm talking about: If these changes aren't made, using the affected functions will crash due to ABI mismatches. |
This commit might be directly incompatible with zig-gamedev#51
This commit might be directly incompatible with zig-gamedev#51
There was a problem hiding this comment.
Pull Request Overview
This pull request upgrades Dear ImGui from version 1.91 to 1.92.1 with accompanying API changes and deprecation fixes. The main focus is implementing the new texture reference system and removing deprecated function usage.
- Updates to use TextureRef instead of TextureIdent for improved type safety in the new texture management system
- Implementation of ImGui 1.92.1's new font loader architecture replacing the old font builder system
- Removal of deprecated Dear ImGui functions throughout the codebase
Reviewed Changes
Copilot reviewed 62 out of 66 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/zgui.cpp | Updates C bindings for new texture system and font APIs, removes deprecated window font scale |
| src/gui.zig | Replaces TextureIdent with TextureRef system, updates configuration types and alignment APIs |
| libs/node_editor/imgui_node_editor.cpp | Reverts workaround by re-adding GetKeyIndex calls for key handling |
| libs/implot/* | Updates texture handling and conditional compilation for IMGUI_DISABLE support |
| libs/imguizmo/* | Version bump and API updates for ImGuizmo compatibility |
| libs/imgui_test_engine/* | License headers added and compatibility fixes for new ImGui versions |
| libs/imgui/misc/freetype/* | Major refactor to new font loader architecture with TextureRef support |
Comments suppressed due to low confidence (1)
src/gui.zig:154
- Typo in field name: 'pading0' should be 'padding0' to match the corrected name on line 155.
dock_enable: bool = false,
| viewport_enable: bool = false, | ||
| _pading1: u3 = 0, | ||
| _padding1: u3 = 0, |
There was a problem hiding this comment.
Typo in field name: 'pading1' should be 'padding1' to match the corrected name on line 157.
Copilot uses AI. Check for mistakes.
| has_set_mouse_pos: bool = false, | ||
| renderer_has_vtx_offset: bool = false, | ||
| renderer_has_textures: bool = false, | ||
| _pading0: u5 = 0, |
There was a problem hiding this comment.
Typo in field name: 'pading0' should be 'padding0' for consistency.
| _pading0: u5 = 0, | |
| _padding0: u5 = 0, |
Copilot uses AI. Check for mistakes.
| candidateAction = Paste; | ||
| if (io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(GetKeyIndexForD())) | ||
| candidateAction = Duplicate; | ||
| if (!io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGuiKey_Space)) | ||
| if (!io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_Space))) |
There was a problem hiding this comment.
GetKeyIndex() was removed in ImGui 1.87+ and IsKeyPressed() now takes ImGuiKey directly. This should be ImGui::IsKeyPressed(ImGuiKey_X) instead of ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_X)).
Copilot uses AI. Check for mistakes.
| candidateAction = Paste; | ||
| if (io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(GetKeyIndexForD())) | ||
| candidateAction = Duplicate; | ||
| if (!io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGuiKey_Space)) | ||
| if (!io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_Space))) |
There was a problem hiding this comment.
GetKeyIndex() was removed in ImGui 1.87+ and IsKeyPressed() now takes ImGuiKey directly. This should be ImGui::IsKeyPressed(ImGuiKey_C) instead of ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_C)).
Copilot uses AI. Check for mistakes.
| candidateAction = Copy; | ||
| if (io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGuiKey_V)) | ||
| if (io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_V))) |
There was a problem hiding this comment.
GetKeyIndex() was removed in ImGui 1.87+ and IsKeyPressed() now takes ImGuiKey directly. This should be ImGui::IsKeyPressed(ImGuiKey_V) instead of ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_V)).
Copilot uses AI. Check for mistakes.
| candidateAction = Paste; | ||
| if (io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(GetKeyIndexForD())) | ||
| candidateAction = Duplicate; | ||
| if (!io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGuiKey_Space)) | ||
| if (!io.KeyCtrl && !io.KeyShift && !io.KeyAlt && ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_Space))) |
There was a problem hiding this comment.
GetKeyIndex() was removed in ImGui 1.87+ and IsKeyPressed() now takes ImGuiKey directly. This should be ImGui::IsKeyPressed(ImGuiKey_Space) instead of ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_Space)).
Copilot uses AI. Check for mistakes.
| @@ -4954,8 +4953,7 @@ ed::EditorAction::AcceptResult ed::DeleteItemsAction::Accept(const Control& cont | |||
| return False; | |||
|
|
|||
| auto& io = ImGui::GetIO(); | |||
| // FIX(zig-gamedev): ImGui::GetKeyIndex was removed from imgui since imgui-node-editor v0.9.3 was released. | |||
| if (Editor->CanAcceptUserInput() && ImGui::IsKeyPressed(ImGuiKey_Delete) && Editor->AreShortcutsEnabled()) | |||
| if (Editor->CanAcceptUserInput() && ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_Delete)) && Editor->AreShortcutsEnabled()) | |||
There was a problem hiding this comment.
GetKeyIndex() was removed in ImGui 1.87+ and IsKeyPressed() now takes ImGuiKey directly. This should be ImGui::IsKeyPressed(ImGuiKey_Delete) instead of ImGui::IsKeyPressed(ImGui::GetKeyIndex(ImGuiKey_Delete)).
Copilot uses AI. Check for mistakes.
|
Re. ImTextureID and ImTextureRef: ocornut/imgui@0e6e876 |
|
I have another small issue to bring up - not fatal like an ABI mismatch or anything, but some of the new imgui backend code is triggering the memory leak / static memory warning in gui.zig when an application exits. It's a case of "guess when this static memory gets a destructor call" that everyone loves from C++. The static variable to blame is here in imgui_impl_glfw.cpp The memory check, for reference, is here And this is the change I've tentatively made to deal with it for my own code, but there may be a nicer way to do it that doesn't require patching the imgui code like that, not sure. This could probably be filed as a "don't care, won't fix," but I hate seeing warnings on exit even if they don't matter functionally, so I just wanted to note it here. |
Remaining TODOs: