The Fate and Purpose of BufferSlice

[cwfitzgerald]

There's been a bit of discussion in the community about the difficulties in the current BufferSlice api. As such, we have been thinking about removing it entirely. However there was some discussion about potential use cases of it in #6309, so we figured we should make an issue to discuss what the api of BufferSlice should be, if it is to exist at all.

cc @kpreid

Some Arguments Against the Current Api:

• It is a deviation from the spec.
• The name BufferSlice implies that you can access the data inside the buffer.
• Users try to keep the BufferSlice around, but because of the strict lifetime, you can't keep it for more than a short period.

[kpreid]

I’ll try to have some thoughts about what would be an “actually actively-good version of BufferSlice”, but I don't have them right now. Some comments:

It is a deviation from the spec.

Note that it's easier in JavaScript to create an ad-hoc object, copy or inherit properties, etc., so it may be easier to deal with not having useful predefined struct types there.

Users try to keep the BufferSlice around, but because of the strict lifetime, you can't keep it for more than a short period.

This could be fixed by making BufferSlice generic or Cow-ish to allow owning the buffer, now that wgpu objects are all Clone-with-refcounting.

[cwfitzgerald]

Note no rush, this isn't a ticking timebomb, we'll let the discussion take its course.

[nical]

It would be useful to list what BufferSlice is useful for (if anything).

[kpreid]

Thanks for the prompt, @nical. Here's where BufferSlice appears in the API (extracted from rustdoc parameter & return type search):

• Creation:
  • Buffer::slice() -> BufferSlice
• Usage in mapping:
  • BufferSlice::{map_async, get_mapped_range, get_mapped_range_mut}
• Usage in command encoding:
  • {RenderPass, RenderBundleEncoder}::{set_index_buffer, set_vertex_buffer}
• Usage in utilities:
  • util::DownloadBuffer::read_buffer
  • util::RenderEncoder::{set_index_buffer, set_vertex_buffer}
• The thing that kicked this all off was proposing to add conversion of a BufferSlice into BufferBinding.

It seems to me that the current direct value of BufferSlice is that it allows command encoding to take “named” parameters concisely without inserting a *Descriptor for each case. In particular, we don't need both a SetIndexBufferDescriptor and a SetVertexBufferDescriptor. This is an an unusually conveniently concise case compared to most of the rest of wgpu's API. (Which has its reasons, but I don't think is actually better in all cases.)

The indirect value of BufferSlice, not fully realized today due to its limited API, is that sub-allocating wgpu middleware (including StagingBelt, in my own plans) can benefit from being able to express “this part of don't-you-worry-about-which-buffer” — the same as slice references are useful everywhere else in Rust. This value is not fully realized because not everything you might want to do with a buffer is possible to do through BufferSlice.

If I could design everything however I liked, we would:

• Add a method to convert BufferSlice into BufferBinding (which would need a note that the conversion is only meaningful in the absence of dynamic offsets)
• Add methods to convert BufferSlice into arguments for copy-to/from-buffer operations
• Add a BufferSlice::slice()
• Probably expose the fields of BufferSlice (they effectively would be anyway via the proposed conversion to BufferBinding unless we make BufferBinding own a BufferSlice instead of a buffer and offset)
• Change BufferSlice to be generic over Deref<Target = Buffer>, so that it can be created in an Arc-rather-than-lifetime form, allowing it to be stored, passed in channels, etc. like a TextureView can be.

Happy to write a PR for all that :)

[nical]

Thanks for the writeup!

It would be great if (some of) BufferSlice's features could be implemented as sugar on top of (or next to) a base API that looks more like the standard (WebGPU) API.

I see the value in having some syntactic sugar for some use cases. When I look at my own usage of wgpu, slice is only used as a temporary when passing parameters to various API calls and I end up with the same information passed as if we were following the standard, but in a slightly non-standard way. The WebGPU specification is a great source of documentation for wgpu, except for anything involving BufferSlice which trips me up regularly.

Example:

// Version 1:
buffer.slice(..).map_async(mode, callback);

Versus

// version 2:
buffer.map_async(mode, .., callback);

Some folks might find version 1 more aesthetically pleasing, but anyone who uses the WebGPU specification for documentation will expect something close to version 2 to work.

I admit to a double-standard here: I'm happy to have rust ranges instead of offset+size parameters, and the callback being a parameter instead of a promise returned. On the other hand the map_async method being absent from Buffer is a problem for me.
The way compiler errors and IDEs work, it's much simpler/quicker to fix "You passed these parameters, but this API expects those instead" than "This function does not exist".

That's not a huge issue of course, but version 1 does not (in my opinion) provide value over version 2 for this type of usage, or at least not enough to warrant straying away from WebGPU.

For that reason, I find the usage in mapping more problematic than, say, the usage in command encoding, even if I would prefer set_index_buffer and set_vertex_buffer to take a buffer and a range.

I'd be much happier with at least map_async, get_mapped_range and get_mapped_range_mut existing both on Buffer and BufferSlice with one being a trivial implementation on top of the other.

The indirect value of BufferSlice, not fully realized today due to its limited API, is that sub-allocating wgpu middleware (including StagingBelt, in my own plans) can benefit from being able to express “this part of don't-you-worry-about-which-buffer”

It sounds like this would be simple to add on top of wgpu using a struct that contains a buffer and a range, without having BufferSlice be the only way to use the API.

the same as slice references are useful everywhere else in Rust.

If I may extend this analogy a bit further, it wouldn't be great if you could only interact with a vector via a slice. Most of what you can do with the slice you can do on the vector directly.

After having written all of this My feeling is that there is a better compromise where perhaps BufferSlice exists as optional convenience instead of being the only way to map/access/bind a buffer.

[kpreid]

On the other hand the map_async method being absent from Buffer is a problem for me.

This makes lots of sense. But how about we fix that by adding a Buffer::map_async()? It seems like that’s the main pain point for you.

It sounds like this would be simple to add on top of wgpu using a struct that contains a buffer and a range, without having BufferSlice be the only way to use the API.

I see this perspective, but I also think that wgpu would be worse as a Rust API if we took BufferSlice out of the signature of the render encoding functions, as would be necessary in order to move it out of wgpu. I agree that wgpu should be obviously matched to the WebGPU API — I just don’t think doing it in a systematic absolutely-no-exceptions way is wise.

Proposal: I will send PRs to:

• Add Buffer::{map_async, get_mapped_range} that does not use BufferSlice (but with docs noting the equivalence).
• Add other BufferSlice conveniences (that are not inserting it into more parts of the API, outside of wgpu::util).

And after those have been out for a while, we can revisit whether BufferSlice is providing value after having given it a better chance than in its current minimal state of “you have to use it and it doesn’t do much for you except being an irregular sort of argument-bundle”.

Any objections?

[nical]

Sounds great!

[cwfitzgerald]

Yeah getting map_async, get_mapped_range(_mut) on Buffer, and making BufferSlice additive seems like a good baseline.