Wgpu: Possible to delete a buffer that is mapped asynchronously

Created on 3 Dec 2019  路  7Comments  路  Source: gfx-rs/wgpu

The library doesn't correctly detect if a buffer is mapped for the matter of cleaning it up. We are only checking for the pending map operation on the GPU: https://github.com/gfx-rs/wgpu/blob/4fdb54d33a38632ee111465e4c8282dc95994aa8/wgpu-core/src/device.rs#L302

Instead, we need to turn this into an enum:

enum MapState {
  Waiting, // waiting for GPU to be done before mapping
  Active, // mapped
  Idle, // not mapped
}
good first issue bug

All 7 comments

Hi! I want to get more familiar with the structure of the API and I think this would be a good start for the same. I would like to try and solve this one.

I think mapped/unmaped is much more descriptive than active/idle. I find it easier to think about it as the state of the buffer instead of the state of the buffer mapping.

enum BufferState {
  // GPU has in-flight commands that are still referencing this buffer
  Referenced { map_pending: bool },
  Mapped, 
  Unmapped,  
}

And then we can short circuit the mapping of buffers that are not in flight:

(pseudo code)

``rust async fn map_write(&self) -> Result<BufferWriteMapping, Error> { match get_buffer_state(self.id) { // Update the state to mapped and resolve immediately BufferState::Unmapped => set_buffer_state(self.id, BufferState::Mapped), BufferState::Mapped => Err("buffer is already mapped"), BufferState::Referenced { map_pending: true } => Err("buffer mapping already requested"), // request_write_map would setmap_pending` to true and return a future
// that resolves once the mapping is complete
BufferState::Referenced { map_pending: false } => request_write_map(self.id),
}
}

Just a thought...instead of storing a bool in map_pending, I can simply store Option<BufferPendingMapping> and remove https://github.com/gfx-rs/wgpu/blob/306554600ab7479ec3e54d0c076c71f02474237a/wgpu-core/src/resource.rs#L89

I am a bit confused. The map_write you mentioned above will be implemented in wgpu-rs, right? So the check here https://github.com/gfx-rs/wgpu/blob/306554600ab7479ec3e54d0c076c71f02474237a/wgpu-core/src/device/mod.rs#L2082-L2085 will need to be moved with it. Do we need that? Or am I think something wrong here?

Also I am not sure when to set the BufferMapState to Referenced.

Here is where we scan through resources used in a submission.

This seems to be more confusing than I thought :sweat_smile:. I have,therefore, opened a draft PR for initial review #547 .

Was this page helpful?
0 / 5 - 0 ratings

Related issues

unrelentingtech picture unrelentingtech  路  14Comments

cloudhead picture cloudhead  路  15Comments

fintelia picture fintelia  路  14Comments

kvark picture kvark  路  15Comments

lordnoriyuki picture lordnoriyuki  路  17Comments