fonts: Use OS/2 table to determine if font face is already bold when synthesizing bold face for FreeType platform#39681
Merged
mrobinson merged 7 commits intoservo:mainfrom Oct 7, 2025
Conversation
Signed-off-by: Minghua Wu <[email protected]>
servo-highfive
added
the
S-awaiting-review
Signed-off-by: Minghua Wu <[email protected]>
mrobinson
reviewed
Oct 6, 2025
Comment on lines
+134
to
+143
| let face_is_bold = unsafe { | ||
| let ptr = FT_Get_Sfnt_Table(face.as_ptr(), ft_sfnt_os2); | ||
| let os2_table: *mut TT_OS2 = std::mem::transmute(ptr); | ||
| if os2_table.is_null() { | ||
| false | ||
| } else { | ||
| (*os2_table).usWeightClass >= BOLD_THRESHOLD | ||
| } | ||
| }; | ||
|
|
Member
There was a problem hiding this comment.
Isn't it possible to use fontations here?
Contributor
Author
There was a problem hiding this comment.
Interesting. I haven't thought about that. Let me give it a shot
Contributor
Author
Signed-off-by: Minghua Wu <[email protected]>
minghuaw
changed the title
Signed-off-by: Minghua Wu <[email protected]>
mrobinson
approved these changes
Oct 7, 2025
Member
mrobinson
left a comment
mrobinson
left a comment
There was a problem hiding this comment.
Does this also need to happen for web fonts?
servo-highfive
removed
the
S-awaiting-review
Co-authored-by: Martin Robinson <[email protected]> Signed-off-by: minghuaw <[email protected]>
servo-highfive
added
the
S-awaiting-review
Co-authored-by: Martin Robinson <[email protected]> Signed-off-by: minghuaw <[email protected]>
Signed-off-by: Minghua Wu <[email protected]>
Contributor
Author
Yeah, it should. I thought this would be already handled by |
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Contributor
Author
@mrobinson I just realized that I forgot to implement this for Edit: I've openend a new PR for the webfont #39713 |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 9, 2025
…t on FreeType platform (#39713) This is to fix my oversight in the previous PR #39681 where the use of OS/2 table to determine a synthetic bold face should be applied is not applied to web font. Testing: Added a new test `tests/wpt/mozilla/tests/css/font_synthesis_weight_webfont_bold.html` that checks if an already-bold webfont is "double emboldened" Part of #39637 --------- Signed-off-by: Minghua Wu <[email protected]>
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
4 participants
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.
The previous implementation in #39519 mistakenly used the
FontTemplateDescriptorto determine whether the font face itself is already bold. This PR fixes that by using theusWeightClassin theOS/2table of the font face to determine if the font face is bold already.Testing: A new testcase
/tests/wpt/mozilla/tests/css/font_synthesis_weight_static_bold.htmlis getting added in #39633. This test checks whether a bold font face gets "double emboldened"Depends on: #39633