fix(text): resolve a declared font weight in one place, so paint and measure agree - #469
Merged
Merged
Conversation
…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.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A numeric
font-weightof 600 or more was painted as 700. The painter collapsed it toFontWeight::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.
boldermeasured 800 and painted 700.lightermeasured 300 and painted 400.gradient_textalready passed numbers through, so it was correct on the numeric case and wrong on both keywords — which is why fixing only the>= 600arm 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_u16for measurement, and six times as amatchintext,caption,counter,number_wheel,rich_textandgradient_text.renderer::css_font_weightis 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
FontWeightthey were using as an intermediary. It stays where it is genuinely the input rather than a re-encoding of aCssStyle:shape's embedded text config, andrich_text's per-span override, both deserialised from JSON.rich_text::make_fontnow takes a resolvedWeight, so the span path converts its own schema value through the exactto_skia_weightand the default path goes throughcss_font_weight.shape::text_in_shape_font_styleis untouched; its mapping was already exact.Verification. Reproduced on
mainoffline, since Inter 400-900 are already in the font cache and a cached face fetches nothing. Varying onlyfonts[0].weightson the issue's scenario, the title's ink rows and black ink mass:[900][800, 900][500, 900][400 … 900]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 assertsfont-weight: 900reaches 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 onCaption::resolve_font_style, which is a real paint path: one over 600/700/800/900, one onbolderandlighter. The two existing tests there covered onlyboldand 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
mainand its diff is this change alone.Closes #466