fix: new_from_descriptor lose font variations, such as font-weight#430
fix: new_from_descriptor lose font variations, such as font-weight#430kaiwk wants to merge 1 commit into
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. |
| 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
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.