Auto merge of #7786 - mbrubeck:harfbuzz-sys, r=pcwalton

Use Harfbuzz 1.0 and unicode-script for text shaping

Depends on servo/rust-harfbuzz#53 and introduces a dependency on the new servo/unicode-script crate.  r? @pcwalton

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7786)
<!-- Reviewable:end -->

components/gfx/Cargo.toml

@@ -12,7 +12,7 @@ path = "lib.rs"
 bitflags = "0.3"
 euclid = "0.2"
 fnv = "1.0"
-harfbuzz = "0.1"
+harfbuzz-sys = "0.1"
 lazy_static = "0.1"
 libc = "0.1"
 log = "0.3"
@@ -23,6 +23,7 @@ serde_macros = "0.5"
 smallvec = "0.1"
 string_cache = "0.1"
 time = "0.1.12"
+unicode-script = { version = "0.1", features = ["harfbuzz"] }
 
 [dependencies.plugins]
 path = "../plugins"

components/gfx/font.rs

@@ -18,6 +18,7 @@ use style::properties::style_structs::Font as FontStyle;
 use text::Shaper;
 use text::glyph::{GlyphId, GlyphStore};
 use text::shaping::ShaperMethods;
+use unicode_script::Script;
 use util::cache::HashCache;
 use util::geometry::Au;
 
@@ -117,6 +118,8 @@ pub struct ShapingOptions {
     pub letter_spacing: Option<Au>,
     /// Spacing to add between each word. Corresponds to the CSS 2.1 `word-spacing` property.
     pub word_spacing: Au,
+    /// The Unicode script property of the characters in this run.
+    pub script: Script,
     /// Various flags.
     pub flags: ShapingFlags,
 }

components/gfx/lib.rs

@@ -52,7 +52,7 @@ extern crate gfx_traits;
 
 // Eventually we would like the shaper to be pluggable, as many operating systems have their own
 // shapers. For now, however, this is a hard dependency.
-extern crate harfbuzz;
+extern crate harfbuzz_sys as harfbuzz;
 
 extern crate ipc_channel;
 extern crate layers;
@@ -72,6 +72,7 @@ extern crate smallvec;
 extern crate string_cache;
 extern crate style;
 extern crate time;
+extern crate unicode_script;
 extern crate url;
 
 

components/gfx/text/shaping/harfbuzz.rs

@@ -2,34 +2,32 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-extern crate harfbuzz;
-
 use euclid::Point2D;
 use font::{DISABLE_KERNING_SHAPING_FLAG, Font, FontHandleMethods, FontTableMethods, FontTableTag};
 use font::{IGNORE_LIGATURES_SHAPING_FLAG, RTL_FLAG, ShapingOptions};
 use harfbuzz::{HB_DIRECTION_LTR, HB_DIRECTION_RTL, HB_MEMORY_MODE_READONLY};
-use harfbuzz::{RUST_hb_blob_create, RUST_hb_face_create_for_tables};
+use harfbuzz::{hb_blob_create, hb_face_create_for_tables};
-use harfbuzz::{RUST_hb_buffer_add_utf8};
-use harfbuzz::{RUST_hb_buffer_create, RUST_hb_font_destroy};
-use harfbuzz::{RUST_hb_buffer_destroy};
-use harfbuzz::{RUST_hb_buffer_get_glyph_infos, RUST_hb_shape};
-use harfbuzz::{RUST_hb_buffer_get_glyph_positions};
-use harfbuzz::{RUST_hb_buffer_get_length};
-use harfbuzz::{RUST_hb_buffer_set_direction};
-use harfbuzz::{RUST_hb_face_destroy};
-use harfbuzz::{RUST_hb_font_create};
-use harfbuzz::{RUST_hb_font_funcs_create};
-use harfbuzz::{RUST_hb_font_funcs_set_glyph_func};
-use harfbuzz::{RUST_hb_font_funcs_set_glyph_h_advance_func};
-use harfbuzz::{RUST_hb_font_funcs_set_glyph_h_kerning_func};
-use harfbuzz::{RUST_hb_font_set_funcs};
-use harfbuzz::{RUST_hb_font_set_ppem};
-use harfbuzz::{RUST_hb_font_set_scale};
 use harfbuzz::{hb_blob_t};
 use harfbuzz::{hb_bool_t};
