Skip to content

Correct HiDPI scaling via Win32 API and centralize geometry calculations#1394

Merged
amilcarlucas merged 6 commits intomasterfrom
scale_window_sizes
Mar 21, 2026
Merged

Correct HiDPI scaling via Win32 API and centralize geometry calculations#1394
amilcarlucas merged 6 commits intomasterfrom
scale_window_sizes

Conversation

@amilcarlucas
Copy link
Collaborator

  • Add _get_win32_system_dpi() to bypass DPI virtualization on Windows
  • Add calculate_scaled_geometry() helper to BaseWindow; use it in all windows
  • Remove double-scaling of point-based fonts (Tk handles them automatically)
  • Refactor show_about_window() function into AboutWindow(BaseWindow) class
  • Fix Treeview column-width and heading-padding measurements in TemplateOverviewWindow

Implements #544

Copilot AI review requested due to automatic review settings March 20, 2026 03:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves HiDPI behavior in the Tkinter frontend, particularly on Windows, by bypassing DPI virtualization, centralizing geometry scaling, and removing font double-scaling, while also refactoring the About popup into a dedicated window class.

Changes:

  • Add Win32 DPI detection and a calculate_scaled_geometry() helper to standardize window sizing across the app.
  • Remove double-scaling of point-based fonts; adjust style and Treeview measurements to use Tk’s logical units correctly.
  • Refactor the About popup from a function into an AboutWindow(BaseWindow) class and update tests accordingly.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
ardupilot_methodic_configurator/frontend_tkinter_base_window.py Adds Win32 DPI query, normalizes scaling detection, and introduces calculate_scaled_geometry() plus improved centering.
ardupilot_methodic_configurator/frontend_tkinter_about_popup_window.py Refactors About popup into AboutWindow using BaseWindow scaling + centering.
ardupilot_methodic_configurator/frontend_tkinter_template_overview.py Switches to centralized geometry scaling; fixes Treeview font/padding and column width measurements.
ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py Uses AboutWindow and scaled geometry; centers main window.
ardupilot_methodic_configurator/frontend_tkinter_project_opener.py Uses scaled geometry + centers window.
ardupilot_methodic_configurator/frontend_tkinter_project_creator.py Scales geometry inline and centers window on screen.
ardupilot_methodic_configurator/frontend_tkinter_software_update.py Uses scaled geometry + centers window on screen.
ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py Uses scaled geometry + centers window on screen.
ardupilot_methodic_configurator/frontend_tkinter_flightcontroller_info.py Uses scaled geometry + centers window on screen.
ardupilot_methodic_configurator/frontend_tkinter_motor_test.py Switches motor test window sizing to calculate_scaled_geometry().
ardupilot_methodic_configurator/frontend_tkinter_battery_monitor.py Switches battery monitor sizing to calculate_scaled_geometry().
ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py Uses scaled geometry for “Add parameter(s)” dialog.
ardupilot_methodic_configurator/frontend_tkinter_progress_window.py Adds DPI-based size scaling and refactors centering logic into _center_progress_window().
tests/test_frontend_tkinter_base_window.py Updates DPI tests to suppress Win32 DPI path and adapts centering tests to new winfo_width/height usage.
tests/unit_frontend_tkinter_software_update.py Patches centering to avoid extra update calls and adjusts geometry assertions.
tests/test_frontend_tkinter_template_overview.py Updates mocks for new centering behavior and Treeview style configuration changes.
tests/test_frontend_tkinter_usage_popup_window.py Makes size assertions DPI-factor aware.
tests/gui_frontend_tkinter_parameter_editor.py Updates About popup invocation and makes geometry assertions DPI-tolerant.
tests/conftest.py Patches center_window_on_screen when tkinter is patched in tests.
tests/test_frontend_tkinter_project_creator.py Patches centering to keep tests stable.
tests/test_frontend_tkinter_project_opener.py Patches centering to keep tests stable.
tests/test_frontend_tkinter_project_creator_integration.py Patches centering and adds missing tk.StringVar patch for opener module.
tests/test_frontend_tkinter_progress_window.py Adds test covering centering behavior when parent is withdrawn.
tests/test_frontend_tkinter_parameter_editor.py Adds Win32 DPI suppression and additional root winfo mocks for centering.
tests/test_frontend_tkinter_connection_selection.py Adds winfo mocks + dpi_scaling_factor for centering/scaling changes.
tests/test_frontend_tkinter_motor_test.py Ensures test BaseWindow init stubs provide dpi_scaling_factor.
tests/test_frontend_tkinter_component_editor_base.py Updates geometry assertions and aligns style expectations with removal of font double-scaling.
ardupilot_methodic_configurator/frontend_tkinter_component_editor_base.py Uses scaled geometry + centers window; stops font double-scaling in styles.

try:
dpi = self.progress_window.winfo_fpixels("1i")
tk_scaling = float(self.progress_window.tk.call("tk", "scaling"))
dpi_scaling_factor = max(dpi / 96.0, tk_scaling)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tk scaling is not normalized to a 96-DPI baseline here (unlike BaseWindow._get_dpi_scaling_factor). On systems where tk scaling is ~1.333 at 96 DPI, this will incorrectly upscale the progress window at 100% scaling. Align this calculation with BaseWindow._get_dpi_scaling_factor (including the tk_scaling * 72/96 normalization and the max(1.0, ...) floor), or reuse the same scaling helper logic to avoid inconsistent sizing across windows.

Suggested change
dpi_scaling_factor = max(dpi / 96.0, tk_scaling)
normalized_tk_scaling = tk_scaling * 72.0 / 96.0
dpi_scaling_factor = max(1.0, dpi / 96.0, normalized_tk_scaling)

