Skip to content

Always fallback fonts using the font-family priority list - #1036

Open
bczhc wants to merge 3 commits into
linebender:mainfrom
bczhc:priority-list-font-resolving
Open

bczhc wants to merge 3 commits into
linebender:mainfrom
bczhc:priority-list-font-resolving

Conversation

@bczhc

@bczhc bczhc commented Mar 8, 2026 •

Copy link
Copy Markdown

The old font resolving approach is to scan the whole tspan text and try to find a font to match all the text. According to CSS font-family font resolving schema, it should resolve and fallback using the family name one by one, making font uses more ergonomic.

Component values are a comma-separated list indicating alternatives. A user agent iterates through the list of family names until it matches an available font that contains a glyph for the character to be rendered.

This implements a simple resolving algorithm doing what it states above.

Fix #301, #916.

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg
        width="100.147198mm"
        height="32.596107mm"
        viewBox="0 0 57.147198 30.596108"
        version="1.1"
        id="svg1"
        xmlns="http://www.w3.org/2000/svg"
>
    <text
            xml:space="preserve"
            x="14.375127"
            id="text1"><tspan
       id="tspan1"
       style="font-family: 'Noto Sans', 'Noto Sans CJK SC', 'Noto Color Emoji';"
       x="14.375127"
       y="20.459293">Text文字😀</tspan></text>
</svg>

Tests

Before:
3

After:
new

@bczhc
bczhc force-pushed the priority-list-font-resolving branch from 1f6c1f4 to 1c66211 Compare March 10, 2026 09:36
thekingofcity and others added 3 commits March 10, 2026 17:38
Co-authored-by: Zhai Can <bczhc0@126.com>
This is not rendered correctly before. Just ignore
and make the test happy for now.
@bczhc
bczhc force-pushed the priority-list-font-resolving branch from 1c66211 to 3e7475e Compare March 10, 2026 09:40
@bczhc

bczhc commented Mar 10, 2026 •

Copy link
Copy Markdown
Author

The CI outputs:

---- render::text_font_family_fallback_3 stdout ----
Warning (in usvg::text:183): Fallback from Noto Sans to Noto Color Emoji.
Warning (in usvg::text:183): Fallback from Noto Serif to Noto Color Emoji.

On my machine it passes. Falling back to Twitter Color Emoji:

---- render::text_font_family_fallback_3 stdout ----
Warning (in usvg::text:183): Fallback from Noto Sans to Twitter Color Emoji.
Warning (in usvg::text:183): Fallback from Noto Serif to Twitter Color Emoji.

successes:
    render::text_font_family_fallback_3

Indeterministic font fallback causes the test failure. Honestly I have no idea why is that. Solution thoughts?

@luisbg

luisbg commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Why does this PR change rtl.png? It isn't clear to me.

Thank you.

@bczhc

bczhc commented Sep 15, 2026

Copy link
Copy Markdown
Author

Hmm because it fails. -- Ok though I've lost some context for this pr now but I may be sure it's because this PR that changes font resolving logic has effects on rtl.svg (this svg uses a comma-separated font list; before the font resolving logic change, resvg will only use the first font to render it, which is not correct).

Also btw, rtl.png rendered by resvg is not correct itself now. The expected result is:

image

because currently (at least at the time I made the pr) resvg does not have a correct rtl handing.

@luisbg

luisbg commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

True, #583 does track that RTL is mishandled.

The golde test image changing makes sense to me. The PR changes which font supplies which run, so rtl.svg does render differently. My only ask is that this gets written down in the PR description to make it clear.

@luisbg

luisbg commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@nicoburns and @DJMcNab , I think this PR might be hitting a CI/harness issue. It sometimes passes and sometimes not because of the iteration order of std::fs::read_dir.

I am going to open a PR to replace load_fonts_dir in the test harness with an explicit sorted load.

@nicoburns

Copy link
Copy Markdown
Collaborator

@luisbg please can you stop tagging me in these. Tagging me does not give me any more time to actually review these PRs.

Actual review comments that explain what review steps you have performed (reviewing the implementation, comparing it with various specs, testing the code against SVGs, etc), and/or reasons for why you think a given PR is good/bad are helpful.

@luisbg

luisbg commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Harness fix at #1136

What I checked to review this PR:

  • Read the issue and the code fix to familiarize myself with things.
  • The bug is real and still present. I applied the PR's test files alone to unmodified main and two tests fail.
  • Applying the code changes fixes those two tests. Since there is a conflict I had to do this by hand.
  • On rtl.png, I also confirmed renders are visibly wrong, most of the Arabic is missing either way, which is Incorrect dx/dy handling for RTL scripts #583.
  • I saw CI was not passing and I investigated why. Since it's outside the scope of this PR I fixed it in a new one. Note: a new golden image will need to be generated for this PR to pass the deterministic order.
  • I had an LLM assist me on running tests and figuring out the CI issue.
  • I did not read specs.

