windows: Fix incorrect font rendering (#11545)

张小白 created

Previously, `DirectWrite` had been following the text system
implementation on `macOS`, using the font's postscript name to
differentiate between different font faces. However, I noticed
occasional rendering issues, such as fonts incorrectly rendering as
italics. Later, I discovered that on `Windows`, the postscript name is
**not** unique. Surprisingly, even the same font can have different
postscript names on macOS and Windows! It's hard to believe! The
postscript name of a font face should be obtained from the same font
table. Why would the same font face have different names?

For example, the postscript name of ZedMono on `macOS` is
`Zed-Mono-Bold-Extended-Italic`, while on `Windows`, it is
`Zed-Mono-Extended`, missing weight and style information, leading to
incorrect rendering.

This PR introduces a `FontIdentifier` struct to uniquely identify font
faces.

Release Notes:

- N/A

Change summary

crates/gpui/src/platform/windows/direct_write.rs | 128 +++++++++++------
1 file changed, 83 insertions(+), 45 deletions(-)

Detailed changes

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

@@ -58,7 +58,14 @@ struct DirectWriteState {
     custom_font_collection: IDWriteFontCollection1,
     fonts: Vec<FontInfo>,
     font_selections: HashMap<Font, FontId>,
-    font_id_by_postscript_name: HashMap<String, FontId>,
+    font_id_by_identifier: HashMap<FontIdentifier, FontId>,
+}
+
+#[derive(Debug, Clone, Hash, PartialEq, Eq)]
+struct FontIdentifier {
+    postscript_name: String,
+    weight: i32,
+    style: i32,
 }
 
 impl DirectWriteComponent {
@@ -118,7 +125,7 @@ impl DirectWriteTextSystem {
             custom_font_collection,
             fonts: Vec::new(),
             font_selections: HashMap::default(),
-            font_id_by_postscript_name: HashMap::default(),
+            font_id_by_identifier: HashMap::default(),
         })))
     }
 }
