Skip to content

dispatch2: Add fn data_create() which copies a slice #710

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

Closed
wants to merge 1 commit into from

Conversation

MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented Feb 5, 2025

Fixes #699

TODO

  • Add a variant that takes ownership over a Box (or T?) and calls its destructor (EDIT: This requires a raw Block pointer, which we can probably get via a Deref on GlobalBlock).
  • Make regenerated Metal code import/use the correct dispatch_data_t type (preferably taking &DispatchObject and calling .as_raw()?).
  • Ensure calling set_finalizer() is still a valid thing to do?
  • Can the header translator parse #define or are we expected to redefine them ourselves?

@madsmtm
Copy link
Owner

madsmtm commented Feb 6, 2025

Add a variant that takes ownership over a Box (or T?) and calls its destructor (EDIT: This requires a raw Block pointer, which we can probably get via a Deref on GlobalBlock).

Yeah, could be useful. A variant which takes &'static [u8] could also be, and so could taking Arc<[u8]>.

Maybe it could even be generic over T: AsRef<[u8]> + 'static? Though a difficulty here is in thread-safety, I'm not sure, but I think + Send would also be required (maybe depending on the queue the destructor is run on)?

Hmm... It actually seems like the block passed to dispatch_data_create doesn't take any parameters, so it would have to keep a pointer to the original data inside the block? That seems odd? Maybe I don't understand this API enough?

  • Make regenerated Metal code import/use the correct dispatch_data_t type (preferably taking &DispatchObject and calling .as_raw()?).

See #681; I intend to start work on that soon, that should resolve this kind of issue.

Can the header translator parse #define or are we expected to redefine them ourselves?

See #609.

@madsmtm madsmtm added enhancement New feature or request A-framework Affects the framework crates and the translator for them A-dispatch2 Affects the `dispatch2` crate and removed A-framework Affects the framework crates and the translator for them labels Feb 6, 2025
@MarijnS95 MarijnS95 force-pushed the dispatch-data-create branch from 104471d to 997890c Compare March 1, 2025 15:54
@MarijnS95
Copy link
Contributor Author

Hmm... It actually seems like the block passed to dispatch_data_create doesn't take any parameters, so it would have to keep a pointer to the original data inside the block? That seems odd? Maybe I don't understand this API enough?

That's actually useful for DSTs like [u8] which need a fat pointer with size to reconstruct the size. moveing a raw pointer into a new closure solves this naturally (but we do need to allocate a new block for each call, which could be circumvented if the API returned a pointer plus size).

I'll experiment with your suggestions for AsRef<[u8]> + 'static later, that does sound great! Let me invesigate the threading on these queues, I'm not too familiar with them yet.

  • Make regenerated Metal code import/use the correct dispatch_data_t type (preferably taking &DispatchObject and calling .as_raw()?).

See #681; I intend to start work on that soon, that should resolve this kind of issue.

Cool yeah, I see that adds a trait AsRawDispatchObject to get the raw object pointer, but see no generator changes: are those still needed to support converting the object on the fly?

I got the header-translator to run and the output tries to use the low-level FFI bindings imported from use dispatch2::*; but simple typedefs are (obviously) not publicly reexported from the dispatch2 root: https://github.com/MarijnS95/objc2-generated/compare/mtl-dispatch

@madsmtm
Copy link
Owner

madsmtm commented Mar 1, 2025

Sorry, still haven't gotten around to finishing the dispatch2 work. It's on the milestone for the next patch release though, so hopefully I'll get it done this month.

madsmtm added a commit that referenced this pull request Apr 19, 2025
Replaces #710.

Co-authored-by: Marijn Suijten <[email protected]>
@madsmtm
Copy link
Owner

madsmtm commented Apr 19, 2025

I've integrated this in b5e5403, using this PR as a base to build upon. Ended up renaming the methods a bit to better fit with the rest of the objc2 crates.

Thanks for your help Marijn!

Will be released shortly in the next version.

@madsmtm madsmtm closed this Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dispatch2 Affects the `dispatch2` crate enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for MTLDevice.newLibraryWithData:error
2 participants