Font weight support #22038
Font weight support #22038
Conversation
Added a `weight: Weight` field to `TextFont`.
`clamp` is called on `Weight` in the `From` impl for `cosmic_text::Weight`
ickshonpe
added
A-Text
ThierryBerger
left a comment
ThierryBerger
left a comment
There was a problem hiding this comment.
Looks good! But the noto font should probably be removed indeed ; Is there any reason not to use fira font ? or another one already present ?
| section_info.5 = (metrics.underline_offset * scalar).round(); | ||
| } | ||
| if let Some((id, _)) = self.map_handle_to_font_id.get(§ion_info.0) | ||
| && let Some(font) = font_system.get_font(*id, cosmic_text::Weight(section_info.6)) |
All the fonts we include atm are static, so they won't show any changes. |
Co-authored-by: Thierry Berger <contact@thierryberger.com>
|
Maybe there is some light weight minimalistic variable font we can include instead, I'll have a look. |
There was a problem hiding this comment.
Approving ; if font size is a problem we may:
- look into bringing a smaller font, maybe inter being 875kb (~half noto?)
- or choose a more "powerful"/"featureful" font to anticipate further needs (but I doubt that makes sense, anticipating unclear goals is usually a no-no)
But it quickly gets into bikeshed territory, and assets management is a wild topic which I wouldn't want to block on here.
|
Switched to MonaSans variable font. It's only 384kb, which seems reasonable. If the extra size is a problem, we could remove the static EBGaramond font. |
|
So if I understand correctly, this new font weight attribute only affects variable fonts, it has no affect on non-variable fonts? If so, this should be made clear in the documentation. |
Sort of. To use this with static fonts, you need to load multiple fonts with different weights, so that it can choose the one with the nearest font-weight. |
How does that work? Because we are still assigning a font handle to each individual text span, how can it choose between different handles? I mean, obviously this is how we want it to work, but it's not clear that this PR works that way now. In any case, my point stands: if there are limitations then they should be called out in the docs. |
alice-i-cecile
added
the
S-Needs-Review
pablo-lua
left a comment
pablo-lua
left a comment
There was a problem hiding this comment.
Very cool feature, of course will only work with variables fonts (Because those are the only type of fonts that have multiple weights), but it works well, I tested with other font here, and even comes with some simplifying in the code, LGTM
IIUC, we just rely on cosmic text to work behind the scenes and take the right gliphs. Because of this, comes with the limitation of just working with a font with variable weights on the same file (kinda rare, but not so much), but it'll not do any magic like looking for the file of a bold font in assets, in case of a not variable kind of ttf, sadly. |
pablo-lua
added
S-Ready-For-Final-Review
|
@ickshonpe can you please resolve merge conflicts? |
alice-i-cecile
left a comment
alice-i-cecile
left a comment
There was a problem hiding this comment.
Very useful, surprisingly easy, as we delegate all the hard bits.
|
@alice-i-cecile Should be good now, assuming it passes CI. |
6 participants
Objective
Add font weight support.
Solution
FontWeightstruct that newtypes au16.font: FontWeightfield onTextFont.TextPipelineduring text updates.MonSans-VariableFont.ttf. This needs a variable font for testing.Doesn't support
lighterandbolderas that would require text style inheritance, which we don't support yet.I added stretch and slant as well, but split them off from this PR. Swash only has limited variable-font support and there's no way to demonstrate that they work without other changes.
Testing
Added a basic example:
cargo run --example font_weightsShowcase