Conversation
63af709 to
5dadadd
Compare
Ralith
requested changes
Nov 10, 2021
Collaborator
Ralith
left a comment
There was a problem hiding this comment.
Should we keep this function similar to get_image_sparse_memory_requirements2 (Vulkan 1.1) with just an out: &mut [] array and separate _len function?
I think it's best to be consistent there, yeah.
5dadadd to
c9d50f9
Compare
Ralith
approved these changes
Nov 11, 2021
MarijnS95
commented
Nov 19, 2021
| &mut count, | ||
| out.as_mut_ptr(), | ||
| ); | ||
| assert_eq!(count, out.len() as u32); |
Collaborator
Author
There was a problem hiding this comment.
@Ralith I slipped in an assert_eq here without mentioning. I have a change locally to apply this to the other functions of this sort - do you think it's worth adding that everywhere consistently?
It seems very unlikely for this to ever change, hence an assert might be a good idea in the strange event that a driver happens to overwrite it after all (ie. playing around with beta drivers etc).
filnet
reviewed
Nov 19, 2021
73 tasks
MarijnS95
added a commit
that referenced
this pull request
Dec 27, 2021
Originally introduced in [#489] this inserts the array-length equality check everywhere else: in the supposedly invalid and inexistant event where Vulkan suddenly returns less items (`count` has been modified) than were originally queried through respective `_len()` functions (or more likely: a slice of invalid length passed by the user) and some elements at the end of the slice are left uninitialized, panic. Wherever there is valid concern or possibility for this to happen - or to circumvent assertions and panics altogether - mutable references to mutable slices should be passed allowing the length to be promptly updated. [#489]: #489 (comment)
MarijnS95
added a commit
that referenced
this pull request
Jan 5, 2022
Originally introduced in [#489] this inserts the array-length equality check everywhere else: in the supposedly invalid and inexistant event where Vulkan suddenly returns less items (`count` has been modified) than were originally queried through respective `_len()` functions (or more likely: a slice of invalid length passed by the user) and some elements at the end of the slice are left uninitialized, panic. Wherever there is valid concern or possibility for this to happen - or to circumvent assertions and panics altogether - mutable references to mutable slices should be passed allowing the length to be promptly updated. [#489]: #489 (comment)
MarijnS95
added a commit
that referenced
this pull request
Jan 5, 2022
Originally introduced in [#489] this inserts the array-length equality check everywhere else: in the supposedly invalid and inexistant event where Vulkan suddenly returns less items (`count` has been modified) than were originally queried through respective `_len()` functions (or more likely: a slice of invalid length passed by the user) and some elements at the end of the slice are left uninitialized, panic. Wherever there is valid concern or possibility for this to happen - or to circumvent assertions and panics altogether - mutable references to mutable slices should be passed allowing the length to be promptly updated. [#489]: #489 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trying a new approach for getter functions with
pNextstructures. seems like this could work for the single item inget_device_image_memory_requirements,but it is probably a bit too weird for the array inget_device_image_sparse_memory_requirements:Length ofnextarray provides the count, so you'll have to fill that withNone(ie.vec![None; get_device_image_sparse_memory_requirements_len()]) if nopNextpointers are used;Slice is an array of borrows, so you'll need to create your (list of) next object(s) in one place, and create a new vector or slice just to hold the borrows to it;Slice could perhaps be an iterator instead.Never mind,
SparseImageMemoryRequirements2currently doesn't have anything that extends it. Should we keep this function similar toget_image_sparse_memory_requirements2(Vulkan 1.1) with just anout: &mut []array and separate_lenfunction?