Adopt IMGUI DPI to Replace Existing Solution#892
Merged
Conversation
Remove the bespoke DPI plumbing (rocprofvis_view_set_dpi, GLFW content scale callback, SettingsManager DPI state, dpi_based_scaling setting, and FontManager::GetDPIScaledFontIndex) in favor of ImGui's built-in DPI font scaling (ConfigDpiScaleFonts/ConfigDpiScaleViewports + multi-viewport). The font size setting becomes a plain user-controlled slider clamped to a supported range, and fixed window sizes no longer multiply by a manual DPI factor. The status bar height now follows the current font size via a single guarded check that handles both user- and DPI-driven font changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Collapse the verbose multi-line comments and remove the defensive static_assert block added with the DPI rework so the comments stay terse and consistent with the rest of the view layer. No behavior change. Co-authored-by: Cursor <cursoragent@cursor.com>
Sajeeth-Wimalasuriyan
requested review from
a team,
amokiche-amd,
drchen-amd and
tomk-amd
as code owners
June 18, 2026 20:30
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Remove the multi-viewport/windowing enablement (ViewportsEnable and the platform-window render pass) that main does not have, and revert the font-size slider redesign, font-range clamp, extra FontManager helpers, and incidental gui_helpers rewrite. The branch now only swaps the custom DPI handling for ImGui's built-in DPI font scaling (ConfigDpiScaleFonts), keeping the status bar sizing and banner scaling needed under it. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the font-size dropdown with a slider (in points) bounded to a supported range, and clamp the saved index on load/apply. The upper bound keeps the UI font from growing too large under ImGui DPI font scaling. Co-authored-by: Cursor <cursoragent@cursor.com>
tomk-amd
reviewed
Jun 22, 2026
Detect the applied font size once per frame in FontManager::Update so both user-selected sizes and ImGui auto-DPI scaling notify every kFontSizeChanged subscriber, and revert AppWindow's private status-bar poll back to a normal subscription. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
I think the font size slider should have a wider range of values. Some people might use the tool on a huge monitor with a lower resolution, while others might use it on a laptop. Right now, if I run the app on a laptop and set the font size to the maximum, I'm comfortable with it, but I can easily imagine people who might find the text too small. |
Widen the user font-size range, fold GetClosestFontSizeIndex into a member using std::lower_bound, use std::clamp for index clamping, and compare effective font size with an epsilon in Update(). Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
I think Derek mentioned this on the mulit-view PR that had a version of this DPI refactor included... The mini-map is tiny. The mini map window needs to be bigger. 
|
Co-authored-by: Cursor <cursoragent@cursor.com>
tomk-amd
reviewed
Jul 15, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
tomk-amd
approved these changes
Jul 21, 2026
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/view/src/rocprofvis_appwindow.cpp # src/view/src/rocprofvis_settings_manager.cpp # src/view/src/rocprofvis_settings_manager.h # src/view/src/widgets/rocprofvis_gui_helpers.cpp
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Replaces the project's custom DPI handling with Dear ImGui's built-in (1.92+) per-monitor DPI font scaling, and turns the font-size preference into a simple user slider.
rocprofvis_view_set_dpi, the GLFWcontent_scale_callback/glfwSetWindowContentScaleCallbackwiring,SettingsManager::Get/SetDPI+m_display_dpi, thedpi_based_scalinguser setting (+ its JSON key), andFontManager::GetDPIScaledFontIndex.io.ConfigDpiScaleFonts/io.ConfigDpiScaleViewports, enables multi-viewport (ImGuiConfigFlags_ViewportsEnable) with theUpdatePlatformWindows()/RenderPlatformWindowsDefault()pump in the main loop, andstyle.FontScaleMain = 1.0f.SettingsPanel), clamped to a supported range; previously persisted indices are clamped on load for backward compatibility.* dpimultipliers on fixed window sizes (shutdown dialog, minimap popup) and makes the internal-build banner viewport/font-scale aware instead of usingio.DisplaySize.UpdateStatusBar(), covering both user-driven and DPI-driven font changes (ImGui auto-DPI resizes fonts without firingkFontSizeChanged).