From cc0a54dbdb7d16ca508119c6f0dda95da9d0e5cb Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Tue, 20 Mar 2018 20:56:39 -0400 Subject: [PATCH] Implement MapWrite except in the wire. Also this MapWrite doesn't zero out memory yet. --- generator/templates/api.h | 1 + generator/templates/mock_api.cpp | 13 ++ generator/templates/mock_api.h | 6 + generator/templates/wire/WireClient.cpp | 5 + next.json | 13 ++ src/backend/Buffer.cpp | 90 +++++++-- src/backend/Buffer.h | 13 +- src/backend/d3d12/BufferD3D12.cpp | 33 +++- src/backend/d3d12/BufferD3D12.h | 14 +- src/backend/d3d12/D3D12Backend.cpp | 10 +- src/backend/d3d12/D3D12Backend.h | 6 +- src/backend/metal/BufferMTL.h | 12 +- src/backend/metal/BufferMTL.mm | 34 +++- src/backend/metal/MetalBackend.h | 6 +- src/backend/metal/MetalBackend.mm | 12 +- src/backend/null/NullBackend.cpp | 22 ++- src/backend/null/NullBackend.h | 5 +- src/backend/opengl/BufferGL.cpp | 10 +- src/backend/opengl/BufferGL.h | 1 + src/backend/vulkan/BufferVk.cpp | 33 +++- src/backend/vulkan/BufferVk.h | 13 +- src/backend/vulkan/VulkanBackend.cpp | 12 +- src/backend/vulkan/VulkanBackend.h | 6 +- src/tests/end2end/BufferTests.cpp | 80 ++++++++ .../validation/BufferValidationTests.cpp | 173 +++++++++++++++++- 25 files changed, 521 insertions(+), 102 deletions(-) diff --git a/generator/templates/api.h b/generator/templates/api.h index f88f8f3c81..a185b51336 100644 --- a/generator/templates/api.h +++ b/generator/templates/api.h @@ -38,6 +38,7 @@ typedef uint64_t nxtCallbackUserdata; typedef void (*nxtDeviceErrorCallback)(const char* message, nxtCallbackUserdata userdata); typedef void (*nxtBuilderErrorCallback)(nxtBuilderErrorStatus status, const char* message, nxtCallbackUserdata userdata1, nxtCallbackUserdata userdata2); typedef void (*nxtBufferMapReadCallback)(nxtBufferMapAsyncStatus status, const void* data, nxtCallbackUserdata userdata); +typedef void (*nxtBufferMapWriteCallback)(nxtBufferMapAsyncStatus status, void* data, nxtCallbackUserdata userdata); #ifdef __cplusplus extern "C" { diff --git a/generator/templates/mock_api.cpp b/generator/templates/mock_api.cpp index b9af99f588..1d6574a18e 100644 --- a/generator/templates/mock_api.cpp +++ b/generator/templates/mock_api.cpp @@ -64,6 +64,14 @@ void ProcTableAsClass::BufferMapReadAsync(nxtBuffer self, uint32_t start, uint32 OnBufferMapReadAsyncCallback(self, start, size, callback, userdata); } +void ProcTableAsClass::BufferMapWriteAsync(nxtBuffer self, uint32_t start, uint32_t size, nxtBufferMapWriteCallback callback, nxtCallbackUserdata userdata) { + auto object = reinterpret_cast(self); + object->mapWriteCallback = callback; + object->userdata1 = userdata; + + OnBufferMapWriteAsyncCallback(self, start, size, callback, userdata); +} + void ProcTableAsClass::CallDeviceErrorCallback(nxtDevice device, const char* message) { auto object = reinterpret_cast(device); object->deviceErrorCallback(message, object->userdata1); @@ -77,6 +85,11 @@ void ProcTableAsClass::CallMapReadCallback(nxtBuffer buffer, nxtBufferMapAsyncSt object->mapReadCallback(status, data, object->userdata1); } +void ProcTableAsClass::CallMapWriteCallback(nxtBuffer buffer, nxtBufferMapAsyncStatus status, void* data) { + auto object = reinterpret_cast(buffer); + object->mapWriteCallback(status, data, object->userdata1); +} + {% for type in by_category["object"] if type.is_builder %} void ProcTableAsClass::{{as_MethodSuffix(type.name, Name("set error callback"))}}({{as_cType(type.name)}} self, nxtBuilderErrorCallback callback, nxtCallbackUserdata userdata1, nxtCallbackUserdata userdata2) { auto object = reinterpret_cast(self); diff --git a/generator/templates/mock_api.h b/generator/templates/mock_api.h index b8b620c9c2..30a89d79a6 100644 --- a/generator/templates/mock_api.h +++ b/generator/templates/mock_api.h @@ -58,22 +58,27 @@ class ProcTableAsClass { // Stores callback and userdata and calls the On* methods void DeviceSetErrorCallback(nxtDevice self, nxtDeviceErrorCallback callback, nxtCallbackUserdata userdata); void BufferMapReadAsync(nxtBuffer self, uint32_t start, uint32_t size, nxtBufferMapReadCallback callback, nxtCallbackUserdata userdata); + void BufferMapWriteAsync(nxtBuffer self, uint32_t start, uint32_t size, nxtBufferMapWriteCallback callback, nxtCallbackUserdata userdata); + // Special cased mockable methods virtual void OnDeviceSetErrorCallback(nxtDevice device, nxtDeviceErrorCallback callback, nxtCallbackUserdata userdata) = 0; virtual void OnBuilderSetErrorCallback(nxtBufferBuilder builder, nxtBuilderErrorCallback callback, nxtCallbackUserdata userdata1, nxtCallbackUserdata userdata2) = 0; virtual void OnBufferMapReadAsyncCallback(nxtBuffer buffer, uint32_t start, uint32_t size, nxtBufferMapReadCallback callback, nxtCallbackUserdata userdata) = 0; + virtual void OnBufferMapWriteAsyncCallback(nxtBuffer buffer, uint32_t start, uint32_t size, nxtBufferMapWriteCallback callback, nxtCallbackUserdata userdata) = 0; // Calls the stored callbacks void CallDeviceErrorCallback(nxtDevice device, const char* message); void CallBuilderErrorCallback(void* builder , nxtBuilderErrorStatus status, const char* message); void CallMapReadCallback(nxtBuffer buffer, nxtBufferMapAsyncStatus status, const void* data); + void CallMapWriteCallback(nxtBuffer buffer, nxtBufferMapAsyncStatus status, void* data); struct Object { ProcTableAsClass* procs = nullptr; nxtDeviceErrorCallback deviceErrorCallback = nullptr; nxtBuilderErrorCallback builderErrorCallback = nullptr; nxtBufferMapReadCallback mapReadCallback = nullptr; + nxtBufferMapWriteCallback mapWriteCallback = nullptr; nxtCallbackUserdata userdata1 = 0; nxtCallbackUserdata userdata2 = 0; }; @@ -104,6 +109,7 @@ class MockProcTable : public ProcTableAsClass { MOCK_METHOD3(OnDeviceSetErrorCallback, void(nxtDevice device, nxtDeviceErrorCallback callback, nxtCallbackUserdata userdata)); MOCK_METHOD4(OnBuilderSetErrorCallback, void(nxtBufferBuilder builder, nxtBuilderErrorCallback callback, nxtCallbackUserdata userdata1, nxtCallbackUserdata userdata2)); MOCK_METHOD5(OnBufferMapReadAsyncCallback, void(nxtBuffer buffer, uint32_t start, uint32_t size, nxtBufferMapReadCallback callback, nxtCallbackUserdata userdata)); + MOCK_METHOD5(OnBufferMapWriteAsyncCallback, void(nxtBuffer buffer, uint32_t start, uint32_t size, nxtBufferMapWriteCallback callback, nxtCallbackUserdata userdata)); }; #endif // MOCK_NXT_H diff --git a/generator/templates/wire/WireClient.cpp b/generator/templates/wire/WireClient.cpp index bb4c80b439..9ea2f52698 100644 --- a/generator/templates/wire/WireClient.cpp +++ b/generator/templates/wire/WireClient.cpp @@ -352,6 +352,11 @@ namespace wire { *allocCmd = cmd; } + void ClientBufferMapWriteAsync(Buffer*, uint32_t, uint32_t, nxtBufferMapWriteCallback, nxtCallbackUserdata) { + // TODO(cwallez@chromium.org): Implement the wire for BufferMapWriteAsync + ASSERT(false); + } + void ProxyClientBufferUnmap(Buffer* buffer) { //* Invalidate the local pointer, and cancel all other in-flight requests that would turn into //* errors anyway (you can't double map). This prevents race when the following happens, where diff --git a/next.json b/next.json index eb9da5dc86..47917ef251 100644 --- a/next.json +++ b/next.json @@ -210,6 +210,16 @@ {"name": "userdata", "type": "callback userdata"} ] }, + { + "_comment": "Contrary to set sub data, this is in char size", + "name": "map write async", + "args": [ + {"name": "start", "type": "uint32_t"}, + {"name": "size", "type": "uint32_t"}, + {"name": "callback", "type": "buffer map write callback"}, + {"name": "userdata", "type": "callback userdata"} + ] + }, { "name": "unmap" }, @@ -257,6 +267,9 @@ "buffer map read callback": { "category": "natively defined" }, + "buffer map write callback": { + "category": "natively defined" + }, "buffer map async status": { "category": "enum", "values": [ diff --git a/src/backend/Buffer.cpp b/src/backend/Buffer.cpp index 4a4ebbe8c1..3cc638054a 100644 --- a/src/backend/Buffer.cpp +++ b/src/backend/Buffer.cpp @@ -33,7 +33,8 @@ namespace backend { BufferBase::~BufferBase() { if (mIsMapped) { - CallMapReadCallback(mMapReadSerial, NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); + CallMapReadCallback(mMapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); + CallMapWriteCallback(mMapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); } } @@ -60,12 +61,26 @@ namespace backend { void BufferBase::CallMapReadCallback(uint32_t serial, nxtBufferMapAsyncStatus status, const void* pointer) { - if (mMapReadCallback && serial == mMapReadSerial) { + if (mMapReadCallback != nullptr && serial == mMapSerial) { + ASSERT(mMapWriteCallback == nullptr); // Tag the callback as fired before firing it, otherwise it could fire a second time if // for example buffer.Unmap() is called inside the application-provided callback. nxtBufferMapReadCallback callback = mMapReadCallback; mMapReadCallback = nullptr; - callback(status, pointer, mMapReadUserdata); + callback(status, pointer, mMapUserdata); + } + } + + void BufferBase::CallMapWriteCallback(uint32_t serial, + nxtBufferMapAsyncStatus status, + void* pointer) { + if (mMapWriteCallback != nullptr && serial == mMapSerial) { + ASSERT(mMapReadCallback == nullptr); + // Tag the callback as fired before firing it, otherwise it could fire a second time if + // for example buffer.Unmap() is called inside the application-provided callback. + nxtBufferMapWriteCallback callback = mMapWriteCallback; + mMapWriteCallback = nullptr; + callback(status, pointer, mMapUserdata); } } @@ -87,30 +102,40 @@ namespace backend { uint32_t size, nxtBufferMapReadCallback callback, nxtCallbackUserdata userdata) { - if (start + size > GetSize()) { - mDevice->HandleError("Buffer map read out of range"); + if (!ValidateMapBase(start, size, nxt::BufferUsageBit::MapRead)) { callback(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); return; } - if (!(mCurrentUsage & nxt::BufferUsageBit::MapRead)) { - mDevice->HandleError("Buffer needs the map read usage bit"); - callback(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); - return; - } - - if (mIsMapped) { - mDevice->HandleError("Buffer already mapped"); - callback(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); - return; - } + ASSERT(mMapWriteCallback == nullptr); // TODO(cwallez@chromium.org): what to do on wraparound? Could cause crashes. - mMapReadSerial++; + mMapSerial++; mMapReadCallback = callback; - mMapReadUserdata = userdata; - MapReadAsyncImpl(mMapReadSerial, start, size); + mMapUserdata = userdata; mIsMapped = true; + + MapReadAsyncImpl(mMapSerial, start, size); + } + + void BufferBase::MapWriteAsync(uint32_t start, + uint32_t size, + nxtBufferMapWriteCallback callback, + nxtCallbackUserdata userdata) { + if (!ValidateMapBase(start, size, nxt::BufferUsageBit::MapWrite)) { + callback(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); + return; + } + + ASSERT(mMapReadCallback == nullptr); + + // TODO(cwallez@chromium.org): what to do on wraparound? Could cause crashes. + mMapSerial++; + mMapWriteCallback = callback; + mMapUserdata = userdata; + mIsMapped = true; + + MapWriteAsyncImpl(mMapSerial, start, size); } void BufferBase::Unmap() { @@ -121,9 +146,13 @@ namespace backend { // A map request can only be called once, so this will fire only if the request wasn't // completed before the Unmap - CallMapReadCallback(mMapReadSerial, NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); + CallMapReadCallback(mMapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); + CallMapWriteCallback(mMapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); UnmapImpl(); mIsMapped = false; + mMapReadCallback = nullptr; + mMapWriteCallback = nullptr; + mMapUserdata = 0; } bool BufferBase::IsFrozen() const { @@ -248,6 +277,27 @@ namespace backend { mPropertiesSet |= BUFFER_PROPERTY_SIZE; } + bool BufferBase::ValidateMapBase(uint32_t start, + uint32_t size, + nxt::BufferUsageBit requiredUsage) { + if (start + size > GetSize()) { + mDevice->HandleError("Buffer map read out of range"); + return false; + } + + if (mIsMapped) { + mDevice->HandleError("Buffer already mapped"); + return false; + } + + if (!(mCurrentUsage & requiredUsage)) { + mDevice->HandleError("Buffer needs the correct map usage bit"); + return false; + } + + return true; + } + // BufferViewBase BufferViewBase::BufferViewBase(BufferViewBuilder* builder) diff --git a/src/backend/Buffer.h b/src/backend/Buffer.h index 0b0e49a141..2b3d100269 100644 --- a/src/backend/Buffer.h +++ b/src/backend/Buffer.h @@ -46,6 +46,10 @@ namespace backend { uint32_t size, nxtBufferMapReadCallback callback, nxtCallbackUserdata userdata); + void MapWriteAsync(uint32_t start, + uint32_t size, + nxtBufferMapWriteCallback callback, + nxtCallbackUserdata userdata); void Unmap(); void TransitionUsage(nxt::BufferUsageBit usage); void FreezeUsage(nxt::BufferUsageBit usage); @@ -54,22 +58,27 @@ namespace backend { void CallMapReadCallback(uint32_t serial, nxtBufferMapAsyncStatus status, const void* pointer); + void CallMapWriteCallback(uint32_t serial, nxtBufferMapAsyncStatus status, void* pointer); private: virtual void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) = 0; virtual void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t size) = 0; + virtual void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t size) = 0; virtual void UnmapImpl() = 0; virtual void TransitionUsageImpl(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage) = 0; + bool ValidateMapBase(uint32_t start, uint32_t size, nxt::BufferUsageBit requiredUsage); + DeviceBase* mDevice; uint32_t mSize; nxt::BufferUsageBit mAllowedUsage = nxt::BufferUsageBit::None; nxt::BufferUsageBit mCurrentUsage = nxt::BufferUsageBit::None; nxtBufferMapReadCallback mMapReadCallback = nullptr; - nxtCallbackUserdata mMapReadUserdata = 0; - uint32_t mMapReadSerial = 0; + nxtBufferMapWriteCallback mMapWriteCallback = nullptr; + nxtCallbackUserdata mMapUserdata = 0; + uint32_t mMapSerial = 0; bool mIsFrozen = false; bool mIsMapped = false; diff --git a/src/backend/d3d12/BufferD3D12.cpp b/src/backend/d3d12/BufferD3D12.cpp index 961e1e4df8..cf49ab2b2f 100644 --- a/src/backend/d3d12/BufferD3D12.cpp +++ b/src/backend/d3d12/BufferD3D12.cpp @@ -144,8 +144,12 @@ namespace backend { namespace d3d12 { return mResource->GetGPUVirtualAddress(); } - void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data) { - CallMapReadCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); + void Buffer::OnMapCommandSerialFinished(uint32_t mapSerial, void* data, bool isWrite) { + if (isWrite) { + CallMapWriteCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); + } else { + CallMapReadCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); + } } void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) { @@ -158,8 +162,17 @@ namespace backend { namespace d3d12 { char* data = nullptr; ASSERT_SUCCESS(mResource->Map(0, &readRange, reinterpret_cast(&data))); - MapReadRequestTracker* tracker = ToBackend(GetDevice())->GetMapReadRequestTracker(); - tracker->Track(this, serial, data + start); + MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapRequestTracker(); + tracker->Track(this, serial, data + start, false); + } + + void Buffer::MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) { + D3D12_RANGE readRange = {start, start + count}; + char* data = nullptr; + ASSERT_SUCCESS(mResource->Map(0, &readRange, reinterpret_cast(&data))); + + MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapRequestTracker(); + tracker->Track(this, serial, data + start, true); } void Buffer::UnmapImpl() { @@ -204,25 +217,27 @@ namespace backend { namespace d3d12 { return mUavDesc; } - MapReadRequestTracker::MapReadRequestTracker(Device* device) : mDevice(device) { + MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) { } - MapReadRequestTracker::~MapReadRequestTracker() { + MapRequestTracker::~MapRequestTracker() { ASSERT(mInflightRequests.Empty()); } - void MapReadRequestTracker::Track(Buffer* buffer, uint32_t mapSerial, const void* data) { + void MapRequestTracker::Track(Buffer* buffer, uint32_t mapSerial, void* data, bool isWrite) { Request request; request.buffer = buffer; request.mapSerial = mapSerial; request.data = data; + request.isWrite = isWrite; mInflightRequests.Enqueue(std::move(request), mDevice->GetSerial()); } - void MapReadRequestTracker::Tick(Serial finishedSerial) { + void MapRequestTracker::Tick(Serial finishedSerial) { for (auto& request : mInflightRequests.IterateUpTo(finishedSerial)) { - request.buffer->OnMapReadCommandSerialFinished(request.mapSerial, request.data); + request.buffer->OnMapCommandSerialFinished(request.mapSerial, request.data, + request.isWrite); } mInflightRequests.ClearUpTo(finishedSerial); } diff --git a/src/backend/d3d12/BufferD3D12.h b/src/backend/d3d12/BufferD3D12.h index fd18a87e1e..d3153616fe 100644 --- a/src/backend/d3d12/BufferD3D12.h +++ b/src/backend/d3d12/BufferD3D12.h @@ -35,7 +35,7 @@ namespace backend { namespace d3d12 { bool GetResourceTransitionBarrier(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage, D3D12_RESOURCE_BARRIER* barrier); - void OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data); + void OnMapCommandSerialFinished(uint32_t mapSerial, void* data, bool isWrite); private: Device* mDevice; @@ -44,6 +44,7 @@ namespace backend { namespace d3d12 { // NXT API void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override; void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; + void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; void UnmapImpl() override; void TransitionUsageImpl(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage) override; @@ -62,12 +63,12 @@ namespace backend { namespace d3d12 { D3D12_UNORDERED_ACCESS_VIEW_DESC mUavDesc; }; - class MapReadRequestTracker { + class MapRequestTracker { public: - MapReadRequestTracker(Device* device); - ~MapReadRequestTracker(); + MapRequestTracker(Device* device); + ~MapRequestTracker(); - void Track(Buffer* buffer, uint32_t mapSerial, const void* data); + void Track(Buffer* buffer, uint32_t mapSerial, void* data, bool isWrite); void Tick(Serial finishedSerial); private: @@ -76,7 +77,8 @@ namespace backend { namespace d3d12 { struct Request { Ref buffer; uint32_t mapSerial; - const void* data; + void* data; + bool isWrite; }; SerialQueue mInflightRequests; }; diff --git a/src/backend/d3d12/D3D12Backend.cpp b/src/backend/d3d12/D3D12Backend.cpp index f27d601693..0648b6707a 100644 --- a/src/backend/d3d12/D3D12Backend.cpp +++ b/src/backend/d3d12/D3D12Backend.cpp @@ -137,7 +137,7 @@ namespace backend { namespace d3d12 { // Initialize backend services mCommandAllocatorManager = new CommandAllocatorManager(this); mDescriptorHeapAllocator = new DescriptorHeapAllocator(this); - mMapReadRequestTracker = new MapReadRequestTracker(this); + mMapRequestTracker = new MapRequestTracker(this); mResourceAllocator = new ResourceAllocator(this); mResourceUploader = new ResourceUploader(this); @@ -153,7 +153,7 @@ namespace backend { namespace d3d12 { delete mCommandAllocatorManager; delete mDescriptorHeapAllocator; - delete mMapReadRequestTracker; + delete mMapRequestTracker; delete mResourceAllocator; delete mResourceUploader; } @@ -174,8 +174,8 @@ namespace backend { namespace d3d12 { return mDescriptorHeapAllocator; } - MapReadRequestTracker* Device::GetMapReadRequestTracker() const { - return mMapReadRequestTracker; + MapRequestTracker* Device::GetMapRequestTracker() const { + return mMapRequestTracker; } ResourceAllocator* Device::GetResourceAllocator() { @@ -215,7 +215,7 @@ namespace backend { namespace d3d12 { mResourceAllocator->Tick(lastCompletedSerial); mCommandAllocatorManager->Tick(lastCompletedSerial); mDescriptorHeapAllocator->Tick(lastCompletedSerial); - mMapReadRequestTracker->Tick(lastCompletedSerial); + mMapRequestTracker->Tick(lastCompletedSerial); mUsedComObjectRefs.ClearUpTo(lastCompletedSerial); ExecuteCommandLists({}); NextSerial(); diff --git a/src/backend/d3d12/D3D12Backend.h b/src/backend/d3d12/D3D12Backend.h index 48fba23882..c76b0507a1 100644 --- a/src/backend/d3d12/D3D12Backend.h +++ b/src/backend/d3d12/D3D12Backend.h @@ -49,7 +49,7 @@ namespace backend { namespace d3d12 { class CommandAllocatorManager; class DescriptorHeapAllocator; - class MapReadRequestTracker; + class MapRequestTracker; class ResourceAllocator; class ResourceUploader; @@ -116,7 +116,7 @@ namespace backend { namespace d3d12 { ComPtr GetCommandQueue(); DescriptorHeapAllocator* GetDescriptorHeapAllocator(); - MapReadRequestTracker* GetMapReadRequestTracker() const; + MapRequestTracker* GetMapRequestTracker() const; ResourceAllocator* GetResourceAllocator(); ResourceUploader* GetResourceUploader(); @@ -143,7 +143,7 @@ namespace backend { namespace d3d12 { CommandAllocatorManager* mCommandAllocatorManager = nullptr; DescriptorHeapAllocator* mDescriptorHeapAllocator = nullptr; - MapReadRequestTracker* mMapReadRequestTracker = nullptr; + MapRequestTracker* mMapRequestTracker = nullptr; ResourceAllocator* mResourceAllocator = nullptr; ResourceUploader* mResourceUploader = nullptr; diff --git a/src/backend/metal/BufferMTL.h b/src/backend/metal/BufferMTL.h index 710bcb51e1..60f58f3e67 100644 --- a/src/backend/metal/BufferMTL.h +++ b/src/backend/metal/BufferMTL.h @@ -31,11 +31,12 @@ namespace backend { namespace metal { id GetMTLBuffer(); - void OnMapReadCommandSerialFinished(uint32_t mapSerial, uint32_t offset); + void OnMapCommandSerialFinished(uint32_t mapSerial, uint32_t offset, bool isWrite); private: void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override; void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; + void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; void UnmapImpl() override; void TransitionUsageImpl(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage) override; @@ -48,12 +49,12 @@ namespace backend { namespace metal { BufferView(BufferViewBuilder* builder); }; - class MapReadRequestTracker { + class MapRequestTracker { public: - MapReadRequestTracker(Device* device); - ~MapReadRequestTracker(); + MapRequestTracker(Device* device); + ~MapRequestTracker(); - void Track(Buffer* buffer, uint32_t mapSerial, uint32_t offset); + void Track(Buffer* buffer, uint32_t mapSerial, uint32_t offset, bool isWrite); void Tick(Serial finishedSerial); private: @@ -63,6 +64,7 @@ namespace backend { namespace metal { Ref buffer; uint32_t mapSerial; uint32_t offset; + bool isWrite; }; SerialQueue mInflightRequests; }; diff --git a/src/backend/metal/BufferMTL.mm b/src/backend/metal/BufferMTL.mm index d9c5d41674..9152668540 100644 --- a/src/backend/metal/BufferMTL.mm +++ b/src/backend/metal/BufferMTL.mm @@ -40,9 +40,13 @@ namespace backend { namespace metal { return mMtlBuffer; } - void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, uint32_t offset) { - const char* data = reinterpret_cast([mMtlBuffer contents]); - CallMapReadCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data + offset); + void Buffer::OnMapCommandSerialFinished(uint32_t mapSerial, uint32_t offset, bool isWrite) { + char* data = reinterpret_cast([mMtlBuffer contents]); + if (isWrite) { + CallMapWriteCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data + offset); + } else { + CallMapReadCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data + offset); + } } void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) { @@ -52,8 +56,13 @@ namespace backend { namespace metal { } void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t) { - MapReadRequestTracker* tracker = ToBackend(GetDevice())->GetMapReadTracker(); - tracker->Track(this, serial, start); + MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapTracker(); + tracker->Track(this, serial, start, false); + } + + void Buffer::MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t) { + MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapTracker(); + tracker->Track(this, serial, start, true); } void Buffer::UnmapImpl() { @@ -66,25 +75,30 @@ namespace backend { namespace metal { BufferView::BufferView(BufferViewBuilder* builder) : BufferViewBase(builder) { } - MapReadRequestTracker::MapReadRequestTracker(Device* device) : mDevice(device) { + MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) { } - MapReadRequestTracker::~MapReadRequestTracker() { + MapRequestTracker::~MapRequestTracker() { ASSERT(mInflightRequests.Empty()); } - void MapReadRequestTracker::Track(Buffer* buffer, uint32_t mapSerial, uint32_t offset) { + void MapRequestTracker::Track(Buffer* buffer, + uint32_t mapSerial, + uint32_t offset, + bool isWrite) { Request request; request.buffer = buffer; request.mapSerial = mapSerial; request.offset = offset; + request.isWrite = isWrite; mInflightRequests.Enqueue(std::move(request), mDevice->GetPendingCommandSerial()); } - void MapReadRequestTracker::Tick(Serial finishedSerial) { + void MapRequestTracker::Tick(Serial finishedSerial) { for (auto& request : mInflightRequests.IterateUpTo(finishedSerial)) { - request.buffer->OnMapReadCommandSerialFinished(request.mapSerial, request.offset); + request.buffer->OnMapCommandSerialFinished(request.mapSerial, request.offset, + request.isWrite); } mInflightRequests.ClearUpTo(finishedSerial); } diff --git a/src/backend/metal/MetalBackend.h b/src/backend/metal/MetalBackend.h index af95438b47..f32e7919de 100644 --- a/src/backend/metal/MetalBackend.h +++ b/src/backend/metal/MetalBackend.h @@ -81,7 +81,7 @@ namespace backend { namespace metal { return ToBackendBase(common); } - class MapReadRequestTracker; + class MapRequestTracker; class ResourceUploader; class Device : public DeviceBase { @@ -117,7 +117,7 @@ namespace backend { namespace metal { void SubmitPendingCommandBuffer(); Serial GetPendingCommandSerial(); - MapReadRequestTracker* GetMapReadTracker() const; + MapRequestTracker* GetMapTracker() const; ResourceUploader* GetResourceUploader() const; private: @@ -125,7 +125,7 @@ namespace backend { namespace metal { id mMtlDevice = nil; id mCommandQueue = nil; - MapReadRequestTracker* mMapReadTracker; + MapRequestTracker* mMapTracker; ResourceUploader* mResourceUploader; Serial mFinishedCommandSerial = 0; diff --git a/src/backend/metal/MetalBackend.mm b/src/backend/metal/MetalBackend.mm index 0d1e1df0c1..8649cc239a 100644 --- a/src/backend/metal/MetalBackend.mm +++ b/src/backend/metal/MetalBackend.mm @@ -45,7 +45,7 @@ namespace backend { namespace metal { Device::Device(id mtlDevice) : mMtlDevice(mtlDevice), - mMapReadTracker(new MapReadRequestTracker(this)), + mMapTracker(new MapRequestTracker(this)), mResourceUploader(new ResourceUploader(this)) { [mMtlDevice retain]; mCommandQueue = [mMtlDevice newCommandQueue]; @@ -65,8 +65,8 @@ namespace backend { namespace metal { [mPendingCommands release]; mPendingCommands = nil; - delete mMapReadTracker; - mMapReadTracker = nullptr; + delete mMapTracker; + mMapTracker = nullptr; delete mResourceUploader; mResourceUploader = nullptr; @@ -138,7 +138,7 @@ namespace backend { namespace metal { void Device::TickImpl() { mResourceUploader->Tick(mFinishedCommandSerial); - mMapReadTracker->Tick(mFinishedCommandSerial); + mMapTracker->Tick(mFinishedCommandSerial); // Code above might have added GPU work, submit it. This also makes sure // that even when no GPU work is happening, the serial number keeps incrementing. @@ -186,8 +186,8 @@ namespace backend { namespace metal { return mPendingCommandSerial; } - MapReadRequestTracker* Device::GetMapReadTracker() const { - return mMapReadTracker; + MapRequestTracker* Device::GetMapTracker() const { + return mMapTracker; } ResourceUploader* Device::GetResourceUploader() const { diff --git a/src/backend/null/NullBackend.cpp b/src/backend/null/NullBackend.cpp index 79a8082d09..3933243c81 100644 --- a/src/backend/null/NullBackend.cpp +++ b/src/backend/null/NullBackend.cpp @@ -113,12 +113,13 @@ namespace backend { namespace null { struct BufferMapReadOperation : PendingOperation { virtual void Execute() { - buffer->MapReadOperationCompleted(serial, ptr); + buffer->MapReadOperationCompleted(serial, ptr, isWrite); } Ref buffer; - const void* ptr; + void* ptr; uint32_t serial; + bool isWrite; }; Buffer::Buffer(BufferBuilder* builder) : BufferBase(builder) { @@ -131,8 +132,12 @@ namespace backend { namespace null { Buffer::~Buffer() { } - void Buffer::MapReadOperationCompleted(uint32_t serial, const void* ptr) { - CallMapReadCallback(serial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, ptr); + void Buffer::MapReadOperationCompleted(uint32_t serial, void* ptr, bool isWrite) { + if (isWrite) { + CallMapWriteCallback(serial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, ptr); + } else { + CallMapReadCallback(serial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, ptr); + } } void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) { @@ -142,6 +147,14 @@ namespace backend { namespace null { } void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) { + MapAsyncImplCommon(serial, start, count, false); + } + + void Buffer::MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) { + MapAsyncImplCommon(serial, start, count, true); + } + + void Buffer::MapAsyncImplCommon(uint32_t serial, uint32_t start, uint32_t count, bool isWrite) { ASSERT(start + count <= GetSize()); ASSERT(mBackingData); @@ -149,6 +162,7 @@ namespace backend { namespace null { operation->buffer = this; operation->ptr = mBackingData.get() + start; operation->serial = serial; + operation->isWrite = isWrite; ToBackend(GetDevice())->AddPendingOperation(std::unique_ptr(operation)); } diff --git a/src/backend/null/NullBackend.h b/src/backend/null/NullBackend.h index 3a26d4d66e..6ca125ddb0 100644 --- a/src/backend/null/NullBackend.h +++ b/src/backend/null/NullBackend.h @@ -132,15 +132,18 @@ namespace backend { namespace null { Buffer(BufferBuilder* builder); ~Buffer(); - void MapReadOperationCompleted(uint32_t serial, const void* ptr); + void MapReadOperationCompleted(uint32_t serial, void* ptr, bool isWrite); private: void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override; void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; + void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; void UnmapImpl() override; void TransitionUsageImpl(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage) override; + void MapAsyncImplCommon(uint32_t serial, uint32_t start, uint32_t count, bool isWrite); + std::unique_ptr mBackingData; }; diff --git a/src/backend/opengl/BufferGL.cpp b/src/backend/opengl/BufferGL.cpp index ce7cd17917..a783c18f09 100644 --- a/src/backend/opengl/BufferGL.cpp +++ b/src/backend/opengl/BufferGL.cpp @@ -38,13 +38,19 @@ namespace backend { namespace opengl { void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) { // TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high // version of OpenGL that would let us map the buffer unsynchronized. - // TODO(cwallez@chromium.org): this crashes on Mac NVIDIA, use GetBufferSubData there - // instead? glBindBuffer(GL_ARRAY_BUFFER, mBuffer); void* data = glMapBufferRange(GL_ARRAY_BUFFER, start, count, GL_MAP_READ_BIT); CallMapReadCallback(serial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); } + void Buffer::MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) { + // TODO(cwallez@chromium.org): this does GPU->CPU synchronization, we could require a high + // version of OpenGL that would let us map the buffer unsynchronized. + glBindBuffer(GL_ARRAY_BUFFER, mBuffer); + void* data = glMapBufferRange(GL_ARRAY_BUFFER, start, count, GL_MAP_WRITE_BIT); + CallMapWriteCallback(serial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); + } + void Buffer::UnmapImpl() { glBindBuffer(GL_ARRAY_BUFFER, mBuffer); glUnmapBuffer(GL_ARRAY_BUFFER); diff --git a/src/backend/opengl/BufferGL.h b/src/backend/opengl/BufferGL.h index 4dfa96a96a..7a45dd52d5 100644 --- a/src/backend/opengl/BufferGL.h +++ b/src/backend/opengl/BufferGL.h @@ -32,6 +32,7 @@ namespace backend { namespace opengl { private: void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override; void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; + void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; void UnmapImpl() override; void TransitionUsageImpl(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage) override; diff --git a/src/backend/vulkan/BufferVk.cpp b/src/backend/vulkan/BufferVk.cpp index 17a9b60703..7268f29ff2 100644 --- a/src/backend/vulkan/BufferVk.cpp +++ b/src/backend/vulkan/BufferVk.cpp @@ -154,6 +154,10 @@ namespace backend { namespace vulkan { CallMapReadCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); } + void Buffer::OnMapWriteCommandSerialFinished(uint32_t mapSerial, void* data) { + CallMapWriteCallback(mapSerial, NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, data); + } + VkBuffer Buffer::GetHandle() const { return mHandle; } @@ -186,11 +190,19 @@ namespace backend { namespace vulkan { } void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t /*count*/) { - const uint8_t* memory = mMemoryAllocation.GetMappedPointer(); + uint8_t* memory = mMemoryAllocation.GetMappedPointer(); ASSERT(memory != nullptr); - MapReadRequestTracker* tracker = ToBackend(GetDevice())->GetMapReadRequestTracker(); - tracker->Track(this, serial, memory + start); + MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapRequestTracker(); + tracker->Track(this, serial, memory + start, false); + } + + void Buffer::MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t /*count*/) { + uint8_t* memory = mMemoryAllocation.GetMappedPointer(); + ASSERT(memory != nullptr); + + MapRequestTracker* tracker = ToBackend(GetDevice())->GetMapRequestTracker(); + tracker->Track(this, serial, memory + start, true); } void Buffer::UnmapImpl() { @@ -203,25 +215,30 @@ namespace backend { namespace vulkan { RecordBarrier(commands, currentUsage, targetUsage); } - MapReadRequestTracker::MapReadRequestTracker(Device* device) : mDevice(device) { + MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) { } - MapReadRequestTracker::~MapReadRequestTracker() { + MapRequestTracker::~MapRequestTracker() { ASSERT(mInflightRequests.Empty()); } - void MapReadRequestTracker::Track(Buffer* buffer, uint32_t mapSerial, const void* data) { + void MapRequestTracker::Track(Buffer* buffer, uint32_t mapSerial, void* data, bool isWrite) { Request request; request.buffer = buffer; request.mapSerial = mapSerial; request.data = data; + request.isWrite = isWrite; mInflightRequests.Enqueue(std::move(request), mDevice->GetSerial()); } - void MapReadRequestTracker::Tick(Serial finishedSerial) { + void MapRequestTracker::Tick(Serial finishedSerial) { for (auto& request : mInflightRequests.IterateUpTo(finishedSerial)) { - request.buffer->OnMapReadCommandSerialFinished(request.mapSerial, request.data); + if (request.isWrite) { + request.buffer->OnMapWriteCommandSerialFinished(request.mapSerial, request.data); + } else { + request.buffer->OnMapReadCommandSerialFinished(request.mapSerial, request.data); + } } mInflightRequests.ClearUpTo(finishedSerial); } diff --git a/src/backend/vulkan/BufferVk.h b/src/backend/vulkan/BufferVk.h index bf05e46b97..2f5301f970 100644 --- a/src/backend/vulkan/BufferVk.h +++ b/src/backend/vulkan/BufferVk.h @@ -31,6 +31,7 @@ namespace backend { namespace vulkan { ~Buffer(); void OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data); + void OnMapWriteCommandSerialFinished(uint32_t mapSerial, void* data); VkBuffer GetHandle() const; @@ -41,6 +42,7 @@ namespace backend { namespace vulkan { private: void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override; void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; + void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; void UnmapImpl() override; void TransitionUsageImpl(nxt::BufferUsageBit currentUsage, nxt::BufferUsageBit targetUsage) override; @@ -49,12 +51,12 @@ namespace backend { namespace vulkan { DeviceMemoryAllocation mMemoryAllocation; }; - class MapReadRequestTracker { + class MapRequestTracker { public: - MapReadRequestTracker(Device* device); - ~MapReadRequestTracker(); + MapRequestTracker(Device* device); + ~MapRequestTracker(); - void Track(Buffer* buffer, uint32_t mapSerial, const void* data); + void Track(Buffer* buffer, uint32_t mapSerial, void* data, bool isWrite); void Tick(Serial finishedSerial); private: @@ -63,7 +65,8 @@ namespace backend { namespace vulkan { struct Request { Ref buffer; uint32_t mapSerial; - const void* data; + void* data; + bool isWrite; }; SerialQueue mInflightRequests; }; diff --git a/src/backend/vulkan/VulkanBackend.cpp b/src/backend/vulkan/VulkanBackend.cpp index cc597a4138..44efd0b445 100644 --- a/src/backend/vulkan/VulkanBackend.cpp +++ b/src/backend/vulkan/VulkanBackend.cpp @@ -142,7 +142,7 @@ namespace backend { namespace vulkan { mBufferUploader = new BufferUploader(this); mDeleter = new FencedDeleter(this); - mMapReadRequestTracker = new MapReadRequestTracker(this); + mMapRequestTracker = new MapRequestTracker(this); mMemoryAllocator = new MemoryAllocator(this); } @@ -181,8 +181,8 @@ namespace backend { namespace vulkan { delete mDeleter; mDeleter = nullptr; - delete mMapReadRequestTracker; - mMapReadRequestTracker = nullptr; + delete mMapRequestTracker; + mMapRequestTracker = nullptr; delete mMemoryAllocator; mMemoryAllocator = nullptr; @@ -267,7 +267,7 @@ namespace backend { namespace vulkan { CheckPassedFences(); RecycleCompletedCommands(); - mMapReadRequestTracker->Tick(mCompletedSerial); + mMapRequestTracker->Tick(mCompletedSerial); mBufferUploader->Tick(mCompletedSerial); mMemoryAllocator->Tick(mCompletedSerial); @@ -307,8 +307,8 @@ namespace backend { namespace vulkan { return mQueue; } - MapReadRequestTracker* Device::GetMapReadRequestTracker() const { - return mMapReadRequestTracker; + MapRequestTracker* Device::GetMapRequestTracker() const { + return mMapRequestTracker; } MemoryAllocator* Device::GetMemoryAllocator() const { diff --git a/src/backend/vulkan/VulkanBackend.h b/src/backend/vulkan/VulkanBackend.h index f71a9b0158..002cf1c24e 100644 --- a/src/backend/vulkan/VulkanBackend.h +++ b/src/backend/vulkan/VulkanBackend.h @@ -55,7 +55,7 @@ namespace backend { namespace vulkan { class BufferUploader; class FencedDeleter; - class MapReadRequestTracker; + class MapRequestTracker; class MemoryAllocator; struct VulkanBackendTraits { @@ -103,7 +103,7 @@ namespace backend { namespace vulkan { BufferUploader* GetBufferUploader() const; FencedDeleter* GetFencedDeleter() const; - MapReadRequestTracker* GetMapReadRequestTracker() const; + MapRequestTracker* GetMapRequestTracker() const; MemoryAllocator* GetMemoryAllocator() const; Serial GetSerial() const; @@ -170,7 +170,7 @@ namespace backend { namespace vulkan { BufferUploader* mBufferUploader = nullptr; FencedDeleter* mDeleter = nullptr; - MapReadRequestTracker* mMapReadRequestTracker = nullptr; + MapRequestTracker* mMapRequestTracker = nullptr; MemoryAllocator* mMemoryAllocator = nullptr; VkFence GetUnusedFence(); diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp index 1d6c534505..20367800e7 100644 --- a/src/tests/end2end/BufferTests.cpp +++ b/src/tests/end2end/BufferTests.cpp @@ -102,6 +102,86 @@ TEST_P(BufferMapReadTests, LargeRead) { NXT_INSTANTIATE_TEST(BufferMapReadTests, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend) +class BufferMapWriteTests : public NXTTest { + protected: + + static void MapWriteCallback(nxtBufferMapAsyncStatus status, void* data, nxtCallbackUserdata userdata) { + ASSERT_EQ(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, status); + ASSERT_NE(nullptr, data); + + auto test = reinterpret_cast(static_cast(userdata)); + test->mappedData = data; + } + + void* MapWriteAsyncAndWait(const nxt::Buffer& buffer, uint32_t start, uint32_t offset) { + buffer.MapWriteAsync(start, offset, MapWriteCallback, static_cast(reinterpret_cast(this))); + + while (mappedData == nullptr) { + WaitABit(); + } + + return mappedData; + } + + private: + void* mappedData = nullptr; +}; + +// Test that the simplest map write (one u32 at offset 0) works. +TEST_P(BufferMapWriteTests, SmallWriteAtZero) { + nxt::Buffer buffer = device.CreateBufferBuilder() + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite | nxt::BufferUsageBit::TransferSrc) + .SetInitialUsage(nxt::BufferUsageBit::MapWrite) + .GetResult(); + + uint32_t myData = 2934875; + void* mappedData = MapWriteAsyncAndWait(buffer, 0, 4); + memcpy(mappedData, &myData, sizeof(myData)); + buffer.Unmap(); + + EXPECT_BUFFER_U32_EQ(myData, buffer, 0); +} + +// Test mapping a buffer at an offset. +TEST_P(BufferMapWriteTests, SmallWriteAtOffset) { + nxt::Buffer buffer = device.CreateBufferBuilder() + .SetSize(4000) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite | nxt::BufferUsageBit::TransferSrc) + .SetInitialUsage(nxt::BufferUsageBit::MapWrite) + .GetResult(); + + uint32_t myData = 2934875; + void* mappedData = MapWriteAsyncAndWait(buffer, 2048, 4); + memcpy(mappedData, &myData, sizeof(myData)); + buffer.Unmap(); + + EXPECT_BUFFER_U32_EQ(myData, buffer, 2048); +} + +// Test mapping large ranges of a buffer. +TEST_P(BufferMapWriteTests, LargeWrite) { + constexpr uint32_t kDataSize = 1000 * 1000; + std::vector myData; + for (uint32_t i = 0; i < kDataSize; ++i) { + myData.push_back(i); + } + + nxt::Buffer buffer = device.CreateBufferBuilder() + .SetSize(static_cast(kDataSize * sizeof(uint32_t))) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite | nxt::BufferUsageBit::TransferSrc) + .SetInitialUsage(nxt::BufferUsageBit::MapWrite) + .GetResult(); + + void* mappedData = MapWriteAsyncAndWait(buffer, 0, kDataSize * sizeof(uint32_t)); + memcpy(mappedData, myData.data(), kDataSize * sizeof(uint32_t)); + buffer.Unmap(); + + EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), buffer, 0, kDataSize); +} + +NXT_INSTANTIATE_TEST(BufferMapWriteTests, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend) + class BufferSetSubDataTests : public NXTTest { }; diff --git a/src/tests/unittests/validation/BufferValidationTests.cpp b/src/tests/unittests/validation/BufferValidationTests.cpp index 03b0d2071f..d9381829ff 100644 --- a/src/tests/unittests/validation/BufferValidationTests.cpp +++ b/src/tests/unittests/validation/BufferValidationTests.cpp @@ -29,6 +29,17 @@ static void ToMockBufferMapReadCallback(nxtBufferMapAsyncStatus status, const vo mockBufferMapReadCallback->Call(status, reinterpret_cast(ptr), userdata); } +class MockBufferMapWriteCallback { + public: + MOCK_METHOD3(Call, void(nxtBufferMapAsyncStatus status, uint32_t* ptr, nxtCallbackUserdata userdata)); +}; + +static MockBufferMapWriteCallback* mockBufferMapWriteCallback = nullptr; +static void ToMockBufferMapWriteCallback(nxtBufferMapAsyncStatus status, void* ptr, nxtCallbackUserdata userdata) { + // Assume the data is uint32_t to make writing matchers easier + mockBufferMapWriteCallback->Call(status, reinterpret_cast(ptr), userdata); +} + class BufferValidationTest : public ValidationTest { protected: nxt::Buffer CreateMapReadBuffer(uint32_t size) { @@ -38,6 +49,13 @@ class BufferValidationTest : public ValidationTest { .SetInitialUsage(nxt::BufferUsageBit::MapRead) .GetResult(); } + nxt::Buffer CreateMapWriteBuffer(uint32_t size) { + return device.CreateBufferBuilder() + .SetSize(size) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite) + .SetInitialUsage(nxt::BufferUsageBit::MapWrite) + .GetResult(); + } nxt::Buffer CreateSetSubDataBuffer(uint32_t size) { return device.CreateBufferBuilder() .SetSize(size) @@ -53,11 +71,13 @@ class BufferValidationTest : public ValidationTest { ValidationTest::SetUp(); mockBufferMapReadCallback = new MockBufferMapReadCallback; + mockBufferMapWriteCallback = new MockBufferMapWriteCallback; queue = device.CreateQueueBuilder().GetResult(); } void TearDown() override { delete mockBufferMapReadCallback; + delete mockBufferMapWriteCallback; ValidationTest::TearDown(); } @@ -177,7 +197,7 @@ TEST_F(BufferValidationTest, CreationMapUsageRestrictions) { } } -// Test the success cause for mapping buffer for reading +// Test the success case for mapping buffer for reading TEST_F(BufferValidationTest, MapReadSuccess) { nxt::Buffer buf = CreateMapReadBuffer(4); @@ -191,6 +211,20 @@ TEST_F(BufferValidationTest, MapReadSuccess) { buf.Unmap(); } +// Test the success case for mapping buffer for writing +TEST_F(BufferValidationTest, MapWriteSuccess) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40598; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Ne(nullptr), userdata)) + .Times(1); + queue.Submit(0, nullptr); + + buf.Unmap(); +} + // Test map reading out of range causes an error TEST_F(BufferValidationTest, MapReadOutOfRange) { nxt::Buffer buf = CreateMapReadBuffer(4); @@ -202,6 +236,17 @@ TEST_F(BufferValidationTest, MapReadOutOfRange) { ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 5, ToMockBufferMapReadCallback, userdata)); } +// Test map writing out of range causes an error +TEST_F(BufferValidationTest, MapWriteOutOfRange) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40599; + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + ASSERT_DEVICE_ERROR(buf.MapWriteAsync(0, 5, ToMockBufferMapWriteCallback, userdata)); +} + // Test map reading a buffer with wrong current usage TEST_F(BufferValidationTest, MapReadWrongUsage) { nxt::Buffer buf = device.CreateBufferBuilder() @@ -217,6 +262,21 @@ TEST_F(BufferValidationTest, MapReadWrongUsage) { ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata)); } +// Test map writing a buffer with wrong current usage +TEST_F(BufferValidationTest, MapWriteWrongUsage) { + nxt::Buffer buf = device.CreateBufferBuilder() + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite | nxt::BufferUsageBit::TransferSrc) + .SetInitialUsage(nxt::BufferUsageBit::TransferSrc) + .GetResult(); + + nxt::CallbackUserdata userdata = 40600; + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + ASSERT_DEVICE_ERROR(buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata)); +} + // Test map reading a buffer that is already mapped TEST_F(BufferValidationTest, MapReadAlreadyMapped) { nxt::Buffer buf = CreateMapReadBuffer(4); @@ -234,7 +294,25 @@ TEST_F(BufferValidationTest, MapReadAlreadyMapped) { queue.Submit(0, nullptr); } -// Test unmapping before having the result gives UNKNOWN +// Test map writing a buffer that is already mapped +TEST_F(BufferValidationTest, MapWriteAlreadyMapped) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata1 = 40601; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata1); + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Ne(nullptr), userdata1)) + .Times(1); + + nxt::CallbackUserdata userdata2 = 40602; + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata2)) + .Times(1); + ASSERT_DEVICE_ERROR(buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata2)); + + queue.Submit(0, nullptr); +} +// TODO(cwallez@chromium.org) Test a MapWrite and already MapRead and vice-versa + +// Test unmapping before having the result gives UNKNOWN - for reading TEST_F(BufferValidationTest, MapReadUnmapBeforeResult) { nxt::Buffer buf = CreateMapReadBuffer(4); @@ -250,7 +328,23 @@ TEST_F(BufferValidationTest, MapReadUnmapBeforeResult) { queue.Submit(0, nullptr); } -// Test destroying the buffer before having the result gives UNKNOWN +// Test unmapping before having the result gives UNKNOWN - for writing +TEST_F(BufferValidationTest, MapWriteUnmapBeforeResult) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40603; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + buf.Unmap(); + + // Submitting the queue makes the null backend process map request, but the callback shouldn't + // be called again + queue.Submit(0, nullptr); +} + +// Test destroying the buffer before having the result gives UNKNOWN - for reading // TODO(cwallez@chromium.org) currently this doesn't work because the buffer doesn't know // when its external ref count reaches 0. TEST_F(BufferValidationTest, DISABLED_MapReadDestroyBeforeResult) { @@ -269,7 +363,26 @@ TEST_F(BufferValidationTest, DISABLED_MapReadDestroyBeforeResult) { queue.Submit(0, nullptr); } -// When a request is cancelled with Unmap it might still be in flight, test doing a new request +// Test destroying the buffer before having the result gives UNKNOWN - for writing +// TODO(cwallez@chromium.org) currently this doesn't work because the buffer doesn't know +// when its external ref count reaches 0. +TEST_F(BufferValidationTest, DISABLED_MapWriteDestroyBeforeResult) { + { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40604; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + } + + // Submitting the queue makes the null backend process map request, but the callback shouldn't + // be called again + queue.Submit(0, nullptr); +} + +// When a MapRead is cancelled with Unmap it might still be in flight, test doing a new request // works as expected and we don't get the cancelled request's data. TEST_F(BufferValidationTest, MapReadUnmapBeforeResultThenMapAgain) { nxt::Buffer buf = CreateMapReadBuffer(4); @@ -289,6 +402,28 @@ TEST_F(BufferValidationTest, MapReadUnmapBeforeResultThenMapAgain) { .Times(1); queue.Submit(0, nullptr); } +// TODO(cwallez@chromium.org) Test a MapWrite and already MapRead and vice-versa + +// When a MapWrite is cancelled with Unmap it might still be in flight, test doing a new request +// works as expected and we don't get the cancelled request's data. +TEST_F(BufferValidationTest, MapWriteUnmapBeforeResultThenMapAgain) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40605; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + buf.Unmap(); + + userdata ++; + + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Ne(nullptr), userdata)) + .Times(1); + queue.Submit(0, nullptr); +} // Test that the MapReadCallback isn't fired twice when unmap() is called inside the callback TEST_F(BufferValidationTest, UnmapInsideMapReadCallback) { @@ -305,6 +440,21 @@ TEST_F(BufferValidationTest, UnmapInsideMapReadCallback) { queue.Submit(0, nullptr); } +// Test that the MapWriteCallback isn't fired twice when unmap() is called inside the callback +TEST_F(BufferValidationTest, UnmapInsideMapWriteCallback) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40678; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Ne(nullptr), userdata)) + .WillOnce(InvokeWithoutArgs([&]() { + buf.Unmap(); + })); + + queue.Submit(0, nullptr); +} + // Test that the MapReadCallback isn't fired twice the buffer external refcount reaches 0 in the callback TEST_F(BufferValidationTest, DestroyInsideMapReadCallback) { nxt::Buffer buf = CreateMapReadBuffer(4); @@ -320,6 +470,21 @@ TEST_F(BufferValidationTest, DestroyInsideMapReadCallback) { queue.Submit(0, nullptr); } +// Test that the MapWriteCallback isn't fired twice the buffer external refcount reaches 0 in the callback +TEST_F(BufferValidationTest, DestroyInsideMapWriteCallback) { + nxt::Buffer buf = CreateMapWriteBuffer(4); + + nxt::CallbackUserdata userdata = 40679; + buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata); + + EXPECT_CALL(*mockBufferMapWriteCallback, Call(NXT_BUFFER_MAP_ASYNC_STATUS_SUCCESS, Ne(nullptr), userdata)) + .WillOnce(InvokeWithoutArgs([&]() { + buf = nxt::Buffer(); + })); + + queue.Submit(0, nullptr); +} + // Test the success case for Buffer::SetSubData TEST_F(BufferValidationTest, SetSubDataSuccess) { nxt::Buffer buf = CreateSetSubDataBuffer(4);