fonts: Make fast shaping determination platform-independent (#33540)

This makes the determination of whether or not to use fast shaping
platform independent. Previously it was less stringent for Windows,
leading to using it in cases where a font had a GSUB or GPOS table --
which broke proper shaping.

In addition, the test is made platform independent and expanded to be
more complete.

Finally, comments are added indicating that "fast shaping" will be
removed.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

6 files changed, 129 insertions, 116 deletions

diff --git a/components/fonts/font.rs b/components/fonts/font.rs
index 3af077b4e06..b4d3df30b6e 100644
--- a/components/fonts/font.rs
+++ b/components/fonts/font.rs
@@ -90,8 +90,6 @@ pub trait PlatformFontMethods: Sized {
     fn glyph_h_advance(&self, _: GlyphId) -> Option<FractionalPixel>;
     fn glyph_h_kerning(&self, glyph0: GlyphId, glyph1: GlyphId) -> FractionalPixel;
 
-    /// Can this font do basic horizontal LTR shaping without Harfbuzz?
-    fn can_do_fast_shaping(&self) -> bool;
     fn metrics(&self) -> FontMetrics;
     fn table_for_tag(&self, _: FontTableTag) -> Option<FontTable>;
     fn typographic_bounds(&self, _: GlyphId) -> Rect<f32>;
@@ -237,6 +235,14 @@ pub struct Font {
     /// essentially equivalent to whether or not we use it for emoji presentation.
     /// This is cached, because getting table data is expensive.
     has_color_bitmap_or_colr_table: OnceLock<bool>,
+
+    /// Whether or not this font can do fast shaping, ie whether or not it has
+    /// a kern table, but no GSUB and GPOS tables. When this is true, Servo will
+    /// shape Latin horizontal left-to-right text without using Harfbuzz.
+    ///
+    /// FIXME: This should be removed entirely in favor of better caching if necessary.
+    /// See <https://github.com/servo/servo/pull/11273#issuecomment-222332873>.
+    can_do_fast_shaping: OnceLock<bool>,
 }
 
 impl malloc_size_of::MallocSizeOf for Font {
@@ -275,6 +281,7 @@ impl Font {
             font_key: FontInstanceKey::default(),
             synthesized_small_caps,
             has_color_bitmap_or_colr_table: OnceLock::new(),
+            can_do_fast_shaping: OnceLock::new(),
         })
     }
 
@@ -390,10 +397,18 @@ impl Font {
             .shape_text(text, options, glyphs);
     }
 
-    fn can_do_fast_shaping(&self, text: &str, options: &ShapingOptions) -> bool {
+    /// Whether not a particular text and [`ShapingOptions`] combination can use
+    /// "fast shaping" ie shaping without Harfbuzz.
+    ///
+    /// Note: This will eventually be removed.
+    pub fn can_do_fast_shaping(&self, text: &str, options: &ShapingOptions) -> bool {
         options.script == Script::Latin &&
             !options.flags.contains(ShapingFlags::RTL_FLAG) &&
-            self.handle.can_do_fast_shaping() &&
+            *self.can_do_fast_shaping.get_or_init(|| {
+                self.table_for_tag(KERN).is_some() &&
+                    self.table_for_tag(GPOS).is_none() &&
+                    self.table_for_tag(GSUB).is_none()
+            }) &&
             text.is_ascii()
     }
 
@@ -892,92 +907,3 @@ pub(crate) fn map_platform_values_to_style_values(mapping: &[(f64, f64)], value:
 
     mapping[mapping.len() - 1].1
 }
-
-#[cfg(test)]
-mod test {
-    use crate::FontData;
-
-    #[cfg(target_os = "windows")]
-    #[test]
-    fn test_shape_text_fast() {
-        use std::fs::File;
-        use std::io::Read;
-        use std::path::PathBuf;
-        use std::sync::Arc;
-
-        use app_units::Au;
-        use euclid::num::Zero;
-        use servo_url::ServoUrl;
-        use style::properties::longhands::font_variant_caps::computed_value::T as FontVariantCaps;
-        use style::values::computed::{FontStretch, FontStyle, FontWeight};
-        use unicode_script::Script;
-
-        use crate::platform::font::PlatformFont;
-        use crate::{
-            Font, FontDescriptor, FontIdentifier, FontTemplate, GlyphStore, PlatformFontMethods,
-            ShapingFlags, ShapingOptions,
-        };
-
-        let path: PathBuf = [
-            env!("CARGO_MANIFEST_DIR"),
-            "tests",
-            "support",
-            "dejavu-fonts-ttf-2.37",
-            "ttf",
-            "DejaVuSans.ttf",
-        ]
-        .iter()
-        .collect();
-
-        let identifier = FontIdentifier::Web(ServoUrl::from_file_path(path.clone()).unwrap());
-        let file = File::open(path).unwrap();
-        let data = Arc::new(FontData::from_bytes(
-            file.bytes().map(|b| b.unwrap()).collect(),
-        ));
-        let platform_font =
-            PlatformFont::new_from_data(identifier.clone(), data.as_arc().clone(), 0, None)
-                .unwrap();
-
-        let template = FontTemplate {
-            identifier,
-            descriptor: platform_font.descriptor(),
-            stylesheet: None,
-        };
-        let descriptor = FontDescriptor {
-            weight: FontWeight::normal(),
-            stretch: FontStretch::hundred(),
-            style: FontStyle::normal(),
-            variant: FontVariantCaps::Normal,
-            pt_size: Au::from_px(24),
-        };
-        let font = Font::new(
-            Arc::new(atomic_refcell::AtomicRefCell::new(template)),
-            descriptor,
-            data,
-            None,
-        )
-        .unwrap();
-
-        let shaping_options = ShapingOptions {
-            letter_spacing: None,
-            word_spacing: Au::zero(),
-            script: Script::Latin,
-            flags: ShapingFlags::empty(),
-        };
-        let text = "WAVE";
-
-        assert!(font.can_do_fast_shaping(text, &shaping_options));
-
-        let mut expected_glyphs = GlyphStore::new(text.len(), false, false, false, false);
-        font.shape_text_harfbuzz(text, &shaping_options, &mut expected_glyphs);
-
-        let mut glyphs = GlyphStore::new(text.len(), false, false, false, false);
-        font.shape_text_fast(text, &shaping_options, &mut glyphs);
-
-        assert_eq!(glyphs.len(), expected_glyphs.len());
-        assert_eq!(
-            glyphs.total_advance().to_nearest_px(),
-            expected_glyphs.total_advance().to_nearest_px()
-        );
-    }
-}
diff --git a/components/fonts/platform/freetype/font.rs b/components/fonts/platform/freetype/font.rs
index 43885556942..827d02cca8f 100644
--- a/components/fonts/platform/freetype/font.rs
+++ b/components/fonts/platform/freetype/font.rs
@@ -75,7 +75,6 @@ pub struct PlatformFont {
     face: ReentrantMutex<FT_Face>,
     requested_face_size: Au,
     actual_face_size: Au,
-    can_do_fast_shaping: bool,
 }
 
 // FT_Face can be used in multiple threads, but from only one thread at a time.
@@ -135,15 +134,12 @@ impl PlatformFontMethods for PlatformFont {
             Some(requested_size) => (requested_size, face.set_size(requested_size)?),
             None => (Au::zero(), Au::zero()),
         };
-        let can_do_fast_shaping =
-            face.has_table(KERN) && !face.has_table(GPOS) && !face.has_table(GSUB);
 
         Ok(PlatformFont {
             face: ReentrantMutex::new(face),
             font_data: data,
             requested_face_size,
             actual_face_size,
-            can_do_fast_shaping,
         })
     }
 
@@ -215,10 +211,6 @@ impl PlatformFontMethods for PlatformFont {
         fixed_26_dot_6_to_float(delta.x) * self.unscalable_font_metrics_scale()
     }
 
-    fn can_do_fast_shaping(&self) -> bool {
-        self.can_do_fast_shaping
-    }
-
     fn glyph_h_advance(&self, glyph: GlyphId) -> Option<FractionalPixel> {
         let face = self.face.lock();
         assert!(!face.is_null());
diff --git a/components/fonts/platform/macos/font.rs b/components/fonts/platform/macos/font.rs
index 13af8ae034e..e2a68d340ce 100644
--- a/components/fonts/platform/macos/font.rs
+++ b/components/fonts/platform/macos/font.rs
@@ -26,7 +26,7 @@ use super::core_text_font_cache::CoreTextFontCache;
 use crate::{
     map_platform_values_to_style_values, FontIdentifier, FontMetrics, FontTableMethods,
     FontTableTag, FontTemplateDescriptor, FractionalPixel, GlyphId, PlatformFontMethods, CBDT,
-    COLR, GPOS, GSUB, KERN, SBIX,
+    COLR, KERN, SBIX,
 };
 
 const KERN_PAIR_LEN: usize = 6;
@@ -59,7 +59,6 @@ pub struct PlatformFont {
     /// data stays alive of the lifetime of this struct.
     _data: Arc<Vec<u8>>,
     h_kern_subtable: Option<CachedKernTable>,
-    can_do_fast_shaping: bool,
 }
 
 // From https://developer.apple.com/documentation/coretext:
@@ -189,12 +188,8 @@ impl PlatformFontMethods for PlatformFont {
             _data: data,
             ctfont: core_text_font.clone_with_font_size(size),
             h_kern_subtable: None,
-            can_do_fast_shaping: false,
         };
         handle.h_kern_subtable = handle.find_h_kern_subtable();
-        handle.can_do_fast_shaping = handle.h_kern_subtable.is_some() &&
-            handle.table_for_tag(GPOS).is_none() &&
-            handle.table_for_tag(GSUB).is_none();
         Ok(handle)
     }
 
@@ -239,10 +234,6 @@ impl PlatformFontMethods for PlatformFont {
         0.0
     }
 
-    fn can_do_fast_shaping(&self) -> bool {
-        self.can_do_fast_shaping
-    }
-
     fn glyph_h_advance(&self, glyph: GlyphId) -> Option<FractionalPixel> {
         let glyphs = [glyph as CGGlyph];
         let advance = unsafe {
diff --git a/components/fonts/platform/windows/font.rs b/components/fonts/platform/windows/font.rs
index 4974a225054..db7c94cd85a 100644
--- a/components/fonts/platform/windows/font.rs
+++ b/components/fonts/platform/windows/font.rs
@@ -211,11 +211,6 @@ impl PlatformFontMethods for PlatformFont {
         Some(f)
     }
 
-    /// Can this font do basic horizontal LTR shaping without Harfbuzz?
-    fn can_do_fast_shaping(&self) -> bool {
-        self.face.has_kerning_pairs()
-    }
-
     fn glyph_h_kerning(&self, first_glyph: GlyphId, second_glyph: GlyphId) -> FractionalPixel {
         let adjustment = self
             .face
diff --git a/components/fonts/tests/font.rs b/components/fonts/tests/font.rs
new file mode 100644
index 00000000000..2f735fb229e
--- /dev/null
+++ b/components/fonts/tests/font.rs
@@ -0,0 +1,109 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+use std::fs::File;
+use std::io::Read;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+use app_units::Au;
+use euclid::num::Zero;
+use fonts::platform::font::PlatformFont;
+use fonts::{
+    Font, FontData, FontDescriptor, FontIdentifier, FontTemplate, PlatformFontMethods,
+    ShapingFlags, ShapingOptions,
+};
+use servo_url::ServoUrl;
+use style::properties::longhands::font_variant_caps::computed_value::T as FontVariantCaps;
+use style::values::computed::{FontStretch, FontStyle, FontWeight};
+use unicode_script::Script;
+
+fn make_font(path: PathBuf) -> Font {
+    let identifier = FontIdentifier::Web(ServoUrl::from_file_path(path.clone()).unwrap());
+    let file = File::open(path).unwrap();
+    let data = Arc::new(FontData::from_bytes(
+        file.bytes().map(Result::unwrap).collect(),
+    ));
+    let platform_font =
+        PlatformFont::new_from_data(identifier.clone(), data.as_arc().clone(), 0, None).unwrap();
+
+    let template = FontTemplate {
+        identifier,
+        descriptor: platform_font.descriptor(),
+        stylesheet: None,
+    };
+    let descriptor = FontDescriptor {
+        weight: FontWeight::normal(),
+        stretch: FontStretch::hundred(),
+        style: FontStyle::normal(),
+        variant: FontVariantCaps::Normal,
+        pt_size: Au::from_px(24),
+    };
+    Font::new(
+        Arc::new(atomic_refcell::AtomicRefCell::new(template)),
+        descriptor,
+        data,
+        None,
+    )
+    .unwrap()
+}
+
+#[test]
+fn test_font_can_do_fast_shaping() {
+    let dejavu_sans = make_font(
+        [
+            env!("CARGO_MANIFEST_DIR"),
+            "tests",
+            "support",
+            "dejavu-fonts-ttf-2.37",
+            "ttf",
+            "DejaVuSans.ttf",
+        ]
+        .iter()
+        .collect(),
+    );
+
+    let dejavu_sans_fast_shapeable = make_font(
+        [
+            env!("CARGO_MANIFEST_DIR"),
+            "tests",
+            "support",
+            "dejavu-fonts-ttf-2.37",
+            "ttf",
+            "DejaVuSansNoGSUBNoGPOS.ttf",
+        ]
+        .iter()
+        .collect(),
+    );
+
+    // Fast shaping requires a font with a kern table and no GPOS or GSUB tables.
+    let shaping_options = ShapingOptions {
+        letter_spacing: None,
+        word_spacing: Au::zero(),
+        script: Script::Latin,
+        flags: ShapingFlags::empty(),
+    };
+    assert!(!dejavu_sans.can_do_fast_shaping("WAVE", &shaping_options));
+    assert!(dejavu_sans_fast_shapeable.can_do_fast_shaping("WAVE", &shaping_options));
+
+    // Non-Latin script should never have fast shaping.
+    let shaping_options = ShapingOptions {
+        letter_spacing: None,
+        word_spacing: Au::zero(),
+        script: Script::Cherokee,
+        flags: ShapingFlags::empty(),
+    };
+    assert!(!dejavu_sans.can_do_fast_shaping("WAVE", &shaping_options));
+    assert!(!dejavu_sans_fast_shapeable.can_do_fast_shaping("WAVE", &shaping_options));
+
+    // Right-to-left text should never use fast shaping.
+    let shaping_options = ShapingOptions {
+        letter_spacing: None,
+        word_spacing: Au::zero(),
+        script: Script::Latin,
+        flags: ShapingFlags::RTL_FLAG,
+    };
+    assert!(!dejavu_sans.can_do_fast_shaping("WAVE", &shaping_options));
+    assert!(!dejavu_sans_fast_shapeable.can_do_fast_shaping("WAVE", &shaping_options));
+}
diff --git a/components/fonts/tests/support/dejavu-fonts-ttf-2.37/ttf/DejaVuSansNoGSUBNoGPOS.ttf b/components/fonts/tests/support/dejavu-fonts-ttf-2.37/ttf/DejaVuSansNoGSUBNoGPOS.ttf
new file mode 100644
index 00000000000..b2a40d73e42
--- /dev/null
+++ b/components/fonts/tests/support/dejavu-fonts-ttf-2.37/ttf/DejaVuSansNoGSUBNoGPOS.ttf