fix(layout): reject bogus Xwayland physical-size reports that shrink the font to microscopic - #285
Conversation
…the font to microscopic nested/remote Xwayland sessions (neomacs-in-neomacs, over mosh, a rootful xwayland server) hand back a physical display height in mm that isn't real — usually absurdly large. fallback_frame_res_y trusted it and computed a geometric DPI of ~30, so the whole frame's font came out microscopic. compute the DPI as before but only trust it when it lands in the 50..=400 range a real monitor can actually produce; anything outside that is a broken report, not a display, so fall back to 96 (the modern desktop neutral) instead of letting it shrink the text. the no-physical-size case (mm == 0) also goes to 96 now rather than 100. new assertions in frame_res_fallback_uses_display_height_and_mm cover the plausible monitor (used as-is), mm == 0, the huge-mm nested-xwayland report, and the tiny-mm implausibly-high reading. layout-engine fontconfig suite green (30/30).
There was a problem hiding this comment.
🟢 Approval recommended
The behavioral change is narrowly scoped, test-covered, and addresses a real failure mode without affecting normal displays.
Pull request overview
This PR hardens the font DPI fallback logic in neomacs-layout-engine to avoid microscopic fonts when Xwayland reports bogus physical display sizes (common in nested/remote Xwayland setups). It keeps the existing geometric DPI calculation, but rejects implausible DPI results and uses a neutral fallback.
Changes:
- Reject out-of-range geometric DPI derived from X server physical-size (mm) reports and fall back to 96 DPI.
- Align the “no physical size” (
mm < 1) case to also fall back to 96 DPI. - Extend the existing DPI fallback unit test with additional plausible/implausible scenarios.
File summaries
| File | Description |
|---|---|
| neomacs-layout-engine/src/font/fontconfig.rs | Adds plausibility bounds to geometric DPI fallback and switches fallback value to 96 DPI. |
| neomacs-layout-engine/src/font/fontconfig_test.rs | Expands tests to cover plausible DPI, missing mm, and implausible low/high DPI cases. |
Review details
Suppressed comments (1)
neomacs-layout-engine/src/font/fontconfig.rs:549
- The
xft_dpidoc comment still says bogus mm dimensions fall back to 100 DPI, butfallback_frame_res_ynow rejects implausible readings and returns 96. Update the comment so it matches the new behavior (and optionally mention that a full X11 query failure still usesunwrap_or(100.0)).
}
}
/// Get the effective DPI for font sizing.
///
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if display_height_mm < 1 { | ||
| 100.0 | ||
| return 96.0; | ||
| } | ||
| let dpi = display_height_px as f32 * 25.4 / display_height_mm as f32; | ||
| if dpi.is_finite() && (50.0..=400.0).contains(&dpi) { |
eval-exec
left a comment
eval-exec
left a comment
There was a problem hiding this comment.
I do not think this should merge as-is.
The reported nested/remote Xwayland failure is real and worth fixing, but this patch changes the general X11 DPI contract rather than identifying the broken measurement source. GNU Emacs uses exactly (mm < 1) ? 100 : pixels * 25.4 / mm; it does not clamp the geometric result and it does not use 96 for the missing-size case:
That leaves two compatibility problems here:
mm < 1changes from GNU 100 DPI to 96 DPI, although this does not address the reported bogus-nonzero-mm failure.- The global
50..=400clamp rejects every unusual geometric result, including potentially legitimate low/high-DPI, projector, virtual, or future display configurations. This contradicts the README GNU-oracle contract while the function still claims to mirror GNU.
The ideal fix is source-aware: retain GNU behavior for the normal X11 path, treat an explicit Xft.dpi resource as authoritative, and apply a compatibility fallback only when we have evidence that the nested/remote Xwayland measurement is broken (or expose an explicit override). I would model this as a typed observation/policy such as DpiObservation::{XResource, Geometry, BrokenXwayland, Missing} so measurement and fallback policy are not hidden in three raw constants.
Please keep GNU-oracle tests for mm < 1 -> 100 and unrestricted geometric DPI, then add a separate Xwayland-specific regression proving the broken-source detection and fallback. The current tests instead codify the global divergence.
no linked issue — hit this myself running neomacs inside neomacs / over a remote xwayland.
what was wrong
launch neomacs into a nested or remote Xwayland session (neomacs-in-neomacs, over mosh, a rootful xwayland server) and the whole frame comes up with a microscopic font — everything a few pixels tall, unreadable.
it traces to
fallback_frame_res_yinneomacs-layout-engine/src/font/fontconfig.rs. it derives the vertical DPI geometrically from the display height in px over the physical height in mm the X server reports. on a real monitor that gives ~100 DPI. but nested/remote Xwayland hands back a bogus mm height — usually absurdly large — so1080px / 800mmworks out to ~34 DPI, and the font gets scaled down to match. GNU only ever falls back when there is no size at all (mm < 1), so it never trips this, but here the server does report a size, it is just fiction.neomacs -Qon a real display is fine (~100 DPI), which is what pointed at "the mm the nested server reports is not real" as the trigger.the fix
compute the geometric DPI exactly as before, but only trust it when it lands in the range a real monitor can physically produce. a display is essentially never below ~50 or above ~400 DPI, so a reading outside
50..=400is a broken report, not a screen — fall back to 96 (the modern desktop neutral) instead of letting it shrink the text. a plausible reading passes through untouched, so nothing changes on a normal display.one small side change: the no-physical-size case (
mm < 1) now returns 96 too, not 100, to match the neutral used elsewhere in the layout engine.testing
frame_res_fallback_uses_display_height_and_mmwith four cases: a plausible monitor (1080/274 ≈ 100DPI, used as-is),mm == 0, the huge-mm nested-xwayland report (1080/800, rejected), and a tiny-mm implausibly-high reading (2160/100, rejected).neomacs-layout-enginefontconfig suite green, 30/30.