+use harfbuzz::{hb_buffer_add_utf8};
+use harfbuzz::{hb_buffer_create, hb_font_destroy};
+use harfbuzz::{hb_buffer_destroy};
+use harfbuzz::{hb_buffer_get_glyph_infos, hb_shape};
+use harfbuzz::{hb_buffer_get_glyph_positions};
+use harfbuzz::{hb_buffer_get_length};
+use harfbuzz::{hb_buffer_set_direction, hb_buffer_set_script};
 use harfbuzz::{hb_buffer_t, hb_codepoint_t, hb_font_funcs_t};
+use harfbuzz::{hb_face_destroy};
 use harfbuzz::{hb_face_t, hb_font_t};
 use harfbuzz::{hb_feature_t};
+use harfbuzz::{hb_font_create};
+use harfbuzz::{hb_font_funcs_create};
+use harfbuzz::{hb_font_funcs_set_glyph_func};
+use harfbuzz::{hb_font_funcs_set_glyph_h_advance_func};
+use harfbuzz::{hb_font_funcs_set_glyph_h_kerning_func};
+use harfbuzz::{hb_font_set_funcs};
+use harfbuzz::{hb_font_set_ppem};
+use harfbuzz::{hb_font_set_scale};
 use harfbuzz::{hb_glyph_info_t};
 use harfbuzz::{hb_glyph_position_t};
 use harfbuzz::{hb_position_t, hb_tag_t};
