Skip to content

fix(qt): handle pixel-sized fonts when scaling widgets#7315

Draft
thepastaclaw wants to merge 1 commit into
dashpay:developfrom
thepastaclaw:tracker-1251
Draft

fix(qt): handle pixel-sized fonts when scaling widgets#7315
thepastaclaw wants to merge 1 commit into
dashpay:developfrom
thepastaclaw:tracker-1251

Conversation

@thepastaclaw
Copy link
Copy Markdown

Pixel-sized font scaling fix

Summary

  • handle Qt widgets whose stylesheet fonts are pixel-sized when
    GUIUtil::updateFonts() runs
  • convert pixel sizes to point-size equivalents before applying the existing
    font-scaling path
  • avoid the macOS assertion abort seen while opening the recovery phrase dialog

Fixes #7281.

Validation

  • git diff --check upstream/develop...HEAD
  • code-review gate: ship for upstream/develop..tracker-1251
  • Static validation only. I do not have a macOS Tahoe GUI environment available
    to reproduce the original crash locally.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@thepastaclaw
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw
Copy link
Copy Markdown
Author

thepastaclaw commented May 9, 2026

✅ Review complete (commit 9c94795)

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cee2f4de-fc71-4e78-af7b-7c0c27f974f3

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd84aa and 9c94795.

📒 Files selected for processing (1)
  • src/qt/guiutil_font.cpp

Walkthrough

This change improves the robustness of font size detection in GUIUtil::updateFonts() within the Qt GUI utility layer. The function now computes baseline font sizes using QFont::pointSizeF() as the primary source and, when that yields a non-positive value but pixel size is available, derives the point size from pixelSize * 72 / logicalDpiY(). Widgets with resulting non-positive sizes are skipped rather than triggering an assertion. The default font size stored in the mapping is now the computed double value instead of the int result from pointSize().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: handling pixel-sized fonts in Qt widget scaling.
Description check ✅ Passed The description is directly related to the changeset, explaining the problem, solution, and validation approach.
Linked Issues check ✅ Passed The changes address the root cause of issue #7281 by fixing font scaling logic to handle pixel-sized fonts, preventing the macOS crash.
Out of Scope Changes check ✅ Passed All changes in src/qt/guiutil_font.cpp are directly related to the linked issue's objective of fixing GUI font handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

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

Code Review

Small, focused Qt fix replacing a fatal assert with a graceful pixel-to-point conversion path for stylesheet-driven pixel-sized fonts. Logic is sound; idempotency via mapWidgetDefaultFontSizes::emplace is preserved. No blocking issues — only two low-impact nitpicks worth surfacing.

Reviewed commit: 9c94795

💬 2 nitpick(s)

Comment thread src/qt/guiutil_font.cpp
Comment on lines +583 to +586
if (font_size <= 0 && font.pixelSize() > 0) {
font_size = font.pixelSize() * 72.0 / w->logicalDpiY();
font.setPointSizeF(font_size);
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

💬 Nitpick: setPointSizeF on line 585 is redundant

The local font is either fully replaced by getFont(...) on line 604 (when an entry exists in mapFontUpdates) or has its size overwritten by setPointSizeF(GetScaledFontSize(...)) on line 606. The explicit font.setPointSizeF(font_size) on line 585 has no observable effect on the font ultimately stored in mapWidgetFonts or compared at line 609. Removing it would tighten the code; keeping it is harmless as a defensive normalization.

source: ['claude']

Comment thread src/qt/guiutil_font.cpp
Comment on lines +583 to +589
if (font_size <= 0 && font.pixelSize() > 0) {
font_size = font.pixelSize() * 72.0 / w->logicalDpiY();
font.setPointSizeF(font_size);
}
if (font_size <= 0) {
continue;
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

💬 Nitpick: logicalDpiY() result is cached for the lifetime of the widget

w->logicalDpiY() correctly reports the DPI of the widget's current screen, but the derived point-size is then cached in mapWidgetDefaultFontSizes as the widget's 'default font size'. If the widget later migrates to a screen with a different DPI, the cached value will no longer correspond to the original pixel size. The same caching limitation exists in the pre-PR code paths, so this is not a regression — just a corner case worth noting.

source: ['claude']

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.

macOS Tahoe 26.3: Dash Core 23.1.2 crashes when showing recovery phrase on legacy wallet

1 participant