From 4a8b659228e3d512d664d6e400058125064d6489 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 3 Dec 2024 19:02:53 +0100 Subject: [PATCH] Fix MonitorHandle PartialEq and Hash on iOS (#4013) --- src/changelog/unreleased.md | 1 + src/platform_impl/apple/uikit/monitor.rs | 39 ++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 3034746ca3..9eefbbd3d6 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -213,3 +213,4 @@ changelog entry. - On macOS, fixed the scancode conversion for `IntlBackslash`. - On macOS, fixed redundant `SurfaceResized` event at window creation. - On macOS, fix crash when pressing Caps Lock in certain configurations. +- On iOS, fixed `MonitorHandle`'s `PartialEq` and `Hash` implementations. diff --git a/src/platform_impl/apple/uikit/monitor.rs b/src/platform_impl/apple/uikit/monitor.rs index b87d5a26ce..7973ed5669 100644 --- a/src/platform_impl/apple/uikit/monitor.rs +++ b/src/platform_impl/apple/uikit/monitor.rs @@ -101,13 +101,20 @@ impl Clone for MonitorHandle { impl hash::Hash for MonitorHandle { fn hash(&self, state: &mut H) { - (self as *const Self).hash(state); + // SAFETY: Only getting the pointer. + let mtm = unsafe { MainThreadMarker::new_unchecked() }; + Retained::as_ptr(self.ui_screen.get(mtm)).hash(state); } } impl PartialEq for MonitorHandle { fn eq(&self, other: &Self) -> bool { - ptr::eq(self, other) + // SAFETY: Only getting the pointer. + let mtm = unsafe { MainThreadMarker::new_unchecked() }; + ptr::eq( + Retained::as_ptr(self.ui_screen.get(mtm)), + Retained::as_ptr(other.ui_screen.get(mtm)), + ) } } @@ -121,8 +128,10 @@ impl PartialOrd for MonitorHandle { impl Ord for MonitorHandle { fn cmp(&self, other: &Self) -> std::cmp::Ordering { + // SAFETY: Only getting the pointer. // TODO: Make a better ordering - (self as *const Self).cmp(&(other as *const Self)) + let mtm = unsafe { MainThreadMarker::new_unchecked() }; + Retained::as_ptr(self.ui_screen.get(mtm)).cmp(&Retained::as_ptr(other.ui_screen.get(mtm))) } } @@ -240,3 +249,27 @@ pub fn uiscreens(mtm: MainThreadMarker) -> VecDeque { #[allow(deprecated)] UIScreen::screens(mtm).into_iter().map(MonitorHandle::new).collect() } + +#[cfg(test)] +mod tests { + use objc2_foundation::NSSet; + + use super::*; + + // Test that UIScreen pointer comparisons are correct. + #[test] + #[allow(deprecated)] + fn screen_comparisons() { + // Test code, doesn't matter that it's not thread safe + let mtm = unsafe { MainThreadMarker::new_unchecked() }; + + assert!(ptr::eq(&*UIScreen::mainScreen(mtm), &*UIScreen::mainScreen(mtm))); + + let main = UIScreen::mainScreen(mtm); + assert!(UIScreen::screens(mtm).iter().any(|screen| ptr::eq(screen, &*main))); + + assert!(unsafe { + NSSet::setWithArray(&UIScreen::screens(mtm)).containsObject(&UIScreen::mainScreen(mtm)) + }); + } +}