From 0daef56b493f0bf58dc2aeda20fe8e216f54f911 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Tue, 14 Jun 2022 16:13:26 +0000 Subject: [PATCH] dawn::wire::client::ObjectBase: encapsulate remaining members. ObjectBase::refcount is no longer exposed and instead Reference() and Release() methods are available to operate on it. The client is now exposed through a GetClient() accessor. Bug: dawn:1451 Change-Id: Ia47ed2de0a8f03ccc3275cb81d6335d2125060cb Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93442 Commit-Queue: Corentin Wallez Reviewed-by: Austin Eng Kokoro: Kokoro Reviewed-by: Loko Kung --- .../templates/dawn/wire/client/ApiProcs.cpp | 15 +++++++-------- src/dawn/wire/client/Adapter.cpp | 2 ++ src/dawn/wire/client/Buffer.cpp | 12 +++++++++--- src/dawn/wire/client/Device.cpp | 10 ++++++++-- src/dawn/wire/client/Instance.cpp | 2 ++ src/dawn/wire/client/ObjectBase.cpp | 18 +++++++++++++++++- src/dawn/wire/client/ObjectBase.h | 10 +++++++--- src/dawn/wire/client/QuerySet.cpp | 2 +- src/dawn/wire/client/Queue.cpp | 5 +++-- src/dawn/wire/client/ShaderModule.cpp | 1 + src/dawn/wire/client/Texture.cpp | 2 +- 11 files changed, 58 insertions(+), 21 deletions(-) diff --git a/generator/templates/dawn/wire/client/ApiProcs.cpp b/generator/templates/dawn/wire/client/ApiProcs.cpp index a637efbf48..ef46293def 100644 --- a/generator/templates/dawn/wire/client/ApiProcs.cpp +++ b/generator/templates/dawn/wire/client/ApiProcs.cpp @@ -58,7 +58,7 @@ namespace dawn::wire::client { //* For object creation, store the object ID the client will use for the result. {% if method.return_type.category == "object" %} - auto* returnObject = self->client->{{method.return_type.name.CamelCase()}}Allocator().New(self->client); + auto* returnObject = self->GetClient()->{{method.return_type.name.CamelCase()}}Allocator().New(self->GetClient()); cmd.result = returnObject->GetWireHandle(); {% endif %} @@ -69,7 +69,7 @@ namespace dawn::wire::client { {% endfor %} //* Allocate space to send the command and copy the value args over. - self->client->SerializeCommand(cmd); + self->GetClient()->SerializeCommand(cmd); {% if method.return_type.category == "object" %} return ToAPI(returnObject); @@ -86,9 +86,8 @@ namespace dawn::wire::client { //* When an object's refcount reaches 0, notify the server side of it and delete it. void Client{{as_MethodSuffix(type.name, Name("release"))}}({{cType}} cObj) { {{Type}}* obj = reinterpret_cast<{{Type}}*>(cObj); - obj->refcount --; - if (obj->refcount > 0) { + if (!obj->Release()) { return; } @@ -96,13 +95,13 @@ namespace dawn::wire::client { cmd.objectType = ObjectType::{{type.name.CamelCase()}}; cmd.objectId = obj->GetWireId(); - obj->client->SerializeCommand(cmd); - obj->client->{{type.name.CamelCase()}}Allocator().Free(obj); + Client* client = obj->GetClient(); + client->SerializeCommand(cmd); + client->{{type.name.CamelCase()}}Allocator().Free(obj); } void Client{{as_MethodSuffix(type.name, Name("reference"))}}({{cType}} cObj) { - {{Type}}* obj = reinterpret_cast<{{Type}}*>(cObj); - obj->refcount ++; + reinterpret_cast<{{Type}}*>(cObj)->Reference(); } {% endfor %} diff --git a/src/dawn/wire/client/Adapter.cpp b/src/dawn/wire/client/Adapter.cpp index 0aa81633ce..ceab7b07e3 100644 --- a/src/dawn/wire/client/Adapter.cpp +++ b/src/dawn/wire/client/Adapter.cpp @@ -65,6 +65,7 @@ void Adapter::GetProperties(WGPUAdapterProperties* properties) const { void Adapter::RequestDevice(const WGPUDeviceDescriptor* descriptor, WGPURequestDeviceCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { callback(WGPURequestDeviceStatus_Error, nullptr, "GPU connection lost", userdata); return; @@ -108,6 +109,7 @@ bool Adapter::OnRequestDeviceCallback(uint64_t requestSerial, return false; } + Client* client = GetClient(); Device* device = client->DeviceAllocator().GetObject(request.deviceObjectId); // If the return status is a failure we should give a null device to the callback and diff --git a/src/dawn/wire/client/Buffer.cpp b/src/dawn/wire/client/Buffer.cpp index df93a94f74..b66e49b113 100644 --- a/src/dawn/wire/client/Buffer.cpp +++ b/src/dawn/wire/client/Buffer.cpp @@ -26,7 +26,7 @@ namespace dawn::wire::client { // static WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor) { - Client* wireClient = device->client; + Client* wireClient = device->GetClient(); bool mappable = (descriptor->usage & (WGPUBufferUsage_MapRead | WGPUBufferUsage_MapWrite)) != 0 || @@ -125,7 +125,9 @@ WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor // static WGPUBuffer Buffer::CreateError(Device* device, const WGPUBufferDescriptor* descriptor) { - Buffer* buffer = device->client->BufferAllocator().New(device->client); + Client* client = device->GetClient(); + + Buffer* buffer = client->BufferAllocator().New(client); buffer->mDevice = device; buffer->mDeviceIsAlive = device->GetAliveWeakPtr(); buffer->mSize = descriptor->size; @@ -134,7 +136,7 @@ WGPUBuffer Buffer::CreateError(Device* device, const WGPUBufferDescriptor* descr DeviceCreateErrorBufferCmd cmd; cmd.self = ToAPI(device); cmd.result = buffer->GetWireHandle(); - device->client->SerializeCommand(cmd); + client->SerializeCommand(cmd); return ToAPI(buffer); } @@ -161,6 +163,7 @@ void Buffer::MapAsync(WGPUMapModeFlags mode, size_t size, WGPUBufferMapCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { return callback(WGPUBufferMapAsyncStatus_DeviceLost, userdata); } @@ -288,6 +291,7 @@ void Buffer::Unmap() { // - Server -> Client: Result of MapRequest1 // - Unmap locally on the client // - Server -> Client: Result of MapRequest2 + Client* client = GetClient(); // mWriteHandle can still be nullptr if buffer has been destroyed before unmap if ((mMapState == MapState::MappedForWrite || mMapState == MapState::MappedAtCreation) && @@ -350,6 +354,8 @@ void Buffer::Unmap() { } void Buffer::Destroy() { + Client* client = GetClient(); + // Remove the current mapping and destroy Read/WriteHandles. FreeMappedData(); diff --git a/src/dawn/wire/client/Device.cpp b/src/dawn/wire/client/Device.cpp index 4140bc19f0..c09cb3b670 100644 --- a/src/dawn/wire/client/Device.cpp +++ b/src/dawn/wire/client/Device.cpp @@ -151,6 +151,7 @@ void Device::SetDeviceLostCallback(WGPUDeviceLostCallback callback, void* userda bool Device::PopErrorScope(WGPUErrorCallback callback, void* userdata) { // TODO(crbug.com/dawn/1324) Replace bool return with void when users are updated. + Client* client = GetClient(); if (client->IsDisconnected()) { callback(WGPUErrorType_DeviceLost, "GPU device disconnected", userdata); return true; @@ -192,7 +193,7 @@ void Device::InjectError(WGPUErrorType type, const char* message) { cmd.self = ToAPI(this); cmd.type = type; cmd.message = message; - client->SerializeCommand(cmd); + GetClient()->SerializeCommand(cmd); } WGPUBuffer Device::CreateBuffer(const WGPUBufferDescriptor* descriptor) { @@ -219,6 +220,7 @@ WGPUQueue Device::GetQueue() { // on construction. if (mQueue == nullptr) { // Get the primary queue for this device. + Client* client = GetClient(); mQueue = client->QueueAllocator().New(client); DeviceGetQueueCmd cmd; @@ -228,13 +230,14 @@ WGPUQueue Device::GetQueue() { client->SerializeCommand(cmd); } - mQueue->refcount++; + mQueue->Reference(); return ToAPI(mQueue); } void Device::CreateComputePipelineAsync(WGPUComputePipelineDescriptor const* descriptor, WGPUCreateComputePipelineAsyncCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { return callback(WGPUCreatePipelineAsyncStatus_DeviceLost, nullptr, "GPU device disconnected", userdata); @@ -266,6 +269,7 @@ bool Device::OnCreateComputePipelineAsyncCallback(uint64_t requestSerial, return false; } + Client* client = GetClient(); auto* pipelineAllocation = client->ComputePipelineAllocator().GetObject(request.pipelineObjectID); @@ -286,6 +290,7 @@ bool Device::OnCreateComputePipelineAsyncCallback(uint64_t requestSerial, void Device::CreateRenderPipelineAsync(WGPURenderPipelineDescriptor const* descriptor, WGPUCreateRenderPipelineAsyncCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { return callback(WGPUCreatePipelineAsyncStatus_DeviceLost, nullptr, "GPU device disconnected", userdata); @@ -317,6 +322,7 @@ bool Device::OnCreateRenderPipelineAsyncCallback(uint64_t requestSerial, return false; } + Client* client = GetClient(); auto* pipelineAllocation = client->RenderPipelineAllocator().GetObject(request.pipelineObjectID); diff --git a/src/dawn/wire/client/Instance.cpp b/src/dawn/wire/client/Instance.cpp index a2c70daca1..9812f2ef89 100644 --- a/src/dawn/wire/client/Instance.cpp +++ b/src/dawn/wire/client/Instance.cpp @@ -35,6 +35,7 @@ void Instance::CancelCallbacksForDisconnect() { void Instance::RequestAdapter(const WGPURequestAdapterOptions* options, WGPURequestAdapterCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { callback(WGPURequestAdapterStatus_Error, nullptr, "GPU connection lost", userdata); return; @@ -80,6 +81,7 @@ bool Instance::OnRequestAdapterCallback(uint64_t requestSerial, return false; } + Client* client = GetClient(); Adapter* adapter = client->AdapterAllocator().GetObject(request.adapterObjectId); // If the return status is a failure we should give a null adapter to the callback and diff --git a/src/dawn/wire/client/ObjectBase.cpp b/src/dawn/wire/client/ObjectBase.cpp index 1930b40fe6..953a443b55 100644 --- a/src/dawn/wire/client/ObjectBase.cpp +++ b/src/dawn/wire/client/ObjectBase.cpp @@ -14,10 +14,12 @@ #include "dawn/wire/client/ObjectBase.h" +#include "dawn/common/Assert.h" + namespace dawn::wire::client { ObjectBase::ObjectBase(const ObjectBaseParams& params) - : client(params.client), refcount(1), mHandle(params.handle) {} + : mClient(params.client), mHandle(params.handle), mRefcount(1) {} ObjectBase::~ObjectBase() { RemoveFromList(); @@ -35,4 +37,18 @@ ObjectGeneration ObjectBase::GetWireGeneration() const { return mHandle.generation; } +Client* ObjectBase::GetClient() const { + return mClient; +} + +void ObjectBase::Reference() { + mRefcount++; +} + +bool ObjectBase::Release() { + ASSERT(mRefcount != 0); + mRefcount--; + return mRefcount == 0; +} + } // namespace dawn::wire::client diff --git a/src/dawn/wire/client/ObjectBase.h b/src/dawn/wire/client/ObjectBase.h index f91149adba..cd6042da25 100644 --- a/src/dawn/wire/client/ObjectBase.h +++ b/src/dawn/wire/client/ObjectBase.h @@ -44,13 +44,17 @@ class ObjectBase : public LinkNode { const ObjectHandle& GetWireHandle() const; ObjectId GetWireId() const; ObjectGeneration GetWireGeneration() const; + Client* GetClient() const; - // TODO(dawn:1451): Make these members private. - Client* const client; - uint32_t refcount; + void Reference(); + // Returns true if it was the last reference, indicating that the caller must destroy the + // object. + [[nodiscard]] bool Release(); private: + Client* const mClient; const ObjectHandle mHandle; + uint32_t mRefcount; }; } // namespace dawn::wire::client diff --git a/src/dawn/wire/client/QuerySet.cpp b/src/dawn/wire/client/QuerySet.cpp index 82d6079a7c..903c65ddca 100644 --- a/src/dawn/wire/client/QuerySet.cpp +++ b/src/dawn/wire/client/QuerySet.cpp @@ -21,7 +21,7 @@ namespace dawn::wire::client { // static WGPUQuerySet QuerySet::Create(Device* device, const WGPUQuerySetDescriptor* descriptor) { - Client* wireClient = device->client; + Client* wireClient = device->GetClient(); QuerySet* querySet = wireClient->QuerySetAllocator().New(wireClient); // Copy over descriptor data for reflection. diff --git a/src/dawn/wire/client/Queue.cpp b/src/dawn/wire/client/Queue.cpp index 83c1281d80..bc5a4bbd28 100644 --- a/src/dawn/wire/client/Queue.cpp +++ b/src/dawn/wire/client/Queue.cpp @@ -36,6 +36,7 @@ bool Queue::OnWorkDoneCallback(uint64_t requestSerial, WGPUQueueWorkDoneStatus s void Queue::OnSubmittedWorkDone(uint64_t signalValue, WGPUQueueWorkDoneCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { callback(WGPUQueueWorkDoneStatus_DeviceLost, userdata); return; @@ -61,7 +62,7 @@ void Queue::WriteBuffer(WGPUBuffer cBuffer, uint64_t bufferOffset, const void* d cmd.data = static_cast(data); cmd.size = size; - client->SerializeCommand(cmd); + GetClient()->SerializeCommand(cmd); } void Queue::WriteTexture(const WGPUImageCopyTexture* destination, @@ -77,7 +78,7 @@ void Queue::WriteTexture(const WGPUImageCopyTexture* destination, cmd.dataLayout = dataLayout; cmd.writeSize = writeSize; - client->SerializeCommand(cmd); + GetClient()->SerializeCommand(cmd); } void Queue::CancelCallbacksForDisconnect() { diff --git a/src/dawn/wire/client/ShaderModule.cpp b/src/dawn/wire/client/ShaderModule.cpp index 43f0c8d174..bcf1d6fbfa 100644 --- a/src/dawn/wire/client/ShaderModule.cpp +++ b/src/dawn/wire/client/ShaderModule.cpp @@ -23,6 +23,7 @@ ShaderModule::~ShaderModule() { } void ShaderModule::GetCompilationInfo(WGPUCompilationInfoCallback callback, void* userdata) { + Client* client = GetClient(); if (client->IsDisconnected()) { callback(WGPUCompilationInfoRequestStatus_DeviceLost, nullptr, userdata); return; diff --git a/src/dawn/wire/client/Texture.cpp b/src/dawn/wire/client/Texture.cpp index 60a123e8c3..99ea97fb7f 100644 --- a/src/dawn/wire/client/Texture.cpp +++ b/src/dawn/wire/client/Texture.cpp @@ -21,7 +21,7 @@ namespace dawn::wire::client { // static WGPUTexture Texture::Create(Device* device, const WGPUTextureDescriptor* descriptor) { - Client* wireClient = device->client; + Client* wireClient = device->GetClient(); Texture* texture = wireClient->TextureAllocator().New(wireClient); // Copy over descriptor data for reflection.