From 4cb45e63f40ac21bd3f933d8cc2d3b98a97b089e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=B0=8F=E7=99=BD?= <364772080@qq.com> Date: Sat, 15 Jun 2024 01:12:20 +0800 Subject: [PATCH] windows: Update `windows-rs` crate and better error handling in `DirectWrite` (#12818) - 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 --- Cargo.lock | 34 +-- Cargo.toml | 2 +- crates/gpui/Cargo.toml | 2 +- .../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(-) diff --git a/Cargo.lock b/Cargo.lock index ed9a82fa75..e49abe9f50 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index 56bae1d7b3..4d61614f33 100644 --- a/Cargo.toml +++ b/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", diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index ee72f08302..ebaace06c0 100644 --- a/crates/gpui/Cargo.toml +++ b/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" diff --git a/crates/gpui/src/platform/windows/direct_write.rs b/crates/gpui/src/platform/windows/direct_write.rs index 615a0e717f..f3823d9357 100644 --- a/crates/gpui/src/platform/windows/direct_write.rs +++ b/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 { 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 { 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 { +fn get_postscript_name(font_face: &IDWriteFontFace3, locale: &str) -> Result { 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 { +fn get_name(string: IDWriteLocalizedStrings, locale: &str) -> Result { 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] diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index 17498305f8..57b85c5ca5 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -162,16 +162,11 @@ impl Platform for WindowsPlatform { fn run(&self, on_finish_launching: Box) { 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 { diff --git a/crates/gpui/src/platform/windows/util.rs b/crates/gpui/src/platform/windows/util.rs index a9c0c00c3e..2c8ccf5e08 100644 --- a/crates/gpui/src/platform/windows/util.rs +++ b/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 { - Ok(OwnedHandle::new(unsafe { - CreateEventW(None, false, false, None)? - })) -} - pub(crate) fn windows_credentials_target_name(url: &str) -> String { format!("zed:url={}", url) }