Conversation
|
the last commit also allows accessing array fields on structs |
|
Yes, I've read the thing quickly but I have a work thing to finish by tomorrow, will be more available to review the PRs after that. Two comments meanwhile:
|
romgrk
left a comment
There was a problem hiding this comment.
This adds length-field handling for the field getters, but do you know how that handling works for field setters? Should we be setting the length when we set an array field?
|
wait, the struct setters work with arrays? I thought they weren't primitives and you could only get the array, not set it edit: according to the code, plain C arrays (the only kind of arrays where |
oh don't worry, the comment was just to keep track of the things in the PR. these PRs can perfectly wait :) I'll fix the typo and the other two things you mentioned (test case and ERROR) when I find some time |
|
in the meanwhile I've done some improvements to the GArray -> V8 conversion, I don't think there's an easy way to implement conversions for arbitrary GArrays for now, because we don't have info about the element GType |
for input (v8 -> gi) conversion: - if fixed-size is set, validate that this size matches the size of the array that has been passed to us - fix bugs in typed array conversion into C array - implement typed array conversion into GArray as well - validate that the element size of the user's typedarray matches the element-size of the array we're emitting - for now, use g_assert_not_reached() because Nan exceptions are not propagated. in the future we could use MaybeLocal for output (gi -> v8) conversion: - validate that element-size (if present) matches the type tag
we still memcpy(), but that's much more efficient than element-by-element conversion to V8 for large buffers of data
- use the (now deprecated) GetContents() to get better compatibility across Node.js versions - convert to Buffer instead of a plain Uint8Array
651f84d to
86bb580
Compare
See the commit messages for full change list, but basically
GArray/GByteArrayWe still need a memcpy, but transferring large data buffers should be much faster now.