Fully use NonZero in gfx-memory #20
Conversation
![monocodus[bot]](https://avatars.githubusercontent.com/in/28526?s=60&v=4){kind=small}
|
I had a strong desire to try this out, and got the code fully converted. Now I'm not sure if this was worth it :) |
|
I don't really have strong feelings either way. From an ideological perspective, if the contract is that it needs to be NonZero, it should be NonZero. On the other hand it's a largely internal api and this seems to add a lot of |
There was a problem hiding this comment.
I think being able to specify at the API boundary that sizes must be nonzero is valuable.
I'm happy for this change to go ahead.
Alternatively we could add documentation for each exposed non zero field, but I cant think of a reason we shouldn't use a type for this.
| @@ -146,16 +146,13 @@ impl<B: hal::Backend> Heaps<B> { | |||
| device, | |||
| memory_index as u32, | |||
| kind, | |||
| requirements.size, | |||
| requirements.alignment, | |||
| Size::new(requirements.size).unwrap(), | |||
There was a problem hiding this comment.
should we then change gfx::memory::Requirements size/alignment to use NonZeroU64 as well?
There was a problem hiding this comment.
Yeah, that's what I've been considering as well. I'm thinking of trying some NonZero in gfx-hal and seeing how it goes.
It's ok to let this PR to sit for a bit.
It could be nice to expose the NonZero contract for the API side when passing down allocation sizes and alignments.
Previously, the conditions on parameters where:
Now, it's just (3), size is aligned.