Refactor `all_font_names` (#15345)

张小白 created 1 year ago

In the current code implementation, it seems that the only difference
between `all_font_names` and `all_font_families` is whether dynamically
loaded font resources are included. Specifically, `all_font_families`
returns the names of all system fonts, while `all_font_names` includes
both the system font names and the dynamically loaded font names. In
other words, `all_font_families` is a strict subset of `all_font_names`.
This is what I observed in my tests on macOS.

<img width="682" alt="截屏2024-07-28 00 49 29"
src="https://github.com/user-attachments/assets/47317c28-0074-49d2-bcfa-052cab13e335">

Related codes:
```rust
let x: HashSet<_> = self.all_font_names().into_iter().collect();
let y: HashSet<_> = self.all_font_families().into_iter().collect();
let only_in_x = x.difference(&y).collect::<Vec<_>>();
let only_in_y = y.difference(&x).collect::<Vec<_>>();
println!("=====================================");
println!("1 -> {:?}", only_in_x);
println!("-------------------------------------");
println!("2 -> {:?}", only_in_y);
```

Release Notes:

- N/A

Change summary

crates/gpui/src/platform.rs                      |  1 -
crates/gpui/src/platform/linux/text_system.rs    | 11 -----------
crates/gpui/src/platform/mac/text_system.rs      | 16 ++++------------
crates/gpui/src/platform/windows/direct_write.rs |  8 --------
crates/gpui/src/text_system.rs                   | 13 +++++--------
5 files changed, 9 insertions(+), 40 deletions(-)

Detailed changes

crates/gpui/src/platform.rs

@@ -412,7 +412,6 @@ pub trait PlatformDispatcher: Send + Sync {
 pub(crate) trait PlatformTextSystem: Send + Sync {
     fn add_fonts(&self, fonts: Vec<Cow<'static, [u8]>>) -> Result<()>;
     fn all_font_names(&self) -> Vec<String>;
-    fn all_font_families(&self) -> Vec<String>;
     fn font_id(&self, descriptor: &Font) -> Result<FontId>;
     fn font_metrics(&self, font_id: FontId) -> FontMetrics;
     fn typographic_bounds(&self, font_id: FontId, glyph_id: GlyphId) -> Result<Bounds<f32>>;

crates/gpui/src/platform/linux/text_system.rs

@@ -77,17 +77,6 @@ impl PlatformTextSystem for CosmicTextSystem {
         result
     }
 
-    fn all_font_families(&self) -> Vec<String> {
-        self.0
-            .read()
-            .font_system
-            .db()
-            .faces()
-            // todo(linux) this will list the same font family multiple times
-            .filter_map(|face| face.families.first().map(|family| family.0.clone()))
-            .collect_vec()
-    }
-
     fn font_id(&self, font: &Font) -> Result<FontId> {
         // todo(linux): Do we need to use CosmicText's Font APIs? Can we consolidate this to use font_kit?
         let mut state = self.0.write();

crates/gpui/src/platform/mac/text_system.rs

@@ -5,7 +5,7 @@ use crate::{
 };
 use anyhow::anyhow;
 use cocoa::appkit::{CGFloat, CGPoint};
-use collections::{BTreeSet, HashMap};
+use collections::HashMap;
 use core_foundation::{
     attributed_string::CFMutableAttributedString,
     base::{CFRange, TCFType},
@@ -93,26 +93,18 @@ impl PlatformTextSystem for MacTextSystem {
     }
 
     fn all_font_names(&self) -> Vec<String> {
+        let mut names = Vec::new();
         let collection = core_text::font_collection::create_for_all_families();
         let Some(descriptors) = collection.get_descriptors() else {
-            return Vec::new();
+            return names;
         };
-        let mut names = BTreeSet::new();
         for descriptor in descriptors.into_iter() {
             names.extend(lenient_font_attributes::family_name(&descriptor));
         }
         if let Ok(fonts_in_memory) = self.0.read().memory_source.all_families() {
             names.extend(fonts_in_memory);
         }
-        names.into_iter().collect()
-    }
-
-    fn all_font_families(&self) -> Vec<String> {
-        self.0
-            .read()
-            .system_source
-            .all_families()
-            .expect("core text should never return an error")
+        names
     }
 
     fn font_id(&self, font: &Font) -> Result<FontId> {

crates/gpui/src/platform/windows/direct_write.rs

@@ -177,10 +177,6 @@ impl PlatformTextSystem for DirectWriteTextSystem {
         self.0.read().all_font_names()
     }
 
-    fn all_font_families(&self) -> Vec<String> {
-        self.0.read().all_font_families()
-    }
-
     fn font_id(&self, font: &Font) -> Result<FontId> {
         let lock = self.0.upgradable_read();
         if let Some(font_id) = lock.font_selections.get(font) {
@@ -962,10 +958,6 @@ impl DirectWriteState {
         ));
         result
     }
-
-    fn all_font_families(&self) -> Vec<String> {
-        get_font_names_from_collection(&self.system_font_collection, &self.components.locale)
-    }
 }
 
 impl Drop for DirectWriteState {

crates/gpui/src/text_system.rs

@@ -17,7 +17,7 @@ use crate::{
     StrikethroughStyle, UnderlineStyle,
 };
 use anyhow::anyhow;
-use collections::{BTreeSet, FxHashMap};
+use collections::FxHashMap;
 use core::fmt;
 use derive_more::Deref;
 use itertools::Itertools;
@@ -78,18 +78,15 @@ impl TextSystem {
 
     /// Get a list of all available font names from the operating system.
     pub fn all_font_names(&self) -> Vec<String> {
-        let mut names: BTreeSet<_> = self
-            .platform_text_system
-            .all_font_names()
-            .into_iter()
-            .collect();
-        names.extend(self.platform_text_system.all_font_families());
+        let mut names = self.platform_text_system.all_font_names();
         names.extend(
             self.fallback_font_stack
                 .iter()
                 .map(|font| font.family.to_string()),
         );
-        names.into_iter().collect()
+        names.sort();
+        names.dedup();
+        names
     }
 
     /// Add a font's data to the text system.