Skip to content

[Merged by Bors] - Documenting BufferVec. #4673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions crates/bevy_render/src/render_resource/buffer_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,29 @@ use bevy_core::{cast_slice, Pod};
use copyless::VecHelper;
use wgpu::BufferUsages;

/// A structure for storing raw bytes that have already been properly formatted
/// for use by the GPU.
///
/// "Properly formatted" means that item data already meets the alignment and padding
/// requirements for how it will be used on the GPU.
///
/// Index, vertex, and instance-rate vertex buffers have no alignment nor padding requirements and
/// so this helper type is a good choice for them. Uniform buffers must adhere to std140
/// alignment/padding requirements, and storage buffers to std430. There are helper types for such
/// buffers:
/// - Uniform buffers
Copy link
Member

@james7132 james7132 Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small usability nit: Please update these other types' docs to point back to this more detailed one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? Over at UniformBuffer add a link to here? Or you mean for each of the helpers, link to the others?

I definitely think we should have some bevy book docs for how to get data into your shader, and how to decide which way is appropriate for your needs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over at UniformBuffer add a link to here?

This was my understanding, and what I would like to see.

/// - Plain: [`UniformBuffer`](crate::render_resource::UniformBuffer)
/// - Dynamic offsets: [`DynamicUniformBuffer`](crate::render_resource::DynamicUniformBuffer)
/// - Storage buffers
/// - Plain: [`StorageBuffer`](crate::render_resource::StorageBuffer)
/// - Dynamic offsets: [`DynamicStorageBuffer`](crate::render_resource::DynamicStorageBuffer)
///
/// The item type must implement [`Pod`] for its data representation to be directly copyable.
///
/// The contained data is stored in system RAM. Calling [`reserve`](crate::render_resource::BufferVec::reserve)
/// allocates VRAM from the [`RenderDevice`](crate::renderer::RenderDevice).
/// [`write_buffer`](crate::render_resource::BufferVec::write_buffer) queues copying of the data
/// from system RAM to VRAM.
pub struct BufferVec<T: Pod> {
values: Vec<T>,
buffer: Option<Buffer>,
Expand Down Expand Up @@ -60,6 +83,17 @@ impl<T: Pod> BufferVec<T> {
index
}

/// Creates a [`Buffer`](crate::render_resource::Buffer) on the [`RenderDevice`](crate::renderer::RenderDevice) with size
/// at least `std::mem::size_of::<T>() * capacity`, unless a such a buffer already exists.
///
/// If a [`Buffer`](crate::render_resource::Buffer) exists, but is too small, references to it will be discarded,
/// and a new [`Buffer`](crate::render_resource::Buffer) will be created. Any previously created [`Buffer`](crate::render_resource::Buffer)s
/// that are no longer referenced will be deleted by the [`RenderDevice`](crate::renderer::RenderDevice)
/// once it is done using them (typically 1-2 frames).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice attention to detail!

///
/// In addition to any [`BufferUsages`](crate::render_resource::BufferUsages) provided when
/// the `BufferVec` was created, the buffer on the [`RenderDevice`](crate::renderer::RenderDevice)
/// is marked as [`BufferUsages::COPY_DST`](crate::render_resource::BufferUsages).
pub fn reserve(&mut self, capacity: usize, device: &RenderDevice) {
if capacity > self.capacity {
self.capacity = capacity;
Expand All @@ -73,6 +107,11 @@ impl<T: Pod> BufferVec<T> {
}
}

/// Queues writing of data from system RAM to VRAM using the [`RenderDevice`](crate::renderer::RenderDevice)
/// and the provided [`RenderQueue`](crate::renderer::RenderQueue).
///
/// Before queuing the write, a [`reserve`](crate::render_resource::BufferVec::reserve) operation
/// is executed.
pub fn write_buffer(&mut self, device: &RenderDevice, queue: &RenderQueue) {
if self.values.is_empty() {
return;
Expand Down