dawn::wire::client: Make object constructors less barebones.
This uses the variadic template arguments to Client::Make to
pass more data to object constructors and move the copying of data from
descriptors into the constructors.
This required adding a descriptor to the textures created in
ReserveTexture. The descriptor should come from the outside to give the
correct reflection data to textures (for example textures from
GPUCanvasContext). So a descriptor argument is added to ReserveTexture
that currently defaults to nullptr, but will be required once current
uses are updated.
Bug: dawn:1451
Change-Id: I44cbd5718b8d75fdde3ab1321d24f69a8e2486de
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93142
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/include/dawn/wire/WireClient.h b/include/dawn/wire/WireClient.h
index c1adfb1..9b425b6 100644
--- a/include/dawn/wire/WireClient.h
+++ b/include/dawn/wire/WireClient.h
@@ -70,7 +70,10 @@
const volatile char* HandleCommands(const volatile char* commands, size_t size) override;
- ReservedTexture ReserveTexture(WGPUDevice device);
+ // TODO(dawn:1451): Remove the defaulting of descriptor once the callers are updated to provide
+ // one.
+ ReservedTexture ReserveTexture(WGPUDevice device,
+ const WGPUTextureDescriptor* descriptor = nullptr);
ReservedSwapChain ReserveSwapChain(WGPUDevice device);
ReservedDevice ReserveDevice();
ReservedInstance ReserveInstance();
diff --git a/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp b/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp
index baabaa5..3438d9b 100644
--- a/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp
@@ -26,12 +26,16 @@
public:
WireInjectTextureTests() {}
~WireInjectTextureTests() override = default;
+
+ // A placeholder texture format for ReserveTexture. The data in it doesn't matter as long as
+ // we don't call texture reflection methods.
+ WGPUTextureDescriptor placeholderDesc = {};
};
// Test that reserving and injecting a texture makes calls on the client object forward to the
// server object correctly.
TEST_F(WireInjectTextureTests, CallAfterReserveInject) {
- ReservedTexture reservation = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc);
WGPUTexture apiTexture = api.GetNewTexture();
EXPECT_CALL(api, TextureReference(apiTexture));
@@ -46,8 +50,8 @@
// Test that reserve correctly returns different IDs each time.
TEST_F(WireInjectTextureTests, ReserveDifferentIDs) {
- ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device);
- ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device, &placeholderDesc);
+ ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device, &placeholderDesc);
ASSERT_NE(reservation1.id, reservation2.id);
ASSERT_NE(reservation1.texture, reservation2.texture);
@@ -55,7 +59,7 @@
// Test that injecting the same id without a destroy first fails.
TEST_F(WireInjectTextureTests, InjectExistingID) {
- ReservedTexture reservation = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc);
WGPUTexture apiTexture = api.GetNewTexture();
EXPECT_CALL(api, TextureReference(apiTexture));
@@ -70,7 +74,7 @@
// Test that the server only borrows the texture and does a single reference-release
TEST_F(WireInjectTextureTests, InjectedTextureLifetime) {
- ReservedTexture reservation = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc);
// Injecting the texture adds a reference
WGPUTexture apiTexture = api.GetNewTexture();
@@ -93,17 +97,17 @@
TEST_F(WireInjectTextureTests, ReclaimTextureReservation) {
// Test that doing a reservation and full release is an error.
{
- ReservedTexture reservation = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc);
wgpuTextureRelease(reservation.texture);
FlushClient(false);
}
// Test that doing a reservation and then reclaiming it recycles the ID.
{
- ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device, &placeholderDesc);
GetWireClient()->ReclaimTextureReservation(reservation1);
- ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device);
+ ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device, &placeholderDesc);
// The ID is the same, but the generation is still different.
ASSERT_EQ(reservation1.id, reservation2.id);
@@ -114,4 +118,26 @@
}
}
+// Test the reflection of texture creation parameters for reserved textures.
+TEST_F(WireInjectTextureTests, ReservedTextureReflection) {
+ WGPUTextureDescriptor desc = {};
+ desc.size = {10, 11, 12};
+ desc.format = WGPUTextureFormat_R32Float;
+ desc.dimension = WGPUTextureDimension_3D;
+ desc.mipLevelCount = 1000;
+ desc.sampleCount = 3;
+ desc.usage = WGPUTextureUsage_RenderAttachment;
+
+ ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &desc);
+ WGPUTexture texture = reservation.texture;
+
+ ASSERT_EQ(desc.size.width, wgpuTextureGetWidth(texture));
+ ASSERT_EQ(desc.size.height, wgpuTextureGetHeight(texture));
+ ASSERT_EQ(desc.size.depthOrArrayLayers, wgpuTextureGetDepthOrArrayLayers(texture));
+ ASSERT_EQ(desc.format, wgpuTextureGetFormat(texture));
+ ASSERT_EQ(desc.dimension, wgpuTextureGetDimension(texture));
+ ASSERT_EQ(desc.mipLevelCount, wgpuTextureGetMipLevelCount(texture));
+ ASSERT_EQ(desc.sampleCount, wgpuTextureGetSampleCount(texture));
+}
+
} // namespace dawn::wire
diff --git a/src/dawn/wire/WireClient.cpp b/src/dawn/wire/WireClient.cpp
index 624cc03..9845b38 100644
--- a/src/dawn/wire/WireClient.cpp
+++ b/src/dawn/wire/WireClient.cpp
@@ -28,8 +28,9 @@
return mImpl->HandleCommands(commands, size);
}
-ReservedTexture WireClient::ReserveTexture(WGPUDevice device) {
- return mImpl->ReserveTexture(device);
+ReservedTexture WireClient::ReserveTexture(WGPUDevice device,
+ const WGPUTextureDescriptor* descriptor) {
+ return mImpl->ReserveTexture(device, descriptor);
}
ReservedSwapChain WireClient::ReserveSwapChain(WGPUDevice device) {
diff --git a/src/dawn/wire/client/Buffer.cpp b/src/dawn/wire/client/Buffer.cpp
index aaf6282..32da663 100644
--- a/src/dawn/wire/client/Buffer.cpp
+++ b/src/dawn/wire/client/Buffer.cpp
@@ -74,11 +74,7 @@
// Create the buffer and send the creation command.
// This must happen after any potential device->CreateErrorBuffer()
// as server expects allocating ids to be monotonically increasing
- Buffer* buffer = wireClient->Make<Buffer>();
- buffer->mDevice = device;
- buffer->mDeviceIsAlive = device->GetAliveWeakPtr();
- buffer->mSize = descriptor->size;
- buffer->mUsage = static_cast<WGPUBufferUsage>(descriptor->usage);
+ Buffer* buffer = wireClient->Make<Buffer>(device, descriptor);
buffer->mDestructWriteHandleOnUnmap = false;
if (descriptor->mappedAtCreation) {
@@ -126,12 +122,7 @@
// static
WGPUBuffer Buffer::CreateError(Device* device, const WGPUBufferDescriptor* descriptor) {
Client* client = device->GetClient();
-
- Buffer* buffer = client->Make<Buffer>();
- buffer->mDevice = device;
- buffer->mDeviceIsAlive = device->GetAliveWeakPtr();
- buffer->mSize = descriptor->size;
- buffer->mUsage = static_cast<WGPUBufferUsage>(descriptor->usage);
+ Buffer* buffer = client->Make<Buffer>(device, descriptor);
DeviceCreateErrorBufferCmd cmd;
cmd.self = ToAPI(device);
@@ -141,6 +132,14 @@
return ToAPI(buffer);
}
+Buffer::Buffer(const ObjectBaseParams& params,
+ Device* device,
+ const WGPUBufferDescriptor* descriptor)
+ : ObjectBase(params),
+ mSize(descriptor->size),
+ mUsage(static_cast<WGPUBufferUsage>(descriptor->usage)),
+ mDeviceIsAlive(device->GetAliveWeakPtr()) {}
+
Buffer::~Buffer() {
ClearAllCallbacks(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
FreeMappedData();
diff --git a/src/dawn/wire/client/Buffer.h b/src/dawn/wire/client/Buffer.h
index 8484164..8040016 100644
--- a/src/dawn/wire/client/Buffer.h
+++ b/src/dawn/wire/client/Buffer.h
@@ -31,7 +31,7 @@
static WGPUBuffer Create(Device* device, const WGPUBufferDescriptor* descriptor);
static WGPUBuffer CreateError(Device* device, const WGPUBufferDescriptor* descriptor);
- using ObjectBase::ObjectBase;
+ Buffer(const ObjectBaseParams& params, Device* device, const WGPUBufferDescriptor* descriptor);
~Buffer() override;
bool OnMapAsyncCallback(uint64_t requestSerial,
@@ -63,8 +63,6 @@
void FreeMappedData();
- Device* mDevice;
-
enum class MapRequestType { None, Read, Write };
enum class MapState {
diff --git a/src/dawn/wire/client/Client.cpp b/src/dawn/wire/client/Client.cpp
index 6c5e631..9b9fc1a 100644
--- a/src/dawn/wire/client/Client.cpp
+++ b/src/dawn/wire/client/Client.cpp
@@ -81,8 +81,22 @@
}
}
-ReservedTexture Client::ReserveTexture(WGPUDevice device) {
- Texture* texture = Make<Texture>();
+ReservedTexture Client::ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor) {
+ // Make a fake descriptor so that data returned by wgpu::Texture getters isn't garbage.
+ // TODO(dawn:1451): Remove this defaulting once the descriptor is required for ReserveTexture.
+ WGPUTextureDescriptor defaultDescriptor = {};
+ if (descriptor == nullptr) {
+ defaultDescriptor.size = {1, 1, 1};
+ defaultDescriptor.mipLevelCount = 1;
+ defaultDescriptor.sampleCount = 1;
+ defaultDescriptor.dimension = WGPUTextureDimension_1D;
+ defaultDescriptor.format = WGPUTextureFormat_RGBA8Unorm;
+ defaultDescriptor.usage = 0;
+
+ descriptor = &defaultDescriptor;
+ }
+
+ Texture* texture = Make<Texture>(descriptor);
ReservedTexture result;
result.texture = ToAPI(texture);
diff --git a/src/dawn/wire/client/Client.h b/src/dawn/wire/client/Client.h
index d7e29d3..8648522 100644
--- a/src/dawn/wire/client/Client.h
+++ b/src/dawn/wire/client/Client.h
@@ -70,7 +70,7 @@
MemoryTransferService* GetMemoryTransferService() const { return mMemoryTransferService; }
- ReservedTexture ReserveTexture(WGPUDevice device);
+ ReservedTexture ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor);
ReservedSwapChain ReserveSwapChain(WGPUDevice device);
ReservedDevice ReserveDevice();
ReservedInstance ReserveInstance();
diff --git a/src/dawn/wire/client/QuerySet.cpp b/src/dawn/wire/client/QuerySet.cpp
index 4a22a0c..6ecbaa4 100644
--- a/src/dawn/wire/client/QuerySet.cpp
+++ b/src/dawn/wire/client/QuerySet.cpp
@@ -22,11 +22,7 @@
// static
WGPUQuerySet QuerySet::Create(Device* device, const WGPUQuerySetDescriptor* descriptor) {
Client* wireClient = device->GetClient();
- QuerySet* querySet = wireClient->Make<QuerySet>();
-
- // Copy over descriptor data for reflection.
- querySet->mType = descriptor->type;
- querySet->mCount = descriptor->count;
+ QuerySet* querySet = wireClient->Make<QuerySet>(descriptor);
// Send the Device::CreateQuerySet command without modifications.
DeviceCreateQuerySetCmd cmd;
@@ -39,6 +35,9 @@
return ToAPI(querySet);
}
+QuerySet::QuerySet(const ObjectBaseParams& params, const WGPUQuerySetDescriptor* descriptor)
+ : ObjectBase(params), mType(descriptor->type), mCount(descriptor->count) {}
+
QuerySet::~QuerySet() = default;
WGPUQueryType QuerySet::GetType() const {
diff --git a/src/dawn/wire/client/QuerySet.h b/src/dawn/wire/client/QuerySet.h
index fc5a73e..84c8099 100644
--- a/src/dawn/wire/client/QuerySet.h
+++ b/src/dawn/wire/client/QuerySet.h
@@ -27,7 +27,7 @@
public:
static WGPUQuerySet Create(Device* device, const WGPUQuerySetDescriptor* descriptor);
- using ObjectBase::ObjectBase;
+ QuerySet(const ObjectBaseParams& params, const WGPUQuerySetDescriptor* descriptor);
~QuerySet() override;
// Note that these values can be arbitrary since they aren't validated in the wire client.
diff --git a/src/dawn/wire/client/Texture.cpp b/src/dawn/wire/client/Texture.cpp
index 1fffde2..40a9873 100644
--- a/src/dawn/wire/client/Texture.cpp
+++ b/src/dawn/wire/client/Texture.cpp
@@ -22,15 +22,7 @@
// static
WGPUTexture Texture::Create(Device* device, const WGPUTextureDescriptor* descriptor) {
Client* wireClient = device->GetClient();
- Texture* texture = wireClient->Make<Texture>();
-
- // Copy over descriptor data for reflection.
- texture->mSize = descriptor->size;
- texture->mMipLevelCount = descriptor->mipLevelCount;
- texture->mSampleCount = descriptor->sampleCount;
- texture->mDimension = descriptor->dimension;
- texture->mFormat = descriptor->format;
- texture->mUsage = static_cast<WGPUTextureUsage>(descriptor->usage);
+ Texture* texture = wireClient->Make<Texture>(descriptor);
// Send the Device::CreateTexture command without modifications.
DeviceCreateTextureCmd cmd;
@@ -43,6 +35,15 @@
return ToAPI(texture);
}
+Texture::Texture(const ObjectBaseParams& params, const WGPUTextureDescriptor* descriptor)
+ : ObjectBase(params),
+ mSize(descriptor->size),
+ mMipLevelCount(descriptor->mipLevelCount),
+ mSampleCount(descriptor->sampleCount),
+ mDimension(descriptor->dimension),
+ mFormat(descriptor->format),
+ mUsage(static_cast<WGPUTextureUsage>(descriptor->usage)) {}
+
Texture::~Texture() = default;
uint32_t Texture::GetWidth() const {
diff --git a/src/dawn/wire/client/Texture.h b/src/dawn/wire/client/Texture.h
index 10bc5fa..4cbd230 100644
--- a/src/dawn/wire/client/Texture.h
+++ b/src/dawn/wire/client/Texture.h
@@ -27,7 +27,7 @@
public:
static WGPUTexture Create(Device* device, const WGPUTextureDescriptor* descriptor);
- using ObjectBase::ObjectBase;
+ Texture(const ObjectBaseParams& params, const WGPUTextureDescriptor* descriptor);
~Texture() override;
// Note that these values can be arbitrary since they aren't validated in the wire client.
