gfx-rs / wgpu-rs

Rust bindings to wgpu native library
https://wgpu.rs
Mozilla Public License 2.0
1.69k stars 186 forks source link

Specify size in `util::BufferInitDescriptor` #817

Closed luiswirth closed 3 years ago

luiswirth commented 3 years ago

This PR adds the ability, to specify a size in the util::BufferInitDescriptor. This can be of convenience, if you want to write new data after initialization, which is bigger than the original contents.

For this change a size: Option<BufferAddress> field was introduced it the util::BufferInitDescriptor and the code in the util::DeviceExt::create_buffer_init trait function impl for Device was adjusted.

If the size is unspecified (None), then the contents.len() is used.

The size must be at least as big as contents.len(). If this is not respected then the buffer creation panics.

I'm open for suggestions.

luiswirth commented 3 years ago

The build failed because of gfx-rs/wgpu#1285. This is unrelated to the PR.

kvark commented 3 years ago

Could you share a bit more info on the use case here? You have a large buffer, and you only want to initialize a part of it?

luiswirth commented 3 years ago

Often you want to update the data in an existing buffer (e.g. for a vertex buffer), but sometimes the new data isn't the same size as the initial data. Then you want to set some initial data on buffer creation and also reserve some more memory for when the buffer needs to get updated.

I assume here, that it's faster to reserve some memory in advance and just write into the existing buffer, than allocating a new buffer with the right size every time... right?

In my particular case, I want to implement a dynamic buffer, which automatically resizes based on the uploaded contents (like a Vec). In the implementation process I noticed, that when the buffer needs to be resized, which means creating a new buffer with some data (create_buffer_init), I can't specify a bigger size than the size of the initial contents and therefore have to resize every time the new data gets only sightly bigger. Instead I would like to reserve some memory prior and therefore resize less frequently.

Of course I could write my own abstraction, but I thought this would be a nice extension to the existing API.

luiswirth commented 3 years ago

Could somebody please re-run the CI? gfx-rs/wgpu#1285 has been fixed.

kvark commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

kvark commented 3 years ago

@LU15W1R7H

I expect dominating use case for create_buffer_init to be initializing the whole buffer. Given that it's an utility function (!), it doesn't have to cover all the cases. If for a few cases you have to use the mapped_at_creation: true flag manually, that's totally cool. You can see how it's implemented, and you can just use the same/similar code in place where you need different semantics.

You could do mapped_at_creation, or just plain call write_buffer for uploading the data partially.

luiswirth commented 3 years ago

@kvark

Okay, if you feel like this makes the utility function less ergonomic, then I can't argue against that.

You can close the PR if you feel like this is unnecessary.

kvark commented 3 years ago

Yes, thank you for understanding! We have to make judgment calls from time to time. If this was a built-in function, we'd consider this more heavily. Fwiw, you may want to use mapped_at_creation even if you have all the data on CPU, since it allows you to write that data into the destination. So if your data is not already a Vec, you could save a copy. vange-rs does it in one or two places.