Skip to content

docs: Add more overview documentation to Buffer and usages. #8075

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

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

kpreid
Copy link
Collaborator

@kpreid kpreid commented Aug 10, 2025

Description
I thought that “how do I actually use a Buffer” could use some more explanation. Added:

  • Mention that buffers are untyped allocations of bytes.
  • Mention that multiple things can be stored in one buffer.
  • Summarize all the ways provided by wgpu to write to a buffer.
  • Mention 2 reasons why you should keep your BufferUsages small.

Testing
cargo doc --open && 👀

Squash or Rebase?
One commit!

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@kpreid kpreid requested a review from a team as a code owner August 10, 2025 22:24
@kpreid kpreid added type: enhancement New feature or request area: documentation Documentation for crate items, public or private labels Aug 10, 2025
@kpreid kpreid changed the title Add more overview documentation to Buffer and usages. docs: Add more overview documentation to Buffer and usages. Aug 11, 2025
@ErichDonGubler ErichDonGubler self-requested a review August 11, 2025 14:48
@ErichDonGubler ErichDonGubler self-assigned this Aug 11, 2025
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Nits but nice improvements!

Comment on lines 5154 to 5156
/// Specifying only usage the application will actually perform may increase performance.
/// Additionally, on the WebGL backend, there are restrictions on [`BufferUsages::INDEX`];
/// see [`DownlevelFlags::UNRESTRICTED_INDEX_BUFFER`] for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Specifying only usage the application will actually perform may increase performance.
/// Additionally, on the WebGL backend, there are restrictions on [`BufferUsages::INDEX`];
/// see [`DownlevelFlags::UNRESTRICTED_INDEX_BUFFER`] for more information.
/// Specifying only usages that the application will actually use may theoretically increase performance.
/// Additionally, on the WebGL backend, there are restrictions on [`BufferUsages::INDEX`];
/// see [`DownlevelFlags::UNRESTRICTED_INDEX_BUFFER`] for more information.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Usages that the application will [] use" is poor English to my ear; it’s wrong like like saying "consumptions that you will eat” would be wrong. I’ll add the plural, but “perform” is appropriate here either way. If you still disagree, please let me know what distinction you are aiming at here and I’ll find another phrasing.

Comment on lines 5277 to 5279
/// Specifying only usage the application will actually perform may increase performance.
/// Additionally, on the WebGL backend, there are restrictions on [`BufferUsages::INDEX`];
/// see [`DownlevelFlags::UNRESTRICTED_INDEX_BUFFER`] for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Specifying only usage the application will actually perform may increase performance.
/// Additionally, on the WebGL backend, there are restrictions on [`BufferUsages::INDEX`];
/// see [`DownlevelFlags::UNRESTRICTED_INDEX_BUFFER`] for more information.
/// Specifying only usages that the application will actually use may theoretically increase performance.
/// Additionally, on the WebGL backend, there are restrictions on [`BufferUsages::INDEX`];
/// see [`DownlevelFlags::UNRESTRICTED_INDEX_BUFFER`] for more information.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed the plural, but I don’t see how adding “theoretically” does any work that “may” does not. Do you see this statement as too strong? If so, can you describe what the distinction is?

@kpreid kpreid force-pushed the bufferdoc branch 2 times, most recently from 2bdf4be to c6957de Compare August 14, 2025 03:07
@kpreid kpreid requested a review from cwfitzgerald August 14, 2025 03:09
Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

Highly nitpicky stuff.

/// This only works when the buffer is created and has not yet been used by
/// the GPU, but it is all you need for buffers whose contents do not change
/// after creation.
/// * You may use [`DeviceExt::create_buffer_init`] as a convenient way to do
Copy link
Member

Choose a reason for hiding this comment

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

style: You refer to every other method/function using parens. Let's do the same here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

However! The entire rest of the project generally doesn’t use parens, so arguably this is making it more inconsistent. I'd be up for adding them across the whole project. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation Documentation for crate items, public or private type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants