Win32: Various internal implementation improvements

src/platform/win/drop_target.rs

@@ -10,15 +10,15 @@ use windows::Win32::System::Com::{IDataObject, DVASPECT_CONTENT, FORMATETC, TYME
 use windows::Win32::System::Ole::*;
 use windows::Win32::System::SystemServices::MODIFIERKEYS_FLAGS;
 use windows_core::Ref;
-use windows_sys::Win32::{
-    Foundation::POINT, Graphics::Gdi::ScreenToClient, UI::Shell::DragQueryFileW,
-};
+use windows_sys::Win32::UI::Shell::DragQueryFileW;
 
 use super::window_state::WindowState;
+use crate::wrappers::win32::window::HWnd;
 use crate::{DropData, DropEffect, Event, EventStatus, MouseEvent};
 
 #[implement(IDropTarget)]
 pub(crate) struct DropTarget {
+    hwnd: HWnd,
     window_state: Weak<WindowState>,
 
     // These are cached since DragOver and DragLeave callbacks don't provide them,
@@ -28,8 +28,9 @@ pub(crate) struct DropTarget {
 }
 
 impl DropTarget {
-    pub(crate) fn new(window_state: Weak<WindowState>) -> Self {
+    pub(crate) fn new(window_state: Weak<WindowState>, hwnd: HWnd) -> Self {
         Self {
+            hwnd,
             window_state,
             drag_position: Cell::new(PhysicalPosition::new(0, 0)),
             drop_data: RefCell::new(DropData::None),
@@ -42,29 +43,27 @@ impl DropTarget {
             return;
         };
 
-        unsafe {
-            let event = Event::Mouse(event);
-            let event_status = window_state.handle_event(event);
-
-            if let Some(pdwEffect) = pdwEffect {
-                match event_status {
-                    EventStatus::AcceptDrop(DropEffect::Copy) => *pdwEffect = DROPEFFECT_COPY,
-                    EventStatus::AcceptDrop(DropEffect::Move) => *pdwEffect = DROPEFFECT_MOVE,
-                    EventStatus::AcceptDrop(DropEffect::Link) => *pdwEffect = DROPEFFECT_LINK,
-                    EventStatus::AcceptDrop(DropEffect::Scroll) => *pdwEffect = DROPEFFECT_SCROLL,
-                    _ => *pdwEffect = DROPEFFECT_NONE,
-                }
-            }
+        let event = Event::Mouse(event);
+        let event_status = window_state.handle_event(event);
+
+        let effect = match event_status {
+            EventStatus::AcceptDrop(DropEffect::Copy) => DROPEFFECT_COPY,
+            EventStatus::AcceptDrop(DropEffect::Move) => DROPEFFECT_MOVE,
+            EventStatus::AcceptDrop(DropEffect::Link) => DROPEFFECT_LINK,
+            EventStatus::AcceptDrop(DropEffect::Scroll) => DROPEFFECT_SCROLL,
+            _ => DROPEFFECT_NONE,
+        };
+
+        if let Some(pdwEffect) = pdwEffect {
+            unsafe { pdwEffect.write(effect) };
         }
     }
 
     fn parse_coordinates(&self, pt: POINTL) {
-        let Some(window_state) = self.window_state.upgrade() else {
+        let Ok(phy_point) = self.hwnd.screen_to_client(PhysicalPosition::new(pt.x, pt.y)) else {
             return;
         };
-        let mut pt = POINT { x: pt.x, y: pt.y };
-        unsafe { ScreenToClient(window_state.hwnd, &mut pt as *mut POINT) };
-        let phy_point = PhysicalPosition::new(pt.x, pt.y);
+
         self.drag_position.set(phy_point);
     }
 

src/platform/win/window.rs

@@ -52,15 +52,15 @@ const WIN_FRAME_TIMER: NonZeroUsize = match NonZeroUsize::new(4242) {
 };
 
 pub struct WindowHandle {
-    hwnd: Cell<Option<HWND>>,
+    hwnd: Cell<Option<HWnd>>,
     is_open: Rc<Cell<bool>>,
 }
 
 impl WindowHandle {
     pub fn close(&self) {
         if let Some(hwnd) = self.hwnd.take() {
             unsafe {
-                PostMessageW(hwnd, BV_WINDOW_MUST_CLOSE, 0, 0);
+                PostMessageW(hwnd.as_raw(), BV_WINDOW_MUST_CLOSE, 0, 0);
             }
         }
     }
@@ -127,7 +127,7 @@ impl WindowImpl for BaseviewWindow {
             }
         }
 
-        let drop_target = ComObject::new(DropTarget::new(Rc::downgrade(window_state)));
+        let drop_target = ComObject::new(DropTarget::new(Rc::downgrade(window_state), window));
         self._drop_target.set(Some(drop_target.clone()));
 
         ole_initialize()?;
@@ -457,12 +457,8 @@ impl Window {
             let extended_user_32 = extended_user_32.clone();
 
             move |hwnd: HWnd| {
-                let window_state = Rc::new(WindowState::new(
-                    hwnd.as_raw(),
-                    window_size,
-                    options.scale,
-                    extended_user_32,
-                ));
+                let window_state =
+                    Rc::new(WindowState::new(hwnd, window_size, options.scale, extended_user_32));
 
                 BaseviewWindow {
                     window_state,
@@ -479,13 +475,10 @@ impl Window {
             }
         };
 
-        let hwnd =
+        let window =
             create_window(&title, style, rect.size(), parent as *mut _, &dpi_ctx, initializer)
                 .unwrap();
 
-        // SAFETY: this handle should be safe to use
-        let window = unsafe { HWnd::from_raw(hwnd) };
-
         // FIXME: this SetTimer call could be in after_create, but for some reason it changes the ordering
         // for a parent+child window situation, which results in the parent drawing over the child.
         // This timer should be replaced by proper window redrawing/damage/vsync handling, but this
@@ -495,7 +488,7 @@ impl Window {
 
         window.show_and_activate();
 
-        WindowHandle { hwnd: Some(hwnd).into(), is_open: Rc::clone(&is_open) }
+        WindowHandle { hwnd: Some(window).into(), is_open: Rc::clone(&is_open) }
     }
 }
 

src/platform/win/window_state.rs

@@ -9,13 +9,12 @@ use dpi::{PhysicalSize, Size};
 use raw_window_handle::{DisplayHandle, Win32WindowHandle};
 use std::cell::{Cell, OnceCell, Ref, RefCell};
 use std::num::NonZeroIsize;
-use windows_sys::Win32::Foundation::HWND;
 use windows_sys::Win32::UI::WindowsAndMessaging::PostMessageW;
 
 /// All data associated with the window.
 pub(crate) struct WindowState {
     /// The HWND belonging to this window.
-    pub hwnd: HWND,
+    pub hwnd: HWnd,
     pub current_size: Cell<PhysicalSize<u32>>,
     pub current_dpi: Cell<Dpi>, // None if in non-system scale policy
     pub keyboard_state: RefCell<KeyboardState>,
@@ -34,7 +33,7 @@ pub(crate) struct WindowState {
 
 impl WindowState {
     pub fn new(
-        hwnd: HWND, current_size: PhysicalSize<u32>, scale_policy: WindowScalePolicy,
+        hwnd: HWnd, current_size: PhysicalSize<u32>, scale_policy: WindowScalePolicy,
         user32: ExtendedUser32,
     ) -> Self {
         Self {
@@ -92,21 +91,21 @@ impl WindowState {
 
     pub fn request_close(&self) {
         unsafe {
-            PostMessageW(self.hwnd, crate::platform::win::window::BV_WINDOW_MUST_CLOSE, 0, 0);
+            PostMessageW(
+                self.hwnd.as_raw(),
+                crate::platform::win::window::BV_WINDOW_MUST_CLOSE,
+                0,
+                0,
+            );
         }
     }
 
     pub fn has_focus(&self) -> bool {
-        HWnd::get_focused_window() == self.hwnd
+        HWnd::get_focused_window() == self.hwnd.as_raw()
     }
 
     pub fn focus(&self) {
-        self.hwnd().set_focus().unwrap()
-    }
-
-    fn hwnd(&self) -> HWnd {
-        // SAFETY: this handle should be safe to use
-        unsafe { HWnd::from_raw(self.hwnd) }
+        self.hwnd.set_focus().unwrap()
     }
 
     pub fn resize(&self, size: Size) {
@@ -115,7 +114,7 @@ impl WindowState {
         let dpi = self.current_dpi.get();
         let new_size = size.to_physical(dpi.scale_factor());
 
-        self.hwnd().resize_and_activate(new_size, dpi, &self.user32).unwrap();
+        self.hwnd.resize_and_activate(new_size, dpi, &self.user32).unwrap();
     }
 
     pub fn set_mouse_cursor(&self, mouse_cursor: MouseCursor) {
@@ -132,7 +131,7 @@ impl WindowState {
     }
 
     pub fn window_handle(&self) -> Option<raw_window_handle::WindowHandle<'_>> {
-        let Some(hwnd) = NonZeroIsize::new(self.hwnd as _) else { unreachable!() };
+        let Some(hwnd) = NonZeroIsize::new(self.hwnd.as_raw() as _) else { unreachable!() };
         let mut handle = Win32WindowHandle::new(hwnd);
         handle.hinstance = Some(HInstance::get_from_dll().addr());
 
@@ -144,7 +143,7 @@ impl WindowState {
     }
 
     pub fn platform_handle(&self) -> PlatformHandle {
-        let Some(hwnd) = NonZeroIsize::new(self.hwnd as _) else { unreachable!() };
+        let Some(hwnd) = NonZeroIsize::new(self.hwnd.as_raw() as _) else { unreachable!() };
         PlatformHandle { hwnd }
     }
 }

src/wrappers/win32/window.rs

@@ -7,7 +7,7 @@ use data::WindowData;
 use dpi::PhysicalSize;
 pub use handle::HWnd;
 pub use proc::wnd_proc;
-use std::ptr::null_mut;
+use std::ptr::{null_mut, NonNull};
 use std::rc::Rc;
 use window_class::RegisteredClass;
 use windows_core::{Error, Result, HSTRING};
@@ -53,7 +53,7 @@ pub trait WindowImpl: 'static {
 pub fn create_window<W: WindowImpl>(
     title: &HSTRING, style: WindowStyle, nc_size: PhysicalSize<u32>, parent: HWND,
     _dpi_ctx: &DpiAwarenessContext, initializer: impl FnOnce(HWnd) -> W + 'static,
-) -> Result<HWND> {
+) -> Result<HWnd> {
     let instance = HInstance::get_from_dll();
     let window_class = RegisteredClass::register_new(instance, Some(wnd_proc::<W>))?;
 
@@ -76,9 +76,9 @@ pub fn create_window<W: WindowImpl>(
         )
     };
 
-    if hwnd.is_null() {
-        return Err(Error::from_thread());
-    }
+    let Some(hwnd) = NonNull::new(hwnd) else { return Err(Error::from_thread()) };
+    // SAFETY: This Hwnd is valid since it came from CreateWindowExW
+    let hwnd = unsafe { HWnd::from_raw(hwnd) };
 
     Ok(hwnd)
 }