fonts: Add color emoji support for FreeType #32278
Merged
Conversation
This file contains 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
mrobinson
force-pushed
the
freetype-bitmap-fonts
branch
from
4d10fd1
to
d18a498
Compare
mrobinson
added
T-linux-wpt-2013
github-actions
bot
removed
T-linux-wpt-2013
|
🔨 Triggering try run (#9075152141) for Linux WPT |
|
Test results for linux-wpt-layout-2020 from try job (#9075152141): Flaky unexpected result (13)
Stable unexpected results that are known to be intermittent (13)
Stable unexpected results (1)
|
|
Test results for linux-wpt-layout-2013 from try job (#9075152141): Flaky unexpected result (15)
Stable unexpected results that are known to be intermittent (11)
Stable unexpected results (2)
|
|
|
mukilan
reviewed
May 14, 2024
| let face_flags = unsafe { (*self).face_flags }; | ||
| if (face_flags & (FT_FACE_FLAG_FIXED_SIZES as FT_Long)) != 0 { | ||
| // We only set FT_LOAD_COLOR if there are bitmap strikes; | ||
| // COLR (color-layer) fonts are handled internally by Gecko, and |
There was a problem hiding this comment.
Suggested change
| // COLR (color-layer) fonts are handled internally by Gecko, and | |
| // COLR (color-layer) fonts are handled internally by Servo, and |
There was a problem hiding this comment.
I've updated this comment and also make it reflect the fact that we don't yet have support for COLR layers -- but will soon.
| // if the face has color bitmaps, it should be treated as scalable | ||
| // and scaled to the desired size. Metrics based on y_ppem need | ||
| // to be rescaled for the adjusted size. This makes metrics agree | ||
| // with the scales we pass to Cairo for Fontconfig fonts. |
There was a problem hiding this comment.
Should the reference to Cairo be removed?
There was a problem hiding this comment.
Yep, I've removed this sentence.
| let face = create_face(data.clone(), face_index, pt_size)?; | ||
| let mut handle = PlatformFont { | ||
| let face = create_face(data.clone(), face_index)?; | ||
| let requested_face_size = requested_size.unwrap_or_else(|| Au::from_f64_px(12.)); |
There was a problem hiding this comment.
Is the default value of 12.0 based on a CSS specification?
There was a problem hiding this comment.
Nope...this should only be None when instantiating a PlatformFont without a size -- which happens only when creating a descriptor for system fonts. I've modified this though to only set the size when the requested size isn't None and giving a 0 size to the font itself in this case. This should make more obvious what it is going on.
mrobinson
force-pushed
the
freetype-bitmap-fonts
branch
from
d18a498
to
14ef7e4
Compare
mrobinson
added
T-linux-wpt-2013
github-actions
bot
removed
T-linux-wpt-2013
|
🔨 Triggering try run (#9083092275) for Linux WPT |
mrobinson
force-pushed
the
freetype-bitmap-fonts
branch
from
14ef7e4
to
89c3f3f
Compare
|
Test results for linux-wpt-layout-2020 from try job (#9083092275): Flaky unexpected result (13)
Stable unexpected results that are known to be intermittent (12)
|
|
Test results for linux-wpt-layout-2013 from try job (#9083092275): Flaky unexpected result (10)
Stable unexpected results that are known to be intermittent (8)
Stable unexpected results (1)
|
|
|
mrobinson
force-pushed
the
freetype-bitmap-fonts
branch
from
89c3f3f
to
bcc1468
Compare
mukilan
approved these changes
May 17, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Color emoji support with "Noto Color Emoji" requires two things: 1. Support for bitmap fonts in the FreeType backend. This requires specially handling bitmap fonts which have different characteristics in the FreeType API (such as requiring metrics scaling). This support is generally ported from Gecko's implementation. 2. When a character is an emoji it "Noto Color Emoji" needs to be in the fallback list. Ensure that this is high on the list -- this will be improved in a later PR.
mrobinson
force-pushed
the
freetype-bitmap-fonts
branch
from
bcc1468
to
bf97c70
Compare
|
Fixed the Android build and sending this back to the MQ. |
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.
Color emoji support with "Noto Color Emoji" requires two things:
specially handling bitmap fonts which have different characteristics
in the FreeType API (such as requiring metrics scaling). This support
is generally ported from Gecko's implementation.
fallback list. Ensure that this is high on the list -- this will be
improved in a later PR.
This change causes one test to start failing, but this is actually a
progression. Before none of the glyphs were shown, but now since they do the
failure is visible. In general, it is difficult to test emoji support.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors