feat: cmake + bring up to date with working commits#4
Conversation
- Replace dock menu arrow with hamburger icon - Set tab spacing to 0, center tab labels - Taller tab bar with 8px padding for docked windows - Simplified close/collapse button hover (no background rect) - Tab border: simple left-side vertical line - Opaque dock window menu popup Authored by Noah (noahxbowers@gmail.com)
…ingWindowNewFrame, which would break docking when the windowing system doesn't match the requested window position
premake5's xcode generator treats systemversion "latest" as a Windows SDK version and fails on macOS. Gate it behind a system:windows filter so non-Windows builds skip it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ry specific part of the tab itself
Extend the imported hazel-cmake CMakeLists to compile the ImPlot sources this lineage carries (implot.cpp, implot_items.cpp), mark the target position- independent (it links into the shared engine), and group it under Dependencies. SYSTEM include suppresses third-party header warnings. The <imgui/...> include root is a vendor-layout concern and stays in the consuming CMakeLists.
WalkthroughThis PR integrates ImPlot as a submodule with build support in CMake and premake, removes Wayland-specific viewport/DPI workarounds in the GLFW backend, adds a ConfigViewportsNoFloatingWindows option, reworks docking title bar/tab-bar/button rendering geometry, and updates FreeType's Lunasvg backend for v3+ multi-glyph rendering. ChangesBuild System and ImPlot Integration
Viewport, Docking, and Widget Rendering Updates
FreeType Lunasvg v3 Multi-Glyph Rendering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ImGui
participant PlatformIO
participant OS_Window
User->>ImGui: drag window title bar
ImGui->>PlatformIO: Platform_SetWindowPos(requested pos)
PlatformIO->>OS_Window: set position
ImGui->>PlatformIO: Platform_GetWindowPos()
PlatformIO-->>ImGui: actual OS position
ImGui->>ImGui: SetWindowPos(actual position)
ImGui->>ImGui: sync Viewport->Pos, LastPlatformPos from window->Pos
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)imgui_draw.cppast-grep timed out on this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
imgui_widgets.cpp (1)
10828-10828: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded black tab border ignores theme.
The border is drawn with a fixed
IM_COL32(0, 0, 0, 255)rather than a style color (e.g.ImGuiCol_Border). On light themes or custom palettes this black separator will look out of place. Consider sourcing the color from the style so it adapts.♻️ Suggested change
- draw_list->AddLine(ImVec2(bb.Min.x + 0.5f, y1), ImVec2(bb.Min.x + 0.5f, y2), IM_COL32(0, 0, 0, 255), g.Style.TabBorderSize); + draw_list->AddLine(ImVec2(bb.Min.x + 0.5f, y1), ImVec2(bb.Min.x + 0.5f, y2), GetColorU32(ImGuiCol_Border), g.Style.TabBorderSize);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@imgui_widgets.cpp` at line 10828, The tab border drawn in the tab bar rendering path uses a hardcoded black color, which bypasses the current theme. Update the border color source in the code that calls draw_list->AddLine in the tab drawing logic to use the style palette instead, such as the border color from the current ImGui style, so the separator adapts correctly across themes and custom palettes.CMakeLists.txt (1)
31-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePIC enabled unconditionally, unlike premake's Linux-only scoping.
Premake only sets
pic "On"underfilter "system:linux", but this CMake target setsPOSITION_INDEPENDENT_CODE ONfor all platforms. Functionally harmless (ignored on Windows, generally default-on on macOS), but worth aligning for consistency between the two build systems.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CMakeLists.txt` at line 31, Align the ImGui target’s PIC handling with premake by scoping the POSITION_INDEPENDENT_CODE setting to Linux only instead of applying it unconditionally. Update the CMake target configuration around set_target_properties for ImGui so it matches the existing Linux-only behavior used in the premake setup, keeping the FOLDER assignment unchanged.imgui.cpp (1)
8074-8075: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
8.0fpadding is duplicated across three coupled sites and won't scale with DPI/font scaling.The docked title-bar padding here must stay equal to
tab_padding_yinDockNodeCalcTabBarLayout(Line 20453) and the drag hit region inBeginDockableDragDropSource(Line 21999) — otherwise the docked title-bar height, tab-bar rect, and tear-off hit test diverge. Extract a single shared constant used by all three. Additionally, unlike theFramePadding.yit replaces (scaled viaScaleAllSizes), a bare8.0fdoes not scale withFontScaleDpi/style scaling, so docked tabs will look cramped on high-DPI displays — relevant given this PR removes Wayland content scaling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@imgui.cpp` around lines 8074 - 8075, The docked title-bar padding currently uses a hardcoded 8.0f that is duplicated across the title-bar height calculation, DockNodeCalcTabBarLayout, and BeginDockableDragDropSource, so the docked title-bar height, tab-bar rect, and drag hit test can drift apart. Replace these separate literals with one shared padding value used consistently by all three code paths, and make sure it scales with FontScaleDpi/style scaling like FramePadding.y does instead of remaining a fixed pixel value.misc/freetype/imgui_freetype.cpp (1)
648-659: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse the rendered bitmap’s stride here. A bulk
memcpyassumesrenderToBitmap()returns a tightly packedwidth * 4buffer; if it pads rows, this will blend padding into the glyph output. Copy per row usingrendered.stride()(and clamp the row count if needed).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@misc/freetype/imgui_freetype.cpp` around lines 648 - 659, The glyph blit path in imgui_freetype.cpp uses a single memcpy after lunasvg::Bitmap::renderToBitmap, which assumes tightly packed rows and can copy padding into the output. Update the state->glyphElement.renderToBitmap branch to copy row-by-row using the rendered bitmap’s stride() rather than a bulk buffer copy, and ensure the copied row count is bounded by slot->bitmap.rows so the output matches the destination bitmap layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 27-29: The target_compile_definitions call for ImGui is passing a
generator expression with space-separated macros, which CMake splits into broken
arguments before evaluation. Update the target_compile_definitions invocation
for ImGui so the $<$<CONFIG:Dist>:...> expression is quoted and the definitions
inside it are expressed as a semicolon-separated list, keeping the macros
grouped correctly.
In `@imgui.cpp`:
- Around line 5422-5443: The position sync in the moving-window path should not
rely on an immediate Platform_GetWindowPos/readback after Platform_SetWindowPos,
since GLFW can return a stale value and cause drag lag or jitter. Update the
logic in the moving_window viewport handling to use the requested pos directly
unless the backend is known to apply window moves synchronously, and keep the
SetWindowPos/Viewport->Pos synchronization in sync with that choice.
---
Nitpick comments:
In `@CMakeLists.txt`:
- Line 31: Align the ImGui target’s PIC handling with premake by scoping the
POSITION_INDEPENDENT_CODE setting to Linux only instead of applying it
unconditionally. Update the CMake target configuration around
set_target_properties for ImGui so it matches the existing Linux-only behavior
used in the premake setup, keeping the FOLDER assignment unchanged.
In `@imgui_widgets.cpp`:
- Line 10828: The tab border drawn in the tab bar rendering path uses a
hardcoded black color, which bypasses the current theme. Update the border color
source in the code that calls draw_list->AddLine in the tab drawing logic to use
the style palette instead, such as the border color from the current ImGui
style, so the separator adapts correctly across themes and custom palettes.
In `@imgui.cpp`:
- Around line 8074-8075: The docked title-bar padding currently uses a hardcoded
8.0f that is duplicated across the title-bar height calculation,
DockNodeCalcTabBarLayout, and BeginDockableDragDropSource, so the docked
title-bar height, tab-bar rect, and drag hit test can drift apart. Replace these
separate literals with one shared padding value used consistently by all three
code paths, and make sure it scales with FontScaleDpi/style scaling like
FramePadding.y does instead of remaining a fixed pixel value.
In `@misc/freetype/imgui_freetype.cpp`:
- Around line 648-659: The glyph blit path in imgui_freetype.cpp uses a single
memcpy after lunasvg::Bitmap::renderToBitmap, which assumes tightly packed rows
and can copy padding into the output. Update the
state->glyphElement.renderToBitmap branch to copy row-by-row using the rendered
bitmap’s stride() rather than a bulk buffer copy, and ensure the copied row
count is bounded by slot->bitmap.rows so the output matches the destination
bitmap layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9d148f1-44c8-4d6e-b02a-eccfb7b6f3dc
📒 Files selected for processing (10)
.gitmodulesCMakeLists.txtbackends/imgui_impl_glfw.cppimgui.cppimgui.himgui_draw.cppimgui_widgets.cppimplotmisc/freetype/imgui_freetype.cpppremake5.lua
💤 Files with no reviewable changes (1)
- backends/imgui_impl_glfw.cpp
| target_compile_definitions(ImGui PUBLIC | ||
| $<$<CONFIG:Dist>:NDEBUG IMGUI_DISABLE_DEMO_WINDOWS IMGUI_DISABLE_DEBUG_TOOLS> | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CMakeLists.txt excerpt ==="
nl -ba CMakeLists.txt | sed -n '20,35p'
echo
echo "=== cmake availability ==="
command -v cmake >/dev/null && cmake --version | head -n 1 || echo "cmake not available"
echo
echo "=== target_compile_definitions help (if available) ==="
if command -v cmake >/dev/null; then
cmake --help-command target_compile_definitions | sed -n '1,220p'
fiRepository: StudioCherno/imgui
Length of output: 227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("CMakeLists.txt")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 20 <= i <= 35:
print(f"{i:4}: {line}")
PYRepository: StudioCherno/imgui
Length of output: 844
Quote the generator expression and use semicolons
target_compile_definitions() splits unquoted spaces before generator-expression evaluation, so this becomes broken arguments. Wrap it in quotes and keep the macros in a semicolon list:
Proposed fix
target_compile_definitions(ImGui PUBLIC IMGUI_USE_WCHAR32)
target_compile_definitions(ImGui PUBLIC
- $<$<CONFIG:Dist>:NDEBUG IMGUI_DISABLE_DEMO_WINDOWS IMGUI_DISABLE_DEBUG_TOOLS>
+ "$<$<CONFIG:Dist>:NDEBUG;IMGUI_DISABLE_DEMO_WINDOWS;IMGUI_DISABLE_DEBUG_TOOLS>"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| target_compile_definitions(ImGui PUBLIC | |
| $<$<CONFIG:Dist>:NDEBUG IMGUI_DISABLE_DEMO_WINDOWS IMGUI_DISABLE_DEBUG_TOOLS> | |
| ) | |
| target_compile_definitions(ImGui PUBLIC | |
| "$<$<CONFIG:Dist>:NDEBUG;IMGUI_DISABLE_DEMO_WINDOWS;IMGUI_DISABLE_DEBUG_TOOLS>" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CMakeLists.txt` around lines 27 - 29, The target_compile_definitions call for
ImGui is passing a generator expression with space-separated macros, which CMake
splits into broken arguments before evaluation. Update the
target_compile_definitions invocation for ImGui so the $<$<CONFIG:Dist>:...>
expression is quoted and the definitions inside it are expressed as a
semicolon-separated list, keeping the macros grouped correctly.
| // Use the actual window location returned by the window system if possible, | ||
| // since windows can snap to borders and things like that which result in a final | ||
| // window location that is not exactly the same as what was requested. | ||
| ImVec2 actual = pos; | ||
| if (moving_window->Viewport && moving_window->ViewportOwned | ||
| && g.PlatformIO.Platform_SetWindowPos && g.PlatformIO.Platform_GetWindowPos) | ||
| { | ||
| g.PlatformIO.Platform_SetWindowPos(moving_window->Viewport, pos); | ||
| actual = g.PlatformIO.Platform_GetWindowPos(moving_window->Viewport); | ||
| } | ||
| SetWindowPos(moving_window, actual, ImGuiCond_Always); | ||
| if (moving_window->Viewport && moving_window->ViewportOwned) // Synchronize viewport immediately because some overlays may relies on clipping rectangle before we Begin() into the window. | ||
| { | ||
| moving_window->Viewport->Pos = pos; | ||
| // Use moving_window->Pos (which SetWindowPos truncated via | ||
| // ImTrunc) rather than raw `actual`. If viewport->Pos kept | ||
| // the untruncated value, a later sync site would overwrite it | ||
| // with window->Pos (truncated), and UpdatePlatformWindows | ||
| // would see the mismatch against LastPlatformPos and push the | ||
| // truncated position to the OS — which maps to a different | ||
| // OS-pixel than what we just set, causing oscillation. | ||
| moving_window->Viewport->Pos = moving_window->Pos; | ||
| moving_window->Viewport->LastPlatformPos = moving_window->Pos; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
glfwGetWindowPos stale value immediately after glfwSetWindowPos X11 Wayland asynchronous
💡 Result:
The observation that glfwGetWindowPos returns a stale value immediately after calling glfwSetWindowPos is a known consequence of the asynchronous nature of window management on X11 and the structural limitations on Wayland [1][2][3]. On X11, window positioning requests are asynchronous [2][4]. When you call glfwSetWindowPos, GLFW sends a request to the X Window Manager (WM) [2][4]. The application does not immediately move; instead, it must wait for the WM to process the request and send a ConfigureNotify event back to the application to confirm the new position [2][5]. Because glfwGetWindowPos reads the state currently tracked by the GLFW window object, it will report the "old" position until that event is received and processed, which typically happens during the next event-polling cycle (e.g., glfwPollEvents or glfwWaitEvents) [2][3]. Additionally, window managers may clamp, override, or ignore these requests (e.g., if a window is snapped to an edge or if decorations interfere), causing the "actual" final position to differ from what you requested [1][5][4]. On Wayland, this behavior is fundamentally different because the Wayland protocol restricts an application's ability to control its own position [6]. Applications generally cannot set their global screen position, and attempting to do so is often unsupported [7][6]. Consequently, glfwGetWindowPos and glfwSetWindowPos may be effectively non-functional or report static/default values depending on the specific compositor implementation [7][6]. To mitigate these issues in your application: 1. Allow the event loop to process: Ensure you call glfwPollEvents or glfwWaitEvents after calling glfwSetWindowPos to allow the platform to update the window state [3]. 2. Account for WM interference: Recognize that the window manager is the final authority on window placement [1][4]. If you are implementing features like draggable windows, consider tracking position changes relative to cursor input rather than relying on querying the window position after a set command [3]. 3. Use window callbacks: Instead of querying the position immediately, rely on the GLFW window position callback (glfwSetWindowPosCallback) to receive the authoritative, updated position once the window manager has finished the move [1][2].
Citations:
- 1: GLFW multi-viewport position mismatch when window manager clamps secondary viewport ocornut/imgui#9442
- 2: GLFW: can we stop ignoring X11 window events for less Jitter? ocornut/imgui#9356
- 3: https://stackoverflow.com/questions/46203809/glfw-mouse-event-lag-with-window-drag
- 4: glfwSetWindowPos can not move window outside of desktop bounds glfw/glfw#1817
- 5: glfwGetWindowPos and glfwSetWindowPos do not ignore decorations glfw/glfw#800
- 6: [rcore] Overview of Wayland, Xwayland and X11 support on Linux raysan5/raylib#4371
- 7: Wayland: The platform does not provide the window position glfw/glfw#2621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant symbols and find their usages/backends.
printf '\n== imgui.cpp outline ==\n'
ast-grep outline imgui.cpp --view expanded | sed -n '1,220p'
printf '\n== Platform_SetWindowPos / Platform_GetWindowPos usages ==\n'
rg -n "Platform_SetWindowPos|Platform_GetWindowPos|LastPlatformPos|SetWindowPos\\(" -S . --glob '!**/build/**' --glob '!**/dist/**'
printf '\n== Candidate backend files mentioning glfwGetWindowPos / glfwSetWindowPos ==\n'
rg -n "glfwGetWindowPos|glfwSetWindowPos|Platform_GetWindowPos|Platform_SetWindowPos" -S . --glob '!**/build/**' --glob '!**/dist/**'Repository: StudioCherno/imgui
Length of output: 29464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant region of imgui.cpp with line numbers.
sed -n '5400,5465p' imgui.cpp | cat -nRepository: StudioCherno/imgui
Length of output: 4455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== imgui.cpp outline ==\n'
ast-grep outline imgui.cpp --view expanded | sed -n '1,220p'
printf '\n== Platform_SetWindowPos / Platform_GetWindowPos usages ==\n'
rg -n "Platform_SetWindowPos|Platform_GetWindowPos|LastPlatformPos|SetWindowPos\\(" -S . --glob '!**/build/**' --glob '!**/dist/**'
printf '\n== Candidate backend files mentioning glfwGetWindowPos / glfwSetWindowPos ==\n'
rg -n "glfwGetWindowPos|glfwSetWindowPos|Platform_GetWindowPos|Platform_SetWindowPos" -S . --glob '!**/build/**' --glob '!**/dist/**'
printf '\n== imgui.cpp region ==\n'
sed -n '5400,5465p' imgui.cpp | cat -nRepository: StudioCherno/imgui
Length of output: 33787
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the GLFW backend implementation around the platform position handlers.
sed -n '1200,1555p' backends/imgui_impl_glfw.cpp | cat -nRepository: StudioCherno/imgui
Length of output: 18770
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the GLFW header docs for the semantics around window position callbacks.
sed -n '1970,2555p' examples/libs/glfw/include/GLFW/glfw3.h | cat -n | sed -n '1,180p'Repository: StudioCherno/imgui
Length of output: 8240
Avoid the immediate glfwGetWindowPos() readback here. GLFW can queue window-pos updates until glfwPollEvents() on Linux, so this round-trip can return the previous position and make the drag lag or jitter. Use the requested pos unless the backend guarantees synchronous updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@imgui.cpp` around lines 5422 - 5443, The position sync in the moving-window
path should not rely on an immediate Platform_GetWindowPos/readback after
Platform_SetWindowPos, since GLFW can return a stale value and cause drag lag or
jitter. Update the logic in the moving_window viewport handling to use the
requested pos directly unless the backend is known to apply window moves
synchronously, and keep the SetWindowPos/Viewport->Pos synchronization in sync
with that choice.
3 participants
This adds cmake support, and also pulls in commits used in active development into the main
hazelbranchSummary by CodeRabbit
New Features
Bug Fixes
Build & Platform