fix: new_from_descriptor lose font variations, such as font-weight #430
Conversation
|
Can you add a test? |
|
@jrmuizel I have to add I encountered the issue when using font-kit, font-kit manually unpack 'otc/ttc', I suppose it won't make any difference here. |
|
The test failed on xcode 7.3/9.2 and passed others. |
| @@ -127,7 +129,21 @@ pub fn new_from_descriptor(desc: &CTFontDescriptor, pt_size: f64) -> CTFont { | |||
|
|
|||
| pub fn new_from_buffer(buffer: &[u8]) -> Result<CTFont, ()> { | |||
| let ct_font_descriptor = create_font_descriptor(buffer)?; | |||
| Ok(new_from_descriptor(&ct_font_descriptor, 16.0)) | |||
| Ok(new_from_name(&ct_font_descriptor.font_name(), 16.0)?) | |||
There was a problem hiding this comment.
Won't this break if the font is not installed or if two different fonts use the same name? i.e. new_from_name won't find the font.
There was a problem hiding this comment.
Well, looks like WebRender also creates CTFont from postscript name, see: https://phabricator.services.mozilla.com/D93518
// there's no way great way to go from a CGFont to a CTFontDescriptor // so we use the postscript name. Ideally NativeFontHandle would // just use a CTFontDescriptor let name = native_font_handle.0.postscript_name(); let font = core_text::font_descriptor::new_from_postscript_name(&name); self.ct_font_descs .insert(*font_key, font);
Also, font_name() returns postscript name, and new_fomr_name receives postscript name.
There was a problem hiding this comment.
WebRender only uses the postscript name for installed system fonts. We don't use the name for data fonts:
https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/platform/macos/font.rs#382
kaiwk
force-pushed
the
new-from-name
branch
4 times, most recently
from
a120e4a to
1605589
Compare
use new_from_name instead
|
I also add an api called |
|
☔ The latest upstream changes (presumably #485) made this pull request unmergeable. Please resolve the merge conflicts. |
If we create
CTFontfrom font descriptor, we will lose font variations, and usenew_from_nameinstead will fix it.CTFontDescriptor is essentially a dictionary of font attributes.