Skip to content

Conversation

@jgraef
Copy link

@jgraef jgraef commented Nov 27, 2025

Connections

This addresses #8575. allocate was introduced in #6900

Description

StagingBelt can give you the buffer slice for more advanced usecases. The documentation incorrectly states that you could e.g. use it in a compute pass. Because the buffers are created with MAP_WRITE | COPY_SRC this is incorrect.

Creating buffers with custom buffer usages is possible, although MAPPABLE_PRIMARY_BUFFERS must be enabled.

This PR adds a new_with_buffer_usages method with a third argument for the intended buffer usages. We could also just extend the current new method, if API breakage is possible. The new method verifies that the combination of BufferUsages is possible depending on the MAPPABLE_PRIMARY_BUFFERS feature.

The MAP_WRITE usage is always added to the supplied buffer usages.

I've also stripped the mention of using this with a compute pass from the documentation of allocate. In my opinion using the staging belt with mappable primary buffers is really a niece case.

Testing

I've added 2 test:

  1. staging_belt_panics_with_invalid_buffer_usages goes through all buffer usages except COPY_SRC and MAP_WRITE and checks that the constructor panics.
  2. staging_belt_works_with_non_exclusive_buffer_usages that the new constructor works with COPY_SRC, COPY_SRC | MAP_WRITE and MAP_WRITE.

Furthermore staging_belt_random_test verifies that any previous functionality isn't lost.

Merge or Squash

This should be squashed.

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.
@jgraef
Copy link
Author

jgraef commented Nov 27, 2025

I think the new constructor would need a #[track_caller]. I can add this, but will wait and see if more issues come up.

//! Tests of [`wgpu::util`].
use nanorand::Rng;
use wgpu::BufferUsages;
Copy link
Collaborator

@kpreid kpreid Nov 27, 2025

Choose a reason for hiding this comment

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

I don't think we have a set style on this yet, but since the file currently uses all qualified paths for wgpu, please stick to that; that is, remove this use and update the code to work without it.

let (device, _queue) = wgpu::Device::noop(&wgpu::DeviceDescriptor::default());
let _belt = wgpu::util::StagingBelt::new_with_buffer_usages(device.clone(), 512, usage);
})
.is_err()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest checking at least a substring match of the panic message. In my experience, tests need to check for the specific error they are looking for, or they will start falsely passing in the future when changes accidentally make them trigger a different error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants