Use objc2 instead of objc

[madsmtm]

Replace objc with my fork, objc2.

This crate contains many improvements, in particular:

• Better tools for protecting against UB (see first commit)
• You no longer have to worry about using bool in msg_send!.
• The msg_send_id! macro greatly helps with follow memory management rules (similar to Objective-C ARC), potentially also alleviating a lot of the need for autoreleasepools.
• The extern_protocol! macro is used in place of foreign_obj_type!

Notable breaking changes:

• objc2::Message differs from objc::Message, so message sending is not compatible

• NSUInteger is now usize instead of u32/u64 depending on platform (equivalently for NSInteger)

• There is no longer three types for each class (e.g. MTLFoo, Foo and FooRef); these have been combined into one: Foo. Pointers to this can be used across FFI boundaries, and objc2::rc::Id<Foo, Shared> can be used as a strong pointer. In table form:

  Previously	Now
  *mut MTLFoo	*mut Foo
  &FooRef	&Foo
  Foo	Id<Foo, Shared>

When done, this should resolve the following issues:

• Fixes Host documentation #5 now
• Later: Investigate use of objc_retainAutoreleasedReturnValue #222
• Later: Don't use auto-release pool in string-related methods #218
• Later: Rename methods according to the new convention #128
• Later: Integrate auto-release pool into the type system #86
• Later: Migrate to foreign_type! macro #99
• Later: Make auto-release pools visible #225
• Later: Consider using objc_retain and objc_release instead of sending -retain and -release message #58
• Later: new_render_command_encoder assumes the return value can never be NULL #282
• Later: UB in new_texture if newTextureWithDescriptor fails? #284

[madsmtm]

@kvark @cwfitzgerald I think I'm ready for some feedback on this before I proceed; there's a lot of things going on here, but I'd like you to take a look at e.g. the last commit, where I've changed things to use the extern_class!, extern_protocol! and msg_send_id! macros.

Feel free to ask me to clarify things about objc2 in general, I recognize that the documentation of it is not (yet) ideal.

[i509VCB]

The pull request title appears to be wrong, shouldn't objc and objc2 be swapped in the title?

[madsmtm]

An update on this:

I decided to make an automatically generated wrapper for Objective-C frameworks called icrate. There are a few downsides to an automatically generated library, but most of these can be mitigated by manual additions, which I've explicitly designed icrate to support.

In madsmtm/objc2#329 I've added support for Metal, with the primary addition being the file crates/header-translator/src/data/Metal.rs, which details which methods are safe (methods are naturally unsafe by default).

This library fixes the same issues as described in the PR description, as well as paving a way forwards for many other improvements, such as:

• Automatic API availability / deprecation (so that users only call new methods when available).
• Quick SDK updating (e.g. Adding support for the raytracing. #252 would've just been: Download the new SDK, rerun the build script, and mark safe methods).
• Reduced risk of mistakes that lead to unsoundness (for example, all of metal's enum definitions are unsound, since a future version of the OS is allowed to add more variants, and that would break Rust's assumption that the enum is exhaustive).

I'm still working on improving how protocols are mapped, which has a big effect on Metal - would like some help with this in madsmtm/objc2#291 (and maybe Metal protocols should work differently from other frameworks?).

---

I would also like to discuss with the maintainers (@kvark @cwfitzgerald); what do you think the best way forwards is?

I shall be blunt 😉: My ideal future would be for the metal crate just being a pub use icrate::Metal::*.

To achieve this I'm very open to changing things that you may be unhappy with, like the method naming scheme or adding collaborators to icrate/objc2 if you would be more comfortable with it if the bus factor was decreased?

I'm reachable on Matrix as @madsmtm:matrix.org, and will probably see a ping on #wgpu:matrix.org as well, if you want to have a more synchronous discussion.

[grovesNL]

I would also like to discuss with the maintainers (@kvark @cwfitzgerald); what do you think the best way forwards is?

I shall be blunt 😉: My ideal future would be for the metal crate just being a pub use icrate::Metal::*.

To achieve this I'm very open to changing things that you may be unhappy with, like the madsmtm/objc2#284 or adding collaborators to icrate/objc2 if you would be more comfortable with it if the bus factor was decreased?

Looks really great so far! Awesome work and I'm excited about fixing some of the autorelease pool and selector issues we've discussed over the years.

I'm a little hesitant about generating all bindings and placing them all in icrate. A few other projects have taken a similar approach to replace objc (e.g., objr, fruity, objrs, etc.) but have never achieved broad adoption by the existing ecosystem, so I'm not confident that objc2/icrate will become the standard yet (though I acknowledge that this is a bit of a chicken-and-egg problem).

For example, ideally I don't think it should be the metal's crate responsibility to say that we only work with NSColors that originate from objc2/icrate, but I could see us easily stuck in that position if we switch.

Any of the following would help a lot:

• more collaborators to reduce the bus factor
• confidence that the generated bindings are a net time saver/reduce maintenance complexity
  • e.g., lowering the amount of manual work instead of often having to patch the generator itself or write a lot of manual fixes on top of the output
• clear signs that the ecosystem will really move in this direction
  • e.g., maybe upgrading the existing ecosystem instead of replacing it where possible
• better understanding around increases or decreases in compilation time or runtime performance

I'm also curious if there's a way we could adopt generated bindings internally without breaking the existing API too much (instead of, or as a temporary measure while we work towards being able to do pub use icrate::Metal::*).

Automatic API availability / deprecation (so that users only call new methods when available).

For what it's worth, we wouldn't want compile-time detection for this in all cases. For example, wgpu's Metal backend would want to be able to compile against the highest possible deployment target and decide at runtime which methods to use.

[madsmtm]

Looks really great so far! Awesome work and I'm excited about fixing some of the autorelease pool and selector issues we've discussed over the years.

Thanks! It's great to hear you raise some concerns, I'll try to respond to them below:

I'm a little hesitant about generating all bindings and placing them all in icrate. A few other projects have taken a similar approach to replace objc (e.g., objr, fruity, objrs, etc.) but have never achieved broad adoption by the existing ecosystem, so I'm not confident that objc2/icrate will become the standard yet (though I acknowledge that this is a bit of a chicken-and-egg problem).

Yeah, I'm aware, though still very much a valid point. If it helps, I'm the maintainer of winit and I plan to use it there, which will likely cause pressure on wgpu to at the very least switch to objc2 to reduce the dependency tree.

For example, ideally I don't think it should be the metal's crate responsibility to say that we only work with NSColors that originate from objc2/icrate, but I could see us easily stuck in that position if we switch.

Metal doesn't actually use NSColor, but I get what you mean with NSString, NSArray, NSDictionary, and so on ;).

In my defence, metal is already tied to objc's versioning, though yeah the problem does get worse with a large crate (and probably for a while a fast-moving target) like icrate.

Any of the following would help a lot:

* more collaborators to reduce the bus factor

Yeah, will hopefully get better with time. I'll probably make a call out on TWiR at some point for starters.

* confidence that the generated bindings are a net time saver/reduce maintenance complexity

  * e.g., lowering the amount of manual work instead of often having to patch the generator itself or write a lot of manual fixes on top of the output

I think it actually does really well on this mark already (I was surprised myself, I thought I'd have to implement a bunch more hacks for Metal), the only real burden was in creating the data file for which methods are unsafe, which is as it should be, that part should require attention.

* clear signs that the ecosystem will really move in this direction

That's indeed on my roadmap: madsmtm/objc2#174

  * e.g., maybe upgrading the existing ecosystem instead of replacing it where possible

Could you provide an example of what you mean here?

* better understanding around increases or decreases in compilation time or runtime performance

Wrt. compilation time, I'm working on a system to help with this in madsmtm/objc2#311.

Runtime performance is kinda difficult to prove before the entirety of the migration is done, but I can at least say that the project has assembly-level tests for ensuring that the autorelease optimizations do happen.

I'm also curious if there's a way we could adopt generated bindings internally without breaking the existing API too much (instead of, or as a temporary measure while we work towards being able to do pub use icrate::Metal::*).

It is very much possible to do something like this:

use icrate::Metal::MTLDepthStencilDescriptor;

#[repr(u64)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
pub enum MTLCompareFunction {
    Never = 0,
    Less = 1,
    Equal = 2,
    LessEqual = 3,
    Greater = 4,
    NotEqual = 5,
    GreaterEqual = 6,
    Always = 7,
}

// from_icrate and into_icrate for MTLCompareFunction, unless we simply export icrate's MTLCompareFunction


foreign_obj_type! {
    type CType = MTLDepthStencilDescriptor;
    pub struct DepthStencilDescriptor;
}
// Will now translate to roughly
{
    pub struct DepthStencilDescriptor(Id<MTLDepthStencilDescriptor, Shared>);

    #[repr(transparent)]
    pub struct DepthStencilDescriptorRef(MTLDepthStencilDescriptor);

    impl Deref<Target = DepthStencilDescriptorRef> for DepthStencilDescriptor { ... }

    impl DepthStencilDescriptor {
        // Could also be a `From` implementation, depending on how much you want
        // to expose that you're using `icrate` under the hood.
        fn from_icrate(obj: Id<MTLDepthStencilDescriptor, Shared>) -> Self {
            Self(obj)
        }
    
        fn into_icrate(...) { ... }
    }
    
    // from_icrate and into_icrate for DepthStencilDescriptorRef
}


impl DepthStencilDescriptor {
    pub fn new() -> Self {
        Self(MTLDepthStencilDescriptor::new())
    }
}

impl DepthStencilDescriptorRef {
    pub fn depth_compare_function(&self) -> MTLCompareFunction {
        MTLCompareFunction::from_objc2(self.0.depthCompareFunction())
    }

    pub fn set_depth_compare_function(&self, func: MTLCompareFunction) {
        self.0.setDepthCompareFunction(func.into_objc2());
    }

    pub fn depth_write_enabled(&self) -> bool {
        self.0.isDepthWriteEnabled();
    }

    pub fn set_depth_write_enabled(&self, enabled: bool) {
        self.0.setDepthWriteEnabled(enabled);
    }

    pub fn front_face_stencil(&self) -> Option<StencilDescriptor> {
        self.frontFaceStencil().map(StencilDescriptor::from_objc2)
    }

    pub fn set_front_face_stencil(&self, descriptor: Option<&StencilDescriptorRef>) {
        self.setFrontFaceStencil(descriptor.map(|d| d.into_objc2()))
    }

    // ...

    // Inefficient, but correct! An alternative would be to return a newtype `MetalString(Id<NSString, Shared>)` instead.
    pub fn label(&self) -> String {
        self.0.label().to_string()
    }

    pub fn set_label(&self, label: &str) {
        self.0.setLabel(&NSString::from_str(label));
    }
}

And this -sys pattern is what I would usually do for normal C-APIs; however, Objective-C is different in that they have quite high type-safety, which means, as you can see, a lot of things just end up being duplicated work.

But yes, it would definitely be a way forward, and I could probably do it at some point if you want.

Automatic API availability / deprecation (so that users only call new methods when available).

For what it's worth, we wouldn't want compile-time detection for this in all cases. For example, wgpu's Metal backend would want to be able to compile against the highest possible deployment target and decide at runtime which methods to use.

The plan is to have the checks be made at runtime, but in a way such that you'll get a panic if you use some functionality without having made the runtime check beforehand, similar to Swift's #available (which, yeah, will require a bit of trickery to get to work in Rust).

---

Again, thanks for the kind response!

[madsmtm]

Okay, so to get this moving forwards, I've reduced this PR to only a "move to objc2", and have kept the more fancy features and improvements out for now - they will come later, but I'd like to get the ball rolling if possible.

So, the primary thing that's changed is that all types now implement Encode, which allows us to ensure at compile-time that the type's ABI is valid (e.g. it's not Vec<u8>, &str or similar), and at runtime that the type matches what is expected by the method.
I really should write some better docs on this, but I recently explained it here, that may help - if not, feel free to ask!

Re compilation time, a clean build has not changed much, we've traded the core-graphics-types crate for icrate, and both of those compile faster than syn anyhow. But icrate is big, and not as optimized on that front as it should be, so there's definitely still room for improvement.

---

Note: This PR is a breaking change, both because of the changed Message impls for everything (e.g. code that wanted to send messages manually to metal types now have to use objc2, or have to do a cast), but also because I changed a lot of APIs to no longer directly depend on Objective-C types. Concretely, I made the following public-facing changes:

• Removed NSInteger and NSUInteger type aliases, instead we just use isize and usize.
• Removed NSRange, methods now take Range<usize> instead.
• Use f64 in [set_]contents_scale instead of CGFloat.
• Changed the signature of drawable_size from fn(&self) -> CGSize to fn(&self) -> (f64, f64).
• Changed the signature of set_drawable_size from fn(&self, size: CGSize) to fn(&self, width: f64, height: f64).

[madsmtm]

Just tested, all the examples except the ones in #282 now work (the errors in that issue are caught by debug assertions, which is why they don't).

[madsmtm]

@grovesNL: I'm a bit unsure because I think you asked about a lot of things that are not strictly relevant to this PR anymore (since I've kept the autogenerated bindings out for now), but think I've resolved/answered your questions, are there any outstanding concerns, or something else?

CC @kvark, @cwfitzgerald