Copilot uses AI. Check for mistakes.
Comment on lines +138 to +141
self.root.geometry(
f"{round(800 * self.dpi_scaling_factor)}x{round(window_height * self.dpi_scaling_factor)}"
) # Set the window size
BaseWindow.center_window_on_screen(self.root)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says calculate_scaled_geometry() is used “in all windows”, but this window still performs inline scaling. For consistency (and to keep scaling behavior centralized), switch this to self.calculate_scaled_geometry(800, window_height) and keep the centering call as-is.

Copilot uses AI. Check for mistakes.

# Patch center_window_on_screen to prevent extra root.update() calls
self.center_window_patch = patch.object(BaseWindow, "center_window_on_screen")
self.center_window_patch.start()
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is started in setUp() but is not stopped in the shown diff, which can leak across tests and cause order-dependent failures. Stop it in tearDown() or register cleanup via self.addCleanup(self.center_window_patch.stop) immediately after start().

Suggested change
self.center_window_patch.start()
self.center_window_patch.start()
self.addCleanup(self.center_window_patch.stop)

Copilot uses AI. Check for mistakes.
Comment on lines +160 to +164
# Check that geometry has positive dimensions (exact size depends on DPI scaling)
win_width = about_window.winfo_width()
win_height = about_window.winfo_height()
assert win_width >= 650, f"Window width {win_width} should be at least 650px (unscaled)"
assert win_height >= 340, f"Window height {win_height} should be at least 340px (unscaled)"
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

winfo_width() / winfo_height() often return 1 until the window has been drawn and idle tasks have run, which can make this assertion flaky. Call about_window.update_idletasks() (or root.update_idletasks()) before reading dimensions to ensure the geometry is realized.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

github-actions bot commented Mar 20, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
11780 11041 94% 89% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: c6466f5 by action🐍

@github-actions
Copy link
Contributor

github-actions bot commented Mar 20, 2026

Test Results

     4 files  ±  0       4 suites  ±0   41m 4s ⏱️ -35s
 3 359 tests + 31   3 357 ✅ + 31   2 💤 ±0  0 ❌ ±0 
13 228 runs  +124  13 207 ✅ +124  21 💤 ±0  0 ❌ ±0 

Results for commit c6466f5. ± Comparison against base commit 9383df7.

♻️ This comment has been updated with latest results.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 23328350423

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 56 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.009%) to 93.079%

Files with Coverage Reduction New Missed Lines %
frontend_tkinter_progress_window.py 2 96.97%
frontend_tkinter_template_overview.py 2 91.36%
frontend_tkinter_project_opener.py 9 82.18%
frontend_tkinter_base_window.py 10 94.9%
frontend_tkinter_parameter_editor.py 10 97.09%
frontend_tkinter_component_editor_base.py 23 88.89%
Totals Coverage Status
Change from base Build 23319673898: -0.009%
Covered Lines: 10960
Relevant Lines: 11775

💛 - Coveralls

…y calculations

- Add _get_win32_system_dpi() to bypass DPI virtualization on Windows
- Add calculate_scaled_geometry() helper to BaseWindow; use it in all windows
- Remove double-scaling of point-based fonts (Tk handles them automatically)
- Refactor show_about_window() function into AboutWindow(BaseWindow) class
- Fix Treeview column-width and heading-padding measurements in TemplateOverviewWindow

Implements #544
…w.center_window_on_screen()

Address GitHub Copilot PR #1387 review findings:

- Replace BaseWindow.center_window_on_screen(self.root) with
  self.center_window_on_screen(self.root) in all 8 subclass windows to
  respect polymorphism and allow instance-level mock patching
- Strengthen withdrawn-parent centering test: reset mocks after __init__,
  assert exactly one call via update_progress_bar(), and verify
  window.progress_window is the argument passed
- Add test_uses_rendered_size_when_window_is_mapped to assert that
  winfo_width/height (rendered size) is preferred over winfo_reqwidth/
  winfo_reqheight (content size) when the window is already mapped
…e to prevent segfault

- Normalize tk_scaling with * 72.0/96.0 and add max(1.0, ...) floor in
  ProgressWindow to match BaseWindow._get_dpi_scaling_factor behaviour
- Use calculate_scaled_geometry() in VehicleProjectCreatorWindow instead
  of inline scaling to keep geometry logic centralized
- Defer focus_force() via after(1, ...) in PopupWindow to avoid a
  segfault in Python 3.9 on Linux/X11 in headless CI environments
- Call update_idletasks() before winfo_width/height in GUI test to
  prevent flaky 1px assertions before geometry is realized

Addresses review comments from PR #1394
@amilcarlucas amilcarlucas merged commit 64b3027 into master Mar 21, 2026
31 checks passed
amilcarlucas added a commit that referenced this pull request Mar 21, 2026
…e to prevent segfault

- Normalize tk_scaling with * 72.0/96.0 and add max(1.0, ...) floor in
  ProgressWindow to match BaseWindow._get_dpi_scaling_factor behaviour
- Use calculate_scaled_geometry() in VehicleProjectCreatorWindow instead
  of inline scaling to keep geometry logic centralized
- Defer focus_force() via after(1, ...) in PopupWindow to avoid a
  segfault in Python 3.9 on Linux/X11 in headless CI environments
- Call update_idletasks() before winfo_width/height in GUI test to
  prevent flaky 1px assertions before geometry is realized

Addresses review comments from PR #1394
@amilcarlucas amilcarlucas deleted the scale_window_sizes branch March 21, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants