Skip to content

Include all coverage variants for a family in a query#663

Open
kane50613 wants to merge 3 commits into
linebender:mainfrom
kane50613:fix/322-multi-coverage-query
Open

Include all coverage variants for a family in a query#663
kane50613 wants to merge 3 commits into
linebender:mainfrom
kane50613:fix/322-multi-coverage-query

Conversation

@kane50613

@kane50613 kane50613 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #322

Generated with Claude and reviewed by me

Add matching::match_fonts to collect every font sharing the best match's attributes, and have the query include the whole set. The caller already walks candidates per cluster against each font's charmap, so it now lands on the slice that covers the cluster.

kane50613 added 2 commits July 1, 2026 04:29
A family may hold several fonts with identical width, style and weight that
differ only in unicode coverage, e.g. the per-unicode-range slices of a web
font registered under one family name. The query previously surfaced only the
single best match plus the family default, so the other slices were never
offered to the caller and their codepoints fell back or rendered as tofu.

Add matching::match_fonts to collect every font sharing the best match's
attributes, and have the query yield the whole set. The caller already walks
candidates per cluster against each font's charmap, so it now lands on the
slice that covers the cluster.

Fixes linebender#322.
The single caller iterates once, so skip the intermediate collection and
its allocation when a family spills past four coverage variants.

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm happy enough for this to land as-is; it does address a real case fairly surgically.

However, I do have a couple of follow-up questions:

  1. Is this definitely the right behaviour; we gate the 'later' fonts on being 'exactly as good' as the best match?
  2. Presumably, a better long-term fix would be to actually implement unicode range support. Would we still need this in that case?

@kane50613, let me know when you're happy and I'll merge it.

Comment thread fontique/src/collection/query.rs Outdated
Comment thread fontique/src/collection/query.rs Outdated
Comment thread fontique/src/matching.rs Outdated
Comment thread fontique/src/matching.rs
@kane50613

Copy link
Copy Markdown
Contributor Author

@DJMcNab Thanks for reviewing and I've guided Claude to research on how Blink does this

  1. Blink does the same: FontFaceCache::GetOrCreate groups faces by attributes and font selection picks a single best bucket.
    Worse-attribute faces never fill coverage gaps, those fall through to the next family. Mixing weights/styles within one run would be worse than falling back.

  2. Yes, we'd still need this. Blink builds unicode-range on the same grouping: equal-attribute faces form one CSSSegmentedFontFace, with ranges filtering within the bucket.
    It would also fix a downside of this PR: load_bucket eagerly loads every variant's blob, since without range metadata the only way to check coverage is the charmap. With ranges, subsets outside the needed range are never loaded.

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.

Support resolving multiple font files in the same file covering different unicode ranges

2 participants