Skip to content
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

[wgpu-core] Call flush_mapped_ranges when unmapping write-mapped buffers #6089

Merged
merged 5 commits into from
Aug 12, 2024

Conversation

teoxoy
Copy link
Member

@teoxoy teoxoy commented Aug 8, 2024

Connections
-

Description
flush_mapped_ranges needs to be called for non-coherent write-mappings so that the data becomes visible to the GPU.
We also need to call it for non-coherent read-mappings in cases where the memory needs to be 0 initialized.

This is only needed for Vulkan and GL but even then, I'm not sure how things worked so far.

Vulkan says:

Unmapping non-coherent memory does not implicitly flush the host mapped memory, and host writes that have not been flushed may not ever be visible to the device.

GL says:

If a buffer range is mapped with both GL_MAP_PERSISTENT_BIT and GL_MAP_FLUSH_EXPLICIT_BIT set, then these commands may be called to ensure that data written by the client into the flushed region becomes visible to the server.

Both specs seem to say that users that want to guarantee that the data is flushed should call the function. But they don't say that if users don't call the function the data won't be flushed.

So, it could be that the drivers are flushing internally?

Testing
Existing tests.

@cwfitzgerald
Copy link
Member

This is only needed for Vulkan and GL but even then, I'm not sure how things worked so far.

Most devices have coherent memory in practice

@teoxoy teoxoy self-assigned this Aug 8, 2024
I'm not sure how things worked without this.
`zero_init_needs_flush_now` was always equal to `mapping.is_coherent`
which is not correct but is fixed by the next commit.
`flush_mapped_ranges` needs to be called when mappings are not coherent.
We can also omit flushing for write-mapped buffers since we always flush them on unmap.
`offset` is relative to the start of the mapping not the start of the buffer.
This is done in the same way as in `map_buffer` & `unmap_buffer`.
@ErichDonGubler ErichDonGubler enabled auto-merge (rebase) August 12, 2024 13:05
@ErichDonGubler ErichDonGubler merged commit 7c917ab into gfx-rs:trunk Aug 12, 2024
25 checks passed
@teoxoy teoxoy deleted the fix-unmap branch August 12, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants