windows: Remove Send and Sync implementation of DirectWrite (#15263)

This PR uses the `AgileReference` provided by the `windows-rs` crate to
correctly implement `Send` and `Sync` for `DirectWrite`.

Release Notes:

- N/A
This commit is contained in:
张小白 2024-07-27 07:46:06 +08:00 committed by GitHub
parent a1bd7a1297
commit e6cd1cf22b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 59 additions and 29 deletions

View file

@ -13,6 +13,11 @@ use std::{
rc::{Rc, Weak}, rc::{Rc, Weak},
sync::Arc, sync::Arc,
}; };
#[cfg(target_os = "windows")]
use windows::Win32::{
Graphics::Imaging::{CLSID_WICImagingFactory, IWICImagingFactory},
System::Com::{CoCreateInstance, CLSCTX_INPROC_SERVER},
};
/// TestPlatform implements the Platform trait for use in tests. /// TestPlatform implements the Platform trait for use in tests.
pub(crate) struct TestPlatform { pub(crate) struct TestPlatform {
@ -28,6 +33,8 @@ pub(crate) struct TestPlatform {
pub(crate) prompts: RefCell<TestPrompts>, pub(crate) prompts: RefCell<TestPrompts>,
pub opened_url: RefCell<Option<String>>, pub opened_url: RefCell<Option<String>>,
pub text_system: Arc<dyn PlatformTextSystem>, pub text_system: Arc<dyn PlatformTextSystem>,
#[cfg(target_os = "windows")]
bitmap_factory: std::mem::ManuallyDrop<IWICImagingFactory>,
weak: Weak<Self>, weak: Weak<Self>,
} }
@ -40,10 +47,14 @@ pub(crate) struct TestPrompts {
impl TestPlatform { impl TestPlatform {
pub fn new(executor: BackgroundExecutor, foreground_executor: ForegroundExecutor) -> Rc<Self> { pub fn new(executor: BackgroundExecutor, foreground_executor: ForegroundExecutor) -> Rc<Self> {
#[cfg(target_os = "windows")] #[cfg(target_os = "windows")]
unsafe { let bitmap_factory = unsafe {
windows::Win32::System::Ole::OleInitialize(None) windows::Win32::System::Ole::OleInitialize(None)
.expect("unable to initialize Windows OLE"); .expect("unable to initialize Windows OLE");
} std::mem::ManuallyDrop::new(
CoCreateInstance(&CLSID_WICImagingFactory, None, CLSCTX_INPROC_SERVER)
.expect("Error creating bitmap factory."),
)
};
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
let text_system = Arc::new(crate::platform::mac::MacTextSystem::new()); let text_system = Arc::new(crate::platform::mac::MacTextSystem::new());
@ -52,7 +63,10 @@ impl TestPlatform {
let text_system = Arc::new(crate::platform::linux::CosmicTextSystem::new()); let text_system = Arc::new(crate::platform::linux::CosmicTextSystem::new());
#[cfg(target_os = "windows")] #[cfg(target_os = "windows")]
let text_system = Arc::new(crate::platform::windows::DirectWriteTextSystem::new().unwrap()); let text_system = Arc::new(
crate::platform::windows::DirectWriteTextSystem::new(&bitmap_factory)
.expect("Unable to initialize direct write."),
);
Rc::new_cyclic(|weak| TestPlatform { Rc::new_cyclic(|weak| TestPlatform {
background_executor: executor, background_executor: executor,
@ -66,6 +80,8 @@ impl TestPlatform {
current_primary_item: Mutex::new(None), current_primary_item: Mutex::new(None),
weak: weak.clone(), weak: weak.clone(),
opened_url: Default::default(), opened_url: Default::default(),
#[cfg(target_os = "windows")]
bitmap_factory,
text_system, text_system,
}) })
} }
@ -303,3 +319,13 @@ impl Platform for TestPlatform {
unimplemented!() unimplemented!()
} }
} }
#[cfg(target_os = "windows")]
impl Drop for TestPlatform {
fn drop(&mut self) {
unsafe {
std::mem::ManuallyDrop::drop(&mut self.bitmap_factory);
windows::Win32::System::Ole::OleUninitialize();
}
}
}

View file

@ -1,4 +1,4 @@
use std::{borrow::Cow, mem::ManuallyDrop, sync::Arc}; use std::{borrow::Cow, sync::Arc};
use ::util::ResultExt; use ::util::ResultExt;
use anyhow::{anyhow, Result}; use anyhow::{anyhow, Result};
@ -16,9 +16,9 @@ use windows::{
DirectWrite::*, DirectWrite::*,
Dxgi::Common::*, Dxgi::Common::*,
Gdi::LOGFONTW, Gdi::LOGFONTW,
Imaging::{D2D::IWICImagingFactory2, *}, Imaging::*,
}, },
System::{Com::*, SystemServices::LOCALE_NAME_MAX_LENGTH}, System::SystemServices::LOCALE_NAME_MAX_LENGTH,
UI::WindowsAndMessaging::*, UI::WindowsAndMessaging::*,
}, },
}; };
@ -40,7 +40,7 @@ pub(crate) struct DirectWriteTextSystem(RwLock<DirectWriteState>);
struct DirectWriteComponent { struct DirectWriteComponent {
locale: String, locale: String,
factory: IDWriteFactory5, factory: IDWriteFactory5,
bitmap_factory: ManuallyDrop<IWICImagingFactory2>, bitmap_factory: AgileReference<IWICImagingFactory>,
d2d1_factory: ID2D1Factory, d2d1_factory: ID2D1Factory,
in_memory_loader: IDWriteInMemoryFontFileLoader, in_memory_loader: IDWriteInMemoryFontFileLoader,
builder: IDWriteFontSetBuilder1, builder: IDWriteFontSetBuilder1,
@ -53,10 +53,6 @@ struct GlyphRenderContext {
dc_target: ID2D1DeviceContext4, dc_target: ID2D1DeviceContext4,
} }
// All use of the IUnknown methods should be "thread-safe".
unsafe impl Sync for DirectWriteComponent {}
unsafe impl Send for DirectWriteComponent {}
struct DirectWriteState { struct DirectWriteState {
components: DirectWriteComponent, components: DirectWriteComponent,
system_ui_font_name: SharedString, system_ui_font_name: SharedString,
@ -75,12 +71,10 @@ struct FontIdentifier {
} }
impl DirectWriteComponent { impl DirectWriteComponent {
pub fn new() -> Result<Self> { pub fn new(bitmap_factory: &IWICImagingFactory) -> Result<Self> {
unsafe { unsafe {
let factory: IDWriteFactory5 = DWriteCreateFactory(DWRITE_FACTORY_TYPE_SHARED)?; let factory: IDWriteFactory5 = DWriteCreateFactory(DWRITE_FACTORY_TYPE_SHARED)?;
let bitmap_factory: IWICImagingFactory2 = let bitmap_factory = AgileReference::new(bitmap_factory)?;
CoCreateInstance(&CLSID_WICImagingFactory2, None, CLSCTX_INPROC_SERVER)?;
let bitmap_factory = ManuallyDrop::new(bitmap_factory);
let d2d1_factory: ID2D1Factory = let d2d1_factory: ID2D1Factory =
D2D1CreateFactory(D2D1_FACTORY_TYPE_MULTI_THREADED, None)?; D2D1CreateFactory(D2D1_FACTORY_TYPE_MULTI_THREADED, None)?;
// The `IDWriteInMemoryFontFileLoader` here is supported starting from // The `IDWriteInMemoryFontFileLoader` here is supported starting from
@ -145,8 +139,8 @@ impl GlyphRenderContext {
} }
impl DirectWriteTextSystem { impl DirectWriteTextSystem {
pub(crate) fn new() -> Result<Self> { pub(crate) fn new(bitmap_factory: &IWICImagingFactory) -> Result<Self> {
let components = DirectWriteComponent::new()?; let components = DirectWriteComponent::new(bitmap_factory)?;
let system_font_collection = unsafe { let system_font_collection = unsafe {
let mut result = std::mem::zeroed(); let mut result = std::mem::zeroed();
components components
@ -172,11 +166,6 @@ impl DirectWriteTextSystem {
font_id_by_identifier: HashMap::default(), font_id_by_identifier: HashMap::default(),
}))) })))
} }
pub(crate) fn destroy(&self) {
let mut lock = self.0.write();
unsafe { ManuallyDrop::drop(&mut lock.components.bitmap_factory) };
}
} }
impl PlatformTextSystem for DirectWriteTextSystem { impl PlatformTextSystem for DirectWriteTextSystem {
@ -798,8 +787,9 @@ impl DirectWriteState {
bitmap_dpi = 192.0; bitmap_dpi = 192.0;
} }
let bitmap_factory = self.components.bitmap_factory.resolve()?;
unsafe { unsafe {
let bitmap = self.components.bitmap_factory.CreateBitmap( let bitmap = bitmap_factory.CreateBitmap(
bitmap_width, bitmap_width,
bitmap_height, bitmap_height,
bitmap_format, bitmap_format,
@ -901,7 +891,7 @@ impl DirectWriteState {
pixel[2] = (pixel[2] as f32 / a) as u8; pixel[2] = (pixel[2] as f32 / a) as u8;
} }
} else { } else {
let scaler = self.components.bitmap_factory.CreateBitmapScaler()?; let scaler = bitmap_factory.CreateBitmapScaler()?;
scaler.Initialize( scaler.Initialize(
&bitmap, &bitmap,
bitmap_size.width.0 as u32, bitmap_size.width.0 as u32,

View file

@ -4,6 +4,7 @@
use std::{ use std::{
cell::{Cell, RefCell}, cell::{Cell, RefCell},
ffi::{c_void, OsString}, ffi::{c_void, OsString},
mem::ManuallyDrop,
os::windows::ffi::{OsStrExt, OsStringExt}, os::windows::ffi::{OsStrExt, OsStringExt},
path::{Path, PathBuf}, path::{Path, PathBuf},
rc::Rc, rc::Rc,
@ -21,7 +22,10 @@ use windows::{
Win32::{ Win32::{
Foundation::*, Foundation::*,
Globalization::u_memcpy, Globalization::u_memcpy,
Graphics::Gdi::*, Graphics::{
Gdi::*,
Imaging::{CLSID_WICImagingFactory, IWICImagingFactory},
},
Security::Credentials::*, Security::Credentials::*,
System::{ System::{
Com::*, Com::*,
@ -53,6 +57,7 @@ pub(crate) struct WindowsPlatform {
clipboard_hash_format: u32, clipboard_hash_format: u32,
clipboard_metadata_format: u32, clipboard_metadata_format: u32,
windows_version: WindowsVersion, windows_version: WindowsVersion,
bitmap_factory: ManuallyDrop<IWICImagingFactory>,
} }
pub(crate) struct WindowsPlatformState { pub(crate) struct WindowsPlatformState {
@ -91,8 +96,14 @@ impl WindowsPlatform {
let dispatcher = Arc::new(WindowsDispatcher::new()); let dispatcher = Arc::new(WindowsDispatcher::new());
let background_executor = BackgroundExecutor::new(dispatcher.clone()); let background_executor = BackgroundExecutor::new(dispatcher.clone());
let foreground_executor = ForegroundExecutor::new(dispatcher); let foreground_executor = ForegroundExecutor::new(dispatcher);
let text_system = let bitmap_factory = ManuallyDrop::new(unsafe {
Arc::new(DirectWriteTextSystem::new().expect("Error creating DirectWriteTextSystem")); CoCreateInstance(&CLSID_WICImagingFactory, None, CLSCTX_INPROC_SERVER)
.expect("Error creating bitmap factory.")
});
let text_system = Arc::new(
DirectWriteTextSystem::new(&bitmap_factory)
.expect("Error creating DirectWriteTextSystem"),
);
let icon = load_icon().unwrap_or_default(); let icon = load_icon().unwrap_or_default();
let state = RefCell::new(WindowsPlatformState::new()); let state = RefCell::new(WindowsPlatformState::new());
let raw_window_handles = RwLock::new(SmallVec::new()); let raw_window_handles = RwLock::new(SmallVec::new());
@ -111,6 +122,7 @@ impl WindowsPlatform {
clipboard_hash_format, clipboard_hash_format,
clipboard_metadata_format, clipboard_metadata_format,
windows_version, windows_version,
bitmap_factory,
} }
} }
@ -584,8 +596,10 @@ impl Platform for WindowsPlatform {
impl Drop for WindowsPlatform { impl Drop for WindowsPlatform {
fn drop(&mut self) { fn drop(&mut self) {
self.text_system.destroy(); unsafe {
unsafe { OleUninitialize() }; ManuallyDrop::drop(&mut self.bitmap_factory);
OleUninitialize();
}
} }
} }