[Merged by Bors] - Documenting `BufferVec`. #4673

bzm3r · 2022-05-06T03:29:39Z

Objective

Documents the BufferVec render resource.

BufferVec is a fairly low level object, that will likely be managed by a higher level API (e.g. through encase) in the future. For now, since it is still used by some simple
example crates (e.g. bevy-vertex-pulling), it will be helpful
to provide some simple documentation on what BufferVec does.

Solution

I looked through Discord discussion on BufferVec, and found a comment by @superdump to be particularly helpful, in the general discussion around encase.

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to reserve), and when data writes from host to device are scheduled (using write_buffer calls).

Changelog

Added doc string for BufferVec and two of its methods: reserve and write_buffer.

crates/bevy_render/src/render_resource/buffer_vec.rs

alice-i-cecile

Excellent, these are looking good. Ping @bevyengine/rendering-team for a second review.

bzm3r · 2022-06-10T22:15:44Z

@superdump Could you give this a second review? (not able to ping the bevy rendering-team)

superdump

Thanks for contributing some much-needed clarification! I've tweaked a few bits.

crates/bevy_render/src/render_resource/buffer_vec.rs

superdump · 2022-06-21T10:15:46Z

crates/bevy_render/src/render_resource/buffer_vec.rs

+/// for the [`RenderDevice`](crate::renderer::RenderDevice).
+///
+/// "Properly formatted" means data that is [`#[repr(C)]`](https://doc.rust-lang.org/reference/type-layout.html#the-c-representation), [`Pod`]
+/// and `Zeroable`. Unlike the [`DynamicUniformVec`](crate::render_resource::DynamicUniformVec),


Note that since we now use encase, the helper types were renamed to UniformBuffer, DynamicUniformBuffer, StorageBuffer, and DynamicStorageBuffer. I think I have made the necessary changes in the suggestion below.

crates/bevy_render/src/render_resource/buffer_vec.rs

superdump · 2022-06-21T10:31:19Z

crates/bevy_render/src/render_resource/buffer_vec.rs

+    /// 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).


Nice attention to detail!

crates/bevy_render/src/render_resource/buffer_vec.rs

Co-authored-by: Robert Swain <robert.swain@gmail.com>

…w being merged. Co-authored-by: Robert Swain <robert.swain@gmail.com>

…ce memory" Co-authored-by: Robert Swain <robert.swain@gmail.com>

bzm3r · 2022-06-21T16:38:17Z

@superdump Thanks for the review! (and for including commit suggestions instead of just comments :D that made it much easier to integrate the changes!)

superdump

With this minor mistake, LGTM.

crates/bevy_render/src/render_resource/buffer_vec.rs

Co-authored-by: Robert Swain <robert.swain@gmail.com>

alice-i-cecile · 2022-06-21T22:17:58Z

bors r+

@superdump

# Objective Documents the `BufferVec` render resource. `BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](#4272)) in the future. For now, since it is still used by some simple example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful to provide some simple documentation on what `BufferVec` does. ## Solution I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). --- ## Changelog - Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. Co-authored-by: Brian Merchant <bhmerchant@gmail.com>

bors · 2022-06-21T22:19:28Z

Build failed:

ci

alice-i-cecile · 2022-06-27T16:18:54Z

@bzm3r can you please format this? I'd like to merge it :)

james7132 · 2022-06-27T17:51:45Z

crates/bevy_render/src/render_resource/buffer_vec.rs

+/// 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


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

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.

Over at UniformBuffer add a link to here?

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

alice-i-cecile · 2022-06-28T04:08:47Z

We're going to document the StorageBuffer and UniformBuffer in their own PRs, so will merge this as is.

bors r+

@superdump

# Objective Documents the `BufferVec` render resource. `BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](#4272)) in the future. For now, since it is still used by some simple example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful to provide some simple documentation on what `BufferVec` does. ## Solution I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). --- ## Changelog - Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. Co-authored-by: Brian Merchant <bhmerchant@gmail.com>

bors · 2022-06-28T04:20:53Z

Pull request successfully merged into main.

Build succeeded:

@superdump

# Objective Documents the `BufferVec` render resource. `BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful to provide some simple documentation on what `BufferVec` does. ## Solution I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). --- ## Changelog - Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. Co-authored-by: Brian Merchant <bhmerchant@gmail.com>

@superdump

