Skip to content

Use objc2-* family of crates #334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 15, 2025
Merged

Use objc2-* family of crates #334

merged 3 commits into from
May 15, 2025

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Apr 20, 2025

The objc2-* family of crates provide many benefits over the core-foundation-rs crates, most notably here is that the bindings are auto-generated and thus much more complete (meaning that we can avoid the error-prone msg_send! in almost all cases).

See servo/core-foundation-rs#729 for additional motivation.

Comment on lines +80 to +111
fn surface_bind_to_gl_texture(surface: &IOSurfaceRef, width: i32, height: i32, has_alpha: bool) {
const BGRA: GLenum = 0x80E1;
const RGBA: GLenum = 0x1908;
const RGB: GLenum = 0x1907;
const TEXTURE_RECTANGLE_ARB: GLenum = 0x84F5;
const UNSIGNED_INT_8_8_8_8_REV: GLenum = 0x8367;

unsafe {
let context = CGLGetCurrentContext();
let gl_error = CGLTexImageIOSurface2D(
context,
TEXTURE_RECTANGLE_ARB,
if has_alpha {
RGBA as GLenum
} else {
RGB as GLenum
},
width,
height,
BGRA as GLenum,
UNSIGNED_INT_8_8_8_8_REV,
surface as *const IOSurfaceRef as cgl::IOSurfaceRef,
0,
);

if gl_error != kCGLNoError {
let error_msg = CStr::from_ptr(CGLErrorString(gl_error));
panic!("{}", error_msg.to_string_lossy());
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was previously provided in io_surface::IOSurface::bind_to_gl_texture, but it really mixes two domains (IOSurface and CGL), and requires that we link both of those, so I didn't want to expose it in objc2-io-surface.

Comment on lines 7 to 11
pub(crate) const kIODefaultCache: i32 = 0;
pub(crate) const kIOWriteCombineCache: i32 = 4;
pub(crate) const kIOMapCacheShift: i32 = 8;
pub(crate) const kIOMapDefaultCache: i32 = kIODefaultCache << kIOMapCacheShift;
pub(crate) const kIOMapWriteCombineCache: i32 = kIOWriteCombineCache << kIOMapCacheShift;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provided in objc2-io-kit, but felt like too little benefit to take on the dependency.

let () = msg_send![self.view.0, release];
self.surface
.io_surface
.unlock(IOSurfaceLockOptions::empty(), &mut seed);
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of manual memory management is now unnecessary, the Retained<NSView> in NativeWidget will do it for you.

Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just a couple questions here:

@madsmtm madsmtm requested a review from mrobinson April 25, 2025 09:38
@madsmtm
Copy link
Contributor Author

madsmtm commented May 12, 2025

I've marked this as ready now, since servo/core-foundation-rs#729 seems to be settling on deprecating the core-foundation-rs crates.

Unsure why CI failed, maybe spurious?

@mrobinson mrobinson added this pull request to the merge queue May 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 14, 2025
@mrobinson mrobinson added this pull request to the merge queue May 15, 2025
Merged via the queue into servo:main with commit a085537 May 15, 2025
32 checks passed
@madsmtm madsmtm deleted the objc2 branch May 15, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants