From a82f66826bf50ce5a0022b41a369eda42e961332 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 23 Jan 2023 00:01:45 +0100 Subject: [PATCH] Use a bit less `unsafe` on iOS (#2643) * Use a bit less `unsafe` on iOS I did test this in XCode 11.3's "Debug View Heirarchy", the NSStringRust problem is no longer applicable (likely because Rust got better at emitting correct debug info). * Avoid using `id` on iOS --- src/platform/ios.rs | 6 +-- src/platform_impl/ios/app_state.rs | 24 ++++----- src/platform_impl/ios/event_loop.rs | 15 ++---- src/platform_impl/ios/ffi.rs | 50 ------------------- src/platform_impl/ios/uikit/mod.rs | 14 ++++++ .../ios/uikit/view_controller.rs | 7 ++- src/platform_impl/ios/view.rs | 4 +- src/platform_impl/ios/window.rs | 31 ++++-------- 8 files changed, 51 insertions(+), 100 deletions(-) diff --git a/src/platform/ios.rs b/src/platform/ios.rs index 9b3159f9..3fa92c79 100644 --- a/src/platform/ios.rs +++ b/src/platform/ios.rs @@ -99,17 +99,17 @@ pub trait WindowExtIOS { impl WindowExtIOS for Window { #[inline] fn ui_window(&self) -> *mut c_void { - self.window.ui_window() as _ + self.window.ui_window() } #[inline] fn ui_view_controller(&self) -> *mut c_void { - self.window.ui_view_controller() as _ + self.window.ui_view_controller() } #[inline] fn ui_view(&self) -> *mut c_void { - self.window.ui_view() as _ + self.window.ui_view() } #[inline] diff --git a/src/platform_impl/ios/app_state.rs b/src/platform_impl/ios/app_state.rs index 4d957d6b..8743a7f2 100644 --- a/src/platform_impl/ios/app_state.rs +++ b/src/platform_impl/ios/app_state.rs @@ -15,11 +15,13 @@ use core_foundation::runloop::{ kCFRunLoopCommonModes, CFRunLoopAddTimer, CFRunLoopGetMain, CFRunLoopRef, CFRunLoopTimerCreate, CFRunLoopTimerInvalidate, CFRunLoopTimerRef, CFRunLoopTimerSetNextFireDate, }; -use objc2::foundation::{CGRect, CGSize, NSInteger}; +use objc2::foundation::{CGRect, CGSize, NSInteger, NSProcessInfo}; use objc2::rc::{Id, Shared}; -use objc2::{class, msg_send, sel}; +use objc2::runtime::Object; +use objc2::{msg_send, sel}; use once_cell::sync::Lazy; +use super::uikit::UIView; use super::view::WinitUIWindow; use crate::{ dpi::LogicalSize, @@ -27,7 +29,7 @@ use crate::{ event_loop::ControlFlow, platform_impl::platform::{ event_loop::{EventHandler, EventProxy, EventWrapper, Never}, - ffi::{id, NSOperatingSystemVersion}, + ffi::NSOperatingSystemVersion, }, window::WindowId as RootWindowId, }; @@ -542,7 +544,7 @@ pub unsafe fn did_finish_launching() { // completed. This may result in incorrect visual appearance. // ``` let screen = window.screen(); - let _: () = msg_send![&window, setScreen: 0 as id]; + let _: () = msg_send![&window, setScreen: ptr::null::()]; window.setScreen(&screen); let controller = window.rootViewController(); @@ -830,14 +832,12 @@ fn handle_hidpi_proxy( let logical_size = physical_size.to_logical(scale_factor); let size = CGSize::new(logical_size.width, logical_size.height); let new_frame: CGRect = CGRect::new(screen_frame.origin, size); - unsafe { - let _: () = msg_send![view, setFrame: new_frame]; - } + view.setFrame(new_frame); } -fn get_view_and_screen_frame(window: &WinitUIWindow) -> (id, CGRect) { +fn get_view_and_screen_frame(window: &WinitUIWindow) -> (Id, CGRect) { let view_controller = window.rootViewController().unwrap(); - let view: id = unsafe { msg_send![&view_controller, view] }; + let view = view_controller.view().unwrap(); let bounds = window.bounds(); let screen = window.screen(); let screen_space = screen.coordinateSpace(); @@ -971,9 +971,9 @@ impl NSOperatingSystemVersion { pub fn os_capabilities() -> OSCapabilities { static OS_CAPABILITIES: Lazy = Lazy::new(|| { let version: NSOperatingSystemVersion = unsafe { - let process_info: id = msg_send![class!(NSProcessInfo), processInfo]; + let process_info = NSProcessInfo::process_info(); let atleast_ios_8: bool = msg_send![ - process_info, + &process_info, respondsToSelector: sel!(operatingSystemVersion) ]; // winit requires atleast iOS 8 because no one has put the time into supporting earlier os versions. @@ -984,7 +984,7 @@ pub fn os_capabilities() -> OSCapabilities { // // The minimum required iOS version is likely to grow in the future. assert!(atleast_ios_8, "`winit` requires iOS version 8 or greater"); - msg_send![process_info, operatingSystemVersion] + msg_send![&process_info, operatingSystemVersion] }; version.into() }); diff --git a/src/platform_impl/ios/event_loop.rs b/src/platform_impl/ios/event_loop.rs index 5949bedc..8649c8c7 100644 --- a/src/platform_impl/ios/event_loop.rs +++ b/src/platform_impl/ios/event_loop.rs @@ -14,13 +14,14 @@ use core_foundation::runloop::{ CFRunLoopObserverCreate, CFRunLoopObserverRef, CFRunLoopSourceContext, CFRunLoopSourceCreate, CFRunLoopSourceInvalidate, CFRunLoopSourceRef, CFRunLoopSourceSignal, CFRunLoopWakeUp, }; -use objc2::foundation::MainThreadMarker; +use objc2::foundation::{MainThreadMarker, NSString}; use objc2::rc::{Id, Shared}; use objc2::ClassType; use raw_window_handle::{RawDisplayHandle, UiKitDisplayHandle}; -use super::uikit::{UIApplication, UIDevice, UIScreen}; +use super::uikit::{UIApplication, UIApplicationMain, UIDevice, UIScreen}; use super::view::WinitUIWindow; +use super::{app_state, monitor, view, MonitorHandle}; use crate::{ dpi::LogicalSize, event::Event, @@ -30,12 +31,6 @@ use crate::{ platform::ios::Idiom, }; -use crate::platform_impl::platform::{ - app_state, - ffi::{nil, NSStringRust, UIApplicationMain}, - monitor, view, MonitorHandle, -}; - #[derive(Debug)] pub(crate) enum EventWrapper { StaticEvent(Event<'static, Never>), @@ -132,8 +127,8 @@ impl EventLoop { UIApplicationMain( 0, ptr::null(), - nil, - NSStringRust::alloc(nil).init_str("WinitApplicationDelegate"), + None, + Some(&NSString::from_str("WinitApplicationDelegate")), ); unreachable!() } diff --git a/src/platform_impl/ios/ffi.rs b/src/platform_impl/ios/ffi.rs index b6e52c9b..746a5d99 100644 --- a/src/platform_impl/ios/ffi.rs +++ b/src/platform_impl/ios/ffi.rs @@ -1,19 +1,12 @@ #![allow(non_camel_case_types, non_snake_case, non_upper_case_globals)] use std::convert::TryInto; -use std::ffi::CString; -use std::os::raw::{c_char, c_int}; use objc2::encode::{Encode, Encoding}; use objc2::foundation::{NSInteger, NSUInteger}; -use objc2::runtime::Object; -use objc2::{class, msg_send}; use crate::platform::ios::{Idiom, ScreenEdge}; -pub type id = *mut Object; -pub const nil: id = 0 as id; - #[repr(C)] #[derive(Clone, Debug)] pub struct NSOperatingSystemVersion { @@ -98,46 +91,3 @@ impl From for ScreenEdge { ScreenEdge::from_bits(bits).expect("invalid `ScreenEdge`") } } - -#[link(name = "UIKit", kind = "framework")] -extern "C" { - pub fn UIApplicationMain( - argc: c_int, - argv: *const c_char, - principalClassName: id, - delegateClassName: id, - ) -> c_int; -} - -// This is named NSStringRust rather than NSString because the "Debug View Heirarchy" feature of -// Xcode requires a non-ambiguous reference to NSString for unclear reasons. This makes Xcode happy -// so please test if you change the name back to NSString. -pub trait NSStringRust: Sized { - unsafe fn alloc(_: Self) -> id { - msg_send![class!(NSString), alloc] - } - - unsafe fn initWithUTF8String_(self, c_string: *const c_char) -> id; - unsafe fn stringByAppendingString_(self, other: id) -> id; - unsafe fn init_str(self, string: &str) -> Self; - unsafe fn UTF8String(self) -> *const c_char; -} - -impl NSStringRust for id { - unsafe fn initWithUTF8String_(self, c_string: *const c_char) -> id { - msg_send![self, initWithUTF8String: c_string] - } - - unsafe fn stringByAppendingString_(self, other: id) -> id { - msg_send![self, stringByAppendingString: other] - } - - unsafe fn init_str(self, string: &str) -> id { - let cstring = CString::new(string).unwrap(); - self.initWithUTF8String_(cstring.as_ptr()) - } - - unsafe fn UTF8String(self) -> *const c_char { - msg_send![self, UTF8String] - } -} diff --git a/src/platform_impl/ios/uikit/mod.rs b/src/platform_impl/ios/uikit/mod.rs index ffd8ab91..d4243748 100644 --- a/src/platform_impl/ios/uikit/mod.rs +++ b/src/platform_impl/ios/uikit/mod.rs @@ -2,6 +2,10 @@ #![allow(non_snake_case)] #![allow(non_upper_case_globals)] +use std::os::raw::{c_char, c_int}; + +use objc2::foundation::NSString; + mod application; mod coordinate_space; mod device; @@ -28,3 +32,13 @@ pub(crate) use self::trait_collection::{UIForceTouchCapability, UITraitCollectio pub(crate) use self::view::{UIEdgeInsets, UIView}; pub(crate) use self::view_controller::{UIInterfaceOrientationMask, UIViewController}; pub(crate) use self::window::UIWindow; + +#[link(name = "UIKit", kind = "framework")] +extern "C" { + pub fn UIApplicationMain( + argc: c_int, + argv: *const c_char, + principalClassName: Option<&NSString>, + delegateClassName: Option<&NSString>, + ) -> c_int; +} diff --git a/src/platform_impl/ios/uikit/view_controller.rs b/src/platform_impl/ios/uikit/view_controller.rs index b3665805..57441775 100644 --- a/src/platform_impl/ios/uikit/view_controller.rs +++ b/src/platform_impl/ios/uikit/view_controller.rs @@ -1,6 +1,7 @@ use objc2::encode::{Encode, Encoding}; use objc2::foundation::{NSObject, NSUInteger}; -use objc2::{extern_class, extern_methods, ClassType}; +use objc2::rc::{Id, Shared}; +use objc2::{extern_class, extern_methods, msg_send_id, ClassType}; use super::{UIResponder, UIView}; @@ -28,6 +29,10 @@ extern_methods!( #[sel(setNeedsUpdateOfScreenEdgesDeferringSystemGestures)] pub fn setNeedsUpdateOfScreenEdgesDeferringSystemGestures(&self); + pub fn view(&self) -> Option> { + unsafe { msg_send_id![self, view] } + } + #[sel(setView:)] pub fn setView(&self, view: Option<&UIView>); } diff --git a/src/platform_impl/ios/view.rs b/src/platform_impl/ios/view.rs index 04da35b0..24925aa7 100644 --- a/src/platform_impl/ios/view.rs +++ b/src/platform_impl/ios/view.rs @@ -18,7 +18,7 @@ use crate::{ platform_impl::platform::{ app_state, event_loop::{EventProxy, EventWrapper}, - ffi::{id, UIRectEdge, UIUserInterfaceIdiom}, + ffi::{UIRectEdge, UIUserInterfaceIdiom}, window::PlatformSpecificWindowBuilderAttributes, DeviceId, Fullscreen, }, @@ -487,7 +487,7 @@ declare_class!( // UIApplicationDelegate protocol unsafe impl WinitApplicationDelegate { #[sel(application:didFinishLaunchingWithOptions:)] - fn did_finish_launching(&self, _application: &UIApplication, _: id) -> bool { + fn did_finish_launching(&self, _application: &UIApplication, _: *mut NSObject) -> bool { unsafe { app_state::did_finish_launching(); } diff --git a/src/platform_impl/ios/window.rs b/src/platform_impl/ios/window.rs index 28e2473c..5a82c532 100644 --- a/src/platform_impl/ios/window.rs +++ b/src/platform_impl/ios/window.rs @@ -2,6 +2,7 @@ use std::{ collections::VecDeque, + ffi::c_void, ops::{Deref, DerefMut}, }; @@ -22,7 +23,7 @@ use crate::{ platform_impl::platform::{ app_state, event_loop::{EventProxy, EventWrapper}, - ffi::{id, UIRectEdge}, + ffi::UIRectEdge, monitor, EventLoopWindowTarget, Fullscreen, MonitorHandle, }, window::{ @@ -503,14 +504,14 @@ impl Window { // WindowExtIOS impl Inner { - pub fn ui_window(&self) -> id { - Id::as_ptr(&self.window) as id + pub fn ui_window(&self) -> *mut c_void { + Id::as_ptr(&self.window) as *mut c_void } - pub fn ui_view_controller(&self) -> id { - Id::as_ptr(&self.view_controller) as id + pub fn ui_view_controller(&self) -> *mut c_void { + Id::as_ptr(&self.view_controller) as *mut c_void } - pub fn ui_view(&self) -> id { - Id::as_ptr(&self.view) as id + pub fn ui_view(&self) -> *mut c_void { + Id::as_ptr(&self.view) as *mut c_void } pub fn set_scale_factor(&self, scale_factor: f64) { @@ -613,7 +614,7 @@ impl Inner { #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct WindowId { - window: id, + window: *mut WinitUIWindow, } impl WindowId { @@ -649,20 +650,6 @@ impl From<&Object> for WindowId { } } -impl From<&mut Object> for WindowId { - fn from(window: &mut Object) -> WindowId { - WindowId { - window: window as _, - } - } -} - -impl From for WindowId { - fn from(window: id) -> WindowId { - WindowId { window } - } -} - #[derive(Clone, Default)] pub struct PlatformSpecificWindowBuilderAttributes { pub scale_factor: Option,