From 5103995c32f01469c4b1c317f415877e0b360cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=B0=8F=E7=99=BD?= <364772080@qq.com> Date: Thu, 9 May 2024 01:58:31 +0800 Subject: [PATCH] windows: Fix incorrect font rendering (#11545) 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 --- .../gpui/src/platform/windows/direct_write.rs | 128 ++++++++++++------ 1 file changed, 83 insertions(+), 45 deletions(-) diff --git a/crates/gpui/src/platform/windows/direct_write.rs b/crates/gpui/src/platform/windows/direct_write.rs index 6078d999be..1217d10ebd 100644 --- a/crates/gpui/src/platform/windows/direct_write.rs +++ b/crates/gpui/src/platform/windows/direct_write.rs @@ -58,7 +58,14 @@ struct DirectWriteState { custom_font_collection: IDWriteFontCollection1, fonts: Vec, font_selections: HashMap, - font_id_by_postscript_name: HashMap, + font_id_by_identifier: HashMap, +} + +#[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::().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)) } -unsafe fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Option { - let mut info = std::mem::zeroed(); +#[inline] +fn get_font_identifier(font_face: &IDWriteFontFace3, locale: &str) -> Option { + 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, + }) +} + +#[inline] +fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Option { + 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 { +#[inline] +fn get_name(string: IDWriteLocalizedStrings, locale: &str) -> Option { 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