-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
base: trunk
Are you sure you want to change the base?
Conversation
Buffer
and usages.Buffer
and usages.
There was a problem hiding this 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!
wgpu-types/src/lib.rs
Outdated
/// 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// 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. |
There was a problem hiding this comment.
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.
wgpu-types/src/lib.rs
Outdated
/// 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// 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. |
There was a problem hiding this comment.
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?
2bdf4be
to
c6957de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highly nitpicky stuff.
wgpu/src/api/buffer.rs
Outdated
/// 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Description
I thought that “how do I actually use a
Buffer
” could use some more explanation. Added:wgpu
to write to a buffer.BufferUsages
small.Testing
cargo doc --open && 👀
Squash or Rebase?
One commit!
Checklist
cargo fmt
.taplo format
.cargo clippy --tests
. If applicable, add:--target wasm32-unknown-unknown
cargo xtask test
to run tests.CHANGELOG.md
entry.