From 120457c0b116755bf505c9cbc6c3d61ba1f26005 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 29 Jan 2025 18:12:30 +0100 Subject: [PATCH 1/5] Add layer observer based on raw-window-metal --- CHANGELOG.md | 4 + wgpu-hal/src/metal/layer_observer.rs | 186 ++++++++++++++++++++++++++ wgpu-hal/src/metal/mod.rs | 1 + wgpu-hal/src/metal/surface.rs | 190 ++------------------------- 4 files changed, 200 insertions(+), 181 deletions(-) create mode 100644 wgpu-hal/src/metal/layer_observer.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 07315f2932..a1b984dc36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,10 @@ By @brodycj in [#6924](https://github.com/gfx-rs/wgpu/pull/6924). - Stop naga causing undefined behavior when a ray query misses. By @Vecvec in [#6752](https://github.com/gfx-rs/wgpu/pull/6752). +#### Metal + +- Use resize observers for smoother resizing. By @madsmtm in [#7026](https://github.com/gfx-rs/wgpu/pull/7026). + #### WebGPU - Improve efficiency of dropping read-only buffer mappings. By @kpreid in [#7007](https://github.com/gfx-rs/wgpu/pull/7007). diff --git a/wgpu-hal/src/metal/layer_observer.rs b/wgpu-hal/src/metal/layer_observer.rs new file mode 100644 index 0000000000..b30de289be --- /dev/null +++ b/wgpu-hal/src/metal/layer_observer.rs @@ -0,0 +1,186 @@ +//! A rewrite of `raw-window-metal` using `objc` instead of `objc2`. +//! +//! See that for details: +//! +//! This should be temporary, see . + +use core::ffi::c_void; +use core_graphics_types::base::CGFloat; +use core_graphics_types::geometry::CGRect; +use objc::declare::ClassDecl; +use objc::rc::StrongPtr; +use objc::runtime::{Class, Object, Sel, BOOL, NO}; +use objc::{class, msg_send, sel, sel_impl}; +use std::sync::OnceLock; + +extern "C" { + static NSKeyValueChangeNewKey: &'static Object; +} + +#[allow(non_upper_case_globals)] +const NSKeyValueObservingOptionNew: usize = 0x01; +#[allow(non_upper_case_globals)] +const NSKeyValueObservingOptionInitial: usize = 0x04; + +/// Create a new custom layer that tracks parameters from the given super layer. +/// +/// Same as . +pub unsafe fn new_observer_layer(root_layer: *mut Object) -> StrongPtr { + let this: *mut Object = unsafe { msg_send![class(), new] }; + + // Add the layer as a sublayer of the root layer. + let _: () = unsafe { msg_send![root_layer, addSublayer: this] }; + + // Register for key-value observing. + let key_path: *const Object = + unsafe { msg_send![class!(NSString), stringWithUTF8String: c"contentsScale".as_ptr()] }; + let _: () = unsafe { + msg_send![ + root_layer, + addObserver: this + forKeyPath: key_path + options: NSKeyValueObservingOptionNew | NSKeyValueObservingOptionInitial + context: context_ptr() + ] + }; + + let key_path: *const Object = + unsafe { msg_send![class!(NSString), stringWithUTF8String: c"bounds".as_ptr()] }; + let _: () = unsafe { + msg_send![ + root_layer, + addObserver: this + forKeyPath: key_path + options: NSKeyValueObservingOptionNew | NSKeyValueObservingOptionInitial + context: context_ptr() + ] + }; + + // Uncomment when debugging resize issues. + // extern "C" { + // static kCAGravityTopLeft: *mut Object; + // } + // let _: () = unsafe { msg_send![this, setContentsGravity: kCAGravityTopLeft] }; + + unsafe { StrongPtr::new(this) } +} + +/// Same as . +fn class() -> &'static Class { + static CLASS: OnceLock<&'static Class> = OnceLock::new(); + + CLASS.get_or_init(|| { + let superclass = class!(CAMetalLayer); + let class_name = format!("WgpuObserverLayer@{:p}", &CLASS); + let mut decl = ClassDecl::new(&class_name, superclass).unwrap(); + + // From NSKeyValueObserving. + let sel = sel!(observeValueForKeyPath:ofObject:change:context:); + let method: extern "C" fn( + &Object, + Sel, + *mut Object, + *mut Object, + *mut Object, + *mut c_void, + ) = observe_value; + unsafe { decl.add_method(sel, method) }; + + let sel = sel!(dealloc); + let method: extern "C" fn(&Object, Sel) = dealloc; + unsafe { decl.add_method(sel, method) }; + + decl.register() + }) +} + +/// The unique context pointer for this class. +fn context_ptr() -> *mut c_void { + let ptr: *const Class = class(); + ptr.cast_mut().cast() +} + +/// Same as . +extern "C" fn observe_value( + this: &Object, + _cmd: Sel, + key_path: *mut Object, + object: *mut Object, + change: *mut Object, + context: *mut c_void, +) { + // An unrecognized context must belong to the super class. + if context != context_ptr() { + // SAFETY: The signature is correct, and it's safe to forward to + // the superclass' method when we're overriding the method. + return unsafe { + msg_send![ + super(this, class!(CAMetalLayer)), + observeValueForKeyPath: key_path + ofObject: object + change: change + context: context + ] + }; + } + + assert!(!change.is_null()); + + let key = unsafe { NSKeyValueChangeNewKey }; + let new: *mut Object = unsafe { msg_send![change, objectForKey: key] }; + assert!(!new.is_null()); + + let to_compare: *const Object = + unsafe { msg_send![class!(NSString), stringWithUTF8String: c"contentsScale".as_ptr()] }; + let is_equal: BOOL = unsafe { msg_send![key_path, isEqual: to_compare] }; + if is_equal != NO { + // `contentsScale` is a CGFloat, and so the observed value is always a NSNumber. + let scale_factor: CGFloat = if cfg!(target_pointer_width = "64") { + unsafe { msg_send![new, doubleValue] } + } else { + unsafe { msg_send![new, floatValue] } + }; + + // Set the scale factor of the layer to match the root layer. + let _: () = unsafe { msg_send![this, setContentsScale: scale_factor] }; + return; + } + + let to_compare: *const Object = + unsafe { msg_send![class!(NSString), stringWithUTF8String: c"bounds".as_ptr()] }; + let is_equal: BOOL = unsafe { msg_send![key_path, isEqual: to_compare] }; + if is_equal != NO { + // `bounds` is a CGRect, and so the observed value is always a NSNumber. + let bounds: CGRect = unsafe { msg_send![new, rectValue] }; + + // Set `bounds` and `position` to match the root layer. + // + // This differs from just setting the `bounds`, as it also takes into account any + // translation that the superlayer may have that we'd want to preserve. + let _: () = unsafe { msg_send![this, setFrame: bounds] }; + return; + } + + panic!("unknown observed keypath {key_path:?}"); +} + +extern "C" fn dealloc(this: &Object, _cmd: Sel) { + // Load the root layer if it still exists, and deregister the observer. + // + // This is not entirely sound, as the ObserverLayer _could_ have been + // moved to another layer; but Wgpu does do that, so it should be fine. + // + // `raw-window-metal` uses a weak instance variable to do it correctly: + // https://docs.rs/raw-window-metal/1.1.0/src/raw_window_metal/observer.rs.html#74-132 + // (but that's difficult to do with `objc`). + let root_layer: *mut Object = unsafe { msg_send![this, superlayer] }; + if !root_layer.is_null() { + let key_path: *const Object = + unsafe { msg_send![class!(NSString), stringWithUTF8String: c"contentsScale".as_ptr()] }; + let _: () = unsafe { msg_send![root_layer, removeObserver: this forKeyPath: key_path] }; + + let key_path: *const Object = + unsafe { msg_send![class!(NSString), stringWithUTF8String: c"bounds".as_ptr()] }; + let _: () = unsafe { msg_send![root_layer, removeObserver: this forKeyPath: key_path] }; + } +} diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs index 8a3ff41bbe..316eca6c48 100644 --- a/wgpu-hal/src/metal/mod.rs +++ b/wgpu-hal/src/metal/mod.rs @@ -22,6 +22,7 @@ mod adapter; mod command; mod conv; mod device; +mod layer_observer; mod surface; mod time; diff --git a/wgpu-hal/src/metal/surface.rs b/wgpu-hal/src/metal/surface.rs index 5f4bcaeb81..a8bc91b7bf 100644 --- a/wgpu-hal/src/metal/surface.rs +++ b/wgpu-hal/src/metal/surface.rs @@ -2,7 +2,6 @@ use std::mem::ManuallyDrop; use std::ptr::NonNull; -use std::sync::Once; use std::thread; use core_graphics_types::{ @@ -11,55 +10,17 @@ use core_graphics_types::{ }; use metal::foreign_types::ForeignType; use objc::{ - class, - declare::ClassDecl, - msg_send, + class, msg_send, rc::{autoreleasepool, StrongPtr}, - runtime::{Class, Object, Sel, BOOL, NO, YES}, + runtime::{Object, BOOL, NO, YES}, sel, sel_impl, }; use parking_lot::{Mutex, RwLock}; -#[link(name = "QuartzCore", kind = "framework")] -extern "C" { - #[allow(non_upper_case_globals)] - static kCAGravityResize: *mut Object; -} - -extern "C" fn layer_should_inherit_contents_scale_from_window( - _: &Class, - _: Sel, - _layer: *mut Object, - _new_scale: CGFloat, - _from_window: *mut Object, -) -> BOOL { - YES -} - -static CAML_DELEGATE_REGISTER: Once = Once::new(); +use crate::metal::layer_observer::new_observer_layer; -#[derive(Debug)] -pub struct HalManagedMetalLayerDelegate(&'static Class); - -impl HalManagedMetalLayerDelegate { - pub fn new() -> Self { - let class_name = format!("HalManagedMetalLayerDelegate@{:p}", &CAML_DELEGATE_REGISTER); - - CAML_DELEGATE_REGISTER.call_once(|| { - type Fun = extern "C" fn(&Class, Sel, *mut Object, CGFloat, *mut Object) -> BOOL; - let mut decl = ClassDecl::new(&class_name, class!(NSObject)).unwrap(); - unsafe { - // - decl.add_class_method::( - sel!(layer:shouldInheritContentsScale:fromWindow:), - layer_should_inherit_contents_scale_from_window, - ); - } - decl.register(); - }); - Self(Class::get(&class_name).unwrap()) - } -} +#[link(name = "QuartzCore", kind = "framework")] +extern "C" {} impl super::Surface { fn new(layer: metal::MetalLayer) -> Self { @@ -140,120 +101,10 @@ impl super::Surface { // is the default for most views). // // This case is trickier! We cannot use the existing layer with - // Metal, so we must do something else. There are a few options: - // - // 1. Panic here, and require the user to pass a view with a - // `CAMetalLayer` layer. - // - // While this would "work", it doesn't solve the problem, and - // instead passes the ball onwards to the user and ecosystem to - // figure it out. - // - // 2. Override the existing layer with a newly created layer. - // - // If we overlook that this does not work in UIKit since - // `UIView`'s `layer` is `readonly`, and that as such we will - // need to do something different there anyhow, this is - // actually a fairly good solution, and was what the original - // implementation did. - // - // It has some problems though, due to: - // - // a. `wgpu` in our API design choosing not to register a - // callback with `-[CALayerDelegate displayLayer:]`, but - // instead leaves it up to the user to figure out when to - // redraw. That is, we rely on other libraries' callbacks - // telling us when to render. - // - // (If this were an API only for Metal, we would probably - // make the user provide a `render` closure that we'd call - // in the right situations. But alas, we have to be - // cross-platform here). - // - // b. Overwriting the `layer` on `NSView` makes the view - // "layer-hosting", see [wantsLayer], which disables drawing - // functionality on the view like `drawRect:`/`updateLayer`. - // - // These two in combination makes it basically impossible for - // crates like Winit to provide a robust rendering callback - // that integrates with the system's built-in mechanisms for - // redrawing, exactly because overwriting the layer would be - // implicitly disabling those mechanisms! - // - // [wantsLayer]: https://developer.apple.com/documentation/appkit/nsview/1483695-wantslayer?language=objc - // - // 3. Create a sublayer. - // - // `CALayer` has the concept of "sublayers", which we can use - // instead of overriding the layer. - // - // This is also the recommended solution on UIKit, so it's nice - // that we can use (almost) the same implementation for these. - // - // It _might_, however, perform ever so slightly worse than - // overriding the layer directly. - // - // 4. Create a new `MTKView` (or a custom view), and add it as a - // subview. - // - // Similar to creating a sublayer (see above), but also - // provides a bunch of event handling that we don't need. - // - // Option 3 seems like the most robust solution, so this is what - // we're going to do. - - // Create a new sublayer. - let new_layer: *mut Object = msg_send![class!(CAMetalLayer), new]; - let () = msg_send![root_layer, addSublayer: new_layer]; - - #[cfg(target_os = "macos")] - { - // Automatically resize the sublayer's frame to match the - // superlayer's bounds. - // - // Note that there is a somewhat hidden design decision in this: - // We define the `width` and `height` in `configure` to control - // the `drawableSize` of the layer, while `bounds` and `frame` are - // outside of the user's direct control - instead, though, they - // can control the size of the view (or root layer), and get the - // desired effect that way. - // - // We _could_ also let `configure` set the `bounds` size, however - // that would be inconsistent with using the root layer directly - // (as we may do, see above). - let width_sizable = 1 << 1; // kCALayerWidthSizable - let height_sizable = 1 << 4; // kCALayerHeightSizable - let mask: std::ffi::c_uint = width_sizable | height_sizable; - let () = msg_send![new_layer, setAutoresizingMask: mask]; - } - - // Specify the relative size that the auto resizing mask above - // will keep (i.e. tell it to fill out its superlayer). - let frame: CGRect = msg_send![root_layer, bounds]; - let () = msg_send![new_layer, setFrame: frame]; - - // The gravity to use when the layer's `drawableSize` isn't the - // same as the bounds rectangle. - // - // The desired content gravity is `kCAGravityResize`, because it - // masks / alleviates issues with resizing when - // `present_with_transaction` is disabled, and behaves better when - // moving the window between monitors. - // - // Unfortunately, it also makes it harder to see changes to - // `width` and `height` in `configure`. When debugging resize - // issues, swap this for `kCAGravityTopLeft` instead. - let _: () = msg_send![new_layer, setContentsGravity: unsafe { kCAGravityResize }]; - - // Set initial scale factor of the layer. This is kept in sync by - // `configure` (on UIKit), and the delegate below (on AppKit). - let scale_factor: CGFloat = msg_send![root_layer, contentsScale]; - let () = msg_send![new_layer, setContentsScale: scale_factor]; - - let delegate = HalManagedMetalLayerDelegate::new(); - let () = msg_send![new_layer, setDelegate: delegate.0]; - - unsafe { StrongPtr::new(new_layer) } + // Metal, so we must do something else. There are a few options, + // we do the same as outlined in: + // https://docs.rs/raw-window-metal/1.1.0/raw_window_metal/#reasoning-behind-creating-a-sublayer + unsafe { new_observer_layer(root_layer) } } } @@ -303,29 +154,6 @@ impl crate::Surface for super::Surface { _ => (), } - // AppKit / UIKit automatically sets the correct scale factor for - // layers attached to a view. Our layer, however, may not be directly - // attached to a view; in those cases, we need to set the scale - // factor ourselves. - // - // For AppKit, we do so by adding a delegate on the layer with the - // `layer:shouldInheritContentsScale:fromWindow:` method returning - // `true` - this tells the system to automatically update the scale - // factor when it changes. - // - // For UIKit, we manually update the scale factor from the super layer - // here, if there is one. - // - // TODO: Is there a way that we could listen to such changes instead? - #[cfg(not(target_os = "macos"))] - { - let superlayer: *mut Object = msg_send![render_layer.as_ptr(), superlayer]; - if !superlayer.is_null() { - let scale_factor: CGFloat = msg_send![superlayer, contentsScale]; - let () = msg_send![render_layer.as_ptr(), setContentsScale: scale_factor]; - } - } - let device_raw = device.shared.device.lock(); render_layer.set_device(&device_raw); render_layer.set_pixel_format(caps.map_format(config.format)); From 0edf770123b5f5a1fb61e28e53dc20170951921c Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 29 Jan 2025 22:15:02 +0100 Subject: [PATCH 2/5] Fix MSRV --- wgpu-hal/src/metal/layer_observer.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/wgpu-hal/src/metal/layer_observer.rs b/wgpu-hal/src/metal/layer_observer.rs index b30de289be..0e13061944 100644 --- a/wgpu-hal/src/metal/layer_observer.rs +++ b/wgpu-hal/src/metal/layer_observer.rs @@ -4,7 +4,7 @@ //! //! This should be temporary, see . -use core::ffi::c_void; +use core::ffi::{CStr, c_void}; use core_graphics_types::base::CGFloat; use core_graphics_types::geometry::CGRect; use objc::declare::ClassDecl; @@ -22,6 +22,9 @@ const NSKeyValueObservingOptionNew: usize = 0x01; #[allow(non_upper_case_globals)] const NSKeyValueObservingOptionInitial: usize = 0x04; +const CONTENTS_SCALE: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"contentsScale\0") }; +const BOUNDS: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"bounds\0") }; + /// Create a new custom layer that tracks parameters from the given super layer. /// /// Same as . @@ -33,7 +36,7 @@ pub unsafe fn new_observer_layer(root_layer: *mut Object) -> StrongPtr { // Register for key-value observing. let key_path: *const Object = - unsafe { msg_send![class!(NSString), stringWithUTF8String: c"contentsScale".as_ptr()] }; + unsafe { msg_send![class!(NSString), stringWithUTF8String: CONTENTS_SCALE.as_ptr()] }; let _: () = unsafe { msg_send![ root_layer, @@ -45,7 +48,7 @@ pub unsafe fn new_observer_layer(root_layer: *mut Object) -> StrongPtr { }; let key_path: *const Object = - unsafe { msg_send![class!(NSString), stringWithUTF8String: c"bounds".as_ptr()] }; + unsafe { msg_send![class!(NSString), stringWithUTF8String: BOUNDS.as_ptr()] }; let _: () = unsafe { msg_send![ root_layer, @@ -131,7 +134,7 @@ extern "C" fn observe_value( assert!(!new.is_null()); let to_compare: *const Object = - unsafe { msg_send![class!(NSString), stringWithUTF8String: c"contentsScale".as_ptr()] }; + unsafe { msg_send![class!(NSString), stringWithUTF8String: CONTENTS_SCALE.as_ptr()] }; let is_equal: BOOL = unsafe { msg_send![key_path, isEqual: to_compare] }; if is_equal != NO { // `contentsScale` is a CGFloat, and so the observed value is always a NSNumber. @@ -147,7 +150,7 @@ extern "C" fn observe_value( } let to_compare: *const Object = - unsafe { msg_send![class!(NSString), stringWithUTF8String: c"bounds".as_ptr()] }; + unsafe { msg_send![class!(NSString), stringWithUTF8String: BOUNDS.as_ptr()] }; let is_equal: BOOL = unsafe { msg_send![key_path, isEqual: to_compare] }; if is_equal != NO { // `bounds` is a CGRect, and so the observed value is always a NSNumber. @@ -176,11 +179,11 @@ extern "C" fn dealloc(this: &Object, _cmd: Sel) { let root_layer: *mut Object = unsafe { msg_send![this, superlayer] }; if !root_layer.is_null() { let key_path: *const Object = - unsafe { msg_send![class!(NSString), stringWithUTF8String: c"contentsScale".as_ptr()] }; + unsafe { msg_send![class!(NSString), stringWithUTF8String: CONTENTS_SCALE.as_ptr()] }; let _: () = unsafe { msg_send![root_layer, removeObserver: this forKeyPath: key_path] }; let key_path: *const Object = - unsafe { msg_send![class!(NSString), stringWithUTF8String: c"bounds".as_ptr()] }; + unsafe { msg_send![class!(NSString), stringWithUTF8String: BOUNDS.as_ptr()] }; let _: () = unsafe { msg_send![root_layer, removeObserver: this forKeyPath: key_path] }; } } From b9c5d74f2c628d8d2bcd5846abbc7cd100410a2e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 29 Jan 2025 22:15:07 +0100 Subject: [PATCH 3/5] Fix Foundation linking --- wgpu-hal/src/metal/layer_observer.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/wgpu-hal/src/metal/layer_observer.rs b/wgpu-hal/src/metal/layer_observer.rs index 0e13061944..ac8745cc89 100644 --- a/wgpu-hal/src/metal/layer_observer.rs +++ b/wgpu-hal/src/metal/layer_observer.rs @@ -13,6 +13,7 @@ use objc::runtime::{Class, Object, Sel, BOOL, NO}; use objc::{class, msg_send, sel, sel_impl}; use std::sync::OnceLock; +#[link(name = "Foundation", kind = "framework")] extern "C" { static NSKeyValueChangeNewKey: &'static Object; } From 85cf73143fa89ec0bc36ed3a8f359496fec10d4e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 29 Jan 2025 22:22:54 +0100 Subject: [PATCH 4/5] Fix formatting --- wgpu-hal/src/metal/layer_observer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wgpu-hal/src/metal/layer_observer.rs b/wgpu-hal/src/metal/layer_observer.rs index ac8745cc89..289332c865 100644 --- a/wgpu-hal/src/metal/layer_observer.rs +++ b/wgpu-hal/src/metal/layer_observer.rs @@ -4,7 +4,7 @@ //! //! This should be temporary, see . -use core::ffi::{CStr, c_void}; +use core::ffi::{c_void, CStr}; use core_graphics_types::base::CGFloat; use core_graphics_types::geometry::CGRect; use objc::declare::ClassDecl; From 41b4b0f0074ce5dfb4415b346fbcbbc34d573ff0 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 30 Jan 2025 22:08:17 +0100 Subject: [PATCH 5/5] Fix incorrect comment --- wgpu-hal/src/metal/layer_observer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wgpu-hal/src/metal/layer_observer.rs b/wgpu-hal/src/metal/layer_observer.rs index 289332c865..6f1fade72e 100644 --- a/wgpu-hal/src/metal/layer_observer.rs +++ b/wgpu-hal/src/metal/layer_observer.rs @@ -172,7 +172,7 @@ extern "C" fn dealloc(this: &Object, _cmd: Sel) { // Load the root layer if it still exists, and deregister the observer. // // This is not entirely sound, as the ObserverLayer _could_ have been - // moved to another layer; but Wgpu does do that, so it should be fine. + // moved to another layer; but Wgpu doesn't do that, so it should be fine. // // `raw-window-metal` uses a weak instance variable to do it correctly: // https://docs.rs/raw-window-metal/1.1.0/src/raw_window_metal/observer.rs.html#74-132