wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

[metal] Use `objc2-metal`

Open madsmtm opened this issue 2 months ago • 4 comments

Description

Use the objc2-metal crate instead of the metal crate. This will:

  • Improve memory management and soundness.
  • Make it easier to quickly support new Metal APIs (they're either already generated for you, or is basically just an update of Xcode away).
  • Likely allow reducing the usage of Arc and Mutex, as Metal objects are already reference-counted (depending on thread-safety details, not entirely sure).
  • Likely improve performance, we use objc_retainAutoreleasedReturnValue underneath the hood to avoid putting objects into the autorelease pool when possible, reducing memory pressure.

Background

The metal crate contains bindings to the Metal framework. This uses objc to manually perform the message sends, which is quite error-prone, see https://github.com/gfx-rs/metal-rs/issues/284, https://github.com/gfx-rs/metal-rs/issues/319 and https://github.com/gfx-rs/metal-rs/issues/209 for a few examples of unsoundness (out of many).

To solve such problems in the Rust ecosystem in general, I created a successor to objc called objc2, which contains most notably the smart-pointer Retained and the macro msg_send_id!, which together ensure that Objective-C's memory-management rules are upheld correctly.

This is only part of the solution though - we'd still have to write the bindings manually. To solve this, I created a tool (planning to integrate it with bindgen, but that's likely a multi-year project) to generate such framework crates automatically. In acknowledgement that this tool is by far not perfect, and never will be, I've ensured that there's a bunch of options to modify each generated crate.

The modifications for objc2-metal in particular are currently just a few hundred lines of code, weak evidence that the generator is fairly good at this point. I'll also bring attention to the file where unsafe methods are marked safe - I have plans to investigate ways to semi-automatically figure out if something is safe or not, or at least reduce the burden of doing so, but it's a hard problem to ensure is completely sound, so for now it's a bit verbose.

Connections

gpu-allocator is also transitioning to objc2-metal in https://github.com/Traverse-Research/gpu-allocator/pull/225.

https://github.com/gfx-rs/metal-rs/pull/241 is an old open PR for using objc2 in metal internally instead. There currently isn't really a clear path forwards there, and it's a lot of work for less direct benefits to the ecosystem (wgpu-hal is by far the biggest user of metal). But more fundamentally IMO, it's a problem of separation of concerns; metal defines several Foundation types like NSArray, NSString, NSError and so on, and even CAMetalLayer from QuartzCore, and that's a bad idea for interoperability compared to having separate crates for each of these frameworks.

https://github.com/gfx-rs/wgpu/pull/5752 removed the link feature which is not available in these crates.

Implementation

The first commit implements the actual migration, by using a branch of objc2, with a method naming scheme that (more closely) matches metal, to make it easier to review and test what's changed.

The second commit moves to the real naming scheme that objc2 uses.

I'd strongly recommend you review these two commits separately.

Testing

Tested by using the checklist below, as well as running each example individually, and checking that they seem to work.

During the development of this I made two quite critical typos, which were luckily found by the test suite, but there's bound to be at least one more lurking in here somewhere, please test this thoroughly!

Checklist

  • [x] Run cargo fmt.
  • [x] Run cargo clippy.
  • [x] Run cargo xtask test to run tests.
  • [x] Add change to CHANGELOG.md. See simple instructions inside file.

madsmtm avatar Apr 30 '24 15:04 madsmtm