Add Reserve/InjectBuffer to Wire
Adds reserve and inject functionality for buffers to the wire. Includes basic tests. This is necessary for Chromium to utilize SharedBufferMemory.
Bug: dawn:2382
Change-Id: I9599c1bff32815a0437c21e8810dc0cb5a6623bb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/186825
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Brandon1 Jones <brandon1.jones@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/include/dawn/wire/WireClient.h b/include/dawn/wire/WireClient.h
index a1e268a..82cb2d7 100644
--- a/include/dawn/wire/WireClient.h
+++ b/include/dawn/wire/WireClient.h
@@ -43,6 +43,12 @@
DAWN_WIRE_EXPORT const DawnProcTable& GetProcs();
} // namespace client
+struct ReservedBuffer {
+ WGPUBuffer buffer;
+ Handle handle;
+ Handle deviceHandle;
+};
+
struct ReservedTexture {
WGPUTexture texture;
Handle handle;
@@ -77,11 +83,13 @@
const volatile char* HandleCommands(const volatile char* commands, size_t size) override;
+ ReservedBuffer ReserveBuffer(WGPUDevice device, const WGPUBufferDescriptor* descriptor);
ReservedTexture ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor);
ReservedSwapChain ReserveSwapChain(WGPUDevice device,
const WGPUSwapChainDescriptor* descriptor);
ReservedInstance ReserveInstance(const WGPUInstanceDescriptor* descriptor = nullptr);
+ void ReclaimBufferReservation(const ReservedBuffer& reservation);
void ReclaimTextureReservation(const ReservedTexture& reservation);
void ReclaimSwapChainReservation(const ReservedSwapChain& reservation);
void ReclaimDeviceReservation(const ReservedDevice& reservation);
diff --git a/include/dawn/wire/WireServer.h b/include/dawn/wire/WireServer.h
index a3b9972..eb1a68a 100644
--- a/include/dawn/wire/WireServer.h
+++ b/include/dawn/wire/WireServer.h
@@ -54,6 +54,7 @@
const volatile char* HandleCommands(const volatile char* commands, size_t size) override;
+ bool InjectBuffer(WGPUBuffer buffer, const Handle& handle, const Handle& deviceHandle);
bool InjectTexture(WGPUTexture texture, const Handle& handle, const Handle& deviceHandle);
bool InjectSwapChain(WGPUSwapChain swapchain, const Handle& handle, const Handle& deviceHandle);
bool InjectInstance(WGPUInstance instance, const Handle& handle);
diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn
index e858d83..6158b5d 100644
--- a/src/dawn/tests/BUILD.gn
+++ b/src/dawn/tests/BUILD.gn
@@ -437,6 +437,7 @@
"unittests/wire/WireExtensionTests.cpp",
"unittests/wire/WireFutureTest.cpp",
"unittests/wire/WireFutureTest.h",
+ "unittests/wire/WireInjectBufferTests.cpp",
"unittests/wire/WireInjectInstanceTests.cpp",
"unittests/wire/WireInjectSwapChainTests.cpp",
"unittests/wire/WireInjectTextureTests.cpp",
diff --git a/src/dawn/tests/unittests/wire/WireInjectBufferTests.cpp b/src/dawn/tests/unittests/wire/WireInjectBufferTests.cpp
new file mode 100644
index 0000000..f6abb29
--- /dev/null
+++ b/src/dawn/tests/unittests/wire/WireInjectBufferTests.cpp
@@ -0,0 +1,181 @@
+// Copyright 2024 The Dawn & Tint Authors
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice, this
+// list of conditions and the following disclaimer.
+//
+// 2. Redistributions in binary form must reproduce the above copyright notice,
+// this list of conditions and the following disclaimer in the documentation
+// and/or other materials provided with the distribution.
+//
+// 3. Neither the name of the copyright holder nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "dawn/tests/unittests/wire/WireTest.h"
+
+#include "dawn/wire/WireClient.h"
+#include "dawn/wire/WireServer.h"
+
+namespace dawn::wire {
+namespace {
+
+using testing::Mock;
+using testing::Return;
+
+class WireInjectBufferTests : public WireTest {
+ public:
+ WireInjectBufferTests() {}
+ ~WireInjectBufferTests() override = default;
+
+ // A placeholder buffer format for ReserveBuffer. The data in it doesn't matter as long as
+ // we don't call buffer reflection methods.
+ WGPUBufferDescriptor placeholderDesc = {};
+};
+
+// Test that reserving and injecting a buffer makes calls on the client object forward to the
+// server object correctly.
+TEST_F(WireInjectBufferTests, CallAfterReserveInject) {
+ auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+
+ WGPUBuffer apiBuffer = api.GetNewBuffer();
+ EXPECT_CALL(api, BufferAddRef(apiBuffer));
+ ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+
+ wgpuBufferDestroy(reserved.buffer);
+ EXPECT_CALL(api, BufferDestroy(apiBuffer));
+ FlushClient();
+}
+
+// Test that reserve correctly returns different IDs each time.
+TEST_F(WireInjectBufferTests, ReserveDifferentIDs) {
+ auto reserved1 = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+ auto reserved2 = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+
+ ASSERT_NE(reserved1.handle.id, reserved2.handle.id);
+ ASSERT_NE(reserved1.buffer, reserved2.buffer);
+}
+
+// Test that injecting the same id without a destroy first fails.
+TEST_F(WireInjectBufferTests, InjectExistingID) {
+ auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+
+ WGPUBuffer apiBuffer = api.GetNewBuffer();
+ EXPECT_CALL(api, BufferAddRef(apiBuffer));
+ ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+
+ // ID already in use, call fails.
+ ASSERT_FALSE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+}
+
+// Test that injecting the same id without a destroy first fails.
+TEST_F(WireInjectBufferTests, ReuseIDAndGeneration) {
+ // Do this loop multiple times since the first time, we can't test `generation - 1` since
+ // generation == 0.
+ ReservedBuffer reserved;
+ WGPUBuffer apiBuffer = nullptr;
+ for (int i = 0; i < 2; ++i) {
+ reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+
+ apiBuffer = api.GetNewBuffer();
+ EXPECT_CALL(api, BufferAddRef(apiBuffer));
+ ASSERT_TRUE(
+ GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+
+ // Release the buffer. It should be possible to reuse the ID now, but not the generation
+ wgpuBufferRelease(reserved.buffer);
+ EXPECT_CALL(api, BufferRelease(apiBuffer));
+ FlushClient();
+
+ // Invalid to inject with the same ID and generation.
+ ASSERT_FALSE(
+ GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+ if (i > 0) {
+ EXPECT_GE(reserved.handle.generation, 1u);
+
+ // Invalid to inject with the same ID and lesser generation.
+ reserved.handle.generation -= 1;
+ ASSERT_FALSE(
+ GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+ }
+ }
+
+ // Valid to inject with the same ID and greater generation.
+ EXPECT_CALL(api, BufferAddRef(apiBuffer));
+ reserved.handle.generation += 2;
+ ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+}
+
+// Test that the server only borrows the buffer and does a single reference-release
+TEST_F(WireInjectBufferTests, InjectedBufferLifetime) {
+ auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+
+ // Injecting the buffer adds a reference
+ WGPUBuffer apiBuffer = api.GetNewBuffer();
+ EXPECT_CALL(api, BufferAddRef(apiBuffer));
+ ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle));
+
+ // Releasing the buffer removes a single reference.
+ wgpuBufferRelease(reserved.buffer);
+ EXPECT_CALL(api, BufferRelease(apiBuffer));
+ FlushClient();
+
+ // Deleting the server doesn't release a second reference.
+ DeleteServer();
+ Mock::VerifyAndClearExpectations(&api);
+}
+
+// Test that a buffer reservation can be reclaimed. This is necessary to
+// avoid leaking ObjectIDs for reservations that are never injected.
+TEST_F(WireInjectBufferTests, ReclaimBufferReservation) {
+ // Test that doing a reservation and full release is an error.
+ {
+ auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+ wgpuBufferRelease(reserved.buffer);
+ FlushClient(false);
+ }
+
+ // Test that doing a reservation and then reclaiming it recycles the ID.
+ {
+ auto reserved1 = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+ GetWireClient()->ReclaimBufferReservation(reserved1);
+
+ auto reserved2 = GetWireClient()->ReserveBuffer(device, &placeholderDesc);
+
+ // The ID is the same, but the generation is still different.
+ ASSERT_EQ(reserved1.handle.id, reserved2.handle.id);
+ ASSERT_NE(reserved1.handle.generation, reserved2.handle.generation);
+
+ // No errors should occur.
+ FlushClient();
+ }
+}
+
+// Test the reflection of buffer creation parameters for reserved buffer.
+TEST_F(WireInjectBufferTests, ReservedBufferReflection) {
+ WGPUBufferDescriptor desc = {};
+ desc.size = 10;
+ desc.usage = WGPUBufferUsage_Storage;
+
+ auto reserved = GetWireClient()->ReserveBuffer(device, &desc);
+ WGPUBuffer buffer = reserved.buffer;
+
+ ASSERT_EQ(desc.size, wgpuBufferGetSize(buffer));
+ ASSERT_EQ(desc.usage, wgpuBufferGetUsage(buffer));
+}
+
+} // anonymous namespace
+} // namespace dawn::wire
diff --git a/src/dawn/wire/WireClient.cpp b/src/dawn/wire/WireClient.cpp
index aa15aa2..e616232 100644
--- a/src/dawn/wire/WireClient.cpp
+++ b/src/dawn/wire/WireClient.cpp
@@ -41,6 +41,11 @@
return mImpl->HandleCommands(commands, size);
}
+ReservedBuffer WireClient::ReserveBuffer(WGPUDevice device,
+ const WGPUBufferDescriptor* descriptor) {
+ return mImpl->ReserveBuffer(device, descriptor);
+}
+
ReservedTexture WireClient::ReserveTexture(WGPUDevice device,
const WGPUTextureDescriptor* descriptor) {
return mImpl->ReserveTexture(device, descriptor);
@@ -55,6 +60,10 @@
return mImpl->ReserveInstance(descriptor);
}
+void WireClient::ReclaimBufferReservation(const ReservedBuffer& reservation) {
+ mImpl->ReclaimBufferReservation(reservation);
+}
+
void WireClient::ReclaimTextureReservation(const ReservedTexture& reservation) {
mImpl->ReclaimTextureReservation(reservation);
}
diff --git a/src/dawn/wire/WireServer.cpp b/src/dawn/wire/WireServer.cpp
index 9437e54..9f34ae0 100644
--- a/src/dawn/wire/WireServer.cpp
+++ b/src/dawn/wire/WireServer.cpp
@@ -43,6 +43,10 @@
return mImpl->HandleCommands(commands, size);
}
+bool WireServer::InjectBuffer(WGPUBuffer buffer, const Handle& handle, const Handle& deviceHandle) {
+ return mImpl->InjectBuffer(buffer, handle, deviceHandle) == WireResult::Success;
+}
+
bool WireServer::InjectTexture(WGPUTexture texture,
const Handle& handle,
const Handle& deviceHandle) {
diff --git a/src/dawn/wire/client/Client.cpp b/src/dawn/wire/client/Client.cpp
index 3d9aac3..3cc08d6 100644
--- a/src/dawn/wire/client/Client.cpp
+++ b/src/dawn/wire/client/Client.cpp
@@ -99,6 +99,16 @@
}
}
+ReservedBuffer Client::ReserveBuffer(WGPUDevice device, const WGPUBufferDescriptor* descriptor) {
+ Buffer* buffer = Make<Buffer>(FromAPI(device)->GetEventManagerHandle(), descriptor);
+
+ ReservedBuffer result;
+ result.buffer = ToAPI(buffer);
+ result.handle = buffer->GetWireHandle();
+ result.deviceHandle = FromAPI(device)->GetWireHandle();
+ return result;
+}
+
ReservedTexture Client::ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor) {
Texture* texture = Make<Texture>(descriptor);
@@ -137,6 +147,10 @@
return result;
}
+void Client::ReclaimBufferReservation(const ReservedBuffer& reservation) {
+ Free(FromAPI(reservation.buffer));
+}
+
void Client::ReclaimTextureReservation(const ReservedTexture& reservation) {
Free(FromAPI(reservation.texture));
}
diff --git a/src/dawn/wire/client/Client.h b/src/dawn/wire/client/Client.h
index 0140680..d389bd5 100644
--- a/src/dawn/wire/client/Client.h
+++ b/src/dawn/wire/client/Client.h
@@ -83,11 +83,13 @@
MemoryTransferService* GetMemoryTransferService() const { return mMemoryTransferService; }
+ ReservedBuffer ReserveBuffer(WGPUDevice device, const WGPUBufferDescriptor* descriptor);
ReservedTexture ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor);
ReservedSwapChain ReserveSwapChain(WGPUDevice device,
const WGPUSwapChainDescriptor* descriptor);
ReservedInstance ReserveInstance(const WGPUInstanceDescriptor* descriptor);
+ void ReclaimBufferReservation(const ReservedBuffer& reservation);
void ReclaimTextureReservation(const ReservedTexture& reservation);
void ReclaimSwapChainReservation(const ReservedSwapChain& reservation);
void ReclaimDeviceReservation(const ReservedDevice& reservation);
diff --git a/src/dawn/wire/server/Server.cpp b/src/dawn/wire/server/Server.cpp
index c835a3f..b363c19 100644
--- a/src/dawn/wire/server/Server.cpp
+++ b/src/dawn/wire/server/Server.cpp
@@ -61,6 +61,30 @@
DestroyAllObjects(mProcs);
}
+WireResult Server::InjectBuffer(WGPUBuffer buffer,
+ const Handle& handle,
+ const Handle& deviceHandle) {
+ DAWN_ASSERT(buffer != nullptr);
+ Known<WGPUDevice> device;
+ WIRE_TRY(Objects<WGPUDevice>().Get(deviceHandle.id, &device));
+ if (device->generation != deviceHandle.generation) {
+ return WireResult::FatalError;
+ }
+
+ Reserved<WGPUBuffer> data;
+ WIRE_TRY(Objects<WGPUBuffer>().Allocate(&data, handle));
+
+ data->handle = buffer;
+ data->generation = handle.generation;
+ data->state = AllocationState::Allocated;
+
+ // The Buffer is externally owned so it shouldn't be destroyed when we receive a destroy
+ // message from the client. Add a reference to counterbalance the eventual release.
+ mProcs.bufferAddRef(buffer);
+
+ return WireResult::Success;
+}
+
WireResult Server::InjectTexture(WGPUTexture texture,
const Handle& handle,
const Handle& deviceHandle) {
diff --git a/src/dawn/wire/server/Server.h b/src/dawn/wire/server/Server.h
index ba1b07f..2d29945 100644
--- a/src/dawn/wire/server/Server.h
+++ b/src/dawn/wire/server/Server.h
@@ -180,6 +180,7 @@
// ChunkedCommandHandler implementation
const volatile char* HandleCommandsImpl(const volatile char* commands, size_t size) override;
+ WireResult InjectBuffer(WGPUBuffer buffer, const Handle& handle, const Handle& deviceHandle);
WireResult InjectTexture(WGPUTexture texture, const Handle& handle, const Handle& deviceHandle);
WireResult InjectSwapChain(WGPUSwapChain swapchain,
const Handle& handle,
