From 79c9d125f2a0d12a3c36aa7c4e8417be54b6ae33 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 31 Mar 2021 11:24:42 +0000 Subject: [PATCH] Propagate more errors from reentrant calls This CL propagates errors from reentrant WriteBuffer calls and makes procy methods on BufferBase to show that it is safe to call GetMappedRange and Unmap without handling errors. Bug: dawn:723 Change-Id: I4ea43adc4844505314bf84e2357b2d928f1d1f8c Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/46003 Auto-Submit: Corentin Wallez Commit-Queue: Corentin Wallez Reviewed-by: Stephen White --- src/dawn_native/Buffer.cpp | 16 ++++++------ src/dawn_native/Buffer.h | 4 ++- src/dawn_native/CommandEncoder.cpp | 25 ++++++++++--------- .../CopyTextureForBrowserHelper.cpp | 5 ++-- src/dawn_native/Queue.cpp | 10 ++++---- src/dawn_native/Queue.h | 8 +++--- src/dawn_native/opengl/TextureGL.cpp | 6 ++--- 7 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index 5c2528ad96..e8bb23f50d 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -186,13 +186,11 @@ namespace dawn_native { DeviceBase* device = GetDevice(); if (device->IsToggleEnabled(Toggle::LazyClearResourceOnFirstUse)) { - // TODO(dawn:723): propagate any errors from GetMappedRange. - memset(APIGetMappedRange(0, mSize), uint8_t(0u), mSize); + memset(GetMappedRange(0, mSize), uint8_t(0u), mSize); SetIsDataInitialized(); device->IncrementLazyClearCountForTesting(); } else if (device->IsToggleEnabled(Toggle::NonzeroClearResourcesOnCreationForTesting)) { - // TODO(dawn:723): propagate any errors from GetMappedRange. - memset(APIGetMappedRange(0, mSize), uint8_t(1u), mSize); + memset(GetMappedRange(0, mSize), uint8_t(1u), mSize); } return {}; @@ -294,14 +292,14 @@ namespace dawn_native { } void* BufferBase::APIGetMappedRange(size_t offset, size_t size) { - return GetMappedRangeInternal(true, offset, size); + return GetMappedRange(offset, size, true); } const void* BufferBase::APIGetConstMappedRange(size_t offset, size_t size) { - return GetMappedRangeInternal(false, offset, size); + return GetMappedRange(offset, size, false); } - void* BufferBase::GetMappedRangeInternal(bool writable, size_t offset, size_t size) { + void* BufferBase::GetMappedRange(size_t offset, size_t size, bool writable) { if (!CanGetMappedRange(writable, offset, size)) { return nullptr; } @@ -357,6 +355,10 @@ namespace dawn_native { } void BufferBase::APIUnmap() { + Unmap(); + } + + void BufferBase::Unmap() { UnmapInternal(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback); } diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h index 30d77086cb..8e848da981 100644 --- a/src/dawn_native/Buffer.h +++ b/src/dawn_native/Buffer.h @@ -61,6 +61,9 @@ namespace dawn_native { bool IsDataInitialized() const; void SetIsDataInitialized(); + void* GetMappedRange(size_t offset, size_t size, bool writable = true); + void Unmap(); + // Dawn API void APIMapAsync(wgpu::MapMode mode, size_t offset, @@ -91,7 +94,6 @@ namespace dawn_native { virtual bool IsCPUWritableAtCreation() const = 0; MaybeError CopyFromStagingBuffer(); - void* GetMappedRangeInternal(bool writable, size_t offset, size_t size); void CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status); MaybeError ValidateMap(wgpu::BufferUsage requiredUsage, diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 0e757906be..743bc7c0f4 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -399,11 +399,11 @@ namespace dawn_native { return {}; } - void EncodeTimestampsToNanosecondsConversion(CommandEncoder* encoder, - QuerySetBase* querySet, - uint32_t queryCount, - BufferBase* destination, - uint64_t destinationOffset) { + MaybeError EncodeTimestampsToNanosecondsConversion(CommandEncoder* encoder, + QuerySetBase* querySet, + uint32_t queryCount, + BufferBase* destination, + uint64_t destinationOffset) { DeviceBase* device = encoder->GetDevice(); // The availability got from query set is a reference to vector, need to covert @@ -419,9 +419,9 @@ namespace dawn_native { // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. Ref availabilityBuffer = AcquireRef(device->APICreateBuffer(&availabilityDesc)); - // TODO(dawn:723): propagate any errors from WriteBuffer. - device->GetQueue()->APIWriteBuffer(availabilityBuffer.Get(), 0, availability.data(), - availability.size() * sizeof(uint32_t)); + DAWN_TRY(device->GetQueue()->WriteBuffer(availabilityBuffer.Get(), 0, + availability.data(), + availability.size() * sizeof(uint32_t))); // Timestamp params uniform buffer TimestampParams params = {queryCount, static_cast(destinationOffset), @@ -431,11 +431,12 @@ namespace dawn_native { parmsDesc.size = sizeof(params); // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. Ref paramsBuffer = AcquireRef(device->APICreateBuffer(&parmsDesc)); - // TODO(dawn:723): propagate any errors from WriteBuffer. - device->GetQueue()->APIWriteBuffer(paramsBuffer.Get(), 0, ¶ms, sizeof(params)); + DAWN_TRY( + device->GetQueue()->WriteBuffer(paramsBuffer.Get(), 0, ¶ms, sizeof(params))); EncodeConvertTimestampsToNanoseconds(encoder, destination, availabilityBuffer.Get(), paramsBuffer.Get()); + return {}; } } // namespace @@ -869,8 +870,8 @@ namespace dawn_native { // Encode internal compute pipeline for timestamp query if (querySet->GetQueryType() == wgpu::QueryType::Timestamp && GetDevice()->IsToggleEnabled(Toggle::ConvertTimestampsToNanoseconds)) { - EncodeTimestampsToNanosecondsConversion(this, querySet, queryCount, destination, - destinationOffset); + DAWN_TRY(EncodeTimestampsToNanosecondsConversion(this, querySet, queryCount, + destination, destinationOffset)); } return {}; diff --git a/src/dawn_native/CopyTextureForBrowserHelper.cpp b/src/dawn_native/CopyTextureForBrowserHelper.cpp index c5b145e355..5d20bd5967 100644 --- a/src/dawn_native/CopyTextureForBrowserHelper.cpp +++ b/src/dawn_native/CopyTextureForBrowserHelper.cpp @@ -273,9 +273,8 @@ namespace dawn_native { // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. Ref uniformBuffer = AcquireRef(device->APICreateBuffer(&uniformDesc)); - // TODO(dawn:723): propagate any errors from WriteBuffer. - device->GetQueue()->APIWriteBuffer(uniformBuffer.Get(), 0, uniformData, - sizeof(uniformData)); + DAWN_TRY(device->GetQueue()->WriteBuffer(uniformBuffer.Get(), 0, uniformData, + sizeof(uniformData))); // Prepare binding 1 resource: sampler // Use default configuration, filterMode set to Nearest for min and mag. diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index d319b64503..124832c595 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -266,13 +266,13 @@ namespace dawn_native { uint64_t bufferOffset, const void* data, size_t size) { - GetDevice()->ConsumedError(WriteBufferInternal(buffer, bufferOffset, data, size)); + GetDevice()->ConsumedError(WriteBuffer(buffer, bufferOffset, data, size)); } - MaybeError QueueBase::WriteBufferInternal(BufferBase* buffer, - uint64_t bufferOffset, - const void* data, - size_t size) { + MaybeError QueueBase::WriteBuffer(BufferBase* buffer, + uint64_t bufferOffset, + const void* data, + size_t size) { DAWN_TRY(ValidateWriteBuffer(buffer, bufferOffset, size)); return WriteBufferImpl(buffer, bufferOffset, data, size); } diff --git a/src/dawn_native/Queue.h b/src/dawn_native/Queue.h index d9d9ee4372..e0f59c4fee 100644 --- a/src/dawn_native/Queue.h +++ b/src/dawn_native/Queue.h @@ -57,6 +57,10 @@ namespace dawn_native { const Extent3D* copySize, const CopyTextureForBrowserOptions* options); + MaybeError WriteBuffer(BufferBase* buffer, + uint64_t bufferOffset, + const void* data, + size_t size); void TrackTask(std::unique_ptr task, ExecutionSerial serial); void Tick(ExecutionSerial finishedSerial); void HandleDeviceLoss(); @@ -66,10 +70,6 @@ namespace dawn_native { QueueBase(DeviceBase* device, ObjectBase::ErrorTag tag); private: - MaybeError WriteBufferInternal(BufferBase* buffer, - uint64_t bufferOffset, - const void* data, - size_t size); MaybeError WriteTextureInternal(const ImageCopyTexture* destination, const void* data, size_t dataSize, diff --git a/src/dawn_native/opengl/TextureGL.cpp b/src/dawn_native/opengl/TextureGL.cpp index 208a48e8b0..c7fc9ef74b 100644 --- a/src/dawn_native/opengl/TextureGL.cpp +++ b/src/dawn_native/opengl/TextureGL.cpp @@ -383,10 +383,8 @@ namespace dawn_native { namespace opengl { DAWN_TRY_ASSIGN(srcBuffer, Buffer::CreateInternalBuffer(device, &descriptor, false)); // Fill the buffer with clear color - // TODO(dawn:723): propagate any errors from GetMappedRange. - memset(srcBuffer->APIGetMappedRange(0, descriptor.size), clearColor, descriptor.size); - // TODO(dawn:723): propagate any errors from Unmap. - srcBuffer->APIUnmap(); + memset(srcBuffer->GetMappedRange(0, descriptor.size), clearColor, descriptor.size); + srcBuffer->Unmap(); // Bind buffer and texture, and make the buffer to texture copy gl.PixelStorei(GL_UNPACK_ROW_LENGTH,