Skip to content

fix: new_from_descriptor lose font variations, such as font-weight #430

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: new_from_descriptor lose font variations, such as font-weight #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 58 additions & 4 deletions core-text/src/font.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
use font_descriptor;
use font_descriptor::{CTFontDescriptor, CTFontDescriptorRef, CTFontOrientation};
use font_descriptor::{CTFontSymbolicTraits, CTFontTraits, SymbolicTraitAccessors, TraitAccessors};
use font_manager::create_font_descriptor;
use font_manager::{create_font_descriptor, create_font_descriptors, create_font_descriptor_with_data};

use core_foundation::array::{CFArray, CFArrayRef};
use core_foundation::base::{CFIndex, CFOptionFlags, CFType, CFTypeID, CFTypeRef, TCFType};
use core_foundation::base::{CFIndex, CFOptionFlags, CFType, CFTypeID, CFTypeRef, TCFType, TCFTypeRef};
use core_foundation::data::{CFData, CFDataRef};
use core_foundation::dictionary::{CFDictionary, CFDictionaryRef};
use core_foundation::number::CFNumber;
Expand All @@ -31,6 +31,7 @@ use foreign_types::ForeignType;
use libc::{self, size_t};
use std::os::raw::c_void;
use std::ptr;
use std::sync::Arc;

type CGContextRef = *mut <CGContext as ForeignType>::CType;
type CGFontRef = *mut <CGFont as ForeignType>::CType;
Expand Down Expand Up @@ -127,7 +128,26 @@ 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)?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@kaiwk kaiwk May 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

pub fn new_from_arc_buffer<T: AsRef<[u8]> + Sync + Send>(buffer: Arc<T>) -> Result<CTFont, ()> {
let ct_font_descriptor = create_font_descriptor_with_data(CFData::from_arc(buffer))?;
Ok(new_from_name(&ct_font_descriptor.font_name(), 16.0)?)
}

pub fn new_fonts_from_buffer(buffer: &[u8]) -> Result<Vec<CTFont>, ()> {
let ct_font_descriptors = create_font_descriptors(buffer)?;

let mut ct_fonts = Vec::new();
unsafe {
for descriptor_ptr in ct_font_descriptors.get_all_values().into_iter() {
let descriptor = CTFontDescriptor::wrap_under_get_rule(CTFontDescriptorRef::from_void_ptr(descriptor_ptr));
ct_fonts.push(new_from_name(&descriptor.font_name(), 16.0)?);
}
}

Ok(ct_fonts)
}

pub fn new_from_name(name: &str, pt_size: f64) -> Result<CTFont, ()> {
Expand Down Expand Up @@ -646,6 +666,40 @@ extern {
fn CTFontGetTypeID() -> CFTypeID;
}

#[test]
fn test_font_descriptor_and_variation () {
use std::io::Read;
let mut f = std::fs::File::open("/System/Library/Fonts/Helvetica.ttc").unwrap();
let mut font_data = Vec::new();
f.read_to_end(&mut font_data).unwrap();
let descriptors = create_font_descriptors(&font_data).unwrap();

unsafe {
for descriptor_ptr in descriptors.get_all_values().into_iter() {
let descriptor = CTFontDescriptor::wrap_under_get_rule(CTFontDescriptorRef::from_void_ptr(descriptor_ptr));

// when create ttc/otc fonts from descriptor, all font variations are the same
let font = new_from_descriptor(&descriptor, 12.);
println!("font name= {:?}, desc.name= {:?}", font.face_name(), descriptor.font_name());

let sym = font.symbolic_traits();
assert_eq!(false, sym.is_italic());
assert_eq!(false, sym.is_bold());
assert_eq!(false, sym.is_expanded());
assert_eq!(false, sym.is_condensed());
assert_eq!(false, sym.is_monospace());

let all_traits = font.all_traits();
assert_eq!(0., all_traits.normalized_weight());
assert_eq!(0., all_traits.normalized_width());

// we have to create them from font name
let font = new_from_name(&descriptor.font_name(), 12.).unwrap();
debug_font_traits(&font);
}
}
}

#[test]
fn copy_font() {
use std::io::Read;
Expand Down Expand Up @@ -741,4 +795,4 @@ fn copy_system_font() {
assert!(matching.attributes().find(CFString::from_static_string("NSFontSizeAttribute")).is_none());

assert_eq!(small.postscript_name(), cgfont.postscript_name());
}
}
14 changes: 13 additions & 1 deletion core-text/src/font_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use core_foundation::base::TCFType;
use core_foundation::string::CFString;
use core_foundation::url::CFURLRef;
use core_foundation::data::{CFDataRef, CFData};
use crate::font_descriptor::{CTFontDescriptorRef, CTFontDescriptor};
use crate::font_descriptor::{CTFontDescriptor, CTFontDescriptorRef};

pub fn copy_available_font_family_names() -> CFArray<CFString> {
unsafe {
Expand Down Expand Up @@ -41,6 +41,17 @@ pub fn create_font_descriptor_with_data(data: CFData) -> Result<CTFontDescriptor
}
}

pub fn create_font_descriptors(buffer: &[u8]) -> Result<CFArray<CTFontDescriptor>, ()> {
let cf_data = CFData::from_buffer(buffer);
unsafe {
let ct_font_descriptors_ref = CTFontManagerCreateFontDescriptorsFromData(cf_data.as_concrete_TypeRef());
if ct_font_descriptors_ref.is_null() {
return Err(());
}
Ok(CFArray::wrap_under_create_rule(ct_font_descriptors_ref))
}
}

extern {
/*
* CTFontManager.h
Expand All @@ -55,6 +66,7 @@ extern {
pub fn CTFontManagerCopyAvailablePostScriptNames() -> CFArrayRef;
pub fn CTFontManagerCreateFontDescriptorsFromURL(fileURL: CFURLRef) -> CFArrayRef;
pub fn CTFontManagerCreateFontDescriptorFromData(data: CFDataRef) -> CTFontDescriptorRef;
pub fn CTFontManagerCreateFontDescriptorsFromData(data: CFDataRef) -> CFArrayRef;
//pub fn CTFontManagerCreateFontRequestRunLoopSource
//pub fn CTFontManagerEnableFontDescriptors
//pub fn CTFontManagerGetAutoActivationSetting
Expand Down