encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Update GetMappedRange to not produce validation errors 

GetMappedRange never produces errors and instead returns nullptr when it
is disallowed. When in a correct state, should return a valid pointer as
much as possible, even if the buffer is an error or if the device is
lost.

Adds tests for error buffers and device loss, and modify existing tests
to not expect a device error.

Also removes some dead code in the Vulkan backend and adds a fix for
missing deallocation of VkMemory on device shutdown.

Bug: dawn:445

Change-Id: Ia844ee3493cdaf75083424743dd194fa94faf591
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24160
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This commit is contained in:
Corentin Wallez 2020-07-06 18:08:10 +00:00 committed by Commit Bot service account
parent db34c78910
commit dbf805fe8d
9 changed files with 202 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/dawn_native/Buffer.cpp
View File

@ -311,22 +311,26 @@ namespace dawn_native {
}
void* BufferBase::GetMappedRange() {
if (GetDevice()->ConsumedError(ValidateGetMappedRange(true))) {
return nullptr;
}
if (mStagingBuffer != nullptr) {
return mStagingBuffer->GetMappedPointer();
}
return GetMappedPointerImpl();
return GetMappedRangeInternal(true);
}
const void* BufferBase::GetConstMappedRange() {
if (GetDevice()->ConsumedError(ValidateGetMappedRange(false))) {
return GetMappedRangeInternal(false);
}
// TODO(dawn:445): When CreateBufferMapped is removed, make GetMappedRangeInternal also take
// care of the validation of GetMappedRange.
void* BufferBase::GetMappedRangeInternal(bool writable) {
if (!CanGetMappedRange(writable)) {
return nullptr;
}
if (mStagingBuffer != nullptr) {
return mStagingBuffer->GetMappedPointer();
}
if (mSize == 0) {
return reinterpret_cast<uint8_t*>(intptr_t(0xCAFED00D));
}
return GetMappedPointerImpl();
}
@ -431,24 +435,29 @@ namespace dawn_native {
return {};
}
MaybeError BufferBase::ValidateGetMappedRange(bool writable) const {
DAWN_TRY(GetDevice()->ValidateIsAlive());
DAWN_TRY(GetDevice()->ValidateObject(this));
bool BufferBase::CanGetMappedRange(bool writable) const {
// Note that:
//
// - We don't check that the device is alive because the application can ask for the
// mapped pointer before it knows, and even Dawn knows, that the device was lost, and
// still needs to work properly.
// - We don't check that the object is alive because we need to return mapped pointers
// for error buffers too.
switch (mState) {
// Writeable Buffer::GetMappedRange is always allowed when mapped at creation.
case BufferState::MappedAtCreation:
return {};
return true;
case BufferState::Mapped:
if (writable && !(mUsage & wgpu::BufferUsage::MapWrite)) {
return DAWN_VALIDATION_ERROR("GetMappedRange requires the MapWrite usage");
}
return {};
ASSERT(bool(mUsage & wgpu::BufferUsage::MapRead) ^
bool(mUsage & wgpu::BufferUsage::MapWrite));
return !writable || (mUsage & wgpu::BufferUsage::MapWrite);
case BufferState::Unmapped:
case BufferState::Destroyed:
return DAWN_VALIDATION_ERROR("Buffer is not mapped");
return false;
default:
UNREACHABLE();
}
@ -493,7 +502,7 @@ namespace dawn_native {
}
void BufferBase::OnMapCommandSerialFinished(uint32_t mapSerial, bool isWrite) {
void* data = GetMappedPointerImpl();
void* data = GetMappedRangeInternal(isWrite);
if (isWrite) {
CallMapWriteCallback(mapSerial, WGPUBufferMapAsyncStatus_Success, data, GetSize());
} else {

20
src/dawn_native/Buffer.h
View File

@ -72,15 +72,6 @@ namespace dawn_native {
BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag);
~BufferBase() override;
void CallMapReadCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
const void* pointer,
uint64_t dataLength);
void CallMapWriteCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
void* pointer,
uint64_t dataLength);
void DestroyInternal();
bool IsMapped() const;
@ -95,12 +86,21 @@ namespace dawn_native {
virtual bool IsMapWritable() const = 0;
MaybeError CopyFromStagingBuffer();
void* GetMappedRangeInternal(bool writable);
void CallMapReadCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
const void* pointer,
uint64_t dataLength);
void CallMapWriteCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
void* pointer,
uint64_t dataLength);
MaybeError ValidateMap(wgpu::BufferUsage requiredUsage,
WGPUBufferMapAsyncStatus* status) const;
MaybeError ValidateGetMappedRange(bool writable) const;
MaybeError ValidateUnmap() const;
MaybeError ValidateDestroy() const;
bool CanGetMappedRange(bool writable) const;
uint64_t mSize = 0;
wgpu::BufferUsage mUsage = wgpu::BufferUsage::None;

8
src/dawn_native/vulkan/BufferVk.cpp
View File

@ -176,14 +176,6 @@ namespace dawn_native { namespace vulkan {
DestroyInternal();
}
void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data) {
CallMapReadCallback(mapSerial, WGPUBufferMapAsyncStatus_Success, data, GetSize());
}
void Buffer::OnMapWriteCommandSerialFinished(uint32_t mapSerial, void* data) {
CallMapWriteCallback(mapSerial, WGPUBufferMapAsyncStatus_Success, data, GetSize());
}
VkBuffer Buffer::GetHandle() const {
return mHandle;
}

3
src/dawn_native/vulkan/BufferVk.h
View File

@ -30,9 +30,6 @@ namespace dawn_native { namespace vulkan {
public:
static ResultOrError<Buffer*> Create(Device* device, const BufferDescriptor* descriptor);
void OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data);
void OnMapWriteCommandSerialFinished(uint32_t mapSerial, void* data);
VkBuffer GetHandle() const;
// Transitions the buffer to be used as `usage`, recording any necessary barrier in

1
src/dawn_native/vulkan/DeviceVk.cpp
View File

@ -853,6 +853,7 @@ namespace dawn_native { namespace vulkan {
// Releasing the uploader enqueues buffers to be released.
// Call Tick() again to clear them before releasing the deleter.
mResourceMemoryAllocator->Tick(GetCompletedCommandSerial());
mDeleter->Tick(GetCompletedCommandSerial());
// The VkRenderPasses in the cache can be destroyed immediately since all commands referring

2
src/dawn_wire/client/Buffer.cpp
View File

@ -367,7 +367,6 @@ namespace dawn_wire { namespace client {
void* Buffer::GetMappedRange() {
if (!IsMappedForWriting()) {
device->InjectError(WGPUErrorType_Validation, "Buffer needs to be mapped for writing");
return nullptr;
}
return mMappedData;
@ -375,7 +374,6 @@ namespace dawn_wire { namespace client {
const void* Buffer::GetConstMappedRange() {
if (!IsMappedForWriting() && !IsMappedForReading()) {
device->InjectError(WGPUErrorType_Validation, "Buffer needs to be mapped");
return nullptr;
}
return mMappedData;

54
src/tests/end2end/BufferTests.cpp
View File

@ -127,7 +127,21 @@ TEST_P(BufferMapReadTests, GetMappedRange) {
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
const void* mappedData = MapReadAsyncAndWait(buffer);
ASSERT_EQ(mappedData, buffer.GetConstMappedRange());
ASSERT_EQ(buffer.GetConstMappedRange(), mappedData);
ASSERT_NE(buffer.GetConstMappedRange(), nullptr);
UnmapBuffer(buffer);
}
// Test the result of GetMappedRange when mapped for reading for a zero-sized buffer.
TEST_P(BufferMapReadTests, GetMappedRangeZeroSized) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
const void* mappedData = MapReadAsyncAndWait(buffer);
ASSERT_EQ(buffer.GetConstMappedRange(), mappedData);
ASSERT_NE(buffer.GetConstMappedRange(), nullptr);
UnmapBuffer(buffer);
}
@ -273,8 +287,23 @@ TEST_P(BufferMapWriteTests, GetMappedRange) {
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
void* mappedData = MapWriteAsyncAndWait(buffer);
ASSERT_EQ(mappedData, buffer.GetMappedRange());
ASSERT_EQ(mappedData, buffer.GetConstMappedRange());
ASSERT_EQ(buffer.GetMappedRange(), mappedData);
ASSERT_EQ(buffer.GetMappedRange(), buffer.GetConstMappedRange());
ASSERT_NE(buffer.GetMappedRange(), nullptr);
UnmapBuffer(buffer);
}
// Test the result of GetMappedRange when mapped for writing for a zero-sized buffer.
TEST_P(BufferMapWriteTests, GetMappedRangeZeroSized) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc;
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
void* mappedData = MapWriteAsyncAndWait(buffer);
ASSERT_EQ(buffer.GetMappedRange(), mappedData);
ASSERT_EQ(buffer.GetMappedRange(), buffer.GetConstMappedRange());
ASSERT_NE(buffer.GetMappedRange(), nullptr);
UnmapBuffer(buffer);
}
@ -538,8 +567,23 @@ TEST_P(CreateBufferMappedTests, GetMappedRange) {
wgpu::CreateBufferMappedResult result;
result = device.CreateBufferMapped(&descriptor);
ASSERT_EQ(result.data, result.buffer.GetMappedRange());
ASSERT_EQ(result.data, result.buffer.GetConstMappedRange());
ASSERT_EQ(result.buffer.GetMappedRange(), result.data);
ASSERT_EQ(result.buffer.GetMappedRange(), result.buffer.GetConstMappedRange());
ASSERT_NE(result.buffer.GetMappedRange(), nullptr);
result.buffer.Unmap();
}
// Test the result of GetMappedRange when mapped at creation for a zero-sized buffer.
TEST_P(CreateBufferMappedTests, GetMappedRangeZeroSized) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::CopyDst;
wgpu::CreateBufferMappedResult result;
result = device.CreateBufferMapped(&descriptor);
ASSERT_EQ(result.buffer.GetMappedRange(), result.data);
ASSERT_EQ(result.buffer.GetMappedRange(), result.buffer.GetConstMappedRange());
ASSERT_NE(result.buffer.GetMappedRange(), nullptr);
result.buffer.Unmap();
}

67
src/tests/end2end/DeviceLostTests.cpp
View File

@ -332,6 +332,73 @@ TEST_P(DeviceLostTest, WriteBufferFails) {
ASSERT_DEVICE_ERROR(queue.WriteBuffer(buffer, 0, &data, sizeof(data)));
}
// Test it's possible to GetMappedRange on a buffer created mapped after device loss
// TODO(cwallez@chromium.org): enable after CreateBufferMapped is implemented in terms of
// mappedAtCreation.
TEST_P(DeviceLostTest, DISABLED_GetMappedRange_CreateBufferMappedAfterLoss) {
SetCallbackAndLoseForTesting();
wgpu::BufferDescriptor desc;
desc.size = 4;
desc.usage = wgpu::BufferUsage::CopySrc;
ASSERT_DEVICE_ERROR(wgpu::CreateBufferMappedResult result = device.CreateBufferMapped(&desc));
ASSERT_NE(result.buffer.GetMappedRange(), nullptr);
ASSERT_EQ(result.buffer.GetMappedRange(), result.data);
}
// Test that device loss doesn't change the result of GetMappedRange, createBufferMapped version.
TEST_P(DeviceLostTest, GetMappedRange_CreateBufferMappedBeforeLoss) {
wgpu::BufferDescriptor desc;
desc.size = 4;
desc.usage = wgpu::BufferUsage::CopySrc;
wgpu::CreateBufferMappedResult result = device.CreateBufferMapped(&desc);
void* rangeBeforeLoss = result.buffer.GetMappedRange();
SetCallbackAndLoseForTesting();
ASSERT_NE(result.buffer.GetMappedRange(), nullptr);
ASSERT_EQ(result.buffer.GetMappedRange(), rangeBeforeLoss);
ASSERT_EQ(result.buffer.GetMappedRange(), result.data);
}
// Test that device loss doesn't change the result of GetMappedRange, mapReadAsync version.
TEST_P(DeviceLostTest, GetMappedRange_MapReadAsync) {
wgpu::BufferDescriptor desc;
desc.size = 4;
desc.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
wgpu::Buffer buffer = device.CreateBuffer(&desc);
buffer.MapReadAsync(nullptr, nullptr);
queue.Submit(0, nullptr);
const void* rangeBeforeLoss = buffer.GetConstMappedRange();
SetCallbackAndLoseForTesting();
ASSERT_NE(buffer.GetConstMappedRange(), nullptr);
ASSERT_EQ(buffer.GetConstMappedRange(), rangeBeforeLoss);
}
// Test that device loss doesn't change the result of GetMappedRange, mapReadAsync version.
TEST_P(DeviceLostTest, GetMappedRange_MapWriteAsync) {
wgpu::BufferDescriptor desc;
desc.size = 4;
desc.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc;
wgpu::Buffer buffer = device.CreateBuffer(&desc);
buffer.MapWriteAsync(nullptr, nullptr);
queue.Submit(0, nullptr);
const void* rangeBeforeLoss = buffer.GetConstMappedRange();
SetCallbackAndLoseForTesting();
ASSERT_NE(buffer.GetConstMappedRange(), nullptr);
ASSERT_EQ(buffer.GetConstMappedRange(), rangeBeforeLoss);
}
// mapreadasync + resolve + loss getmappedrange != nullptr.
// mapwriteasync + resolve + loss getmappedrange != nullptr.
// Test that Command Encoder Finish fails when device lost
TEST_P(DeviceLostTest, CommandEncoderFinishFails) {
wgpu::CommandBuffer commands;

65
src/tests/unittests/validation/BufferValidationTests.cpp
View File

@ -672,8 +672,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
desc.usage = wgpu::BufferUsage::CopySrc;
wgpu::Buffer buf = device.CreateBuffer(&desc);
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
// Unmapped after CreateBufferMapped case.
@ -681,8 +681,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
wgpu::Buffer buf = CreateBufferMapped(4, wgpu::BufferUsage::CopySrc).buffer;
buf.Unmap();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
// Unmapped after MapReadAsync case.
@ -696,8 +696,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
queue.Submit(0, nullptr);
buf.Unmap();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
// Unmapped after MapWriteAsync case.
@ -710,8 +710,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
queue.Submit(0, nullptr);
buf.Unmap();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
}
@ -725,8 +725,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
wgpu::Buffer buf = device.CreateBuffer(&desc);
buf.Destroy();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
// Destroyed after CreateBufferMapped case.
@ -734,8 +734,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
wgpu::Buffer buf = CreateBufferMapped(4, wgpu::BufferUsage::CopySrc).buffer;
buf.Destroy();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
// Destroyed after MapReadAsync case.
@ -749,8 +749,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
queue.Submit(0, nullptr);
buf.Destroy();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
// Destroyed after MapWriteAsync case.
@ -763,8 +763,8 @@ TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
queue.Submit(0, nullptr);
buf.Destroy();
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
ASSERT_EQ(nullptr, buf.GetConstMappedRange());
}
}
@ -778,7 +778,7 @@ TEST_F(BufferValidationTest, GetMappedRangeOnMappedForReading) {
.Times(1);
queue.Submit(0, nullptr);
ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
ASSERT_EQ(nullptr, buf.GetMappedRange());
}
// Test valid cases to call GetMappedRange on a buffer.
@ -825,3 +825,34 @@ TEST_F(BufferValidationTest, GetMappedRangeValidCases) {
ASSERT_EQ(buf.GetConstMappedRange(), mappedPointer);
}
}
// Test valid cases to call GetMappedRange on an error buffer.
// TODO(cwallez@chromium.org): enable after CreateBufferMapped is implemented in terms of
// mappedAtCreation.
TEST_F(BufferValidationTest, DISABLED_GetMappedRangeOnErrorBuffer) {
wgpu::BufferDescriptor desc;
desc.size = 4;
desc.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead;
// GetMappedRange after CreateBufferMapped non-OOM returns a non-nullptr.
{
wgpu::CreateBufferMappedResult result;
ASSERT_DEVICE_ERROR(result = CreateBufferMapped(
4, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead));
ASSERT_NE(result.buffer.GetConstMappedRange(), nullptr);
ASSERT_EQ(result.buffer.GetConstMappedRange(), result.buffer.GetMappedRange());
ASSERT_EQ(result.buffer.GetConstMappedRange(), result.data);
}
// GetMappedRange after CreateBufferMapped OOM case returns nullptr.
{
wgpu::CreateBufferMappedResult result;
ASSERT_DEVICE_ERROR(result = CreateBufferMapped(
1 << 31, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead));
ASSERT_EQ(result.buffer.GetConstMappedRange(), nullptr);
ASSERT_EQ(result.buffer.GetConstMappedRange(), result.buffer.GetMappedRange());
ASSERT_EQ(result.buffer.GetConstMappedRange(), result.data);
}
}