Make FontSystem not self-referencing

Cargo.toml

@@ -12,7 +12,7 @@ repository = "https://github.com/pop-os/cosmic-text"
 fontdb = { version = "0.13.0", default-features = false }
 libm = "0.2.6"
 log = "0.4.17"
-ouroboros = "0.15.5"
+ouroboros = { version = "0.15.5", default-features = false }
 rustybuzz = { version = "0.7.0", default-features = false, features = ["libm"] }
 swash = { version = "0.1.6", optional = true }
 syntect = { version = "5.0.0", optional = true }
@@ -35,6 +35,7 @@ no_std = [
 std = [
   "fontdb/memmap",
   "fontdb/std",
+  "ouroboros/std",
   "rustybuzz/std",
   "sys-locale",
   "unicode-bidi/std",

src/font/fallback/mod.rs

@@ -26,7 +26,7 @@ mod platform;
 mod platform;
 
 pub struct FontFallbackIter<'a> {
-    fonts: &'a [Arc<Font<'a>>],
+    fonts: &'a [Arc<Font>],
     default_families: &'a [&'a str],
     default_i: usize,
     scripts: Vec<Script>,
@@ -39,7 +39,7 @@ pub struct FontFallbackIter<'a> {
 
 impl<'a> FontFallbackIter<'a> {
     pub fn new(
-        fonts: &'a [Arc<Font<'a>>],
+        fonts: &'a [Arc<Font>],
         default_families: &'a [&'a str],
         scripts: Vec<Script>,
         locale: &'a str,
@@ -88,7 +88,7 @@ impl<'a> FontFallbackIter<'a> {
 }
 
 impl<'a> Iterator for FontFallbackIter<'a> {
-    type Item = &'a Arc<Font<'a>>;
+    type Item = &'a Arc<Font>;
     fn next(&mut self) -> Option<Self::Item> {
         while self.default_i < self.default_families.len() {
             let default_family = self.default_families[self.default_i];

src/font/matches.rs

@@ -1,14 +0,0 @@
-// SPDX-License-Identifier: MIT OR Apache-2.0
-
-use alloc::sync::Arc;
-#[cfg(not(feature = "std"))]
-use alloc::{string::String, vec::Vec};
-
-use crate::Font;
-
-/// Fonts that match a pattern
-pub struct FontMatches<'a> {
-    pub locale: &'a str,
-    pub default_family: String,
-    pub fonts: Vec<Arc<Font<'a>>>,
-}

src/font/mod.rs

@@ -1,67 +1,119 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
-
-use core::ops::Deref;
-
 pub(crate) mod fallback;
 
-pub use self::matches::*;
+use fontdb::FaceInfo;
-mod matches;
 
 pub use self::system::*;
 mod system;
 
 /// A font
-pub struct Font<'a> {
+pub struct Font(FontInner);
-    pub info: &'a fontdb::FaceInfo,
+
-    pub data: &'a [u8],
+#[ouroboros::self_referencing]
-    pub rustybuzz: rustybuzz::Face<'a>,
+#[allow(dead_code)]
-    #[cfg(feature = "swash")]
+struct FontInner {
-    pub swash: (u32, swash::CacheKey),
+    info: fontdb::FaceInfo,
+    #[borrows(info)]
+    #[covariant]
+    rustybuzz: rustybuzz::Face<'this>,
+    // workaround, since ouroboros does not work with #[cfg(feature = "swash")]
+    swash: SwashKey,
 }
 
-impl<'a> Font<'a> {
+#[cfg(feature = "swash")]
-    pub fn new(info: &'a fontdb::FaceInfo) -> Option<Self> {
+pub type SwashKey = (u32, swash::CacheKey);
+
+#[cfg(not(feature = "swash"))]
+pub type SwashKey = ();
+
+impl Font {
+    pub fn new(info: &fontdb::FaceInfo) -> Option<Self> {
+        #[allow(unused_variables)]
         let data = match &info.source {
-            fontdb::Source::Binary(data) => data.deref().as_ref(),
+            fontdb::Source::Binary(data) => (**data).as_ref(),
             #[cfg(feature = "std")]
             fontdb::Source::File(path) => {
                 log::warn!("Unsupported fontdb Source::File('{}')", path.display());
                 return None;
             }
             #[cfg(feature = "std")]
-            fontdb::Source::SharedFile(_path, data) => data.deref().as_ref(),
+            fontdb::Source::SharedFile(_path, data) => (**data).as_ref(),
         };
+        Some(Self(
+            FontInnerTryBuilder {
+                info: info.clone(),
+                swash: {
+                    #[cfg(feature = "swash")]
+                    let swash = {
+                        let swash =
+                            swash::FontRef::from_index((*data).as_ref(), info.index as usize)?;
+                        (swash.offset, swash.key)
+                    };
+                    #[cfg(not(feature = "swash"))]
+                    let swash = ();
+                    swash
+                },
+                rustybuzz_builder: |info| {
+                    rustybuzz::Face::from_slice(get_data(info), info.index).ok_or(())
+                },
+            }
+            .try_build()
+            .ok()?,
+        ))
+    }
 
-        Some(Self {
+    pub fn info(&self) -> &FaceInfo {
-            info,
+        self.0.borrow_info()
-            data,
+    }
-            rustybuzz: rustybuzz::Face::from_slice(data, info.index)?,
+
-            #[cfg(feature = "swash")]
+    pub fn data(&self) -> &[u8] {
-            swash: {
+        get_data(self.0.borrow_info())
-                let swash = swash::FontRef::from_index(data, info.index as usize)?;
+    }
-                (swash.offset, swash.key)
+
-            },
+    pub fn rustybuzz(&self) -> &rustybuzz::Face {
-        })
+        self.0.borrow_rustybuzz()
     }
 
     pub fn name(&self) -> &str {
-        if let Some((name, _)) = self.info.families.first() {
+        if let Some((name, _)) = self.info().families.first() {
             name
         } else {
-            &self.info.post_script_name
+            &self.info().post_script_name
         }
     }
 
     pub fn contains_family(&self, family: &str) -> bool {
-        self.info.families.iter().any(|(name, _)| name == family)
+        self.info().families.iter().any(|(name, _)| name == family)
     }
 
     #[cfg(feature = "swash")]
     pub fn as_swash(&self) -> swash::FontRef {
+        let info = self.0.borrow_info();
+        let swash = self.0.borrow_swash();
         swash::FontRef {
-            data: self.data,
+            data: get_data(info),
-            offset: self.swash.0,
+            offset: swash.0,
-            key: self.swash.1,
+            key: swash.1,
         }
     }
+
+    // This is used to prevent warnings due to the swash field being unused.
+    #[cfg(not(feature = "swash"))]
+    #[allow(dead_code)]
+    fn as_swash(&self) {
+        self.0.borrow_swash();
+    }
+}
+
+fn get_data(info: &FaceInfo) -> &[u8] {
+    match &info.source {
+        fontdb::Source::Binary(data) => (**data).as_ref(),
+        #[cfg(feature = "std")]
+        fontdb::Source::File(path) => {
+            // This should never happen, because `Font::new` verified the source isn't a file
+            panic!("Unsupported fontdb Source::File('{}')", path.display());
+        }
+        #[cfg(feature = "std")]
+        fontdb::Source::SharedFile(_path, data) => (**data).as_ref(),
+    }
 }

src/font/system/no_std.rs

@@ -6,7 +6,7 @@ use alloc::{
     vec::Vec,
 };
 
-use crate::{Attrs, Font, FontMatches};
+use crate::{Attrs, Font};
 
 /// Access system fonts
 pub struct FontSystem {
@@ -43,9 +43,7 @@ impl FontSystem {
         &self.db
     }
 
-    // Clippy false positive
+    pub fn get_font(&self, id: fontdb::ID) -> Option<Arc<Font>> {
-    #[allow(clippy::needless_lifetimes)]
-    pub fn get_font<'a>(&'a self, id: fontdb::ID) -> Option<Arc<Font<'a>>> {
         let face = self.db.face(id)?;
         match Font::new(face) {
             Some(font) => Some(Arc::new(font)),
@@ -56,7 +54,7 @@ impl FontSystem {
         }
     }
 
-    pub fn get_font_matches<'a>(&'a self, attrs: Attrs) -> Arc<FontMatches<'a>> {
+    pub fn get_font_matches(&self, attrs: Attrs) -> Arc<Vec<Arc<Font>>> {
         let mut fonts = Vec::new();
         for face in self.db.faces() {
             if !attrs.matches(face) {
@@ -68,10 +66,6 @@ impl FontSystem {
             }
         }
 
-        Arc::new(FontMatches {
+        Arc::new(fonts)
-            locale: &self.locale,
-            default_family: self.db.family_name(&attrs.family).to_string(),
-            fonts,
-        })
     }
 }

src/font/system/std.rs

@@ -5,22 +5,15 @@ use std::{
     sync::{Arc, Mutex},
 };
 
-use crate::{Attrs, AttrsOwned, Font, FontMatches};
+use crate::{Attrs, AttrsOwned, Font};
-
-#[ouroboros::self_referencing]
-struct FontSystemInner {
-    locale: String,
-    db: fontdb::Database,
-    #[borrows(db)]
-    #[not_covariant]
-    font_cache: Mutex<HashMap<fontdb::ID, Option<Arc<Font<'this>>>>>,
-    #[borrows(locale, db)]
-    #[not_covariant]
-    font_matches_cache: Mutex<HashMap<AttrsOwned, Arc<FontMatches<'this>>>>,
-}
 
 /// Access system fonts
-pub struct FontSystem(FontSystemInner);
+pub struct FontSystem {
+    locale: String,
+    db: fontdb::Database,
+    font_cache: Mutex<HashMap<fontdb::ID, Option<Arc<Font>>>>,
+    font_matches_cache: Mutex<HashMap<AttrsOwned, Arc<Vec<Arc<Font>>>>>,
+}
 
 impl FontSystem {
     /// Create a new [`FontSystem`], that allows access to any installed system fonts
@@ -92,97 +85,73 @@ impl FontSystem {
             );
         }
 
-        Self(
+        Self {
-            FontSystemInnerBuilder {
+            locale,
-                locale,
+            db,
-                db,
+            font_cache: Mutex::new(HashMap::new()),
-                font_cache_builder: |_| Mutex::new(HashMap::new()),
+            font_matches_cache: Mutex::new(HashMap::new()),
-                font_matches_cache_builder: |_, _| Mutex::new(HashMap::new()),
+        }
-            }
-            .build(),
-        )
     }
 
     pub fn locale(&self) -> &str {
-        self.0.borrow_locale()
+        &self.locale
     }
 
     pub fn db(&self) -> &fontdb::Database {
-        self.0.borrow_db()
+        &self.db
     }
 
     pub fn into_locale_and_db(self) -> (String, fontdb::Database) {
-        let heads = self.0.into_heads();
+        (self.locale, self.db)
-        (heads.locale, heads.db)
     }
 
-    // Clippy false positive
+    pub fn get_font(&self, id: fontdb::ID) -> Option<Arc<Font>> {
-    #[allow(clippy::needless_lifetimes)]
+        self.font_cache
-    pub fn get_font<'a>(&'a self, id: fontdb::ID) -> Option<Arc<Font<'a>>> {
+            .lock()
-        self.0.with(|fields| get_font(&fields, id))
+            .expect("failed to lock font cache")
-    }
+            .entry(id)
-
+            .or_insert_with(|| {
-    pub fn get_font_matches<'a>(&'a self, attrs: Attrs) -> Arc<FontMatches<'a>> {
+                let face = self.db.face(id)?;
-        self.0.with(|fields| {
+                match Font::new(face) {
-            let mut font_matches_cache = fields
+                    Some(font) => Some(Arc::new(font)),
-                .font_matches_cache
+                    None => {
-                .lock()
+                        log::warn!("failed to load font '{}'", face.post_script_name);
-                .expect("failed to lock font matches cache");
+                        None
-            //TODO: do not create AttrsOwned unless entry does not already exist
-            font_matches_cache
-                .entry(AttrsOwned::new(attrs))
-                .or_insert_with(|| {
-                    #[cfg(not(target_arch = "wasm32"))]
-                    let now = std::time::Instant::now();
-
-                    let mut fonts = Vec::new();
-                    for face in fields.db.faces() {
-                        if !attrs.matches(face) {
-                            continue;
-                        }
-
-                        if let Some(font) = get_font(&fields, face.id) {
-                            fonts.push(font);
-                        }
                     }
-
-                    let font_matches = Arc::new(FontMatches {
-                        locale: fields.locale,
-                        default_family: fields.db.family_name(&attrs.family).to_string(),
-                        fonts,
-                    });
-
-                    #[cfg(not(target_arch = "wasm32"))]
-                    {
-                        let elapsed = now.elapsed();
-                        log::debug!("font matches for {:?} in {:?}", attrs, elapsed);
-                    }
-
-                    font_matches
-                })
-                .clone()
-        })
-    }
-}
-
-fn get_font<'b>(
-    fields: &ouroboros_impl_font_system_inner::BorrowedFields<'_, 'b>,
-    id: fontdb::ID,
-) -> Option<Arc<Font<'b>>> {
-    fields
-        .font_cache
-        .lock()
-        .expect("failed to lock font cache")
-        .entry(id)
-        .or_insert_with(|| {
-            let face = fields.db.face(id)?;
-            match Font::new(face) {
-                Some(font) => Some(Arc::new(font)),
-                None => {
-                    log::warn!("failed to load font '{}'", face.post_script_name);
-                    None
                 }
-            }
+            })
-        })
+            .clone()
-        .clone()
+    }
+
+    pub fn get_font_matches(&self, attrs: Attrs) -> Arc<Vec<Arc<Font>>> {
+        self.font_matches_cache
+            .lock()
+            .expect("failed to lock font matches cache")
+            //TODO: do not create AttrsOwned unless entry does not already exist
+            .entry(AttrsOwned::new(attrs))
+            .or_insert_with(|| {
+                #[cfg(not(target_arch = "wasm32"))]
+                let now = std::time::Instant::now();
+
+                let mut fonts = Vec::new();
+                for face in self.db.faces() {
+                    if !attrs.matches(face) {
+                        continue;
+                    }
+
+                    if let Some(font) = self.get_font(face.id) {
+                        fonts.push(font);
+                    }
+                }
+
+                #[cfg(not(target_arch = "wasm32"))]
+                {
+                    let elapsed = now.elapsed();
+                    log::debug!("font matches for {:?} in {:?}", attrs, elapsed);
+                }
+
+                Arc::new(fonts)
+            })
+            .clone()
+    }
 }

