Clean up NativeFont constructors.

src/servo/text/font_cache.rs

@@ -2,6 +2,7 @@ use font::{Font,
            FontStyle,
            FontWeight300,
            test_font_bin};
+use native_font::NativeFont;
 
 struct FontCache {
     native_lib: native::NativeFontCache,
@@ -46,7 +47,7 @@ fn create_font(lib: @FontCache, native_lib: &native::NativeFontCache) -> Result<
         italic: false,
         oblique: false
     };
-    let native_font = native_font::create(native_lib, font_bin, dummy_style.pt_size);
+    let native_font = NativeFont::new(native_lib, font_bin, dummy_style.pt_size);
     let native_font = if native_font.is_ok() {
         result::unwrap(move native_font)
     } else {

src/servo/text/freetype/native_font.rs

@@ -43,14 +43,27 @@ pub struct FreeTypeNativeFont {
     }
 }
 
-pub fn FreeTypeNativeFont(face: FT_Face, buf: @~[u8]) -> FreeTypeNativeFont {
-    assert face.is_not_null();
-    FreeTypeNativeFont { buf: buf, face: face }
-}
-
 pub impl FreeTypeNativeFont {
+    static pub fn new(lib: &FT_Library, buf: @~[u8], pt_size: float) -> Result<FreeTypeNativeFont, ()> {
+        assert lib.is_not_null();
+        let face: FT_Face = null();
+        return vec_as_buf(*buf, |cbuf, _len| {
+            if FT_New_Memory_Face(*lib, cbuf, (*buf).len() as FT_Long,
+                                  0 as FT_Long, addr_of(&face)).succeeded() {
+                let res = FT_Set_Char_Size(face, // the face
+                                           float_to_fixed_ft(pt_size) as FT_F26Dot6, // char width
+                                           float_to_fixed_ft(pt_size) as FT_F26Dot6, // char height
+                                           72, // horiz. DPI
+                                           72); // vert. DPI
+                if !res.succeeded() { fail ~"unable to set font char size" }
+                Ok(FreeTypeNativeFont { face: face, buf: buf })
+            } else {
+                Err(())
+            }
+        })
+    }
 
-    fn glyph_index(codepoint: char) -> Option<GlyphIndex> {
+    pub fn glyph_index(codepoint: char) -> Option<GlyphIndex> {
         assert self.face.is_not_null();
         let idx = FT_Get_Char_Index(self.face, codepoint as FT_ULong);
         return if idx != 0 as FT_UInt {
@@ -61,8 +74,7 @@ pub impl FreeTypeNativeFont {
         };
     }
 
-    // FIXME: What unit is this returning? Let's have a custom type
+    pub fn glyph_h_advance(glyph: GlyphIndex) -> Option<FractionalPixel> {
-    fn glyph_h_advance(glyph: GlyphIndex) -> Option<FractionalPixel> {
         assert self.face.is_not_null();
         let res =  FT_Load_Glyph(self.face, glyph as FT_UInt, 0);
         if res.succeeded() {
@@ -82,9 +94,8 @@ pub impl FreeTypeNativeFont {
         }
     }
 
-    fn get_metrics() -> FontMetrics {
+    pub fn get_metrics() -> FontMetrics {
-        /* TODO: complete me (Issue #76) */
+        /* TODO(Issue #76): complete me */
-
         let face = self.get_face_rec();
 
         let underline_size = self.font_units_to_au(face.underline_thickness as float);
@@ -129,26 +140,6 @@ pub impl FreeTypeNativeFont {
     }
 }
 
-pub fn create(lib: &FT_Library, buf: @~[u8], pt_size: float) -> Result<FreeTypeNativeFont, ()> {
-    assert lib.is_not_null();
-    let face: FT_Face = null();
-    return vec_as_buf(*buf, |cbuf, _len| {
-           if FT_New_Memory_Face(*lib, cbuf, (*buf).len() as FT_Long,
-                                 0 as FT_Long, addr_of(&face)).succeeded() {
-               // FIXME: These values are placeholders
-               let res = FT_Set_Char_Size(face, // the face
-                                          float_to_fixed_ft(pt_size) as FT_F26Dot6, // char width
-                                          float_to_fixed_ft(pt_size) as FT_F26Dot6, // char height
-                                          72, // horiz. DPI
-                                          72); // vert. DPI
-               if !res.succeeded() { fail ~"unable to set font char size" }
-               Ok(FreeTypeNativeFont(face, buf))
-           } else {
-               Err(())
-           }
-    })
-}
-
 trait FTErrorMethods {
     fn succeeded() -> bool;
 }

src/servo/text/native_font.rs

@@ -13,13 +13,23 @@ pub type NativeFont/& = quartz::native_font::QuartzNativeFont;
 #[cfg(target_os = "linux")]
 pub type NativeFont/& = freetype::native_font::FreeTypeNativeFont;
 
-// TODO: this should be part of trait NativeFont
+// TODO: `new` should be part of trait NativeFont
+
+// TODO(Issue #163): this is a workaround for static methods and
+// typedefs not working well together. It should be removed.
+
+// TODO(Rust #1723): #cfg doesn't work for impl methods, so we have
+// to conditionally define the entire impl.
 #[cfg(target_os = "macos")]
-pub fn create(native_lib: &NativeFontCache, buf: @~[u8], pt_size: float) -> Result<NativeFont, ()> {
+impl NativeFont {
-    quartz::native_font::create(native_lib, buf, pt_size)
+    static pub fn new(native_lib: &NativeFontCache, buf: @~[u8], pt_size: float) -> Result<NativeFont, ()> {
+        quartz::native_font::QuartzNativeFont::new(native_lib, buf, pt_size)
+    }
 }
 
 #[cfg(target_os = "linux")]
-pub fn create(native_lib: &NativeFontCache, buf: @~[u8], pt_size: float) -> Result<NativeFont, ()> {
+impl NativeFont {
-    freetype::native_font::create(native_lib, buf, pt_size)
+    static pub fn new(native_lib: &NativeFontCache, buf: @~[u8], pt_size: float) -> Result<NativeFont, ()> {
-}
+        freetype::native_font::FreeTypeNativeFont::new(native_lib, buf, pt_size)
+    }
+}

src/servo/text/quartz/native_font.rs

@@ -67,21 +67,30 @@ pub struct QuartzNativeFont {
     }
 }
 
-fn QuartzNativeFont(fontprov: CGDataProviderRef, cgfont: CGFontRef, pt_size: float) -> QuartzNativeFont {
+pub impl QuartzNativeFont {
-    assert fontprov.is_not_null();
+    static pub fn new(_lib: &NativeFontCache, buf: @~[u8], pt_size: float) -> Result<QuartzNativeFont, ()> {
-    assert cgfont.is_not_null();
+        let fontprov = vec::as_imm_buf(*buf, |cbuf, len| {
+            CGDataProviderCreateWithData(
+                null(),
+                unsafe { transmute(copy cbuf) },
+                len as size_t,
+                null())
+        });
+        if fontprov.is_null() { return Err(()); }
 
-    let ctfont = ctfont_from_cgfont(cgfont, pt_size);
+        let cgfont = CGFontCreateWithDataProvider(fontprov);
-    assert ctfont.is_not_null();
+        if cgfont.is_null() { return Err(()); }
 
-    QuartzNativeFont {
+        let ctfont = ctfont_from_cgfont(cgfont, pt_size);
-        fontprov : fontprov,
+        if ctfont.is_null() { return Err(()); }
-        cgfont : cgfont,
+
-        ctfont : ctfont,
+        Ok(QuartzNativeFont {
+            fontprov : fontprov,
+            cgfont : cgfont,
+            ctfont : ctfont,
+        })
     }
-}
 
-impl QuartzNativeFont {
     fn glyph_index(codepoint: char) -> Option<GlyphIndex> {
         assert self.ctfont.is_not_null();
 
@@ -146,23 +155,4 @@ fn ctfont_from_cgfont(cgfont: CGFontRef, pt_size: float) -> CTFontRef {
     assert cgfont.is_not_null();
 
     CTFontCreateWithGraphicsFont(cgfont, pt_size as CGFloat, null(), null())
-}
+}
-
-pub fn create(_lib: &NativeFontCache, buf: @~[u8], pt_size: float) -> Result<QuartzNativeFont, ()> {
-    let fontprov = vec::as_imm_buf(*buf, |cbuf, len| {
-        CGDataProviderCreateWithData(
-            null(),
-            unsafe { transmute(copy cbuf) },
-            len as size_t,
-            null())
-    });
-    // FIXME: Error handling
-    assert fontprov.is_not_null();
-    let cgfont = CGFontCreateWithDataProvider(fontprov);
-
-    match cgfont.is_not_null() {
-        true => Ok(QuartzNativeFont(fontprov, cgfont, pt_size)),
-        false => Err(())
-    }
-    
-}