Skip to content

Commit

Permalink
Linux: Don't hold onto bytes of system fonts
Browse files Browse the repository at this point in the history
FontTemplateData gets passed over IPC during the communication between
FontContext and FontCacheThread. Serializing and deserializing these
bytes is expensive, so this change ensures that we only do that when the
bytes can't be read from disk. A similar strategy is already used on
macos and windows.

The performance problem was particularly noticeable after implenting
font fallback, where the content process would potentially work through
a list of fonts, trying to find one which contains a certain glyph. That
could result in lots of font bytes going over IPC.
  • Loading branch information
jonleighton committed May 19, 2018
1 parent 4403bcd commit dc683a1
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 51 deletions.
80 changes: 48 additions & 32 deletions components/gfx/platform/freetype/font.rs
Expand Up @@ -5,7 +5,7 @@
use app_units::Au;
use font::{FontHandleMethods, FontMetrics, FontTableMethods};
use font::{FontTableTag, FractionalPixel, GPOS, GSUB, KERN};
use freetype::freetype::{FT_Done_Face, FT_New_Memory_Face};
use freetype::freetype::{FT_Done_Face, FT_New_Face, FT_New_Memory_Face};
use freetype::freetype::{FT_F26Dot6, FT_Face, FT_FaceRec};
use freetype::freetype::{FT_Get_Char_Index, FT_Get_Postscript_Name};
use freetype::freetype::{FT_Get_Kerning, FT_Get_Sfnt_Table, FT_Load_Sfnt_Table};
Expand All @@ -20,6 +20,7 @@ use platform::font_context::FontContextHandle;
use platform::font_template::FontTemplateData;
use servo_atoms::Atom;
use std::{mem, ptr};
use std::ffi::CString;
use std::os::raw::{c_char, c_long};
use std::sync::Arc;
use style::computed_values::font_stretch::T as FontStretch;
Expand Down Expand Up @@ -87,6 +88,39 @@ impl Drop for FontHandle {
}
}

fn create_face(
lib: FT_Library,
template: &FontTemplateData,
pt_size: Option<Au>,
) -> Result<FT_Face, ()> {
unsafe {
let mut face: FT_Face = ptr::null_mut();
let face_index = 0 as FT_Long;

let result = if let Some(ref bytes) = template.bytes {
FT_New_Memory_Face(lib, bytes.as_ptr(), bytes.len() as FT_Long, face_index, &mut face)
} else {
// This will trigger a synchronous file read in the layout thread, which we may want to
// revisit at some point. See discussion here:
//
// https://github.com/servo/servo/pull/20506#issuecomment-378838800

let filename = CString::new(&*template.identifier).expect("filename contains NUL byte!");
FT_New_Face(lib, filename.as_ptr(), face_index, &mut face)
};

if !succeeded(result) || face.is_null() {
return Err(());
}

if let Some(s) = pt_size {
FontHandle::set_char_size(face, s).or(Err(()))?
}

Ok(face)
}
}

impl FontHandleMethods for FontHandle {
fn new_from_template(fctx: &FontContextHandle,
template: Arc<FontTemplateData>,
Expand All @@ -95,37 +129,19 @@ impl FontHandleMethods for FontHandle {
let ft_ctx: FT_Library = fctx.ctx.ctx;
if ft_ctx.is_null() { return Err(()); }

return create_face_from_buffer(ft_ctx, &template.bytes, pt_size).map(|face| {
let mut handle = FontHandle {
face: face,
font_data: template.clone(),
handle: fctx.clone(),
can_do_fast_shaping: false,
};
// TODO (#11310): Implement basic support for GPOS and GSUB.
handle.can_do_fast_shaping = handle.has_table(KERN) &&
!handle.has_table(GPOS) &&
!handle.has_table(GSUB);
handle
});

fn create_face_from_buffer(lib: FT_Library, buffer: &[u8], pt_size: Option<Au>)
-> Result<FT_Face, ()> {
unsafe {
let mut face: FT_Face = ptr::null_mut();
let face_index = 0 as FT_Long;
let result = FT_New_Memory_Face(lib, buffer.as_ptr(), buffer.len() as FT_Long,
face_index, &mut face);

if !succeeded(result) || face.is_null() {
return Err(());
}
if let Some(s) = pt_size {
FontHandle::set_char_size(face, s).or(Err(()))?
}
Ok(face)
}
}
let face = create_face(ft_ctx, &template, pt_size)?;

let mut handle = FontHandle {
face: face,
font_data: template.clone(),
handle: fctx.clone(),
can_do_fast_shaping: false,
};
// TODO (#11310): Implement basic support for GPOS and GSUB.
handle.can_do_fast_shaping = handle.has_table(KERN) &&
!handle.has_table(GPOS) &&
!handle.has_table(GSUB);
Ok(handle)
}

fn template(&self) -> Arc<FontTemplateData> {
Expand Down
37 changes: 18 additions & 19 deletions components/gfx/platform/freetype/font_template.rs
Expand Up @@ -16,34 +16,21 @@ use webrender_api::NativeFontHandle;
pub struct FontTemplateData {
// If you add members here, review the Debug impl below

pub bytes: Vec<u8>,
pub bytes: Option<Vec<u8>>,
pub identifier: Atom,
}

impl fmt::Debug for FontTemplateData {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
fmt.debug_struct("FontTemplateData")
.field("bytes", &format!("[{} bytes]", self.bytes.len()))
.field("bytes", &self.bytes.as_ref().map(|b| format!("[{} bytes]", b.len())))
.field("identifier", &self.identifier)
.finish()
}
}

impl FontTemplateData {
pub fn new(identifier: Atom, font_data: Option<Vec<u8>>) -> Result<FontTemplateData, Error> {
let bytes = match font_data {
Some(bytes) => {
bytes
},
None => {
// TODO: Handle file load failure!
let mut file = File::open(&*identifier)?;
let mut buffer = vec![];
file.read_to_end(&mut buffer).unwrap();
buffer
},
};

pub fn new(identifier: Atom, bytes: Option<Vec<u8>>) -> Result<FontTemplateData, Error> {
Ok(FontTemplateData {
bytes: bytes,
identifier: identifier,
Expand All @@ -54,17 +41,29 @@ impl FontTemplateData {
/// operation (depending on the platform) which performs synchronous disk I/O
/// and should never be done lightly.
pub fn bytes(&self) -> Vec<u8> {
self.bytes.clone()
self.bytes_if_in_memory().unwrap_or_else(|| {
let mut file = File::open(&*self.identifier).expect("Couldn't open font file!");
let mut buffer = vec![];
file.read_to_end(&mut buffer).unwrap();
buffer
})
}

/// Returns a clone of the bytes in this font if they are in memory. This function never
/// performs disk I/O.
pub fn bytes_if_in_memory(&self) -> Option<Vec<u8>> {
Some(self.bytes())
self.bytes.clone()
}

/// Returns the native font that underlies this font template, if applicable.
pub fn native_font(&self) -> Option<NativeFontHandle> {
None
if self.bytes.is_none() {
Some(NativeFontHandle {
pathname: String::from(&*self.identifier),
index: 0,
})
} else {
None
}
}
}

0 comments on commit dc683a1

Please sign in to comment.