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>
diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index 0414159..cc1b6e7 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp
@@ -311,22 +311,26 @@ } 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 @@ 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 @@ } void BufferBase::OnMapCommandSerialFinished(uint32_t mapSerial, bool isWrite) { - void* data = GetMappedPointerImpl(); + void* data = GetMappedRangeInternal(isWrite); if (isWrite) { CallMapWriteCallback(mapSerial, WGPUBufferMapAsyncStatus_Success, data, GetSize()); } else {