[metal] Improve layer initialization and resizing (#6107)

* [metal]: Create a new layer instead of overwriting the existing one

Overriding the `layer` on `NSView` makes the view "layer-hosting", see
[wantsLayer], which disables drawing functionality on the view like
`drawRect:`/`updateLayer`.

This prevents crates like Winit from providing a robust rendering
callback that integrates well with the rest of the system.

Instead, if the layer is not CAMetalLayer, we create a new sublayer, and
render to that instead.

[wantsLayer]: https://developer.apple.com/documentation/appkit/nsview/1483695-wantslayer?language=objc

* [metal]: Fix double-free when re-using layer

* doc: Document the behavior when mis-configuring width/height of Surface

* [metal]: Use kCAGravityResize for smoother resizing

* [metal] Do not keep the view around that the surface was created from

We do not need to use it, and the layer itself is already retained, so
it won't be de-allocated from under our feet.

* Always set delegate on layers created by Wgpu

* More docs on contentsGravity
This commit is contained in:
Mads Marquart 2024-09-08 16:28:14 +02:00 committed by GitHub
parent 9b36a3e129
commit fb0cb1eb11
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 216 additions and 112 deletions

View File

@ -96,9 +96,7 @@ crate::impl_dyn_resource!(
TextureView
);
pub struct Instance {
managed_metal_layer_delegate: surface::HalManagedMetalLayerDelegate,
}
pub struct Instance {}
impl Instance {
pub fn create_surface_from_layer(&self, layer: &metal::MetalLayerRef) -> Surface {
@ -113,9 +111,7 @@ impl crate::Instance for Instance {
profiling::scope!("Init Metal Backend");
// We do not enable metal validation based on the validation flags as it affects the entire
// process. Instead, we enable the validation inside the test harness itself in tests/src/native.rs.
Ok(Instance {
managed_metal_layer_delegate: surface::HalManagedMetalLayerDelegate::new(),
})
Ok(Instance {})
}
unsafe fn create_surface(
@ -126,16 +122,12 @@ impl crate::Instance for Instance {
match window_handle {
#[cfg(target_os = "ios")]
raw_window_handle::RawWindowHandle::UiKit(handle) => {
let _ = &self.managed_metal_layer_delegate;
Ok(unsafe { Surface::from_view(handle.ui_view.as_ptr(), None) })
Ok(unsafe { Surface::from_view(handle.ui_view.cast()) })
}
#[cfg(target_os = "macos")]
raw_window_handle::RawWindowHandle::AppKit(handle) => Ok(unsafe {
Surface::from_view(
handle.ns_view.as_ptr(),
Some(&self.managed_metal_layer_delegate),
)
}),
raw_window_handle::RawWindowHandle::AppKit(handle) => {
Ok(unsafe { Surface::from_view(handle.ns_view.cast()) })
}
_ => Err(crate::InstanceError::new(format!(
"window handle {window_handle:?} is not a Metal-compatible handle"
))),
@ -367,7 +359,6 @@ pub struct Device {
}
pub struct Surface {
view: Option<NonNull<objc::runtime::Object>>,
render_layer: Mutex<metal::MetalLayer>,
swapchain_format: RwLock<Option<wgt::TextureFormat>>,
extent: RwLock<wgt::Extent3d>,

View File

@ -1,26 +1,30 @@
#![allow(clippy::let_unit_value)] // `let () =` being used to constrain result type
use std::{os::raw::c_void, ptr::NonNull, sync::Once, thread};
use std::ffi::c_uint;
use std::mem::ManuallyDrop;
use std::ptr::NonNull;
use std::sync::Once;
use std::thread;
use core_graphics_types::{
base::CGFloat,
geometry::{CGRect, CGSize},
};
use metal::foreign_types::ForeignType;
use objc::{
class,
declare::ClassDecl,
msg_send,
rc::autoreleasepool,
rc::{autoreleasepool, StrongPtr},
runtime::{Class, Object, Sel, BOOL, NO, YES},
sel, sel_impl,
};
use parking_lot::{Mutex, RwLock};
#[cfg(target_os = "macos")]
#[link(name = "QuartzCore", kind = "framework")]
extern "C" {
#[allow(non_upper_case_globals)]
static kCAGravityTopLeft: *mut Object;
static kCAGravityResize: *mut Object;
}
extern "C" fn layer_should_inherit_contents_scale_from_window(
@ -46,6 +50,7 @@ impl HalManagedMetalLayerDelegate {
type Fun = extern "C" fn(&Class, Sel, *mut Object, CGFloat, *mut Object) -> BOOL;
let mut decl = ClassDecl::new(&class_name, class!(NSObject)).unwrap();
unsafe {
// <https://developer.apple.com/documentation/appkit/nsviewlayercontentscaledelegate/3005294-layer?language=objc>
decl.add_class_method::<Fun>(
sel!(layer:shouldInheritContentsScale:fromWindow:),
layer_should_inherit_contents_scale_from_window,
@ -58,9 +63,8 @@ impl HalManagedMetalLayerDelegate {
}
impl super::Surface {
fn new(view: Option<NonNull<Object>>, layer: metal::MetalLayer) -> Self {
fn new(layer: metal::MetalLayer) -> Self {
Self {
view,
render_layer: Mutex::new(layer),
swapchain_format: RwLock::new(None),
extent: RwLock::new(wgt::Extent3d::default()),
@ -71,86 +75,183 @@ impl super::Surface {
/// If not called on the main thread, this will panic.
#[allow(clippy::transmute_ptr_to_ref)]
pub unsafe fn from_view(
view: *mut c_void,
delegate: Option<&HalManagedMetalLayerDelegate>,
) -> Self {
let view = view.cast::<Object>();
let render_layer = {
let layer = unsafe { Self::get_metal_layer(view, delegate) };
let layer = layer.cast::<metal::MetalLayerRef>();
// SAFETY: This pointer…
//
// - …is properly aligned.
// - …is dereferenceable to a `MetalLayerRef` as an invariant of the `metal`
// field.
// - …points to an _initialized_ `MetalLayerRef`.
// - …is only ever aliased via an immutable reference that lives within this
// lexical scope.
unsafe { &*layer }
}
.to_owned();
let _: *mut c_void = msg_send![view, retain];
Self::new(NonNull::new(view), render_layer)
pub unsafe fn from_view(view: NonNull<Object>) -> Self {
let layer = unsafe { Self::get_metal_layer(view) };
let layer = ManuallyDrop::new(layer);
// SAFETY: The layer is an initialized instance of `CAMetalLayer`, and
// we transfer the retain count to `MetalLayer` using `ManuallyDrop`.
let layer = unsafe { metal::MetalLayer::from_ptr(layer.cast()) };
Self::new(layer)
}
pub unsafe fn from_layer(layer: &metal::MetalLayerRef) -> Self {
let class = class!(CAMetalLayer);
let proper_kind: BOOL = msg_send![layer, isKindOfClass: class];
assert_eq!(proper_kind, YES);
Self::new(None, layer.to_owned())
Self::new(layer.to_owned())
}
/// If not called on the main thread, this will panic.
pub(crate) unsafe fn get_metal_layer(
view: *mut Object,
delegate: Option<&HalManagedMetalLayerDelegate>,
) -> *mut Object {
if view.is_null() {
panic!("window does not have a valid contentView");
}
/// Get or create a new `CAMetalLayer` associated with the given `NSView`
/// or `UIView`.
///
/// # Panics
///
/// If called from a thread that is not the main thread, this will panic.
///
/// # Safety
///
/// The `view` must be a valid instance of `NSView` or `UIView`.
pub(crate) unsafe fn get_metal_layer(view: NonNull<Object>) -> StrongPtr {
let is_main_thread: BOOL = msg_send![class!(NSThread), isMainThread];
if is_main_thread == NO {
panic!("get_metal_layer cannot be called in non-ui thread.");
}
let main_layer: *mut Object = msg_send![view, layer];
let class = class!(CAMetalLayer);
let is_valid_layer: BOOL = msg_send![main_layer, isKindOfClass: class];
// Ensure that the view is layer-backed.
// Views are always layer-backed in UIKit.
#[cfg(target_os = "macos")]
let () = msg_send![view.as_ptr(), setWantsLayer: YES];
if is_valid_layer == YES {
main_layer
let root_layer: *mut Object = msg_send![view.as_ptr(), layer];
// `-[NSView layer]` can return `NULL`, while `-[UIView layer]` should
// always be available.
assert!(!root_layer.is_null(), "failed making the view layer-backed");
// NOTE: We explicitly do not touch properties such as
// `layerContentsPlacement`, `needsDisplayOnBoundsChange` and
// `contentsGravity` etc. on the root layer, both since we would like
// to give the user full control over them, and because the default
// values suit us pretty well (especially the contents placement being
// `NSViewLayerContentsRedrawDuringViewResize`, which allows the view
// to receive `drawRect:`/`updateLayer` calls).
let is_metal_layer: BOOL = msg_send![root_layer, isKindOfClass: class!(CAMetalLayer)];
if is_metal_layer == YES {
// The view has a `CAMetalLayer` as the root layer, which can
// happen for example if user overwrote `-[NSView layerClass]` or
// the view is `MTKView`.
//
// This is easily handled: We take "ownership" over the layer, and
// render directly into that; after all, the user passed a view
// with an explicit Metal layer to us, so this is very likely what
// they expect us to do.
unsafe { StrongPtr::retain(root_layer) }
} else {
// If the main layer is not a CAMetalLayer, we create a CAMetalLayer and use it.
let new_layer: *mut Object = msg_send![class, new];
let frame: CGRect = msg_send![main_layer, bounds];
// The view does not have a `CAMetalLayer` as the root layer (this
// 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];
// 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: 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];
#[cfg(target_os = "ios")]
{
// Unlike NSView, UIView does not allow to replace main layer.
let () = msg_send![main_layer, addSublayer: new_layer];
// On iOS, "from_view" may be called before the application initialization is complete,
// `msg_send![view, window]` and `msg_send![window, screen]` will get null.
let screen: *mut Object = msg_send![class!(UIScreen), mainScreen];
let scale_factor: CGFloat = msg_send![screen, nativeScale];
let () = msg_send![view, setContentScaleFactor: scale_factor];
};
#[cfg(target_os = "macos")]
{
let () = msg_send![view, setLayer: new_layer];
let () = msg_send![view, setWantsLayer: YES];
let () = msg_send![new_layer, setContentsGravity: unsafe { kCAGravityTopLeft }];
let window: *mut Object = msg_send![view, window];
if !window.is_null() {
let scale_factor: CGFloat = msg_send![window, backingScaleFactor];
let () = msg_send![new_layer, setContentsScale: scale_factor];
}
};
if let Some(delegate) = delegate {
let () = msg_send![new_layer, setDelegate: delegate.0];
}
new_layer
// 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) }
}
}
@ -171,16 +272,6 @@ impl super::Surface {
}
}
impl Drop for super::Surface {
fn drop(&mut self) {
if let Some(view) = self.view {
unsafe {
let () = msg_send![view.as_ptr(), release];
}
}
}
}
impl crate::Surface for super::Surface {
type A = super::Api;
@ -210,19 +301,30 @@ impl crate::Surface for super::Surface {
_ => (),
}
let device_raw = device.shared.device.lock();
// On iOS, unless the user supplies a view with a CAMetalLayer, we
// create one as a sublayer. However, when the view changes size,
// its sublayers are not automatically resized, and we must resize
// it here. The drawable size and the layer size don't correlate
#[cfg(target_os = "ios")]
// 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"))]
{
if let Some(view) = self.view {
let main_layer: *mut Object = msg_send![view.as_ptr(), layer];
let bounds: CGRect = msg_send![main_layer, bounds];
let () = msg_send![*render_layer, setFrame: bounds];
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));
render_layer.set_framebuffer_only(framebuffer_only);

View File

@ -514,7 +514,7 @@ impl super::Instance {
#[cfg(metal)]
fn create_surface_from_view(
&self,
view: *mut c_void,
view: std::ptr::NonNull<c_void>,
) -> Result<super::Surface, crate::InstanceError> {
if !self.shared.extensions.contains(&ext::metal_surface::NAME) {
return Err(crate::InstanceError::new(String::from(
@ -522,16 +522,17 @@ impl super::Instance {
)));
}
let layer = unsafe {
crate::metal::Surface::get_metal_layer(view.cast::<objc::runtime::Object>(), None)
};
let layer = unsafe { crate::metal::Surface::get_metal_layer(view.cast()) };
// NOTE: The layer is retained by Vulkan's `vkCreateMetalSurfaceEXT`,
// so no need to retain it beyond the scope of this function.
let layer_ptr = (*layer).cast();
let surface = {
let metal_loader =
ext::metal_surface::Instance::new(&self.shared.entry, &self.shared.raw);
let vk_info = vk::MetalSurfaceCreateInfoEXT::default()
.flags(vk::MetalSurfaceCreateFlagsEXT::empty())
.layer(layer.cast());
.layer(layer_ptr);
unsafe { metal_loader.create_metal_surface(&vk_info, None).unwrap() }
};
@ -873,13 +874,13 @@ impl crate::Instance for super::Instance {
(Rwh::AppKit(handle), _)
if self.shared.extensions.contains(&ext::metal_surface::NAME) =>
{
self.create_surface_from_view(handle.ns_view.as_ptr())
self.create_surface_from_view(handle.ns_view)
}
#[cfg(all(target_os = "ios", feature = "metal"))]
(Rwh::UiKit(handle), _)
if self.shared.extensions.contains(&ext::metal_surface::NAME) =>
{
self.create_surface_from_view(handle.ui_view.as_ptr())
self.create_surface_from_view(handle.ui_view)
}
(_, _) => Err(crate::InstanceError::new(format!(
"window handle {window_handle:?} is not a Vulkan-compatible handle"

View File

@ -5547,8 +5547,18 @@ pub struct SurfaceConfiguration<V> {
/// `Bgra8Unorm` and `Bgra8UnormSrgb`
pub format: TextureFormat,
/// Width of the swap chain. Must be the same size as the surface, and nonzero.
///
/// If this is not the same size as the underlying surface (e.g. if it is
/// set once, and the window is later resized), the behaviour is defined
/// but platform-specific, and may change in the future (currently macOS
/// scales the surface, other platforms may do something else).
pub width: u32,
/// Height of the swap chain. Must be the same size as the surface, and nonzero.
///
/// If this is not the same size as the underlying surface (e.g. if it is
/// set once, and the window is later resized), the behaviour is defined
/// but platform-specific, and may change in the future (currently macOS
/// scales the surface, other platforms may do something else).
pub height: u32,
/// Presentation mode of the swap chain. Fifo is the only mode guaranteed to be supported.
/// FifoRelaxed, Immediate, and Mailbox will crash if unsupported, while AutoVsync and