Skip to content
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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarijnS95
Copy link

@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
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.

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