src/shape.rs

@@ -21,7 +21,7 @@ fn shape_fallback(
 ) -> (Vec<ShapeGlyph>, Vec<usize>) {
     let run = &line[start_run..end_run];
 
-    let font_scale = font.rustybuzz.units_per_em() as f32;
+    let font_scale = font.rustybuzz().units_per_em() as f32;
 
     let mut buffer = rustybuzz::UnicodeBuffer::new();
     buffer.set_direction(if span_rtl {
@@ -35,7 +35,7 @@ fn shape_fallback(
     let rtl = matches!(buffer.direction(), rustybuzz::Direction::RightToLeft);
     assert_eq!(rtl, span_rtl);
 
-    let glyph_buffer = rustybuzz::shape(&font.rustybuzz, &[], buffer);
+    let glyph_buffer = rustybuzz::shape(&font.rustybuzz(), &[], buffer);
     let glyph_infos = glyph_buffer.glyph_infos();
     let glyph_positions = glyph_buffer.glyph_positions();
 
@@ -62,7 +62,7 @@ fn shape_fallback(
             y_advance,
             x_offset,
             y_offset,
-            font_id: font.info.id,
+            font_id: font.info().id,
             glyph_id: info.glyph_id.try_into().expect("failed to cast glyph ID"),
             //TODO: color should not be related to shaping
             color_opt: attrs.color_opt,
@@ -123,24 +123,18 @@ fn shape_run(
 
     let attrs = attrs_list.get_span(start_run);
 
-    let font_matches = font_system.get_font_matches(attrs);
+    let fonts = font_system.get_font_matches(attrs);
 
-    let default_families = [font_matches.default_family.as_str()];
+    let db = font_system.db();
-    let mut font_iter = FontFallbackIter::new(
-        &font_matches.fonts,
-        &default_families,
-        scripts,
-        font_matches.locale,
-    );
 
-    let (mut glyphs, mut missing) = shape_fallback(
+    let default_families = [db.family_name(&attrs.family)];
-        font_iter.next().expect("no default font found"),
+    let mut font_iter =
-        line,
+        FontFallbackIter::new(&fonts, &default_families, scripts, font_system.locale());
-        attrs_list,
+
-        start_run,
+    let font = font_iter.next().expect("no default font found");
-        end_run,
+
-        span_rtl,
+    let (mut glyphs, mut missing) =
-    );
+        shape_fallback(font, line, attrs_list, start_run, end_run, span_rtl);
 
     //TODO: improve performance!
     while !missing.is_empty() {