@@ -269,8 +276,7 @@ impl DirectWriteState {
             let Some(font_face) = font_face_ref.CreateFontFace().log_err() else {
                 continue;
             };
-            let Some(postscript_name) = get_postscript_name(&font_face, &self.components.locale)
-            else {
+            let Some(identifier) = get_font_identifier(&font_face, &self.components.locale) else {
                 continue;
             };
             let is_emoji = font_face.IsColorFont().as_bool();
@@ -287,8 +293,7 @@ impl DirectWriteState {
             };
             let font_id = FontId(self.fonts.len());
             self.fonts.push(font_info);
-            self.font_id_by_postscript_name
-                .insert(postscript_name, font_id);
+            self.font_id_by_identifier.insert(identifier, font_id);
             return Some(font_id);
         }
         None
@@ -945,8 +950,8 @@ impl IDWriteTextRenderer_Impl for TextRenderer {
             // This `cast()` action here should never fail since we are running on Win10+, and
             // `IDWriteFontFace3` requires Win10
             let font_face = &font_face.cast::<IDWriteFontFace3>().unwrap();
-            let Some((postscript_name, font_struct, is_emoji)) =
-                get_postscript_name_and_font(font_face, &self.locale)
+            let Some((font_identifier, font_struct, is_emoji)) =
+                get_font_identifier_and_font_struct(font_face, &self.locale)
             else {
                 log::error!("none postscript name found");
                 return Ok(());
@@ -954,8 +959,8 @@ impl IDWriteTextRenderer_Impl for TextRenderer {
 
             let font_id = if let Some(id) = context
                 .text_system
-                .font_id_by_postscript_name
-                .get(&postscript_name)
+                .font_id_by_identifier
+                .get(&font_identifier)
             {
                 *id
             } else {
@@ -1121,39 +1126,60 @@ fn get_font_names_from_collection(
     }
 }
 
-unsafe fn get_postscript_name_and_font(
+fn get_font_identifier_and_font_struct(
     font_face: &IDWriteFontFace3,
     locale: &str,
-) -> Option<(String, Font, bool)> {
+) -> Option<(FontIdentifier, Font, bool)> {
     let Some(postscript_name) = get_postscript_name(font_face, locale) else {
         return None;
     };
-    let Some(localized_family_name) = font_face.GetFamilyNames().log_err() else {
+    let Some(localized_family_name) = (unsafe { font_face.GetFamilyNames().log_err() }) else {
         return None;
     };
     let Some(family_name) = get_name(localized_family_name, locale) else {
         return None;
     };
+    let weight = unsafe { font_face.GetWeight() };
+    let style = unsafe { font_face.GetStyle() };
+    let identifier = FontIdentifier {
+        postscript_name,
+        weight: weight.0,
+        style: style.0,
+    };
     let font_struct = Font {
         family: family_name.into(),
         features: FontFeatures::default(),
-        weight: font_face.GetWeight().into(),
-        style: font_face.GetStyle().into(),
+        weight: weight.into(),
+        style: style.into(),
     };
-    let is_emoji = font_face.IsColorFont().as_bool();
-    Some((postscript_name, font_struct, is_emoji))
+    let is_emoji = unsafe { font_face.IsColorFont().as_bool() };
+    Some((identifier, font_struct, is_emoji))
+}
+
+#[inline]
+fn get_font_identifier(font_face: &IDWriteFontFace3, locale: &str) -> Option<FontIdentifier> {
+    let weight = unsafe { font_face.GetWeight().0 };
+    let style = unsafe { font_face.GetStyle().0 };
+    get_postscript_name(font_face, locale).map(|postscript_name| FontIdentifier {
+        postscript_name,
+        weight,
+        style,
+    })
 }
 
-unsafe fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Option<String> {
-    let mut info = std::mem::zeroed();
+#[inline]
+fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Option<String> {
+    let mut info = None;
     let mut exists = BOOL(0);
-    font_face
-        .GetInformationalStrings(
-            DWRITE_INFORMATIONAL_STRING_POSTSCRIPT_NAME,
-            &mut info,
-            &mut exists,
-        )
-        .log_err();
+    unsafe {
+        font_face
+            .GetInformationalStrings(
+                DWRITE_INFORMATIONAL_STRING_POSTSCRIPT_NAME,
+                &mut info,
+                &mut exists,
+            )
+            .log_err();
+    }
     if !exists.as_bool() || info.is_none() {
         return None;
     }
@@ -1162,7 +1188,7 @@ unsafe fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Opt
 }
 
 // https://learn.microsoft.com/en-us/windows/win32/api/dwrite/ne-dwrite-dwrite_font_feature_tag
-unsafe fn apply_font_features(
+fn apply_font_features(
     direct_write_features: &IDWriteTypography,
     features: &FontFeatures,
 ) -> Result<()> {
@@ -1191,11 +1217,15 @@ unsafe fn apply_font_features(
             continue;
         }
 
-        direct_write_features.AddFontFeature(make_direct_write_feature(&tag, enable))?;
+        unsafe {
+            direct_write_features.AddFontFeature(make_direct_write_feature(&tag, enable))?;
+        }
+    }
+    unsafe {
+        direct_write_features.AddFontFeature(feature_liga)?;
+        direct_write_features.AddFontFeature(feature_clig)?;
+        direct_write_features.AddFontFeature(feature_calt)?;
     }
-    direct_write_features.AddFontFeature(feature_liga)?;
-    direct_write_features.AddFontFeature(feature_clig)?;
-    direct_write_features.AddFontFeature(feature_calt)?;
 
     Ok(())
 }
@@ -1231,32 +1261,39 @@ fn make_direct_write_tag(tag_name: &str) -> DWRITE_FONT_FEATURE_TAG {
     DWRITE_FONT_FEATURE_TAG(make_open_type_tag(tag_name))
 }
 
-unsafe fn get_name(string: IDWriteLocalizedStrings, locale: &str) -> Option<String> {
+#[inline]
+fn get_name(string: IDWriteLocalizedStrings, locale: &str) -> Option<String> {
     let mut locale_name_index = 0u32;
     let mut exists = BOOL(0);
-    string
-        .FindLocaleName(
-            &HSTRING::from(locale),
-            &mut locale_name_index,
-            &mut exists as _,
-        )
-        .log_err();
-    if !exists.as_bool() {
+    unsafe {
         string
             .FindLocaleName(
-                DEFAULT_LOCALE_NAME,
-                &mut locale_name_index as _,
+                &HSTRING::from(locale),
+                &mut locale_name_index,
                 &mut exists as _,
             )
             .log_err();
+    }
+    if !exists.as_bool() {
+        unsafe {
+            string
+                .FindLocaleName(
+                    DEFAULT_LOCALE_NAME,
+                    &mut locale_name_index as _,
+                    &mut exists as _,
+                )
+                .log_err();
+        }
         if !exists.as_bool() {
             return None;
         }
     }
 
-    let name_length = string.GetStringLength(locale_name_index).unwrap() as usize;
+    let name_length = unsafe { string.GetStringLength(locale_name_index).unwrap() } as usize;
     let mut name_vec = vec![0u16; name_length + 1];
-    string.GetString(locale_name_index, &mut name_vec).unwrap();
+    unsafe {
+        string.GetString(locale_name_index, &mut name_vec).unwrap();
+    }
 
     Some(String::from_utf16_lossy(&name_vec[..name_length]))
 }
@@ -1287,7 +1324,8 @@ fn get_system_ui_font_name() -> SharedString {
             // Segoe UI is the Windows font intended for user interface text strings.
             "Segoe UI".into()
         } else {
-            String::from_utf16_lossy(&info.lfFaceName).into()
+            let font_name = String::from_utf16_lossy(&info.lfFaceName);
+            font_name.trim_matches(char::from(0)).to_owned().into()
         };
         log::info!("Use {} as UI font.", font_family);
         font_family