Skip to content

M14: Fix(nixos): discover installed UI fallback fonts on NixOS - #1553

Merged
echelon merged 6 commits into
storytold:mainfrom
tomasmark79:fix/nixos-ui-font-discovery
Oct 15, 2026
Merged

echelon merged 6 commits into
storytold:mainfrom
tomasmark79:fix/nixos-ui-font-discovery

Conversation

@tomasmark79

@tomasmark79 tomasmark79 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

On NixOS, installed fonts live in /nix/store and are registered through Fontconfig. The UI fallback lookup only tries conventional system paths, so it misses configured fonts even when Fontconfig finds them.

Discover configured font directories using the pure-Rust fontconfig-parser XML parser, without a subprocess or a libfontconfig runtime dependency. Retain the existing fallback filename allowlist, candidate order, glyph probes, size limits and screenshot opt-out. Font files are read only at runtime, never copied, embedded or shipped.

Review follow-up for @FredApps and @echelon:

  • Relative includes resolve beside each including config, including nested includes under FONTCONFIG_PATH.
  • Directory includes accept only digit-prefixed *.conf files in lexical order. Disabled files, backups and unnumbered files cannot unexpectedly apply <reset-dirs/>.
  • Both findings have isolated subprocess regressions using the locked parser dependency and the production config_path() / font_files() functions. Both tests failed before the fix and pass after it. Fixtures contain XML and dummy files, no font assets.
  • Additional tests cover valid reset ordering, symlinked config, include cycles, oversized config, include depth/count limits, font-directory symlinks/cycles and candidate ordering. Explicit file includes remain supported regardless of their filename.
  • Merge upstream through e88dd72, preserving the Telugu fallback from Telugu interface: draw with an installed Telugu face when the build has none (part of #1531) #1611. The additional Hebrew candidates proposed in Hebrew file names: draw them with an installed Hebrew face when the build has none #1624 use the same discovery helper when combined.
  • Replace an existing nested if in the forms date parser with its equivalent match guard, so strict Clippy no longer needs the previously reported exception. No lint is suppressed.
  • Updating to current main exposed the existing command-opening regression: the asynchronous open request lost the chosen tool. Carry its continuation in PendingOpen.then, and make the two file-opening command tests wait for completion with a two-second bound. The tool test failed even with that wait before the continuation fix; all 22 command tests pass after it.
  • Remote-link tests also wait for background opening/password retries. Existing picker, recovery, password-details and drop-dispatch tests explicitly select the provided run_inline mode where their assertions require synchronous preparation; dedicated worker tests and the command/remote-link tests still exercise asynchronous opens.

Configuration traversal is bounded to 1 MiB per file, 256 visited configs/directories, depth 16 and 32,768 directory entries. Missing or malformed config preserves conventional fallback candidates. This intentionally discovers directories; it does not implement Fontconfig font matching or alias rules.

Validation on upstream e88dd72:

  • cargo fmt --check passed.
  • cargo clippy --workspace --all-targets --locked --profile low-disk -- -D warnings passed without exceptions.
  • Workspace tests passed: 3,028 passed, 14 ignored, no failures.
  • Full xtask wasm gate passed: 31 crates.
  • Six new configuration regressions, including the two isolated review reproductions; the original four discovery regressions remain passing.

Find existing fallback faces in configured directories, including Nix store paths. Bound and cache discovery while preserving candidate names, glyph checks and the screenshot opt-out.

Assisted-by: OpenAI Codex
@tomasmark79
tomasmark79 marked this pull request as ready for review October 12, 2026 19:42
@tomasmark79 tomasmark79 changed the title Linux: discover installed UI fallback fonts on NixOS Fix: discover installed UI fallback fonts on NixOS Oct 12, 2026
@tomasmark79 tomasmark79 changed the title Fix: discover installed UI fallback fonts on NixOS Fix(nixos): discover installed UI fallback fonts on NixOS Oct 12, 2026
@tomasmark79 tomasmark79 changed the title Fix(nixos): discover installed UI fallback fonts on NixOS M14: Fix(nixos): discover installed UI fallback fonts on NixOS Oct 12, 2026

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested the unmodified config_path() and font_files() from ac8cb6e with the locked fontconfig-parser 0.5.8 dependency in isolated subprocesses, against native Fontconfig 2.15.0. Two cases still miss a configured fallback font:

  1. FONTCONFIG_PATH selects the correct fonts.conf, but a relative conf.d is resolved by the dependency against /etc/fonts rather than that configuration directory. The PR missed the test font; an absolute include and native Fontconfig both found it.
  2. An included directory also loads disabled/backup files. Adding 99-reset.conf.disabled, 99-reset.conf.bak, or reset.conf containing cleared the discovered directories. Native Fontconfig ignored those files and still found the font. Included directory entries should follow Fontconfig's digit-prefixed *.conf filename filter.

Both cases returned Ok(()) from the parser, so they fail silently. Could these include-resolution and filename-filter cases be covered by isolated regressions before merging? The focused harness built and ran successfully; this was not a full UI or NixOS hardware test.

@echelon

echelon commented Oct 13, 2026

Copy link
Copy Markdown
Member

Thanks for the NixOS font discovery, @tomasmark79! Before it lands, please answer @FredApps' review; the current head predates it. Two gaps in particular, each with a regression test:

  1. A relative <include>conf.d</include> should resolve against the directory of the config file that includes it (for example under FONTCONFIG_PATH), not against /etc/fonts.
  2. Included directories currently load *.conf.disabled, *.bak and similar files that Fontconfig itself ignores; it only reads *.conf. One of those files containing <reset-dirs/> could silently clear the font directory list.

Note that #1611 and #1624 touch unix_fonts in system_fonts.rs too, so expect a small rebase.

Please join us on Discord if you haven't already: https://discord.gg/artcraft

Resolve nested relative includes, filter numbered configuration files, and bound traversal. Add isolated regressions for both review findings and retain current upstream font fallbacks. Repair upstream validation regressions from background opening and use strict Clippy without exceptions.

Assisted-by: OpenAI Codex
Select inline document preparation in the existing drop-dispatch test, matching the validation fix used by both Wayland branches. Full workspace, strict Clippy and wasm gates pass.

Assisted-by: OpenAI Codex
Preserve new GoToR consent checks while waiting for background opens and password retries in their regressions. Workspace tests, strict Clippy and full wasm gates pass against e88dd72.

Assisted-by: OpenAI Codex

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed 7c83e2a. Both findings from my earlier review are resolved. I reran the original isolated relative/absolute include and disabled/backup/non-numbered config cases against the production discovery functions and locked dependency; all five now find the configured font, agreeing with native Fontconfig 2.15.0. All 10 Fontconfig unit tests also passed.

The relative include resolution and digit-prefixed *.conf filtering are now covered directly. I found no remaining blocker in the font-discovery changes. This was focused Linux validation, not a NixOS hardware test or a new full-workspace run.

The shared asynchronous PDF-opening repair was also checked on #1580's byte-identical implementation: 22 command tests and 30 document-link tests passed in worker mode. One pre-existing converted-input continuation gap remains outside that PDF repair: selecting a .txt file for Export Text opens the document but does not launch the tool. I reproduced it separately and noted it on #1580; #1634 now includes coverage for that path.

@FredApps

Copy link
Copy Markdown
Contributor

@echelon Passes review and ready to merge

echelon and others added 2 commits October 15, 2026 03:51
The af.rs clippy refactor is already on main, main carries the tool through background opens
(open_path_then) and has its own waits in the touched UI tests, so those hunks and the matching
roadmap entry are dropped. The Fontconfig scan is skipped when PDFCRAFT_SYSTEM_FONTS=0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	ROADMAP.md
#	docs/roadmap.md
@echelon
echelon merged commit ac69c29 into storytold:main Oct 15, 2026
@echelon

echelon commented Oct 15, 2026

Copy link
Copy Markdown
Member

Thank you @tomasmark79 for finding installed UI fallback fonts on NixOS through fontconfig! It's merged into main now.

We trimmed the branch to the Fontconfig change (the other fixes had reached main separately) and skip the scan when installed fonts are turned off. Lander note: the bounded scan (<=32768 entries) still runs once on the UI thread at startup, lazily via OnceLock.

If you haven't already, please come and join us on Discord: https://discord.gg/artcraft

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