Add Buffer::Get[|Const]MappedRange

This CL adds GetMappedRange reusing the existing GetMappedPointerImpl
call in dawn_native. In dawn_wire tracking is added to keep a
Buffer::mMappedData around.

Tests are added to test the result of Get[|Const]MappedRange in all
buffer states.

Bug: dawn:445
Change-Id: I3737dc4d36f31d392839952da0b5c0d10c7c8a88
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/23861
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

dawn.json

@@ -197,6 +197,14 @@
                     {"name": "userdata", "type": "void", "annotation": "*"}
                 ]
             },
+            {
+                "name": "get mapped range",
+                "returns": "void *"
+            },
+            {
+                "name": "get const mapped range",
+                "returns": "void const *"
+            },
             {
                 "name": "unmap"
             },
@@ -1690,6 +1698,12 @@
     "void": {
         "category": "native"
     },
+    "void *": {
+        "category": "native"
+    },
+    "void const *": {
+        "category": "native"
+    },
     "uint32_t": {
         "category": "native"
     },

dawn_wire.json

@@ -97,6 +97,8 @@
             "BufferMapReadAsync",
             "BufferMapWriteAsync",
             "BufferSetSubData",
+            "BufferGetConstMappedRange",
+            "BufferGetMappedRange",
             "DevicePopErrorScope",
             "DeviceSetDeviceLostCallback",
             "DeviceSetUncapturedErrorCallback",

src/dawn_native/Buffer.cpp

@@ -305,6 +305,26 @@ namespace dawn_native {
         tracker->Track(this, mMapSerial, true);
     }
 
