Skip to content

fix(text): resolve a declared font weight in one place, so paint and measure agree - #469

Merged
LeadcodeDev merged 4 commits into
mainfrom
fix/font-weight-466
Oct 1, 2026
Merged

LeadcodeDev merged 4 commits into
mainfrom
fix/font-weight-466

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

A numeric font-weight of 600 or more was painted as 700. The painter collapsed it to FontWeight::Bold, the measurer passed the exact number through, and when several weights of a family are registered the two resolve different files: 900 was drawn in 800 or 700, and the box measured on the 900 face came out a few pixels narrower than the face actually painted. A title that fits on one line then wrapped at paint time while its box kept a single-line height, so the second line was drawn over the next sibling.

Measuring the other direction turned up two divergences the issue does not mention, both the same kind of thing. bolder measured 800 and painted 700. lighter measured 300 and painted 400. gradient_text already passed numbers through, so it was correct on the numeric case and wrong on both keywords — which is why fixing only the >= 600 arm would have left two thirds of the defect in place.

The reason all three existed is that the mapping was written seven times: once in intrinsic.rs::weight_to_u16 for measurement, and six times as a match in text, caption, counter, number_wheel, rich_text and gradient_text. renderer::css_font_weight is now the single expression of it and all seven callers go through it. That is the part worth reviewing rather than the arithmetic: the agreement between measuring and painting stops being a rule each painter has to remember and becomes a thing there is only one place to write.

Five painters dropped the schema FontWeight they were using as an intermediary. It stays where it is genuinely the input rather than a re-encoding of a CssStyle: shape's embedded text config, and rich_text's per-span override, both deserialised from JSON. rich_text::make_font now takes a resolved Weight, so the span path converts its own schema value through the exact to_skia_weight and the default path goes through css_font_weight. shape::text_in_shape_font_style is untouched; its mapping was already exact.

Verification. Reproduced on main offline, since Inter 400-900 are already in the font cache and a cached face fetches nothing. Varying only fonts[0].weights on the issue's scenario, the title's ink rows and black ink mass:

loaded weights before after
[900] 436-548, 1 line, 80885 436-548, 1 line, 80885
[800, 900] 436-698, 2 lines, 67713 436-548, 1 line, 80885
[500, 900] 436-698, 2 lines, 46412 436-548, 1 line, 80885
[400 … 900] 436-698, 2 lines, 60161 436-548, 1 line, 80885

All four now match the single-weight render exactly. The identical ink mass is the point: it shows the painted face is the 900 one, not merely that the wrap stopped. [500, 900] carrying the least ink before is the tie at distance 200 resolving to the first registered variant, as the issue said.

Four tests. A table over every input class of css_font_weight, including the 1..1000 clamp. One that registers two faces at 800 and 900 with distinguishable bytes and asserts font-weight: 900 reaches the 900 one, while asking for 700 — what the painter used to do — lands on the 800 face; that one needs no font files and no network, so it holds on a bare runner. Two on Caption::resolve_font_style, which is a real paint path: one over 600/700/800/900, one on bolder and lighter. The two existing tests there covered only bold and 350, which is why the bug survived them.

cargo fmt --all --check, cargo clippy --workspace --all-targets --features rustmotion/studio -- -D warnings, cargo test --workspace --features rustmotion/studio: 2150 passed, 0 failed, 10 ignored across 52 suites.

The toolchain pin this needed is merged (#467), so this targets main and its diff is this change alone.

Closes #466

…ted From impl

`redundant_field_names` now fires on the two `#[from]` variants whose field is
named `source`: the derive generates `Self::JsonParse { source: source }`, and
1.99 attributes that to our field span. rustc 1.98.1 did not report it.

The allow sits on the module, not on the variants, because the generated
`impl From` is a sibling item of the enum rather than part of it: an attribute
on the variant leaves the lint exactly where it was, which is what the first
attempt measured. Renaming the field is not an option either way, since
`source` is what thiserror reads to implement `Error::source` and what the
`#[error("...{source}")]` strings interpolate.
CI installed the floating `stable` channel, so the version it tested was
whatever had shipped by the time the job ran. On 2026-10-01 that turned main red
with no commit in between: rustc moved 1.98.1 -> 1.99.0, clippy gained a lint on
code the repository had not touched, and the schema PR that was green on its own
branch failed once merged.

`rust-toolchain.toml` is now the single place the version is written.
`rustup toolchain install` with no argument reads that file, including its
`components`, so there is no second copy in the workflow to drift out of step
and no way for CI to check a version the working tree does not. It replaces
`dtolnay/rust-toolchain` in all four CI jobs and in publish: that action's
`toolchain` input is required and defaults to `stable`, and it does not read
the manifest, so keeping it would have meant writing the version five more
times. Runners ship rustup, and dropping the action removes a third-party
dependency from every job.

Relying on rustup's implicit auto-install was the alternative and is rejected:
it works today, but rustup prints a deprecation for it and says it may stop
working, which is the same class of delayed breakage this commit exists to
remove.

The `audit` job stays as it is. It is designed to fail on a new advisory, and
the ten `--ignore` entries are reviewed on a date written next to them. Making
that job unconditionally green would mean not auditing.
…measure agree

A numeric `font-weight` of 600 or more was painted as 700. The painter collapsed
it to `FontWeight::Bold`, the measurer passed the exact number through, and with
several weights of a family registered the two picked different faces: 900 was
drawn in 800 or 700, and the box measured on the 900 face was a few pixels
narrower than the face actually painted, so a title that fit on one line wrapped
at paint time while its box kept a single-line height. The second line landed on
top of the next sibling.

Measuring the other direction turned up two more divergences the issue does not
mention. `bolder` measured 800 and painted 700; `lighter` measured 300 and
painted 400. `gradient_text` already passed numbers through, so it was right on
the numeric row and wrong on both keyword rows.

The cause is that the mapping existed seven times: once in
`intrinsic.rs::weight_to_u16` for measurement and six times as a `match` in the
painters. `renderer::css_font_weight` is now the only expression of it and all
seven callers go through it, so the invariant is not a rule the painters have to
remember. Five of them dropped the schema `FontWeight` they were using as an
intermediary; it stays where it is genuinely the input, which is `shape`'s
embedded text config and `rich_text`'s per-span override, both deserialised from
JSON rather than read off a `CssStyle`.

Verified on the issue's scenario with Inter 400-900 registered. Varying only
`fonts[0].weights`, the title's ink rows and black ink mass were 436-548 / 80885
for `[900]` and 436-698 / 67713, 46412, 60161 for `[800,900]`, `[500,900]` and
`[400..900]`. All four now read 436-548 / 80885, identical to the single-weight
render, which is what shows the painted face is the 900 one rather than merely
that the wrap is gone.

Closes #466
The entry document already carries the same shape of rule for subpixel_font.
A painter that writes the obvious match gets the divergence back.
@LeadcodeDev
LeadcodeDev merged commit 2605ce6 into main Oct 1, 2026
4 checks passed
@LeadcodeDev
LeadcodeDev deleted the fix/font-weight-466 branch October 1, 2026 16:18
@LeadcodeDev LeadcodeDev mentioned this pull request Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

font-weight 800/900 is painted as 700 while measured at its real weight, so text wraps over its next sibling when several weights are loaded

1 participant