Don't rely on null::Queue::Submit resolving mapping operations.

In the validation tests, we relied on Queue.Submit(0, nullptr) to
resolve mapping operations. This is fragile so we replace it with a
FlushMappingOperations() function that uses device.Tick() instead.

This allows removing the mapSerial argument from
Buffer::MapRead/WriteAsyncImpl (which was the actual goal of this CL).

Bug: dawn:445
Change-Id: Id98822287370c371bebb83afb8e290e17f3c1b55
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24381
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Corentin Wallez 2020-07-09 06:12:04 +00:00 committed by Commit Bot service account
parent d761d5a575
commit 1318c603d9
17 changed files with 92 additions and 109 deletions

View File

@ -64,11 +64,11 @@ namespace dawn_native {
return {}; return {};
} }
MaybeError MapReadAsyncImpl(uint32_t serial) override { MaybeError MapReadAsyncImpl() override {
UNREACHABLE(); UNREACHABLE();
return {}; return {};
} }
MaybeError MapWriteAsyncImpl(uint32_t serial) override { MaybeError MapWriteAsyncImpl() override {
UNREACHABLE(); UNREACHABLE();
return {}; return {};
} }
@ -272,7 +272,7 @@ namespace dawn_native {
mMapUserdata = userdata; mMapUserdata = userdata;
mState = BufferState::Mapped; mState = BufferState::Mapped;
if (GetDevice()->ConsumedError(MapReadAsyncImpl(mMapSerial))) { if (GetDevice()->ConsumedError(MapReadAsyncImpl())) {
CallMapReadCallback(mMapSerial, WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0); CallMapReadCallback(mMapSerial, WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0);
return; return;
} }
@ -297,7 +297,7 @@ namespace dawn_native {
mMapUserdata = userdata; mMapUserdata = userdata;
mState = BufferState::Mapped; mState = BufferState::Mapped;
if (GetDevice()->ConsumedError(MapWriteAsyncImpl(mMapSerial))) { if (GetDevice()->ConsumedError(MapWriteAsyncImpl())) {
CallMapWriteCallback(mMapSerial, WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0); CallMapWriteCallback(mMapSerial, WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0);
return; return;
} }

View File

@ -77,8 +77,8 @@ namespace dawn_native {
private: private:
virtual MaybeError MapAtCreationImpl() = 0; virtual MaybeError MapAtCreationImpl() = 0;
virtual MaybeError MapReadAsyncImpl(uint32_t serial) = 0; virtual MaybeError MapReadAsyncImpl() = 0;
virtual MaybeError MapWriteAsyncImpl(uint32_t serial) = 0; virtual MaybeError MapWriteAsyncImpl() = 0;
virtual void UnmapImpl() = 0; virtual void UnmapImpl() = 0;
virtual void DestroyImpl() = 0; virtual void DestroyImpl() = 0;
virtual void* GetMappedPointerImpl() = 0; virtual void* GetMappedPointerImpl() = 0;

View File

@ -269,11 +269,11 @@ namespace dawn_native { namespace d3d12 {
return {}; return {};
} }
MaybeError Buffer::MapReadAsyncImpl(uint32_t serial) { MaybeError Buffer::MapReadAsyncImpl() {
return MapInternal(false, "D3D12 map read async"); return MapInternal(false, "D3D12 map read async");
} }
MaybeError Buffer::MapWriteAsyncImpl(uint32_t serial) { MaybeError Buffer::MapWriteAsyncImpl() {
return MapInternal(true, "D3D12 map write async"); return MapInternal(true, "D3D12 map write async");
} }

View File

@ -49,8 +49,8 @@ namespace dawn_native { namespace d3d12 {
private: private:
~Buffer() override; ~Buffer() override;
// Dawn API // Dawn API
MaybeError MapReadAsyncImpl(uint32_t serial) override; MaybeError MapReadAsyncImpl() override;
MaybeError MapWriteAsyncImpl(uint32_t serial) override; MaybeError MapWriteAsyncImpl() override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override; void DestroyImpl() override;

View File

@ -38,8 +38,8 @@ namespace dawn_native { namespace metal {
MaybeError Initialize(); MaybeError Initialize();
~Buffer() override; ~Buffer() override;
// Dawn API // Dawn API
MaybeError MapReadAsyncImpl(uint32_t serial) override; MaybeError MapReadAsyncImpl() override;
MaybeError MapWriteAsyncImpl(uint32_t serial) override; MaybeError MapWriteAsyncImpl() override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override; void DestroyImpl() override;
void* GetMappedPointerImpl() override; void* GetMappedPointerImpl() override;

View File

@ -113,11 +113,11 @@ namespace dawn_native { namespace metal {
return {}; return {};
} }
MaybeError Buffer::MapReadAsyncImpl(uint32_t serial) { MaybeError Buffer::MapReadAsyncImpl() {
return {}; return {};
} }
MaybeError Buffer::MapWriteAsyncImpl(uint32_t serial) { MaybeError Buffer::MapWriteAsyncImpl() {
return {}; return {};
} }

View File

@ -270,17 +270,6 @@ namespace dawn_native { namespace null {
// Buffer // Buffer
struct BufferMapOperation : PendingOperation {
virtual void Execute() {
buffer->OnMapCommandSerialFinished(serial, isWrite);
}
Ref<Buffer> buffer;
void* ptr;
uint32_t serial;
bool isWrite;
};
Buffer::Buffer(Device* device, const BufferDescriptor* descriptor) Buffer::Buffer(Device* device, const BufferDescriptor* descriptor)
: BufferBase(device, descriptor) { : BufferBase(device, descriptor) {
mBackingData = std::unique_ptr<uint8_t[]>(new uint8_t[GetSize()]); mBackingData = std::unique_ptr<uint8_t[]>(new uint8_t[GetSize()]);
@ -315,28 +304,14 @@ namespace dawn_native { namespace null {
memcpy(mBackingData.get() + bufferOffset, data, size); memcpy(mBackingData.get() + bufferOffset, data, size);
} }
MaybeError Buffer::MapReadAsyncImpl(uint32_t serial) { MaybeError Buffer::MapReadAsyncImpl() {
MapAsyncImplCommon(serial, false);
return {}; return {};
} }
MaybeError Buffer::MapWriteAsyncImpl(uint32_t serial) { MaybeError Buffer::MapWriteAsyncImpl() {
MapAsyncImplCommon(serial, true);
return {}; return {};
} }
void Buffer::MapAsyncImplCommon(uint32_t serial, bool isWrite) {
ASSERT(mBackingData);
auto operation = std::make_unique<BufferMapOperation>();
operation->buffer = this;
operation->ptr = mBackingData.get();
operation->serial = serial;
operation->isWrite = isWrite;
ToBackend(GetDevice())->AddPendingOperation(std::move(operation));
}
void* Buffer::GetMappedPointerImpl() { void* Buffer::GetMappedPointerImpl() {
return mBackingData.get(); return mBackingData.get();
} }

View File

@ -199,14 +199,13 @@ namespace dawn_native { namespace null {
~Buffer() override; ~Buffer() override;
// Dawn API // Dawn API
MaybeError MapReadAsyncImpl(uint32_t serial) override; MaybeError MapReadAsyncImpl() override;
MaybeError MapWriteAsyncImpl(uint32_t serial) override; MaybeError MapWriteAsyncImpl() override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override; void DestroyImpl() override;
bool IsMapWritable() const override; bool IsMapWritable() const override;
MaybeError MapAtCreationImpl() override; MaybeError MapAtCreationImpl() override;
void MapAsyncImplCommon(uint32_t serial, bool isWrite);
void* GetMappedPointerImpl() override; void* GetMappedPointerImpl() override;
std::unique_ptr<uint8_t[]> mBackingData; std::unique_ptr<uint8_t[]> mBackingData;

View File

@ -79,7 +79,7 @@ namespace dawn_native { namespace opengl {
return {}; return {};
} }
MaybeError Buffer::MapReadAsyncImpl(uint32_t serial) { MaybeError Buffer::MapReadAsyncImpl() {
const OpenGLFunctions& gl = ToBackend(GetDevice())->gl; const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
// TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high // TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high
@ -89,7 +89,7 @@ namespace dawn_native { namespace opengl {
return {}; return {};
} }
MaybeError Buffer::MapWriteAsyncImpl(uint32_t serial) { MaybeError Buffer::MapWriteAsyncImpl() {
const OpenGLFunctions& gl = ToBackend(GetDevice())->gl; const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
// TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high // TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high

View File

@ -34,8 +34,8 @@ namespace dawn_native { namespace opengl {
private: private:
~Buffer() override; ~Buffer() override;
// Dawn API // Dawn API
MaybeError MapReadAsyncImpl(uint32_t serial) override; MaybeError MapReadAsyncImpl() override;
MaybeError MapWriteAsyncImpl(uint32_t serial) override; MaybeError MapWriteAsyncImpl() override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override; void DestroyImpl() override;

View File

@ -243,7 +243,7 @@ namespace dawn_native { namespace vulkan {
return {}; return {};
} }
MaybeError Buffer::MapReadAsyncImpl(uint32_t serial) { MaybeError Buffer::MapReadAsyncImpl() {
Device* device = ToBackend(GetDevice()); Device* device = ToBackend(GetDevice());
CommandRecordingContext* recordingContext = device->GetPendingRecordingContext(); CommandRecordingContext* recordingContext = device->GetPendingRecordingContext();
@ -251,7 +251,7 @@ namespace dawn_native { namespace vulkan {
return {}; return {};
} }
MaybeError Buffer::MapWriteAsyncImpl(uint32_t serial) { MaybeError Buffer::MapWriteAsyncImpl() {
Device* device = ToBackend(GetDevice()); Device* device = ToBackend(GetDevice());
CommandRecordingContext* recordingContext = device->GetPendingRecordingContext(); CommandRecordingContext* recordingContext = device->GetPendingRecordingContext();

View File

@ -52,8 +52,8 @@ namespace dawn_native { namespace vulkan {
void ClearBuffer(CommandRecordingContext* recordingContext, uint32_t clearValue); void ClearBuffer(CommandRecordingContext* recordingContext, uint32_t clearValue);
// Dawn API // Dawn API
MaybeError MapReadAsyncImpl(uint32_t serial) override; MaybeError MapReadAsyncImpl() override;
MaybeError MapWriteAsyncImpl(uint32_t serial) override; MaybeError MapWriteAsyncImpl() override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override; void DestroyImpl() override;

View File

@ -172,7 +172,7 @@ TEST_F(BufferValidationTest, MapReadSuccess) {
EXPECT_CALL(*mockBufferMapReadCallback, EXPECT_CALL(*mockBufferMapReadCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
buf.Unmap(); buf.Unmap();
} }
@ -186,7 +186,7 @@ TEST_F(BufferValidationTest, MapWriteSuccess) {
EXPECT_CALL(*mockBufferMapWriteCallback, EXPECT_CALL(*mockBufferMapWriteCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
buf.Unmap(); buf.Unmap();
} }
@ -264,7 +264,7 @@ TEST_F(BufferValidationTest, MapReadAlreadyMapped) {
.Times(1); .Times(1);
ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, this + 1)); ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, this + 1));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// Test map writing a buffer that is already mapped // Test map writing a buffer that is already mapped
@ -281,7 +281,7 @@ TEST_F(BufferValidationTest, MapWriteAlreadyMapped) {
.Times(1); .Times(1);
ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, this + 1)); ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, this + 1));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// TODO(cwallez@chromium.org) Test a MapWrite and already MapRead and vice-versa // TODO(cwallez@chromium.org) Test a MapWrite and already MapRead and vice-versa
@ -295,9 +295,8 @@ TEST_F(BufferValidationTest, MapReadUnmapBeforeResult) {
.Times(1); .Times(1);
buf.Unmap(); buf.Unmap();
// Submitting the queue makes the null backend process map request, but the callback shouldn't // The callback shouldn't be called again.
// be called again WaitForAllOperations(device);
queue.Submit(0, nullptr);
} }
// Test unmapping before having the result gives UNKNOWN - for writing // Test unmapping before having the result gives UNKNOWN - for writing
@ -310,9 +309,8 @@ TEST_F(BufferValidationTest, MapWriteUnmapBeforeResult) {
.Times(1); .Times(1);
buf.Unmap(); buf.Unmap();
// Submitting the queue makes the null backend process map request, but the callback shouldn't // The callback shouldn't be called again.
// be called again WaitForAllOperations(device);
queue.Submit(0, nullptr);
} }
// Test destroying the buffer before having the result gives UNKNOWN - for reading // Test destroying the buffer before having the result gives UNKNOWN - for reading
@ -329,9 +327,8 @@ TEST_F(BufferValidationTest, DISABLED_MapReadDestroyBeforeResult) {
.Times(1); .Times(1);
} }
// Submitting the queue makes the null backend process map request, but the callback shouldn't // The callback shouldn't be called again.
// be called again WaitForAllOperations(device);
queue.Submit(0, nullptr);
} }
// Test destroying the buffer before having the result gives UNKNOWN - for writing // Test destroying the buffer before having the result gives UNKNOWN - for writing
@ -348,9 +345,8 @@ TEST_F(BufferValidationTest, DISABLED_MapWriteDestroyBeforeResult) {
.Times(1); .Times(1);
} }
// Submitting the queue makes the null backend process map request, but the callback shouldn't // The callback shouldn't be called again.
// be called again WaitForAllOperations(device);
queue.Submit(0, nullptr);
} }
// When a MapRead is cancelled with Unmap it might still be in flight, test doing a new request // When a MapRead is cancelled with Unmap it might still be in flight, test doing a new request
@ -370,7 +366,7 @@ TEST_F(BufferValidationTest, MapReadUnmapBeforeResultThenMapAgain) {
EXPECT_CALL(*mockBufferMapReadCallback, EXPECT_CALL(*mockBufferMapReadCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, this + 1)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, this + 1))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// TODO(cwallez@chromium.org) Test a MapWrite and already MapRead and vice-versa // TODO(cwallez@chromium.org) Test a MapWrite and already MapRead and vice-versa
@ -391,7 +387,7 @@ TEST_F(BufferValidationTest, MapWriteUnmapBeforeResultThenMapAgain) {
EXPECT_CALL(*mockBufferMapWriteCallback, EXPECT_CALL(*mockBufferMapWriteCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, this + 1)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, this + 1))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// Test that the MapReadCallback isn't fired twice when unmap() is called inside the callback // Test that the MapReadCallback isn't fired twice when unmap() is called inside the callback
@ -404,7 +400,7 @@ TEST_F(BufferValidationTest, UnmapInsideMapReadCallback) {
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.WillOnce(InvokeWithoutArgs([&]() { buf.Unmap(); })); .WillOnce(InvokeWithoutArgs([&]() { buf.Unmap(); }));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// Test that the MapWriteCallback isn't fired twice when unmap() is called inside the callback // Test that the MapWriteCallback isn't fired twice when unmap() is called inside the callback
@ -417,7 +413,7 @@ TEST_F(BufferValidationTest, UnmapInsideMapWriteCallback) {
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.WillOnce(InvokeWithoutArgs([&]() { buf.Unmap(); })); .WillOnce(InvokeWithoutArgs([&]() { buf.Unmap(); }));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// Test that the MapReadCallback isn't fired twice the buffer external refcount reaches 0 in the callback // Test that the MapReadCallback isn't fired twice the buffer external refcount reaches 0 in the callback
@ -430,7 +426,7 @@ TEST_F(BufferValidationTest, DestroyInsideMapReadCallback) {
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.WillOnce(InvokeWithoutArgs([&]() { buf = wgpu::Buffer(); })); .WillOnce(InvokeWithoutArgs([&]() { buf = wgpu::Buffer(); }));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// Test that the MapWriteCallback isn't fired twice the buffer external refcount reaches 0 in the callback // Test that the MapWriteCallback isn't fired twice the buffer external refcount reaches 0 in the callback
@ -443,7 +439,7 @@ TEST_F(BufferValidationTest, DestroyInsideMapWriteCallback) {
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.WillOnce(InvokeWithoutArgs([&]() { buf = wgpu::Buffer(); })); .WillOnce(InvokeWithoutArgs([&]() { buf = wgpu::Buffer(); }));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
// Test that it is valid to destroy an unmapped buffer // Test that it is valid to destroy an unmapped buffer
@ -482,7 +478,7 @@ TEST_F(BufferValidationTest, DestroyMappedBufferCausesImplicitUnmap) {
Call(WGPUBufferMapAsyncStatus_Unknown, nullptr, 0, this + 0)) Call(WGPUBufferMapAsyncStatus_Unknown, nullptr, 0, this + 0))
.Times(1); .Times(1);
buf.Destroy(); buf.Destroy();
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer buf = CreateMapWriteBuffer(4); wgpu::Buffer buf = CreateMapWriteBuffer(4);
@ -492,7 +488,7 @@ TEST_F(BufferValidationTest, DestroyMappedBufferCausesImplicitUnmap) {
Call(WGPUBufferMapAsyncStatus_Unknown, nullptr, 0, this + 1)) Call(WGPUBufferMapAsyncStatus_Unknown, nullptr, 0, this + 1))
.Times(1); .Times(1);
buf.Destroy(); buf.Destroy();
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
} }
@ -537,13 +533,13 @@ TEST_F(BufferValidationTest, MapMappedBuffer) {
wgpu::Buffer buf = CreateMapReadBuffer(4); wgpu::Buffer buf = CreateMapReadBuffer(4);
buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr); buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr);
ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr)); ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer buf = CreateMapWriteBuffer(4); wgpu::Buffer buf = CreateMapWriteBuffer(4);
buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr); buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr);
ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr)); ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
} }
@ -552,12 +548,12 @@ TEST_F(BufferValidationTest, MapCreateBufferMappedBuffer) {
{ {
wgpu::Buffer buf = CreateBufferMapped(4, wgpu::BufferUsage::MapRead).buffer; wgpu::Buffer buf = CreateBufferMapped(4, wgpu::BufferUsage::MapRead).buffer;
ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr)); ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer buf = CreateBufferMapped(4, wgpu::BufferUsage::MapWrite).buffer; wgpu::Buffer buf = CreateBufferMapped(4, wgpu::BufferUsage::MapWrite).buffer;
ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr)); ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
} }
@ -566,12 +562,12 @@ TEST_F(BufferValidationTest, MapBufferMappedAtCreation) {
{ {
wgpu::Buffer buf = BufferMappedAtCreation(4, wgpu::BufferUsage::MapRead); wgpu::Buffer buf = BufferMappedAtCreation(4, wgpu::BufferUsage::MapRead);
ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr)); ASSERT_DEVICE_ERROR(buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer buf = BufferMappedAtCreation(4, wgpu::BufferUsage::MapWrite); wgpu::Buffer buf = BufferMappedAtCreation(4, wgpu::BufferUsage::MapWrite);
ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr)); ASSERT_DEVICE_ERROR(buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
} }
@ -613,7 +609,7 @@ TEST_F(BufferValidationTest, SubmitMappedBuffer) {
encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4); encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
wgpu::CommandBuffer commands = encoder.Finish(); wgpu::CommandBuffer commands = encoder.Finish();
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer bufA = device.CreateBuffer(&descriptorA); wgpu::Buffer bufA = device.CreateBuffer(&descriptorA);
@ -625,7 +621,7 @@ TEST_F(BufferValidationTest, SubmitMappedBuffer) {
encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4); encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
wgpu::CommandBuffer commands = encoder.Finish(); wgpu::CommandBuffer commands = encoder.Finish();
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer bufA = device.CreateBufferMapped(&descriptorA).buffer; wgpu::Buffer bufA = device.CreateBufferMapped(&descriptorA).buffer;
@ -635,7 +631,7 @@ TEST_F(BufferValidationTest, SubmitMappedBuffer) {
encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4); encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
wgpu::CommandBuffer commands = encoder.Finish(); wgpu::CommandBuffer commands = encoder.Finish();
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::Buffer bufA = device.CreateBuffer(&descriptorA); wgpu::Buffer bufA = device.CreateBuffer(&descriptorA);
@ -645,7 +641,7 @@ TEST_F(BufferValidationTest, SubmitMappedBuffer) {
encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4); encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
wgpu::CommandBuffer commands = encoder.Finish(); wgpu::CommandBuffer commands = encoder.Finish();
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::BufferDescriptor mappedBufferDesc = descriptorA; wgpu::BufferDescriptor mappedBufferDesc = descriptorA;
@ -657,7 +653,7 @@ TEST_F(BufferValidationTest, SubmitMappedBuffer) {
encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4); encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
wgpu::CommandBuffer commands = encoder.Finish(); wgpu::CommandBuffer commands = encoder.Finish();
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
{ {
wgpu::BufferDescriptor mappedBufferDesc = descriptorB; wgpu::BufferDescriptor mappedBufferDesc = descriptorB;
@ -669,7 +665,7 @@ TEST_F(BufferValidationTest, SubmitMappedBuffer) {
encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4); encoder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
wgpu::CommandBuffer commands = encoder.Finish(); wgpu::CommandBuffer commands = encoder.Finish();
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
queue.Submit(0, nullptr); WaitForAllOperations(device);
} }
} }
@ -764,7 +760,7 @@ TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
EXPECT_CALL(*mockBufferMapReadCallback, EXPECT_CALL(*mockBufferMapReadCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
buf.Unmap(); buf.Unmap();
ASSERT_EQ(nullptr, buf.GetMappedRange()); ASSERT_EQ(nullptr, buf.GetMappedRange());
@ -778,7 +774,7 @@ TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
EXPECT_CALL(*mockBufferMapWriteCallback, EXPECT_CALL(*mockBufferMapWriteCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
buf.Unmap(); buf.Unmap();
ASSERT_EQ(nullptr, buf.GetMappedRange()); ASSERT_EQ(nullptr, buf.GetMappedRange());
@ -826,7 +822,7 @@ TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
EXPECT_CALL(*mockBufferMapReadCallback, EXPECT_CALL(*mockBufferMapReadCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
buf.Destroy(); buf.Destroy();
ASSERT_EQ(nullptr, buf.GetMappedRange()); ASSERT_EQ(nullptr, buf.GetMappedRange());
@ -840,7 +836,7 @@ TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
EXPECT_CALL(*mockBufferMapWriteCallback, EXPECT_CALL(*mockBufferMapWriteCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
buf.Destroy(); buf.Destroy();
ASSERT_EQ(nullptr, buf.GetMappedRange()); ASSERT_EQ(nullptr, buf.GetMappedRange());
@ -856,7 +852,7 @@ TEST_F(BufferValidationTest, GetMappedRangeOnMappedForReading) {
EXPECT_CALL(*mockBufferMapReadCallback, EXPECT_CALL(*mockBufferMapReadCallback,
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.Times(1); .Times(1);
queue.Submit(0, nullptr); WaitForAllOperations(device);
ASSERT_EQ(nullptr, buf.GetMappedRange()); ASSERT_EQ(nullptr, buf.GetMappedRange());
} }
@ -889,7 +885,7 @@ TEST_F(BufferValidationTest, GetMappedRangeValidCases) {
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.WillOnce(SaveArg<1>(&mappedPointer)); .WillOnce(SaveArg<1>(&mappedPointer));
queue.Submit(0, nullptr); WaitForAllOperations(device);
ASSERT_NE(buf.GetConstMappedRange(), nullptr); ASSERT_NE(buf.GetConstMappedRange(), nullptr);
ASSERT_EQ(buf.GetConstMappedRange(), mappedPointer); ASSERT_EQ(buf.GetConstMappedRange(), mappedPointer);
@ -905,7 +901,7 @@ TEST_F(BufferValidationTest, GetMappedRangeValidCases) {
Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _)) Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
.WillOnce(SaveArg<1>(&mappedPointer)); .WillOnce(SaveArg<1>(&mappedPointer));
queue.Submit(0, nullptr); WaitForAllOperations(device);
ASSERT_NE(buf.GetConstMappedRange(), nullptr); ASSERT_NE(buf.GetConstMappedRange(), nullptr);
ASSERT_EQ(buf.GetConstMappedRange(), buf.GetMappedRange()); ASSERT_EQ(buf.GetConstMappedRange(), buf.GetMappedRange());

View File

@ -21,13 +21,6 @@ namespace {
class QueueSubmitValidationTest : public ValidationTest { class QueueSubmitValidationTest : public ValidationTest {
}; };
static void StoreTrueMapWriteCallback(WGPUBufferMapAsyncStatus status,
void*,
uint64_t,
void* userdata) {
*static_cast<bool*>(userdata) = true;
}
// Test submitting with a mapped buffer is disallowed // Test submitting with a mapped buffer is disallowed
TEST_F(QueueSubmitValidationTest, SubmitWithMappedBuffer) { TEST_F(QueueSubmitValidationTest, SubmitWithMappedBuffer) {
// Create a map-write buffer. // Create a map-write buffer.
@ -54,11 +47,14 @@ TEST_F(QueueSubmitValidationTest, SubmitWithMappedBuffer) {
queue.Submit(1, &commands); queue.Submit(1, &commands);
// Map the buffer, submitting when the buffer is mapped should fail // Map the buffer, submitting when the buffer is mapped should fail
bool mapWriteFinished = false; buffer.MapWriteAsync(nullptr, nullptr);
buffer.MapWriteAsync(StoreTrueMapWriteCallback, &mapWriteFinished);
queue.Submit(0, nullptr);
ASSERT_TRUE(mapWriteFinished);
// Try submitting before the callback is fired.
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
WaitForAllOperations(device);
// Try submitting after the callback is fired.
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands)); ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
// Unmap the buffer, queue submit should succeed // Unmap the buffer, queue submit should succeed
@ -190,10 +186,10 @@ TEST_F(QueueWriteBufferValidationTest, MappedBuffer) {
{ {
wgpu::BufferDescriptor descriptor; wgpu::BufferDescriptor descriptor;
descriptor.size = 4; descriptor.size = 4;
descriptor.usage = wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::MapRead; descriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::MapWrite;
wgpu::Buffer buf = device.CreateBuffer(&descriptor); wgpu::Buffer buf = device.CreateBuffer(&descriptor);
buf.MapReadAsync(nullptr, nullptr); buf.MapWriteAsync(nullptr, nullptr);
uint32_t value = 0; uint32_t value = 0;
ASSERT_DEVICE_ERROR(queue.WriteBuffer(buf, 0, &value, sizeof(value))); ASSERT_DEVICE_ERROR(queue.WriteBuffer(buf, 0, &value, sizeof(value)));
} }

View File

@ -87,6 +87,21 @@ std::string ValidationTest::GetLastDeviceErrorMessage() const {
return mDeviceErrorMessage; return mDeviceErrorMessage;
} }
void ValidationTest::WaitForAllOperations(const wgpu::Device& device) const {
wgpu::Queue queue = device.GetDefaultQueue();
wgpu::Fence fence = queue.CreateFence();
// Force the currently submitted operations to completed.
queue.Signal(fence, 1);
while (fence.GetCompletedValue() < 1) {
device.Tick();
}
// TODO(cwallez@chromium.org): It's not clear why we need this additional tick. Investigate it
// once WebGPU has defined the ordering of callbacks firing.
device.Tick();
}
// static // static
void ValidationTest::OnDeviceError(WGPUErrorType type, const char* message, void* userdata) { void ValidationTest::OnDeviceError(WGPUErrorType type, const char* message, void* userdata) {
ASSERT(type != WGPUErrorType_NoError); ASSERT(type != WGPUErrorType_NoError);

View File

@ -42,6 +42,8 @@ class ValidationTest : public testing::Test {
bool EndExpectDeviceError(); bool EndExpectDeviceError();
std::string GetLastDeviceErrorMessage() const; std::string GetLastDeviceErrorMessage() const;
void WaitForAllOperations(const wgpu::Device& device) const;
// Helper functions to create objects to test validation. // Helper functions to create objects to test validation.
struct DummyRenderPass : public wgpu::RenderPassDescriptor { struct DummyRenderPass : public wgpu::RenderPassDescriptor {