+    void* BufferBase::GetMappedRange() {
+        if (GetDevice()->ConsumedError(ValidateGetMappedRange(true))) {
+            return nullptr;
+        }
+        if (mStagingBuffer != nullptr) {
+            return mStagingBuffer->GetMappedPointer();
+        }
+        return GetMappedPointerImpl();
+    }
+
+    const void* BufferBase::GetConstMappedRange() {
+        if (GetDevice()->ConsumedError(ValidateGetMappedRange(false))) {
+            return nullptr;
+        }
+        if (mStagingBuffer != nullptr) {
+            return mStagingBuffer->GetMappedPointer();
+        }
+        return GetMappedPointerImpl();
+    }
+
     void BufferBase::Destroy() {
         if (IsError()) {
             // It is an error to call Destroy() on an ErrorBuffer, but we still need to reclaim the
@@ -406,6 +426,29 @@ namespace dawn_native {
         return {};
     }
 
+    MaybeError BufferBase::ValidateGetMappedRange(bool writable) const {
+        DAWN_TRY(GetDevice()->ValidateIsAlive());
+        DAWN_TRY(GetDevice()->ValidateObject(this));
+
+        switch (mState) {
+            // Writeable Buffer::GetMappedRange is always allowed when mapped at creation.
+            case BufferState::MappedAtCreation:
+                return {};
+
+            case BufferState::Mapped:
+                if (writable && !(mUsage & wgpu::BufferUsage::MapWrite)) {
+                    return DAWN_VALIDATION_ERROR("GetMappedRange requires the MapWrite usage");
+                }
+                return {};
+
+            case BufferState::Unmapped:
+            case BufferState::Destroyed:
+                return DAWN_VALIDATION_ERROR("Buffer is not mapped");
+            default:
+                UNREACHABLE();
+        }
+    }
+
     MaybeError BufferBase::ValidateUnmap() const {
         DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));

src/dawn_native/Buffer.h

@@ -61,6 +61,8 @@ namespace dawn_native {
         void SetSubData(uint32_t start, uint32_t count, const void* data);
         void MapReadAsync(WGPUBufferMapReadCallback callback, void* userdata);
         void MapWriteAsync(WGPUBufferMapWriteCallback callback, void* userdata);
+        void* GetMappedRange();
+        const void* GetConstMappedRange();
         void Unmap();
         void Destroy();
 
@@ -94,6 +96,7 @@ namespace dawn_native {
 
         MaybeError ValidateMap(wgpu::BufferUsage requiredUsage,
                                WGPUBufferMapAsyncStatus* status) const;
+        MaybeError ValidateGetMappedRange(bool writable) const;
         MaybeError ValidateUnmap() const;
         MaybeError ValidateDestroy() const;
 

src/dawn_native/metal/BufferMTL.mm

@@ -120,7 +120,7 @@ namespace dawn_native { namespace metal {
     }
 
     void* Buffer::GetMappedPointerImpl() {
-        return reinterpret_cast<uint8_t*>([mMtlBuffer contents]);
+        return [mMtlBuffer contents];
     }
 
     void Buffer::UnmapImpl() {

src/dawn_wire/client/ApiProcs.cpp

@@ -40,6 +40,16 @@ namespace dawn_wire { namespace client {
         buffer->SetSubData(start, count, data);
     }
 
+    void* ClientHandwrittenBufferGetMappedRange(WGPUBuffer cBuffer) {
+        Buffer* buffer = reinterpret_cast<Buffer*>(cBuffer);
+        return buffer->GetMappedRange();
+    }
+
+    const void* ClientHandwrittenBufferGetConstMappedRange(WGPUBuffer cBuffer) {
+        Buffer* buffer = reinterpret_cast<Buffer*>(cBuffer);
+        return buffer->GetConstMappedRange();
+    }
+
     void ClientHandwrittenBufferUnmap(WGPUBuffer cBuffer) {
         Buffer* buffer = reinterpret_cast<Buffer*>(cBuffer);
         buffer->Unmap();

src/dawn_wire/client/Buffer.cpp

@@ -122,6 +122,7 @@ namespace dawn_wire { namespace client {
         buffer->mSize = descriptor->size;
         // Successfully created staging memory. The buffer now owns the WriteHandle.
         buffer->mWriteHandle = std::move(writeHandle);
+        buffer->mMappedData = result.data;
 
         result.buffer = reinterpret_cast<WGPUBuffer>(buffer);
 
@@ -252,8 +253,8 @@ namespace dawn_wire { namespace client {
         // second time. If, for example, buffer.Unmap() is called inside the callback.
         mRequests.erase(requestIt);
 
-        const void* mappedData = nullptr;
         size_t mappedDataLength = 0;
+        const void* mappedData = nullptr;
 
         auto GetMappedData = [&]() -> bool {
             // It is an error for the server to call the read callback when we asked for a map write
@@ -298,7 +299,8 @@ namespace dawn_wire { namespace client {
             request.readCallback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0, request.userdata);
             return false;
         } else {
-            request.readCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mappedData,
+            mMappedData = const_cast<void*>(mappedData);
+            request.readCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mMappedData,
                                  static_cast<uint64_t>(mappedDataLength), request.userdata);
             return true;
         }
@@ -316,8 +318,8 @@ namespace dawn_wire { namespace client {
         // second time. If, for example, buffer.Unmap() is called inside the callback.
         mRequests.erase(requestIt);
 
-        void* mappedData = nullptr;
         size_t mappedDataLength = 0;
+        void* mappedData = nullptr;
 
         auto GetMappedData = [&]() -> bool {
             // It is an error for the server to call the write callback when we asked for a map read
@@ -355,12 +357,31 @@ namespace dawn_wire { namespace client {
                                   request.userdata);
             return false;
         } else {
+            mMappedData = mappedData;
             request.writeCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mappedData,
                                   static_cast<uint64_t>(mappedDataLength), request.userdata);
             return true;
         }
     }
 
+    void* Buffer::GetMappedRange() {
+        if (!IsMappedForWriting()) {
+            ClientDeviceInjectError(reinterpret_cast<WGPUDevice>(device), WGPUErrorType_Validation,
+                                    "Buffer needs to be mapped for writing");
+            return nullptr;
+        }
+        return mMappedData;
+    }
+
+    const void* Buffer::GetConstMappedRange() {
+        if (!IsMappedForWriting() && !IsMappedForReading()) {
+            ClientDeviceInjectError(reinterpret_cast<WGPUDevice>(device), WGPUErrorType_Validation,
+                                    "Buffer needs to be mapped");
+            return nullptr;
+        }
+        return mMappedData;
+    }
+
     void Buffer::Unmap() {
         // 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
@@ -394,6 +415,7 @@ namespace dawn_wire { namespace client {
         } else if (mReadHandle) {
             mReadHandle = nullptr;
         }
+        mMappedData = nullptr;
         ClearMapRequests(WGPUBufferMapAsyncStatus_Unknown);
 
         BufferUnmapCmd cmd;
@@ -405,6 +427,7 @@ namespace dawn_wire { namespace client {
         // Cancel or remove all mappings
         mWriteHandle = nullptr;
         mReadHandle = nullptr;
+        mMappedData = nullptr;
         ClearMapRequests(WGPUBufferMapAsyncStatus_Unknown);
 
         BufferDestroyCmd cmd;
@@ -422,4 +445,12 @@ namespace dawn_wire { namespace client {
         device->GetClient()->SerializeCommand(cmd);
     }
 
+    bool Buffer::IsMappedForReading() const {
+        return mReadHandle != nullptr;
+    }
+
+    bool Buffer::IsMappedForWriting() const {
+        return mWriteHandle != nullptr;
+    }
+
 }}  // namespace dawn_wire::client

src/dawn_wire/client/Buffer.h

@@ -42,12 +42,17 @@ namespace dawn_wire { namespace client {
                                     uint64_t initialDataInfoLength,
                                     const uint8_t* initialDataInfo);
         bool OnMapWriteAsyncCallback(uint32_t requestSerial, uint32_t status);
+        void* GetMappedRange();
+        const void* GetConstMappedRange();
         void Unmap();
 
         void Destroy();
 
         void SetSubData(uint64_t start, uint64_t count, const void* data);
 
+        bool IsMappedForReading() const;
+        bool IsMappedForWriting() const;
+
       private:
         // We want to defer all the validation to the server, which means we could have multiple
         // map request in flight at a single time and need to track them separately.
@@ -70,6 +75,7 @@ namespace dawn_wire { namespace client {
         // TODO(enga): Use a tagged pointer to save space.
         std::unique_ptr<MemoryTransferService::ReadHandle> mReadHandle = nullptr;
         std::unique_ptr<MemoryTransferService::WriteHandle> mWriteHandle = nullptr;
+        void* mMappedData = nullptr;
     };
 
 }}  // namespace dawn_wire::client

src/tests/end2end/BufferTests.cpp

@@ -119,6 +119,18 @@ TEST_P(BufferMapReadTests, ZeroSized) {
     UnmapBuffer(buffer);
 }
 
+// Test the result of GetMappedRange when mapped for reading.
+TEST_P(BufferMapReadTests, GetMappedRange) {
+    wgpu::BufferDescriptor descriptor;
+    descriptor.size = 4;
+    descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
+    wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+
+    const void* mappedData = MapReadAsyncAndWait(buffer);
+    ASSERT_EQ(mappedData, buffer.GetConstMappedRange());
+    UnmapBuffer(buffer);
+}
+
 DAWN_INSTANTIATE_TEST(BufferMapReadTests, D3D12Backend(), MetalBackend(), OpenGLBackend(), VulkanBackend());
 
 class BufferMapWriteTests : public DawnTest {
@@ -253,6 +265,19 @@ TEST_P(BufferMapWriteTests, ManyWrites) {
     }
 }
 
+// Test the result of GetMappedRange when mapped for writing.
+TEST_P(BufferMapWriteTests, GetMappedRange) {
+    wgpu::BufferDescriptor descriptor;
+    descriptor.size = 4;
+    descriptor.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc;
+    wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+
+    void* mappedData = MapWriteAsyncAndWait(buffer);
+    ASSERT_EQ(mappedData, buffer.GetMappedRange());
+    ASSERT_EQ(mappedData, buffer.GetConstMappedRange());
+    UnmapBuffer(buffer);
+}
+
 DAWN_INSTANTIATE_TEST(BufferMapWriteTests, D3D12Backend(), MetalBackend(), OpenGLBackend(), VulkanBackend());
 
 // TODO(enga): These tests should use the testing toggle to initialize resources to 1.
@@ -505,6 +530,19 @@ TEST_P(CreateBufferMappedTests, ZeroSizedErrorBuffer) {
     ASSERT_NE(nullptr, result.data);
 }
 
+// Test the result of GetMappedRange when mapped at creation.
+TEST_P(CreateBufferMappedTests, GetMappedRange) {
+    wgpu::BufferDescriptor descriptor;
+    descriptor.size = 4;
+    descriptor.usage = wgpu::BufferUsage::CopyDst;
+    wgpu::CreateBufferMappedResult result;
+    result = device.CreateBufferMapped(&descriptor);
+
+    ASSERT_EQ(result.data, result.buffer.GetMappedRange());
+    ASSERT_EQ(result.data, result.buffer.GetConstMappedRange());
+    result.buffer.Unmap();
+}
+
 DAWN_INSTANTIATE_TEST(CreateBufferMappedTests,
                       D3D12Backend(),
                       D3D12Backend({}, {"use_d3d12_resource_heap_tier2"}),

src/tests/unittests/validation/BufferValidationTests.cpp

@@ -662,3 +662,166 @@ TEST_F(BufferValidationTest, UnmapUnmappedBuffer) {
         buf.Unmap();
     }
 }
+
+// Test that it is invalid to call GetMappedRange on an unmapped buffer.
+TEST_F(BufferValidationTest, GetMappedRangeOnUnmappedBuffer) {
+    // Unmapped at creation case.
+    {
+        wgpu::BufferDescriptor desc;
+        desc.size = 4;
+        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()));
+    }
+
+    // Unmapped after CreateBufferMapped case.
+    {
+        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()));
+    }
+
+    // Unmapped after MapReadAsync case.
+    {
+        wgpu::Buffer buf = CreateMapReadBuffer(4);
+
+        buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr);
+        EXPECT_CALL(*mockBufferMapReadCallback,
+                    Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+            .Times(1);
+        queue.Submit(0, nullptr);
+        buf.Unmap();
+
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
+    }
+
+    // Unmapped after MapWriteAsync case.
+    {
+        wgpu::Buffer buf = CreateMapWriteBuffer(4);
+        buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr);
+        EXPECT_CALL(*mockBufferMapWriteCallback,
+                    Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+            .Times(1);
+        queue.Submit(0, nullptr);
+        buf.Unmap();
+
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
+    }
+}
+
+// Test that it is invalid to call GetMappedRange on a destroyed buffer.
+TEST_F(BufferValidationTest, GetMappedRangeOnDestroyedBuffer) {
+    // Destroyed after creation case.
+    {
+        wgpu::BufferDescriptor desc;
+        desc.size = 4;
+        desc.usage = wgpu::BufferUsage::CopySrc;
+        wgpu::Buffer buf = device.CreateBuffer(&desc);
+        buf.Destroy();
+
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
+    }
+
+    // Destroyed after CreateBufferMapped case.
+    {
+        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()));
+    }
+
+    // Destroyed after MapReadAsync case.
+    {
+        wgpu::Buffer buf = CreateMapReadBuffer(4);
+
+        buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr);
+        EXPECT_CALL(*mockBufferMapReadCallback,
+                    Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+            .Times(1);
+        queue.Submit(0, nullptr);
+        buf.Destroy();
+
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
+    }
+
+    // Destroyed after MapWriteAsync case.
+    {
+        wgpu::Buffer buf = CreateMapWriteBuffer(4);
+        buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr);
+        EXPECT_CALL(*mockBufferMapWriteCallback,
+                    Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+            .Times(1);
+        queue.Submit(0, nullptr);
+        buf.Destroy();
+
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
+        ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetConstMappedRange()));
+    }
+}
+
+// Test that it is invalid to call GetMappedRange on a buffer afterMapReadAsync
+TEST_F(BufferValidationTest, GetMappedRangeOnMappedForReading) {
+    wgpu::Buffer buf = CreateMapReadBuffer(4);
+
+    buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr);
+    EXPECT_CALL(*mockBufferMapReadCallback,
+                Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+        .Times(1);
+    queue.Submit(0, nullptr);
+
+    ASSERT_DEVICE_ERROR(ASSERT_EQ(nullptr, buf.GetMappedRange()));
+}
+
+// Test valid cases to call GetMappedRange on a buffer.
+TEST_F(BufferValidationTest, GetMappedRangeValidCases) {
+    // GetMappedRange after CreateBufferMapped case.
+    {
+        wgpu::CreateBufferMappedResult result = CreateBufferMapped(4, wgpu::BufferUsage::CopySrc);
+        ASSERT_NE(result.buffer.GetConstMappedRange(), nullptr);
+        ASSERT_EQ(result.buffer.GetConstMappedRange(), result.buffer.GetMappedRange());
+        ASSERT_EQ(result.buffer.GetConstMappedRange(), result.data);
+    }
+
+    // GetMappedRange after MapReadAsync case.
+    {
+        wgpu::Buffer buf = CreateMapReadBuffer(4);
+
+        buf.MapReadAsync(ToMockBufferMapReadCallback, nullptr);
+
+        const void* mappedPointer = nullptr;
+        EXPECT_CALL(*mockBufferMapReadCallback,
+                    Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+            .WillOnce(SaveArg<1>(&mappedPointer));
+
+        queue.Submit(0, nullptr);
+
+        ASSERT_NE(buf.GetConstMappedRange(), nullptr);
+        ASSERT_EQ(buf.GetConstMappedRange(), mappedPointer);
+    }
+
+    // GetMappedRange after MapWriteAsync case.
+    {
+        wgpu::Buffer buf = CreateMapWriteBuffer(4);
+        buf.MapWriteAsync(ToMockBufferMapWriteCallback, nullptr);
+
+        const void* mappedPointer = nullptr;
+        EXPECT_CALL(*mockBufferMapWriteCallback,
+                    Call(WGPUBufferMapAsyncStatus_Success, Ne(nullptr), 4u, _))
+            .WillOnce(SaveArg<1>(&mappedPointer));
+
+        queue.Submit(0, nullptr);
+
+        ASSERT_NE(buf.GetConstMappedRange(), nullptr);
+        ASSERT_EQ(buf.GetConstMappedRange(), buf.GetMappedRange());
+        ASSERT_EQ(buf.GetConstMappedRange(), mappedPointer);
+    }
+}