@@ -70,10 +68,10 @@ impl ShapedGlyphData {
     pub fn new(buffer: *mut hb_buffer_t) -> ShapedGlyphData {
         unsafe {
             let mut glyph_count = 0;
-            let glyph_infos = RUST_hb_buffer_get_glyph_infos(buffer, &mut glyph_count);
+            let glyph_infos = hb_buffer_get_glyph_infos(buffer, &mut glyph_count);
             assert!(!glyph_infos.is_null());
             let mut pos_count = 0;
-            let pos_infos = RUST_hb_buffer_get_glyph_positions(buffer, &mut pos_count);
+            let pos_infos = hb_buffer_get_glyph_positions(buffer, &mut pos_count);
             assert!(!pos_infos.is_null());
             assert!(glyph_count == pos_count);
 
@@ -151,10 +149,10 @@ impl Drop for Shaper {
     fn drop(&mut self) {
         unsafe {
             assert!(!self.hb_face.is_null());
-            RUST_hb_face_destroy(self.hb_face);
+            hb_face_destroy(self.hb_face);
 
             assert!(!self.hb_font.is_null());
-            RUST_hb_font_destroy(self.hb_font);
+            hb_font_destroy(self.hb_font);
         }
     }
 }
@@ -167,24 +165,22 @@ impl Shaper {
                 options: *options,
             };
             let hb_face: *mut hb_face_t =
-                RUST_hb_face_create_for_tables(font_table_func,
+                hb_face_create_for_tables(Some(font_table_func),
-                                          (&mut *font_and_shaping_options)
+                                          &mut *font_and_shaping_options as *mut _ as *mut c_void,
-                                            as *mut FontAndShapingOptions
-                                            as *mut c_void,
                                           None);
-            let hb_font: *mut hb_font_t = RUST_hb_font_create(hb_face);
+            let hb_font: *mut hb_font_t = hb_font_create(hb_face);
 
             // Set points-per-em. if zero, performs no hinting in that direction.
             let pt_size = font.actual_pt_size.to_f64_px();
-            RUST_hb_font_set_ppem(hb_font, pt_size as c_uint, pt_size as c_uint);
+            hb_font_set_ppem(hb_font, pt_size as c_uint, pt_size as c_uint);
 
             // Set scaling. Note that this takes 16.16 fixed point.
-            RUST_hb_font_set_scale(hb_font,
+            hb_font_set_scale(hb_font,
-                                   Shaper::float_to_fixed(pt_size) as c_int,
+                              Shaper::float_to_fixed(pt_size) as c_int,
-                                   Shaper::float_to_fixed(pt_size) as c_int);
+                              Shaper::float_to_fixed(pt_size) as c_int);
 
             // configure static function callbacks.
-            RUST_hb_font_set_funcs(hb_font, **HB_FONT_FUNCS, font as *mut Font as *mut c_void, None);
+            hb_font_set_funcs(hb_font, **HB_FONT_FUNCS, font as *mut Font as *mut c_void, None);
 
             Shaper {
                 hb_face: hb_face,
@@ -212,40 +208,42 @@ impl ShaperMethods for Shaper {
     /// font.
     fn shape_text(&self, text: &str, options: &ShapingOptions, glyphs: &mut GlyphStore) {
         unsafe {
-            let hb_buffer: *mut hb_buffer_t = RUST_hb_buffer_create();
+            let hb_buffer: *mut hb_buffer_t = hb_buffer_create();
-            RUST_hb_buffer_set_direction(hb_buffer, if options.flags.contains(RTL_FLAG) {
+            hb_buffer_set_direction(hb_buffer, if options.flags.contains(RTL_FLAG) {
                 HB_DIRECTION_RTL
             } else {
                 HB_DIRECTION_LTR
             });
 
-            RUST_hb_buffer_add_utf8(hb_buffer,
+            hb_buffer_set_script(hb_buffer, options.script.to_hb_script());
-                                    text.as_ptr() as *const c_char,
+
-                                    text.len() as c_int,
+            hb_buffer_add_utf8(hb_buffer,
-                                    0,
+                               text.as_ptr() as *const c_char,
-                                    text.len() as c_int);
+                               text.len() as c_int,
+                               0,
+                               text.len() as c_int);
 
             let mut features = Vec::new();
             if options.flags.contains(IGNORE_LIGATURES_SHAPING_FLAG) {
                 features.push(hb_feature_t {
-                    _tag: LIGA,
+                    tag: LIGA,
-                    _value: 0,
+                    value: 0,
-                    _start: 0,
+                    start: 0,
-                    _end: RUST_hb_buffer_get_length(hb_buffer),
+                    end: hb_buffer_get_length(hb_buffer),
                 })
             }
             if options.flags.contains(DISABLE_KERNING_SHAPING_FLAG) {
                 features.push(hb_feature_t {
-                    _tag: KERN,
+                    tag: KERN,
-                    _value: 0,
+                    value: 0,
-                    _start: 0,
+                    start: 0,
-                    _end: RUST_hb_buffer_get_length(hb_buffer),
+                    end: hb_buffer_get_length(hb_buffer),
                 })
             }
 
-            RUST_hb_shape(self.hb_font, hb_buffer, features.as_mut_ptr(), features.len() as u32);
+            hb_shape(self.hb_font, hb_buffer, features.as_mut_ptr(), features.len() as u32);
             self.save_glyph_results(text, options, glyphs, hb_buffer);
-            RUST_hb_buffer_destroy(hb_buffer);
+            hb_buffer_destroy(hb_buffer);
         }
     }
 }
@@ -444,6 +442,21 @@ impl Shaper {
                 let character = text.char_at(char_byte_span.begin());
                 if is_bidi_control(character) {
                     glyphs.add_nonglyph_for_char_index(char_idx, false, false);
+                } else if character == '\t' {
+                    // Treat tabs in pre-formatted text as a fixed number of spaces.
+                    //
+                    // TODO: Proper tab stops.
+                    const TAB_COLS: i32 = 8;
+                    let font = self.font_and_shaping_options.font;
+                    let (space_glyph_id, space_advance) = glyph_space_advance(font);
+                    let advance = Au::from_f64_px(space_advance) * TAB_COLS;
+                    let data = GlyphData::new(space_glyph_id,
+                                              advance,
+                                              Default::default(),
+                                              false,
+                                              true,
+                                              true);
+                    glyphs.add_glyph_for_char_index(char_idx, Some(character), &data);
                 } else {
                     let shape = glyph_data.entry_for_glyph(glyph_span.begin(), &mut y_pos);
                     let advance = self.advance_for_shaped_glyph(shape.advance, character, options);
@@ -510,9 +523,6 @@ impl Shaper {
         // We elect to only space the two required code points.
         if character == ' ' || character == '\u{a0}' {
             advance = advance + options.word_spacing
-        } else if character == '\t' {
-            let tab_size = 8f64;
-            advance = Au::from_f64_px(tab_size * glyph_space_advance(self.font_and_shaping_options.font));
         }
 
         advance
@@ -522,12 +532,12 @@ impl Shaper {
 // Callbacks from Harfbuzz when font map and glyph advance lookup needed.
 lazy_static! {
     static ref HB_FONT_FUNCS: ptr::Unique<hb_font_funcs_t> = unsafe {
-        let hb_funcs = RUST_hb_font_funcs_create();
+        let hb_funcs = hb_font_funcs_create();
-        RUST_hb_font_funcs_set_glyph_func(hb_funcs, glyph_func, ptr::null_mut(), None);
+        hb_font_funcs_set_glyph_func(hb_funcs, Some(glyph_func), ptr::null_mut(), None);
-        RUST_hb_font_funcs_set_glyph_h_advance_func(
+        hb_font_funcs_set_glyph_h_advance_func(
-            hb_funcs, glyph_h_advance_func, ptr::null_mut(), None);
+            hb_funcs, Some(glyph_h_advance_func), ptr::null_mut(), None);
-        RUST_hb_font_funcs_set_glyph_h_kerning_func(
+        hb_font_funcs_set_glyph_h_kerning_func(
-            hb_funcs, glyph_h_kerning_func, ptr::null_mut(), ptr::null_mut());
+            hb_funcs, Some(glyph_h_kerning_func), ptr::null_mut(), None);
 
         ptr::Unique::new(hb_funcs)
     };
@@ -568,7 +578,7 @@ extern fn glyph_h_advance_func(_: *mut hb_font_t,
     }
 }
 
-fn glyph_space_advance(font: *mut Font) -> f64 {
+fn glyph_space_advance(font: *mut Font) -> (hb_codepoint_t, f64) {
     let space_unicode = ' ';
     let space_glyph: hb_codepoint_t;
     match unsafe { (*font).glyph_index(space_unicode) } {
@@ -578,7 +588,7 @@ fn glyph_space_advance(font: *mut Font) -> f64 {
         None => panic!("No space info")
     }
     let space_advance = unsafe { (*font).glyph_h_advance(space_glyph as GlyphId) };
-    space_advance
+    (space_glyph, space_advance)
 }
 
 extern fn glyph_h_kerning_func(_: *mut hb_font_t,
@@ -620,11 +630,11 @@ extern fn font_table_func(_: *mut hb_face_t,
                 let mut blob: *mut hb_blob_t = ptr::null_mut();
                 (*font_table_ptr).with_buffer(|buf: *const u8, len: usize| {
                     // HarfBuzz calls `destroy_blob_func` when the buffer is no longer needed.
-                    blob = RUST_hb_blob_create(buf as *const c_char,
+                    blob = hb_blob_create(buf as *const c_char,
-                                               len as c_uint,
+                                          len as c_uint,
-                                               HB_MEMORY_MODE_READONLY,
+                                          HB_MEMORY_MODE_READONLY,
-                                               font_table_ptr as *mut c_void,
+                                          font_table_ptr as *mut c_void,
-                                               destroy_blob_func);
+                                          Some(destroy_blob_func));
                 });
 
                 assert!(!blob.is_null());

components/layout/Cargo.toml

@@ -81,3 +81,4 @@ serde = "0.6"
 serde_macros = "0.5"
 serde_json = "0.5"
 unicode-bidi = "0.2"
+unicode-script = { version = "0.1", features = ["harfbuzz"] }

components/layout/lib.rs

@@ -53,6 +53,7 @@ extern crate smallvec;
 extern crate string_cache;
 extern crate style;
 extern crate unicode_bidi;
+extern crate unicode_script;
 extern crate url;
 
 #[macro_use]

components/layout/text.rs

@@ -23,6 +23,7 @@ use style::computed_values::{white_space};
 use style::properties::ComputedValues;
 use style::properties::style_structs::Font as FontStyle;
 use unicode_bidi::{is_rtl, process_text};
+use unicode_script::{get_script, Script};
 use util::geometry::Au;
 use util::linked_list::split_off_head;
 use util::logical_geometry::{LogicalSize, WritingMode};
@@ -204,8 +205,22 @@ impl TextRunScanner {
                         None => 0
                     };
 
+                    // Break the run if the new character has a different explicit script than the
+                    // previous characters.
+                    //
+                    // TODO: Special handling of paired punctuation characters.
+                    // http://www.unicode.org/reports/tr24/#Common
+                    let script = get_script(character);
+                    let compatible_script = is_compatible(script, run_info.script);
+                    if compatible_script && !is_specific(run_info.script) && is_specific(script) {
+                        run_info.script = script;
+                    }
+
                     // Now, if necessary, flush the mapping we were building up.
-                    if run_info.font_index != font_index || run_info.bidi_level != bidi_level {
+                    if run_info.font_index != font_index ||
+                       run_info.bidi_level != bidi_level ||
+                       !compatible_script
+                    {
                         if end_position > start_position {
                             mapping.flush(&mut mappings,
                                           &mut run_info,
@@ -226,6 +241,7 @@ impl TextRunScanner {
                         }
                         run_info.font_index = font_index;
                         run_info.bidi_level = bidi_level;
+                        run_info.script = script;
                     }
 
                     // Consume this character.
@@ -269,12 +285,14 @@ impl TextRunScanner {
             let options = ShapingOptions {
                 letter_spacing: letter_spacing,
                 word_spacing: word_spacing,
+                script: Script::Common,
                 flags: flags,
             };
 
             // FIXME(https://github.com/rust-lang/rust/issues/23338)
             run_info_list.into_iter().map(|run_info| {
                 let mut options = options;
+                options.script = run_info.script;
                 if is_rtl(run_info.bidi_level) {
                     options.flags.insert(RTL_FLAG);
                 }
@@ -440,6 +458,8 @@ struct RunInfo {
     character_length: usize,
     /// The bidirection embedding level of this text run.
     bidi_level: u8,
+    /// The Unicode script property of this text run.
+    script: Script,
 }
 
 impl RunInfo {
@@ -450,6 +470,7 @@ impl RunInfo {
             font_index: 0,
             character_length: 0,
             bidi_level: 0,
+            script: Script::Common,
         }
     }
 }
@@ -503,9 +524,12 @@ impl RunMapping {
 
         // Account for `text-transform`. (Confusingly, this is not handled in "text
         // transformation" above, but we follow Gecko in the naming.)
+        let is_first_run = *start_position == 0;
         let character_count = apply_style_transform_if_necessary(&mut run_info.text,
                                                                  old_byte_length,
-                                                                 text_transform);
+                                                                 text_transform,
+                                                                 *last_whitespace,
+                                                                 is_first_run);
 
         // Record the position of the insertion point if necessary.
         if let Some(insertion_point) = insertion_point {
@@ -536,7 +560,9 @@ impl RunMapping {
 /// use graphemes instead of characters.
 fn apply_style_transform_if_necessary(string: &mut String,
                                       first_character_position: usize,
-                                      text_transform: text_transform::T)
+                                      text_transform: text_transform::T,
+                                      last_whitespace: bool,
+                                      is_first_run: bool)
                                       -> usize {
     match text_transform {
         text_transform::T::none => string[first_character_position..].chars().count(),
@@ -564,9 +590,7 @@ fn apply_style_transform_if_necessary(string: &mut String,
             let original = string[first_character_position..].to_owned();
             string.truncate(first_character_position);
 
-            // FIXME(pcwalton): This may not always be correct in the case of something like
+            let mut capitalize_next_letter = is_first_run || last_whitespace;
-            // `f<span>oo</span>`.
-            let mut capitalize_next_letter = true;
             let mut count = 0;
             for character in original.chars() {
                 count += 1;
@@ -599,3 +623,13 @@ struct ScannedTextRun {
     run: Arc<TextRun>,
     insertion_point: Option<CharIndex>,
 }
+
+/// Can a character with script `b` continue a text run with script `a`?
+fn is_compatible(a: Script, b: Script) -> bool {
+    a == b || !is_specific(a) || !is_specific(b)
+}
+
+/// Returns true if the script is not invalid or inherited.
+fn is_specific(script: Script) -> bool {
+    script != Script::Common && script != Script::Inherited
+}

components/servo/Cargo.lock

@@ -591,7 +591,7 @@ dependencies = [
  "fontconfig 0.1.0 (git+https://github.com/servo/rust-fontconfig)",
  "freetype 0.1.0 (git+https://github.com/servo/rust-freetype)",
  "gfx_traits 0.0.1",
- "harfbuzz 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "harfbuzz-sys 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipc-channel 0.1.0 (git+https://github.com/pcwalton/ipc-channel)",
  "layers 0.1.0 (git+https://github.com/servo/rust-layers)",
  "lazy_static 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -612,6 +612,7 @@ dependencies = [
  "string_cache 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)",
  "style 0.0.1",
  "time 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
+ "unicode-script 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "url 0.2.37 (registry+https://github.com/rust-lang/crates.io-index)",
  "util 0.0.1",
 ]
@@ -734,11 +735,12 @@ dependencies = [
 ]
 
 [[package]]
-name = "harfbuzz"
+name = "harfbuzz-sys"
-version = "0.1.2"
+version = "0.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "libc 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
+ "pkg-config 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -959,6 +961,7 @@ dependencies = [
  "string_cache_plugin 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "style 0.0.1",
  "unicode-bidi 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "unicode-script 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "url 0.2.37 (registry+https://github.com/rust-lang/crates.io-index)",
  "util 0.0.1",
 ]
@@ -1777,6 +1780,14 @@ dependencies = [
  "matches 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "unicode-script"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "harfbuzz-sys 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "unreachable"
 version = "0.0.2"

tests/ref/text_transform_capitalize_a.html

@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <html>
+<meta charset="UTF-8">
 <!-- Tests that `text-transform: capitalize` works. -->
 <body>
 <h1 style='text-transform: capitalize; font-family: Hiragino Maru Gothic Pro'>ュゥゥゥゥ can do ányThing at ゾムボ.cOm</h1>

tests/ref/text_transform_capitalize_ref.html

@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <html>
+<meta charset="UTF-8">
 <!-- Tests that `text-transform: capitalize` works. -->
 <body>
 <h1 style='font-family: Hiragino Maru Gothic Pro'>ュゥゥゥゥ Can Do ÁnyThing At ゾムボ.cOm</h1>

tests/wpt/metadata-css/css-flexbox-1_dev/html/css-flexbox-column-reverse.htm.ini

@@ -1,4 +1,4 @@
 [css-flexbox-column-reverse.htm]
   type: reftest
   expected:
-    if os == "linux": FAIL
+    FAIL

tests/wpt/metadata-css/css21_dev/html4/bidi-glyph-mirroring-002.htm.ini

@@ -1,4 +0,0 @@
-[bidi-glyph-mirroring-002.htm]
-  type: reftest
-  expected:
-    if os == "mac": FAIL