-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fonts: Add color emoji support for FreeType #32278
Conversation
4d10fd1
to
d18a498
Compare
🔨 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)
|
|
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the reference to Cairo be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the default value of 12.0
based on a CSS specification?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
d18a498
to
14ef7e4
Compare
🔨 Triggering try run (#9083092275) for Linux WPT |
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)
|
|
89c3f3f
to
bcc1468
Compare
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.
bcc1468
to
bf97c70
Compare
Fixed the Android build and sending this back to the MQ. |
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 -d
does not report any errors./mach test-tidy
does not report any errors