# Objective Documents the `BufferVec` render resource. `BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful to provide some simple documentation on what `BufferVec` does. ## Solution I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). --- ## Changelog - Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. Co-authored-by: Brian Merchant <bhmerchant@gmail.com>

@superdump

# Objective Documents the `BufferVec` render resource. `BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful to provide some simple documentation on what `BufferVec` does. ## Solution I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). --- ## Changelog - Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. Co-authored-by: Brian Merchant <bhmerchant@gmail.com>

@superdump

# Objective Documents the `BufferVec` render resource. `BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful to provide some simple documentation on what `BufferVec` does. ## Solution I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). --- ## Changelog - Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. Co-authored-by: Brian Merchant <bhmerchant@gmail.com>

github-actions bot added the S-Needs-Triage This issue needs to be labelled label May 6, 2022

alice-i-cecile added C-Docs An addition or correction to our documentation A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels May 6, 2022

alice-i-cecile reviewed May 6, 2022

View reviewed changes

crates/bevy_render/src/render_resource/buffer_vec.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed May 6, 2022

View reviewed changes

crates/bevy_render/src/render_resource/buffer_vec.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed May 6, 2022

View reviewed changes

crates/bevy_render/src/render_resource/buffer_vec.rs Outdated Show resolved Hide resolved

alice-i-cecile approved these changes May 6, 2022

View reviewed changes

Documenting BufferVec.

11727c5

superdump requested changes Jun 21, 2022

View reviewed changes

bzm3r and others added 3 commits June 21, 2022 09:33

Replace RenderDevice by GPU.

9ef6d24

Co-authored-by: Robert Swain <robert.swain@gmail.com>

State Pod as the direct requirement; and reflect reality of encase no…

5ffd580

…w being merged. Co-authored-by: Robert Swain <robert.swain@gmail.com>

Say "system RAM" instead of "host memory" and "VRAM" instead of "devi…

da1f666

…ce memory" Co-authored-by: Robert Swain <robert.swain@gmail.com>

superdump approved these changes Jun 21, 2022

View reviewed changes

crates/bevy_render/src/render_resource/buffer_vec.rs Outdated Show resolved Hide resolved

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 21, 2022

remove repetition of "data"

e407b97

Co-authored-by: Robert Swain <robert.swain@gmail.com>

run cargo fmt

c50f80c

james7132 requested changes Jun 27, 2022

View reviewed changes

bors bot changed the title ~~Documenting BufferVec.~~ [Merged by Bors] - Documenting BufferVec. Jun 28, 2022

bors bot closed this Jun 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Documenting `BufferVec`. #4673

[Merged by Bors] - Documenting `BufferVec`. #4673

bzm3r commented May 6, 2022

alice-i-cecile left a comment

bzm3r commented Jun 10, 2022

superdump left a comment

superdump Jun 21, 2022

superdump Jun 21, 2022

bzm3r commented Jun 21, 2022

superdump left a comment

alice-i-cecile commented Jun 21, 2022

bors bot commented Jun 21, 2022

alice-i-cecile commented Jun 27, 2022

james7132 Jun 27, 2022 •

edited

Loading

superdump Jun 27, 2022

alice-i-cecile Jun 27, 2022

alice-i-cecile commented Jun 28, 2022

bors bot commented Jun 28, 2022

[Merged by Bors] - Documenting BufferVec. #4673

[Merged by Bors] - Documenting BufferVec. #4673

Conversation

bzm3r commented May 6, 2022

Objective

Solution

Changelog

alice-i-cecile left a comment

Choose a reason for hiding this comment

bzm3r commented Jun 10, 2022

superdump left a comment

Choose a reason for hiding this comment

superdump Jun 21, 2022

Choose a reason for hiding this comment

superdump Jun 21, 2022

Choose a reason for hiding this comment

bzm3r commented Jun 21, 2022

superdump left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Jun 21, 2022

bors bot commented Jun 21, 2022

alice-i-cecile commented Jun 27, 2022

james7132 Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

superdump Jun 27, 2022

Choose a reason for hiding this comment

alice-i-cecile Jun 27, 2022

Choose a reason for hiding this comment

alice-i-cecile commented Jun 28, 2022

bors bot commented Jun 28, 2022

[Merged by Bors] - Documenting `BufferVec`. #4673

[Merged by Bors] - Documenting `BufferVec`. #4673

james7132 Jun 27, 2022 •

edited

Loading