Overall I think this is worth landing: it fixes #301 (open since 2020) and #916 , from what I've read the approach matches how CSS specifies font-family resolution, and the conflict is confined to text/layout.rs

@ZhaoZuohong

Copy link
Copy Markdown

Hi @bczhc, thanks for this PR. We hit this bug while rendering charts with mixed Latin/CJK labels, and I tested your branch against current main.

Reproduction

To make the result independent of directory order, I skipped system fonts and loaded the fonts in a fixed order: NotoSans-Regular.ttf (from tests/fonts), then NotoSerifCJK-Regular.ttc, then NotoSansCJK-Regular.ttc. The test string is Chart 中文图表.

resvg --skip-system-fonts \
  --use-font-file NotoSans-Regular.ttf \
  --use-font-file NotoSerifCJK-Regular.ttc \
  --use-font-file NotoSansCJK-Regular.ttc \
  --sans-serif-family "Noto Sans" \
  in.svg out.png
font-family main (77b12f4) this PR (3e7475e)
'Noto Sans', 'Noto Sans CJK SC' the whole run, Latin included, becomes Noto Serif CJK JP Latin in Noto Sans, CJK in Noto Sans CJK SC ✅
sans-serif whole run in Noto Serif CJK JP Latin in Noto Sans, CJK in Noto Serif CJK JP (as expected: the declared list has no CJK family)
Inter, 'Noto Sans CJK SC', system fonts (includes user-installed fonts) whole run in FZFangSong-Z02 (the first face in fontdb's load order that covers the characters) Latin in Inter, CJK in Noto Sans CJK SC ✅
evidence

On main, the "replace all glyphs when the fallback font matches everything" path also swaps out the Latin text. The PR fixes this as well, which is probably worth mentioning next to the rtl.png note that @luisbg asked for.

Review notes

  1. The custom resolver is bypassed for declared families. The family loop calls fontdb.query directly, so a custom FontResolver::select_font (for example one that loads fonts on demand, as the FontSelectionFn docs suggest) is never consulted. Building a single-family Font for each entry and passing it to resolver.select_font would keep resolvers in control. It would also address the concern behind Prefer declared font-family entries during glyph fallback #1041 without changing the public API.
  2. fonts_cache is now unused. The build warns unused variable: fonts_cache, and the font is re-selected for every span. With font-family="sans-serif" and no match, No match for 'sans-serif' font-family is logged twice where main logs it once. Either drop the cache plumbing or cache per family.
  3. Rebase. Since tests: load test fonts in a deterministic order #1136 landed, the golden images need to be regenerated under the deterministic font order.

Offer

Would you like to keep driving this? If you're short on time, I'm happy to rebase it onto main, apply the points above and regenerate the references. I can push that as a follow-up PR that keeps your commits and authorship, or send you a patch for your branch, whichever you prefer.

Disclosure: I used an AI assistant to help investigate and build the comparison above. I checked the results by hand.

@bczhc

bczhc commented Oct 15, 2026 •

Copy link
Copy Markdown
Author

This PR is only trying to make font get resolved at a character level (instead of old behavior - applying to the whole text) from a deterministic given font-family priority list. I guess it goes to have other issues with things like generic family etc; and this pr may only serve as a workaround now because they claimed there's a wholly new font resolving engine under development anyway.

@ZhaoZuohong

Copy link
Copy Markdown

Thanks for getting back to me, @bczhc, and for the context!

That framing makes sense to me. This PR doesn't try to be a full font cascade, and I don't think it needs to. It fixes the part users run into most: falling back through the font-family list one character at a time instead of swapping out the whole run. From what I can see in #862, the move to fontique is still the long-term plan, but nobody has picked it up yet and RazrFalcon mentioned fontique isn't finished. Since #301 has been open since 2020, I think it's still worth landing this in the meantime. The change is mostly contained in layout.rs and doesn't touch the public API, so it shouldn't make a later migration harder.

On generic families: in my tests, sans-serif behaves as I'd expect with this PR. Characters the resolved family can't cover go to system fallback, and that's where the remaining issues are (picking by load order, ignoring lang). I've brought that part up separately in #862, so I don't think it needs to block this PR.

If you're OK with it, I'll open a follow-up PR based on your branch that keeps your commits and authorship. On top of them, I'd:

  • rebase onto current main
  • route each declared family through resolver.select_font, so custom resolvers stay in control
  • clean up the unused fonts_cache
  • regenerate the reference images under the deterministic font order from tests: load test fonts in a deterministic order #1136, and note the rtl.png change in the description

I'll link it here once it's up. Of course, if you'd rather keep going with this one yourself, just let me know and I'll send the changes your way instead. Thanks again for starting this!

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.

(usvg) Prefer font-family during font fallback

5 participants