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

Reading mapped buffer data yields zeros when the data is written using the mapping API #305

Open
almarklein opened this issue Oct 17, 2023 · 7 comments

Comments

@almarklein
Copy link
Collaborator

To reproduce:

  • Create a buffer using mapped_at_creation=True and the MAP_READ flag.
  • Use wgpuBufferGetMappedRange to get mapped memory and write data to it.
  • Unmap.
  • Map the buffer with MAP_READ.
  • Use wgpuBufferGetMappedRange to get mapped memory and read data from it.
  • Data is all zeros.

If I submit an empty command buffer to the queue between unmap and mapping it again, it does work. So it looks like the written data is not actually submitted yet.

Here's some Python code illustrating the same. It uses an uncommitted version of wgpu-py (I'm working on the buffer mapping API), so consider this pseudo-code to help illustrate the issue.

import wgpu.backends.rs
import wgpu.utils

device = wgpu.utils.get_default_device()
data1 = b"abcdefghijkl"

# Create buffer with data
buf = device.create_buffer(size=len(data1), usage=wgpu.BufferUsage.MAP_READ, mapped_at_creation=True)
buf.write_mapped(data1)
buf.unmap()

# Comment this out to make the problem go away
# encoder = device.create_command_encoder()
# device.queue.submit([encoder.finish()])

# Download from buffer to CPU
buf.map(wgpu.MapMode.READ)
data2 = buf.read_mapped()
buf.unmap()
print(data2.tobytes())
assert data1 == data2

My questions are:

Is this expected behavior (from the POV of the WebGPU spec) or something we should fix?

If the intent is to fix it, where should this happen, here or wgpu-core?

@almarklein
Copy link
Collaborator Author

Addition: the same problem occurs when:

  • Creating a buffer with usage MAP_READ | COPY_DST.
  • Write to it with wgpuQueueWriteBuffer
  • Map it with READ and checking the data.

@nical
Copy link

nical commented Oct 17, 2023

Yes, that's to be expected. You should not write into a buffer mapping with (Edit: apologies: without) the WRITE bit set (and expect things to work). You may observe a different behavior in the browser but that's more an artifact of how things are implemented there due to the multi-process architecture of Firefox, Chrome and Safari.

Addition: the same problem occurs when:

* Creating a buffer with usage `MAP_READ | COPY_DST`.

* Write to it with `wgpuQueueWriteBuffer`

* Map it with READ and checking the data.

This should generate an error. There may be a bit of validation missing. (Edit: sorry again, see below)

@almarklein
Copy link
Collaborator Author

You should not write into a buffer mapping with the WRITE bit set (and expect things to work)

I'm confused.

This should generate an error. There may be a bit of validation missing.

Why? This seems to me like a legit way to work with a buffer. I feel like I am missing something 🤔

@nical
Copy link

nical commented Oct 17, 2023

Sorry, I intended to write without and not with:

You should not write into a buffer mapping without the WRITE bit set.

Why? This seems to me like a legit way to work with a buffer. I feel like I am missing something 🤔

Sorry again my brain was locked on something else, I misread your post and answered too quickly.

queue.writeBuffer should indeed work with a buffer that is COPY_DST, that second test case probably underlines a bug and should work even thought it sounds unlikely to happen in practice (transferring data to the GPU on a buffer that can be only read by the CPU).

@almarklein
Copy link
Collaborator Author

You should not write into a buffer mapping without the WRITE bit set.

What about when using mapped_at_creation=True, it looks like the WRITE bit is not required in that case.

@nical
Copy link

nical commented Oct 17, 2023

It seems that today is one of those days where I should have slept more and not engaged with the outside world because my output's full of nonsense.
mapped_at_creation implies a sort of special writable mapping so it should get the contents of what you wrote into the buffer.

I guess what should happen in your original example (although take it with a grain of salt I still need sleep) , is that mapping the buffer again should not be possible until the device has been polled (typically after submission). I don't know how the map API is implemented in your python example, but in Rust I would expect that map_aync's callback which tells you when the mapping is ready, should probably have fired after submit and that the sequence "map, unmap, map, unmap" with nothing in between should probably not work. But that's just a guess.

It would help to write a test in rust and debug what is happening under the hood.

@almarklein
Copy link
Collaborator Author

almarklein commented Oct 18, 2023

It seems that today is one of those days where I should have slept more and not engaged with the outside world because my output's full of nonsense.

😆 one of those days, eh ...

I guess what should happen in your original example (although take it with a grain of salt I still need sleep) , is that mapping the buffer again should not be possible until the device has been polled (typically after submission).

edited: I did try adding calls to wgpuDevicePoll() but that did not work. Submitting an empty command buffer did.

Anyway ... I think its best if I demonstrate the issue with an example in C or Rust that can easily be reproduced here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants