From c74abcaa5c33ae68a5877ac27deff72fc056d5e4 Mon Sep 17 00:00:00 2001 From: "Brian J. Burg" Date: Wed, 14 Nov 2012 17:10:57 -0800 Subject: [PATCH] Add some RAII and black magic to let harfbuzz determine font table blob lifetimes. --- src/servo-gfx/font.rs | 14 +++++- src/servo-gfx/quartz/font.rs | 64 ++++++++++++++++++--------- src/servo-gfx/text/harfbuzz/shaper.rs | 30 ++++++++++--- 3 files changed, 79 insertions(+), 29 deletions(-) diff --git a/src/servo-gfx/font.rs b/src/servo-gfx/font.rs index b996ea490b2..1bfb35f0d0e 100644 --- a/src/servo-gfx/font.rs +++ b/src/servo-gfx/font.rs @@ -35,7 +35,7 @@ pub trait FontHandleMethods { fn glyph_index(codepoint: char) -> Option; fn glyph_h_advance(GlyphIndex) -> Option; fn get_metrics() -> FontMetrics; - fn get_table_for_tag(FontTableTag) -> Option<~[u8]>; + fn get_table_for_tag(FontTableTag) -> Option; } // TODO(Issue #163): this is a workaround for static methods and @@ -74,6 +74,16 @@ impl FontTableTag : FontTableTagConversions { } } +#[cfg(target_os = "macos")] +pub type FontTable/& = quartz::font::QuartzFontTable; + +#[cfg(target_os = "linux")] +pub type FontTable/& = freetype::font::FreeTypeFontTable; + +trait FontTableMethods { + fn with_buffer(fn&(*u8, uint)); +} + struct FontMetrics { underline_size: Au, underline_offset: Au, @@ -311,7 +321,7 @@ impl Font { shaper } - fn get_table_for_tag(tag: FontTableTag) -> Option<~[u8]> { + fn get_table_for_tag(tag: FontTableTag) -> Option { let result = self.handle.get_table_for_tag(tag); let status = if result.is_some() { "Found" } else { "Didn't find" }; diff --git a/src/servo-gfx/quartz/font.rs b/src/servo-gfx/quartz/font.rs index eddf5ec03fe..f4bb8fb5398 100644 --- a/src/servo-gfx/quartz/font.rs +++ b/src/servo-gfx/quartz/font.rs @@ -2,12 +2,35 @@ extern mod core_foundation; extern mod core_graphics; extern mod core_text; +use cf = core_foundation; +use cf::base::{ + CFIndex, + CFRelease, + CFTypeOps, + CFTypeRef, +}; +use cf::data::{CFData, CFDataRef}; +use cf::string::UniChar; +use cg = core_graphics; + +use cg::base::{CGFloat, CGAffineTransform}; +use cg::data_provider::{CGDataProviderRef, CGDataProvider}; +use cg::font::{CGFontCreateWithDataProvider, CGFontRef, CGFontRelease, CGGlyph}; +use cg::geometry::CGRect; + +use ct = core_text; +use ct::font::CTFont; +use ct::font_descriptor::{kCTFontDefaultOrientation, CTFontSymbolicTraits}; +use ct::font_descriptor::{SymbolicTraitAccessors}; + use font_context::QuartzFontContextHandle; use geometry::Au; use gfx_font::{ CSSFontWeight, FontHandleMethods, FontMetrics, + FontTable, + FontTableMethods, FontTableTag, FontWeight100, FontWeight200, @@ -23,24 +46,25 @@ use gfx_font::{ }; use text::glyph::GlyphIndex; -use cf = core_foundation; -use cf::base::{ - CFIndex, - CFRelease, - CFTypeRef -}; -use cf::data::CFData; -use cf::string::UniChar; -use cg = core_graphics; -use cg::base::{CGFloat, CGAffineTransform}; -use cg::data_provider::{CGDataProviderRef, CGDataProvider}; -use cg::font::{CGFontCreateWithDataProvider, CGFontRef, CGFontRelease, CGGlyph}; -use cg::geometry::CGRect; use core::libc::size_t; -use ct = core_text; -use ct::font::CTFont; -use ct::font_descriptor::{kCTFontDefaultOrientation, CTFontSymbolicTraits}; -use ct::font_descriptor::{SymbolicTraitAccessors}; + +struct QuartzFontTable { + data: CFData, + + drop { } +} + +pub impl QuartzFontTable { + static fn wrap(data: CFData) -> QuartzFontTable { + QuartzFontTable { data: move data } + } +} + +pub impl QuartzFontTable : FontTableMethods { + fn with_buffer(blk: fn&(*u8, uint)) { + blk(self.data.bytes(), self.data.len()); + } +} pub struct QuartzFontHandle { priv mut cgfont: Option, @@ -182,10 +206,10 @@ pub impl QuartzFontHandle : FontHandleMethods { return metrics; } - fn get_table_for_tag(tag: FontTableTag) -> Option<~[u8]> { - let result = self.ctfont.get_font_table(tag); + fn get_table_for_tag(tag: FontTableTag) -> Option { + let result : Option = self.ctfont.get_font_table(tag); return option::chain(move result, |data| { - Some(data.copy_to_buf()) + Some(QuartzFontTable::wrap(move data)) }); } diff --git a/src/servo-gfx/text/harfbuzz/shaper.rs b/src/servo-gfx/text/harfbuzz/shaper.rs index f23bab73a6f..03a3b39c26a 100644 --- a/src/servo-gfx/text/harfbuzz/shaper.rs +++ b/src/servo-gfx/text/harfbuzz/shaper.rs @@ -7,6 +7,7 @@ use au::Au; use font::{ Font, + FontTable, FontTableTag, }; @@ -191,16 +192,31 @@ extern fn get_font_table_func(_face: *hb_face_t, tag: hb_tag_t, user_data: *c_vo let font: *Font = user_data as *Font; assert font.is_not_null(); - // TODO(Issue #197): reuse font table data, which will change return type here. - // it will also require us to provide deletion callbacks to hb_blob_create, so - // that refcounts of shared blobs can be managed. + // TODO(Issue #197): reuse font table data, which will change the unsound trickery here. match (*font).get_table_for_tag(tag as FontTableTag) { None => return ptr::null(), - Some(table_buffer) => { - let blob: *hb_blob_t = vec::as_imm_buf(table_buffer, |buf: *u8, len: uint| { - hb_blob_create(buf as *c_char, len as c_uint, HB_MEMORY_MODE_READONLY, null(), null()) + Some(ref font_table) => { + let skinny_font_table = ~font_table; + let skinny_font_table_ptr = ptr::to_unsafe_ptr(skinny_font_table); + let mut blob: *hb_blob_t = ptr::null(); + (*skinny_font_table_ptr).with_buffer(|buf: *u8, len: uint| { + blob = hb_blob_create(buf as *c_char, + len as c_uint, + HB_MEMORY_MODE_READONLY, + cast::transmute(skinny_font_table_ptr), // private context for below. + destroy_blob_func); // HarfBuzz calls this when blob not needed. }); + assert blob.is_not_null(); return blob; } } -} \ No newline at end of file +} + +// TODO(Issue #197): reuse font table data, which will change the unsound trickery here. +// In particular, we'll need to cast to a boxed, rather than owned, FontTable. + +// even better, should cache the harfbuzz blobs directly instead of recreating a lot. +extern fn destroy_blob_func(user_data: *c_void) unsafe { + // this will cause drop to run. + let _wrapper : &~FontTable = cast::transmute(user_data); +}