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>

include/dawn/wire/WireClient.h

@@ -70,7 +70,10 @@ class DAWN_WIRE_EXPORT WireClient : public CommandHandler {
 
     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();

src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp

@@ -26,12 +26,16 @@ class WireInjectTextureTests : public WireTest {
   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_F(WireInjectTextureTests, CallAfterReserveInject) {
 
 // Test that reserve correctly returns different IDs each time.
 TEST_F(WireInjectTextureTests, ReserveDifferentIDs) {
-    ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device);
+    ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device, &placeholderDesc);
-    ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device);
+    ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device, &placeholderDesc);
 
     ASSERT_NE(reservation1.id, reservation2.id);
     ASSERT_NE(reservation1.texture, reservation2.texture);
@@ -55,7 +59,7 @@ TEST_F(WireInjectTextureTests, ReserveDifferentIDs) {
 
 // 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_F(WireInjectTextureTests, InjectExistingID) {
 
 // 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, InjectedTextureLifetime) {
 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_F(WireInjectTextureTests, ReclaimTextureReservation) {
     }
 }
 
+// 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

src/dawn/wire/WireClient.cpp

@@ -28,8 +28,9 @@ const volatile char* WireClient::HandleCommands(const volatile char* commands, s
     return mImpl->HandleCommands(commands, size);
 }
 
-ReservedTexture WireClient::ReserveTexture(WGPUDevice device) {
+ReservedTexture WireClient::ReserveTexture(WGPUDevice device,
-    return mImpl->ReserveTexture(device);
+                                           const WGPUTextureDescriptor* descriptor) {
+    return mImpl->ReserveTexture(device, descriptor);
 }
 
 ReservedSwapChain WireClient::ReserveSwapChain(WGPUDevice device) {

src/dawn/wire/client/Buffer.cpp

@@ -74,11 +74,7 @@ WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor
     // 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* buffer = wireClient->Make<Buffer>(device, descriptor);
-    buffer->mDevice = device;
-    buffer->mDeviceIsAlive = device->GetAliveWeakPtr();
-    buffer->mSize = descriptor->size;
-    buffer->mUsage = static_cast<WGPUBufferUsage>(descriptor->usage);
     buffer->mDestructWriteHandleOnUnmap = false;
 
     if (descriptor->mappedAtCreation) {
@@ -126,12 +122,7 @@ WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor
 // static
 WGPUBuffer Buffer::CreateError(Device* device, const WGPUBufferDescriptor* descriptor) {
     Client* client = device->GetClient();
-
+    Buffer* buffer = client->Make<Buffer>(device, descriptor);
-    Buffer* buffer = client->Make<Buffer>();
-    buffer->mDevice = device;
-    buffer->mDeviceIsAlive = device->GetAliveWeakPtr();
-    buffer->mSize = descriptor->size;
-    buffer->mUsage = static_cast<WGPUBufferUsage>(descriptor->usage);
 
     DeviceCreateErrorBufferCmd cmd;
     cmd.self = ToAPI(device);
@@ -141,6 +132,14 @@ WGPUBuffer Buffer::CreateError(Device* device, const WGPUBufferDescriptor* descr
     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();

src/dawn/wire/client/Buffer.h

@@ -31,7 +31,7 @@ class Buffer final : public ObjectBase {
     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 @@ class Buffer final : public ObjectBase {
 
     void FreeMappedData();
 
-    Device* mDevice;
-
     enum class MapRequestType { None, Read, Write };
 
     enum class MapState {

src/dawn/wire/client/Client.cpp

@@ -81,8 +81,22 @@ void Client::DestroyAllObjects() {
     }
 }
 
-ReservedTexture Client::ReserveTexture(WGPUDevice device) {
+ReservedTexture Client::ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor) {
-    Texture* texture = Make<Texture>();
+    // 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);

src/dawn/wire/client/Client.h

@@ -70,7 +70,7 @@ class Client : public ClientBase {
 
     MemoryTransferService* GetMemoryTransferService() const { return mMemoryTransferService; }
 
-    ReservedTexture ReserveTexture(WGPUDevice device);
+    ReservedTexture ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor);
     ReservedSwapChain ReserveSwapChain(WGPUDevice device);
     ReservedDevice ReserveDevice();
     ReservedInstance ReserveInstance();

src/dawn/wire/client/QuerySet.cpp

@@ -22,11 +22,7 @@ namespace dawn::wire::client {
 // static
 WGPUQuerySet QuerySet::Create(Device* device, const WGPUQuerySetDescriptor* descriptor) {
     Client* wireClient = device->GetClient();
-    QuerySet* querySet = wireClient->Make<QuerySet>();
+    QuerySet* querySet = wireClient->Make<QuerySet>(descriptor);
-
-    // Copy over descriptor data for reflection.
-    querySet->mType = descriptor->type;
-    querySet->mCount = descriptor->count;
 
     // Send the Device::CreateQuerySet command without modifications.
     DeviceCreateQuerySetCmd cmd;
@@ -39,6 +35,9 @@ WGPUQuerySet QuerySet::Create(Device* device, const WGPUQuerySetDescriptor* desc
     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 {

src/dawn/wire/client/QuerySet.h

@@ -27,7 +27,7 @@ class QuerySet final : public ObjectBase {
   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.

src/dawn/wire/client/Texture.cpp

@@ -22,15 +22,7 @@ namespace dawn::wire::client {
 // static
 WGPUTexture Texture::Create(Device* device, const WGPUTextureDescriptor* descriptor) {
     Client* wireClient = device->GetClient();
-    Texture* texture = wireClient->Make<Texture>();
+    Texture* texture = wireClient->Make<Texture>(descriptor);
-
-    // 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);
 
     // Send the Device::CreateTexture command without modifications.
     DeviceCreateTextureCmd cmd;
@@ -43,6 +35,15 @@ WGPUTexture Texture::Create(Device* device, const WGPUTextureDescriptor* descrip
     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 {

src/dawn/wire/client/Texture.h

@@ -27,7 +27,7 @@ class Texture final : public ObjectBase {
   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.