windows: Update `windows-rs` crate and better error handling in `DirectWrite` (#12818)

张小白 created

- Update `windows-rs` from `0.56` to `0.57`
- Use the newly introduced `Owned` struct in `0.57` to handle the RAII
stuff of `HANDLE`
- Better error handling in `DirectWrite`

Release Notes:

- N/A

Change summary

Cargo.lock                                       |  34 +-
Cargo.toml                                       |   2 
crates/gpui/Cargo.toml                           |   2 
crates/gpui/src/platform/windows/direct_write.rs | 204 ++++++++---------
crates/gpui/src/platform/windows/platform.rs     |  11 
crates/gpui/src/platform/windows/util.rs         |  30 --
6 files changed, 116 insertions(+), 167 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -4238,7 +4238,7 @@ dependencies = [
  "text",
  "time",
  "util",
- "windows 0.56.0",
+ "windows 0.57.0",
 ]
 
 [[package]]
@@ -4558,7 +4558,7 @@ dependencies = [
  "unindent",
  "url",
  "util",
- "windows 0.56.0",
+ "windows 0.57.0",
 ]
 
 [[package]]
@@ -4781,8 +4781,8 @@ dependencies = [
  "wayland-cursor",
  "wayland-protocols",
  "wayland-protocols-plasma",
- "windows 0.56.0",
- "windows-core 0.56.0",
+ "windows 0.57.0",
+ "windows-core 0.57.0",
  "x11rb",
  "xim",
  "xkbcommon",
@@ -6096,7 +6096,7 @@ dependencies = [
  "serde_json",
  "smol",
  "util",
- "windows 0.56.0",
+ "windows 0.57.0",
 ]
 
 [[package]]
@@ -6592,7 +6592,7 @@ dependencies = [
  "tempfile",
  "util",
  "walkdir",
- "windows 0.56.0",
+ "windows 0.57.0",
 ]
 
 [[package]]
@@ -10440,7 +10440,7 @@ dependencies = [
  "theme",
  "thiserror",
  "util",
- "windows 0.56.0",
+ "windows 0.57.0",
 ]
 
 [[package]]
@@ -11368,7 +11368,7 @@ dependencies = [
  "story",
  "strum",
  "theme",
- "windows 0.56.0",
+ "windows 0.57.0",
 ]
 
 [[package]]
@@ -12512,11 +12512,11 @@ dependencies = [
 
 [[package]]
 name = "windows"
-version = "0.56.0"
+version = "0.57.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1de69df01bdf1ead2f4ac895dc77c9351aefff65b2f3db429a343f9cbf05e132"
+checksum = "12342cb4d8e3b046f3d80effd474a7a02447231330ef77d71daa6fbc40681143"
 dependencies = [
- "windows-core 0.56.0",
+ "windows-core 0.57.0",
  "windows-targets 0.52.5",
 ]
 
@@ -12531,9 +12531,9 @@ dependencies = [
 
 [[package]]
 name = "windows-core"
-version = "0.56.0"
+version = "0.57.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4698e52ed2d08f8658ab0c39512a7c00ee5fe2688c65f8c0a4f06750d729f2a6"
+checksum = "d2ed2439a290666cd67ecce2b0ffaad89c2a56b976b736e6ece670297897832d"
 dependencies = [
  "windows-implement",
  "windows-interface",
@@ -12543,9 +12543,9 @@ dependencies = [
 
 [[package]]
 name = "windows-implement"
-version = "0.56.0"
+version = "0.57.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f6fc35f58ecd95a9b71c4f2329b911016e6bec66b3f2e6a4aad86bd2e99e2f9b"
+checksum = "9107ddc059d5b6fbfbffdfa7a7fe3e22a226def0b2608f72e9d552763d3e1ad7"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -12554,9 +12554,9 @@ dependencies = [
 
 [[package]]
 name = "windows-interface"
-version = "0.56.0"
+version = "0.57.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "08990546bf4edef8f431fa6326e032865f27138718c587dc21bc0265bbcb57cc"
+checksum = "29bee4b38ea3cde66011baa44dba677c432a78593e202392d1e9070cf2a7fca7"
 dependencies = [
  "proc-macro2",
  "quote",

Cargo.toml 🔗

@@ -410,7 +410,7 @@ wit-component = "0.201"
 sys-locale = "0.3.1"
 
 [workspace.dependencies.windows]
-version = "0.56.0"
+version = "0.57"
 features = [
     "implement",
     "Foundation_Numerics",

crates/gpui/Cargo.toml 🔗

@@ -139,7 +139,7 @@ xim = { git = "https://github.com/npmania/xim-rs", rev = "27132caffc5b9bc9c432ca
 
 [target.'cfg(windows)'.dependencies]
 windows.workspace = true
-windows-core = "0.56"
+windows-core = "0.57"
 
 [target.'cfg(windows)'.build-dependencies]
 embed-resource = "2.4"

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

@@ -229,7 +229,14 @@ impl PlatformTextSystem for DirectWriteTextSystem {
     }
 
     fn layout_line(&self, text: &str, font_size: Pixels, runs: &[FontRun]) -> LineLayout {
-        self.0.write().layout_line(text, font_size, runs)
+        self.0
+            .write()
+            .layout_line(text, font_size, runs)
+            .log_err()
+            .unwrap_or(LineLayout {
+                font_size,
+                ..Default::default()
+            })
     }
 }
 
@@ -296,20 +303,15 @@ impl DirectWriteState {
         } else {
             &self.custom_font_collection
         };
-        let Some(fontset) = collection.GetFontSet().log_err() else {
-            return None;
-        };
-        let Some(font) = fontset
+        let fontset = collection.GetFontSet().log_err()?;
+        let font = fontset
             .GetMatchingFonts(
                 &HSTRING::from(family_name),
                 font_weight.into(),
                 DWRITE_FONT_STRETCH_NORMAL,
                 font_style.into(),
             )
-            .log_err()
-        else {
-            return None;
-        };
+            .log_err()?;
         let total_number = font.GetFontCount();
         for index in 0..total_number {
             let Some(font_face_ref) = font.GetFontFaceReference(index).log_err() else {
@@ -343,11 +345,15 @@ impl DirectWriteState {
 
     unsafe fn update_system_font_collection(&mut self) {
         let mut collection = std::mem::zeroed();
-        self.components
+        if self
+            .components
             .factory
             .GetSystemFontCollection(false, &mut collection, true)
-            .unwrap();
-        self.system_font_collection = collection.unwrap();
+            .log_err()
+            .is_some()
+        {
+            self.system_font_collection = collection.unwrap();
+        }
     }
 
     fn select_font(&mut self, target_font: &Font) -> FontId {
@@ -402,12 +408,17 @@ impl DirectWriteState {
             })
     }
 
-    fn layout_line(&mut self, text: &str, font_size: Pixels, font_runs: &[FontRun]) -> LineLayout {
+    fn layout_line(
+        &mut self,
+        text: &str,
+        font_size: Pixels,
+        font_runs: &[FontRun],
+    ) -> Result<LineLayout> {
         if font_runs.is_empty() {
-            return LineLayout {
+            return Ok(LineLayout {
                 font_size,
                 ..Default::default()
-            };
+            });
         }
         unsafe {
             let text_renderer = self.components.text_renderer.clone();
@@ -423,25 +434,22 @@ impl DirectWriteState {
                 } else {
                     &self.custom_font_collection
                 };
-                let format = self
-                    .components
-                    .factory
-                    .CreateTextFormat(
-                        &HSTRING::from(&font_info.font_family),
-                        collection,
-                        font_info.font_face.GetWeight(),
-                        font_info.font_face.GetStyle(),
-                        DWRITE_FONT_STRETCH_NORMAL,
-                        font_size.0,
-                        &HSTRING::from(&self.components.locale),
-                    )
-                    .unwrap();
+                let format = self.components.factory.CreateTextFormat(
+                    &HSTRING::from(&font_info.font_family),
+                    collection,
+                    font_info.font_face.GetWeight(),
+                    font_info.font_face.GetStyle(),
+                    DWRITE_FONT_STRETCH_NORMAL,
+                    font_size.0,
+                    &HSTRING::from(&self.components.locale),
+                )?;
 
-                let layout = self
-                    .components
-                    .factory
-                    .CreateTextLayout(&text_wide, &format, f32::INFINITY, f32::INFINITY)
-                    .unwrap();
+                let layout = self.components.factory.CreateTextLayout(
+                    &text_wide,
+                    &format,
+                    f32::INFINITY,
+                    f32::INFINITY,
+                )?;
                 let current_text = &text[utf8_offset..(utf8_offset + first_run.len)];
                 utf8_offset += first_run.len;
                 let current_text_utf16_length = current_text.encode_utf16().count() as u32;
@@ -449,9 +457,7 @@ impl DirectWriteState {
                     startPosition: utf16_offset,
                     length: current_text_utf16_length,
                 };
-                layout
-                    .SetTypography(&font_info.features, text_range)
-                    .unwrap();
+                layout.SetTypography(&font_info.features, text_range)?;
                 utf16_offset += current_text_utf16_length;
 
                 layout
@@ -465,9 +471,7 @@ impl DirectWriteState {
                     first_run = false;
                     let mut metrics = vec![DWRITE_LINE_METRICS::default(); 4];
                     let mut line_count = 0u32;
-                    text_layout
-                        .GetLineMetrics(Some(&mut metrics), &mut line_count as _)
-                        .unwrap();
+                    text_layout.GetLineMetrics(Some(&mut metrics), &mut line_count as _)?;
                     ascent = px(metrics[0].baseline);
                     descent = px(metrics[0].height - metrics[0].baseline);
                     continue;
@@ -487,22 +491,13 @@ impl DirectWriteState {
                     length: current_text_utf16_length,
                 };
                 utf16_offset += current_text_utf16_length;
+                text_layout.SetFontCollection(collection, text_range)?;
                 text_layout
-                    .SetFontCollection(collection, text_range)
-                    .unwrap();
-                text_layout
-                    .SetFontFamilyName(&HSTRING::from(&font_info.font_family), text_range)
-                    .unwrap();
-                text_layout.SetFontSize(font_size.0, text_range).unwrap();
-                text_layout
-                    .SetFontStyle(font_info.font_face.GetStyle(), text_range)
-                    .unwrap();
-                text_layout
-                    .SetFontWeight(font_info.font_face.GetWeight(), text_range)
-                    .unwrap();
-                text_layout
-                    .SetTypography(&font_info.features, text_range)
-                    .unwrap();
+                    .SetFontFamilyName(&HSTRING::from(&font_info.font_family), text_range)?;
+                text_layout.SetFontSize(font_size.0, text_range)?;
+                text_layout.SetFontStyle(font_info.font_face.GetStyle(), text_range)?;
+                text_layout.SetFontWeight(font_info.font_face.GetWeight(), text_range)?;
+                text_layout.SetTypography(&font_info.features, text_range)?;
             }
 
             let mut runs = Vec::new();
@@ -513,24 +508,22 @@ impl DirectWriteState {
                 utf16_index: 0,
                 width: 0.0,
             };
-            text_layout
-                .Draw(
-                    Some(&renderer_context as *const _ as _),
-                    &text_renderer.0,
-                    0.0,
-                    0.0,
-                )
-                .unwrap();
+            text_layout.Draw(
+                Some(&renderer_context as *const _ as _),
+                &text_renderer.0,
+                0.0,
+                0.0,
+            )?;
             let width = px(renderer_context.width);
 
-            LineLayout {
+            Ok(LineLayout {
                 font_size,
                 width,
                 ascent,
                 descent,
                 runs,
                 len: text.len(),
-            }
+            })
         }
     }
 
@@ -979,7 +972,6 @@ impl IDWriteTextRenderer_Impl for TextRenderer {
             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(());
             };
 
@@ -1142,7 +1134,7 @@ fn get_font_names_from_collection(
             let Some(localized_family_name) = font_family.GetFamilyNames().log_err() else {
                 continue;
             };
-            let Some(family_name) = get_name(localized_family_name, locale) else {
+            let Some(family_name) = get_name(localized_family_name, locale).log_err() else {
                 continue;
             };
             result.push(family_name);
@@ -1156,15 +1148,9 @@ fn get_font_identifier_and_font_struct(
     font_face: &IDWriteFontFace3,
     locale: &str,
 ) -> Option<(FontIdentifier, Font, bool)> {
-    let Some(postscript_name) = get_postscript_name(font_face, locale) else {
-        return None;
-    };
-    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 postscript_name = get_postscript_name(font_face, locale).log_err()?;
+    let localized_family_name = unsafe { font_face.GetFamilyNames().log_err() }?;
+    let family_name = get_name(localized_family_name, locale).log_err()?;
     let weight = unsafe { font_face.GetWeight() };
     let style = unsafe { font_face.GetStyle() };
     let identifier = FontIdentifier {
@@ -1186,28 +1172,28 @@ fn get_font_identifier_and_font_struct(
 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,
-    })
+    get_postscript_name(font_face, locale)
+        .log_err()
+        .map(|postscript_name| FontIdentifier {
+            postscript_name,
+            weight,
+            style,
+        })
 }
 
 #[inline]
-fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Option<String> {
+fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Result<String> {
     let mut info = None;
     let mut exists = BOOL(0);
     unsafe {
-        font_face
-            .GetInformationalStrings(
-                DWRITE_INFORMATIONAL_STRING_POSTSCRIPT_NAME,
-                &mut info,
-                &mut exists,
-            )
-            .log_err();
-    }
+        font_face.GetInformationalStrings(
+            DWRITE_INFORMATIONAL_STRING_POSTSCRIPT_NAME,
+            &mut info,
+            &mut exists,
+        )?
+    };
     if !exists.as_bool() || info.is_none() {
-        return None;
+        return Err(anyhow!("No postscript name found for font face"));
     }
 
     get_name(info.unwrap(), locale)
@@ -1281,40 +1267,36 @@ fn make_direct_write_tag(tag_name: &str) -> DWRITE_FONT_FEATURE_TAG {
 }
 
 #[inline]
-fn get_name(string: IDWriteLocalizedStrings, locale: &str) -> Option<String> {
+fn get_name(string: IDWriteLocalizedStrings, locale: &str) -> Result<String> {
     let mut locale_name_index = 0u32;
     let mut exists = BOOL(0);
     unsafe {
-        string
-            .FindLocaleName(
-                &HSTRING::from(locale),
-                &mut locale_name_index,
-                &mut exists as _,
-            )
-            .log_err();
-    }
+        string.FindLocaleName(
+            &HSTRING::from(locale),
+            &mut locale_name_index,
+            &mut exists as _,
+        )?
+    };
     if !exists.as_bool() {
         unsafe {
-            string
-                .FindLocaleName(
-                    DEFAULT_LOCALE_NAME,
-                    &mut locale_name_index as _,
-                    &mut exists as _,
-                )
-                .log_err();
-        }
+            string.FindLocaleName(
+                DEFAULT_LOCALE_NAME,
+                &mut locale_name_index as _,
+                &mut exists as _,
+            )?
+        };
         if !exists.as_bool() {
-            return None;
+            return Err(anyhow!("No localised string for {}", locale));
         }
     }
 
-    let name_length = unsafe { string.GetStringLength(locale_name_index).unwrap() } as usize;
+    let name_length = unsafe { string.GetStringLength(locale_name_index) }? as usize;
     let mut name_vec = vec![0u16; name_length + 1];
     unsafe {
-        string.GetString(locale_name_index, &mut name_vec).unwrap();
+        string.GetString(locale_name_index, &mut name_vec)?;
     }
 
-    Some(String::from_utf16_lossy(&name_vec[..name_length]))
+    Ok(String::from_utf16_lossy(&name_vec[..name_length]))
 }
 
 #[inline]

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

@@ -162,16 +162,11 @@ impl Platform for WindowsPlatform {
 
     fn run(&self, on_finish_launching: Box<dyn 'static + FnOnce()>) {
         on_finish_launching();
-        let vsync_event = create_event().unwrap();
-        begin_vsync(vsync_event.to_raw());
+        let vsync_event = unsafe { Owned::new(CreateEventW(None, false, false, None).unwrap()) };
+        begin_vsync(*vsync_event);
         'a: loop {
             let wait_result = unsafe {
-                MsgWaitForMultipleObjects(
-                    Some(&[vsync_event.to_raw()]),
-                    false,
-                    INFINITE,
-                    QS_ALLINPUT,
-                )
+                MsgWaitForMultipleObjects(Some(&[*vsync_event]), false, INFINITE, QS_ALLINPUT)
             };
 
             match wait_result {

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

@@ -1,7 +1,7 @@
 use std::sync::OnceLock;
 
 use ::util::ResultExt;
-use windows::Win32::{Foundation::*, System::Threading::*, UI::WindowsAndMessaging::*};
+use windows::Win32::{Foundation::*, UI::WindowsAndMessaging::*};
 
 use crate::*;
 
@@ -74,34 +74,6 @@ pub(crate) unsafe fn set_window_long(
     }
 }
 
-#[derive(Debug, Clone)]
-pub(crate) struct OwnedHandle(HANDLE);
-
-impl OwnedHandle {
-    pub(crate) fn new(handle: HANDLE) -> Self {
-        Self(handle)
-    }
-
-    #[inline(always)]
-    pub(crate) fn to_raw(&self) -> HANDLE {
-        self.0
-    }
-}
-
-impl Drop for OwnedHandle {
-    fn drop(&mut self) {
-        if !self.0.is_invalid() {
-            unsafe { CloseHandle(self.0) }.log_err();
-        }
-    }
-}
-
-pub(crate) fn create_event() -> windows::core::Result<OwnedHandle> {
-    Ok(OwnedHandle::new(unsafe {
-        CreateEventW(None, false, false, None)?
-    }))
-}
-
 pub(crate) fn windows_credentials_target_name(url: &str) -> String {
     format!("zed:url={}", url)
 }