From cb2c64f7d93e4453b7b94a584da66f23359d719a Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 1 Apr 2019 21:04:17 +0000 Subject: [PATCH] Nuke Builders Part 2: remove all builder code from wire This removes blocks of code that were obviously builder-specific but also removes the ObjectStorage::valid member that was used to implement the maybe monad on the wire server side. This is no longer needed since dawn_native handles the maybe monad internally now. BUG=dawn:125 Change-Id: I8c30daae9fc70853bc1996d85a860b4877c5976c Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6161 Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- BUILD.gn | 3 - generator/main.py | 3 - generator/templates/dawn_wire/TypeTraits.h | 35 ------- generator/templates/dawn_wire/WireCmd.cpp | 54 ++--------- generator/templates/dawn_wire/WireCmd.h | 8 +- .../templates/dawn_wire/client/ApiProcs.cpp | 23 ----- .../dawn_wire/client/ClientDoers.cpp | 39 -------- .../templates/dawn_wire/server/ServerBase.h | 8 +- .../dawn_wire/server/ServerCallbacks.cpp | 58 ----------- .../dawn_wire/server/ServerDoers.cpp | 5 + .../dawn_wire/server/ServerHandlers.cpp | 42 +------- .../dawn_wire/server/ServerPrototypes.inl | 11 --- generator/wire_cmd.py | 14 --- src/dawn_wire/client/ObjectAllocator.h | 2 - src/dawn_wire/client/ObjectBase.h | 20 ---- src/dawn_wire/server/ObjectStorage.h | 17 +--- src/dawn_wire/server/Server.cpp | 2 - src/dawn_wire/server/ServerBuffer.cpp | 14 +-- .../unittests/wire/WireArgumentTests.cpp | 12 ++- .../unittests/wire/WireBufferMappingTests.cpp | 95 ++++++------------- .../unittests/wire/WireInjectTextureTests.cpp | 3 +- .../unittests/wire/WireOptionalTests.cpp | 10 +- 22 files changed, 67 insertions(+), 411 deletions(-) delete mode 100644 generator/templates/dawn_wire/TypeTraits.h delete mode 100644 generator/templates/dawn_wire/client/ClientDoers.cpp delete mode 100644 generator/templates/dawn_wire/server/ServerCallbacks.cpp diff --git a/BUILD.gn b/BUILD.gn index 836ed1e674..4217394873 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -396,18 +396,15 @@ dawn_component("libdawn_native") { dawn_generator("libdawn_wire_gen") { target = "dawn_wire" outputs = [ - "dawn_wire/TypeTraits_autogen.h", "dawn_wire/WireCmd_autogen.h", "dawn_wire/WireCmd_autogen.cpp", "dawn_wire/client/ApiObjects_autogen.h", "dawn_wire/client/ApiProcs_autogen.cpp", "dawn_wire/client/ApiProcs_autogen.h", "dawn_wire/client/ClientBase_autogen.h", - "dawn_wire/client/ClientDoers_autogen.cpp", "dawn_wire/client/ClientHandlers_autogen.cpp", "dawn_wire/client/ClientPrototypes_autogen.inl", "dawn_wire/server/ServerBase_autogen.h", - "dawn_wire/server/ServerCallbacks_autogen.cpp", "dawn_wire/server/ServerDoers_autogen.cpp", "dawn_wire/server/ServerHandlers_autogen.cpp", "dawn_wire/server/ServerPrototypes_autogen.inl", diff --git a/generator/main.py b/generator/main.py index 679f431a63..8ddcc7df80 100644 --- a/generator/main.py +++ b/generator/main.py @@ -386,18 +386,15 @@ def get_renders_for_targets(api_params, wire_json, targets): }, additional_params ] - renders.append(FileRender('dawn_wire/TypeTraits.h', 'dawn_wire/TypeTraits_autogen.h', wire_params)) renders.append(FileRender('dawn_wire/WireCmd.h', 'dawn_wire/WireCmd_autogen.h', wire_params)) renders.append(FileRender('dawn_wire/WireCmd.cpp', 'dawn_wire/WireCmd_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/client/ApiObjects.h', 'dawn_wire/client/ApiObjects_autogen.h', wire_params)) renders.append(FileRender('dawn_wire/client/ApiProcs.cpp', 'dawn_wire/client/ApiProcs_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/client/ApiProcs.h', 'dawn_wire/client/ApiProcs_autogen.h', wire_params)) renders.append(FileRender('dawn_wire/client/ClientBase.h', 'dawn_wire/client/ClientBase_autogen.h', wire_params)) - renders.append(FileRender('dawn_wire/client/ClientDoers.cpp', 'dawn_wire/client/ClientDoers_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/client/ClientHandlers.cpp', 'dawn_wire/client/ClientHandlers_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/client/ClientPrototypes.inl', 'dawn_wire/client/ClientPrototypes_autogen.inl', wire_params)) renders.append(FileRender('dawn_wire/server/ServerBase.h', 'dawn_wire/server/ServerBase_autogen.h', wire_params)) - renders.append(FileRender('dawn_wire/server/ServerCallbacks.cpp', 'dawn_wire/server/ServerCallbacks_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/server/ServerDoers.cpp', 'dawn_wire/server/ServerDoers_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/server/ServerHandlers.cpp', 'dawn_wire/server/ServerHandlers_autogen.cpp', wire_params)) renders.append(FileRender('dawn_wire/server/ServerPrototypes.inl', 'dawn_wire/server/ServerPrototypes_autogen.inl', wire_params)) diff --git a/generator/templates/dawn_wire/TypeTraits.h b/generator/templates/dawn_wire/TypeTraits.h deleted file mode 100644 index 26a91eeff2..0000000000 --- a/generator/templates/dawn_wire/TypeTraits.h +++ /dev/null @@ -1,35 +0,0 @@ -//* Copyright 2019 The Dawn Authors -//* -//* Licensed under the Apache License, Version 2.0 (the "License"); -//* you may not use this file except in compliance with the License. -//* You may obtain a copy of the License at -//* -//* http://www.apache.org/licenses/LICENSE-2.0 -//* -//* Unless required by applicable law or agreed to in writing, software -//* distributed under the License is distributed on an "AS IS" BASIS, -//* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -//* See the License for the specific language governing permissions and -//* limitations under the License. - -#ifndef DAWNWIRE_TYPETRAITS_AUTOGEN_H_ -#define DAWNWIRE_TYPETRAITS_AUTOGEN_H_ - -#include - -//* This file can be removed when WebGPU error handling is implemented -namespace dawn_wire { - template - struct IsBuilderType { - static constexpr bool value = false; - }; - - {% for type in by_category["object"] if type.is_builder %} - template<> - struct IsBuilderType<{{as_cType(type.name)}}> { - static constexpr bool value = true; - }; - {% endfor %} -} - -#endif // DAWNWIRE_TYPETRAITS_AUTOGEN_H_ diff --git a/generator/templates/dawn_wire/WireCmd.cpp b/generator/templates/dawn_wire/WireCmd.cpp index 0b3f95c674..b1c952f74e 100644 --- a/generator/templates/dawn_wire/WireCmd.cpp +++ b/generator/templates/dawn_wire/WireCmd.cpp @@ -83,7 +83,7 @@ //* Methods are very similar to structures that have one member corresponding to each arguments. //* This macro takes advantage of the similarity to output [de]serialization code for a record //* that is either a structure or a method, with some special cases for each. -{% macro write_record_serialization_helpers(record, name, members, is_cmd=False, is_method=False, is_return_command=False) %} +{% macro write_record_serialization_helpers(record, name, members, is_cmd=False, is_return_command=False) %} {% set Return = "Return" if is_return_command else "" %} {% set Cmd = "Cmd" if is_cmd else "" %} @@ -218,53 +218,14 @@ ASSERT(transfer->commandId == {{Return}}WireCmd::{{name}}); {% endif %} - //* First assign result ObjectHandles: - //* Deserialize guarantees they are filled even if there is an ID for an error object - //* for the Maybe monad mechanism. - //* TODO(enga): This won't need to be done first once we have "WebGPU error handling". - {% set return_handles = members - |selectattr("is_return_value") - |selectattr("annotation", "equalto", "value") - |selectattr("type.dict_name", "equalto", "ObjectHandle") - |list %} - - //* Strip return_handles so we don't deserialize it again - {% set members = members|reject("in", return_handles)|list %} - - {% for member in return_handles %} - {% set memberName = as_varName(member.name) %} - {{deserialize_member(member, "transfer->" + memberName, "record->" + memberName)}} - {% endfor %} - - //* Handle special transfer members for methods - {% if is_method %} - //* First assign selfId: - //* Deserialize guarantees they are filled even if there is an ID for an error object - //* for the Maybe monad mechanism. - //* TODO(enga): This won't need to be done first once we have "WebGPU error handling". - //* We can also remove is_method - record->selfId = transfer->self; - //* This conversion is done after the copying of result* and selfId: Deserialize - //* guarantees they are filled even if there is an ID for an error object for the - //* Maybe monad mechanism. - DESERIALIZE_TRY(resolver.GetFromId(record->selfId, &record->self)); - - //* Strip self so we don't deserialize it again - {% set members = members|rejectattr("name.chunks", "equalto", ["self"])|list %} - - //* The object resolver returns a success even if the object is null because the - //* frontend is responsible to validate that (null objects sometimes have special - //* meanings). However it is never valid to call a method on a null object so we - //* can error out in that case. - if (record->self == nullptr) { - return DeserializeResult::FatalError; - } - {% endif %} - {% if record.extensible %} record->nextInChain = nullptr; {% endif %} + {% if record.derived_method %} + record->selfId = transfer->self; + {% endif %} + //* Value types are directly in the transfer record, objects being replaced with their IDs. {% for member in members if member.annotation == "value" %} {% set memberName = as_varName(member.name) %} @@ -428,15 +389,14 @@ namespace dawn_wire { {% for command in cmd_records["command"] %} {% set name = command.name.CamelCase() %} {{write_record_serialization_helpers(command, name, command.members, - is_cmd=True, is_method=command.derived_method != None)}} + is_cmd=True)}} {% endfor %} //* Output [de]serialization helpers for return commands {% for command in cmd_records["return command"] %} {% set name = command.name.CamelCase() %} {{write_record_serialization_helpers(command, name, command.members, - is_cmd=True, is_method=command.derived_method != None, - is_return_command=True)}} + is_cmd=True, is_return_command=True)}} {% endfor %} } // anonymous namespace diff --git a/generator/templates/dawn_wire/WireCmd.h b/generator/templates/dawn_wire/WireCmd.h index d92de2ce68..ad3a839e98 100644 --- a/generator/templates/dawn_wire/WireCmd.h +++ b/generator/templates/dawn_wire/WireCmd.h @@ -29,7 +29,6 @@ namespace dawn_wire { enum class DeserializeResult { Success, FatalError, - ErrorObject, }; // Interface to allocate more space to deserialize pointed-to data. @@ -40,8 +39,7 @@ namespace dawn_wire { }; // Interface to convert an ID to a server object, if possible. - // Methods return FatalError if the ID is for a non-existent object, ErrorObject if the - // object is an error value and Success otherwise. + // Methods return FatalError if the ID is for a non-existent object and Success otherwise. class ObjectIdResolver { public: {% for type in by_category["object"] %} @@ -101,10 +99,6 @@ namespace dawn_wire { //* Deserialize returns: //* - Success if everything went well (yay!) //* - FatalError is something bad happened (buffer too small for example) - //* - ErrorObject if one if the deserialized object is an error value, for the implementation - //* of the Maybe monad. - //* If the return value is not FatalError, selfId, resultId and resultSerial (if present) are - //* filled. DeserializeResult Deserialize(const char** buffer, size_t* size, DeserializeAllocator* allocator {%- if command.has_dawn_object -%} , const ObjectIdResolver& resolver diff --git a/generator/templates/dawn_wire/client/ApiProcs.cpp b/generator/templates/dawn_wire/client/ApiProcs.cpp index 1537783b3b..5b3fa95444 100644 --- a/generator/templates/dawn_wire/client/ApiProcs.cpp +++ b/generator/templates/dawn_wire/client/ApiProcs.cpp @@ -42,15 +42,6 @@ namespace dawn_wire { namespace client { //* For object creation, store the object ID the client will use for the result. {% if method.return_type.category == "object" %} auto* allocation = self->device->GetClient()->{{method.return_type.name.CamelCase()}}Allocator().New(self->device); - - {% if type.is_builder %} - //* We are in GetResult, so the callback that should be called is the - //* currently set one. Copy it over to the created object and prevent the - //* builder from calling the callback on destruction. - allocation->object->builderCallback = self->builderCallback; - self->builderCallback.canCall = false; - {% endif %} - cmd.result = ObjectHandle{allocation->object->id, allocation->serial}; {% endif %} @@ -70,18 +61,6 @@ namespace dawn_wire { namespace client { {% endif %} {% endfor %} - {% if type.is_builder %} - void Client{{as_MethodSuffix(type.name, Name("set error callback"))}}({{cType}} cSelf, - DawnBuilderErrorCallback callback, - DawnCallbackUserdata userdata1, - DawnCallbackUserdata userdata2) { - {{Type}}* self = reinterpret_cast<{{Type}}*>(cSelf); - self->builderCallback.callback = callback; - self->builderCallback.userdata1 = userdata1; - self->builderCallback.userdata2 = userdata2; - } - {% endif %} - {% if not type.name.canonical_case() == "device" %} //* 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) { @@ -92,8 +71,6 @@ namespace dawn_wire { namespace client { return; } - obj->builderCallback.Call(DAWN_BUILDER_ERROR_STATUS_UNKNOWN, "Unknown"); - DestroyObjectCmd cmd; cmd.objectType = ObjectType::{{type.name.CamelCase()}}; cmd.objectId = obj->id; diff --git a/generator/templates/dawn_wire/client/ClientDoers.cpp b/generator/templates/dawn_wire/client/ClientDoers.cpp deleted file mode 100644 index a76906d4ad..0000000000 --- a/generator/templates/dawn_wire/client/ClientDoers.cpp +++ /dev/null @@ -1,39 +0,0 @@ -//* Copyright 2019 The Dawn Authors -//* -//* Licensed under the Apache License, Version 2.0 (the "License"); -//* you may not use this file except in compliance with the License. -//* You may obtain a copy of the License at -//* -//* http://www.apache.org/licenses/LICENSE-2.0 -//* -//* Unless required by applicable law or agreed to in writing, software -//* distributed under the License is distributed on an "AS IS" BASIS, -//* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -//* See the License for the specific language governing permissions and -//* limitations under the License. - -#include "dawn_wire/client/Client.h" - -#include - -namespace dawn_wire { - namespace client { - {% for type in by_category["object"] if type.is_builder %} - {% set Type = type.name.CamelCase() %} - bool Client::Do{{Type}}ErrorCallback({{type.built_type.name.CamelCase()}}* object, uint32_t status, const char* message) { - //* The object might have been deleted or a new object created with the same ID. - if (object == nullptr) { - return true; - } - bool called = object->builderCallback.Call(static_cast(status), message); - - //* Unhandled builder errors are forwarded to the device - if (!called && status != DAWN_BUILDER_ERROR_STATUS_SUCCESS && status != DAWN_BUILDER_ERROR_STATUS_UNKNOWN) { - mDevice->HandleError(("Unhandled builder error: " + std::string(message)).c_str()); - } - - return true; - } - {% endfor %} - } -} diff --git a/generator/templates/dawn_wire/server/ServerBase.h b/generator/templates/dawn_wire/server/ServerBase.h index f21b4ad081..926c86d173 100644 --- a/generator/templates/dawn_wire/server/ServerBase.h +++ b/generator/templates/dawn_wire/server/ServerBase.h @@ -29,7 +29,7 @@ namespace dawn_wire { namespace server { protected: void DestroyAllObjects(const DawnProcTable& procs) { - //* Free all objects when the server is destroyed + //* Free all objects when the server is destroyed {% for type in by_category["object"] if type.name.canonical_case() != "device" %} { std::vector<{{as_cType(type.name)}}> handles = mKnown{{type.name.CamelCase()}}.AcquireAllHandles(); @@ -68,11 +68,7 @@ namespace dawn_wire { namespace server { } *out = data->handle; - if (data->valid) { - return DeserializeResult::Success; - } else { - return DeserializeResult::ErrorObject; - } + return DeserializeResult::Success; } DeserializeResult GetOptionalFromId(ObjectId id, {{as_cType(type.name)}}* out) const final { diff --git a/generator/templates/dawn_wire/server/ServerCallbacks.cpp b/generator/templates/dawn_wire/server/ServerCallbacks.cpp deleted file mode 100644 index b306bba0c2..0000000000 --- a/generator/templates/dawn_wire/server/ServerCallbacks.cpp +++ /dev/null @@ -1,58 +0,0 @@ -//* Copyright 2019 The Dawn Authors -//* -//* Licensed under the Apache License, Version 2.0 (the "License"); -//* you may not use this file except in compliance with the License. -//* You may obtain a copy of the License at -//* -//* http://www.apache.org/licenses/LICENSE-2.0 -//* -//* Unless required by applicable law or agreed to in writing, software -//* distributed under the License is distributed on an "AS IS" BASIS, -//* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -//* See the License for the specific language governing permissions and -//* limitations under the License. - -#include "common/Assert.h" -#include "dawn_wire/server/Server.h" - -namespace dawn_wire { namespace server { - {% for type in by_category["object"] if type.is_builder%} - void Server::Forward{{type.name.CamelCase()}}(DawnBuilderErrorStatus status, const char* message, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2) { - auto server = reinterpret_cast(static_cast(userdata1)); - uint32_t id = userdata2 & 0xFFFFFFFFu; - uint32_t serial = userdata2 >> uint64_t(32); - server->On{{type.name.CamelCase()}}Error(status, message, id, serial); - } - {% endfor %} - - {% for type in by_category["object"] if type.is_builder%} - {% set Type = type.name.CamelCase() %} - void Server::On{{Type}}Error(DawnBuilderErrorStatus status, const char* message, uint32_t id, uint32_t serial) { - auto* builder = {{Type}}Objects().Get(id); - - if (builder == nullptr || builder->serial != serial) { - return; - } - - if (status != DAWN_BUILDER_ERROR_STATUS_SUCCESS) { - builder->valid = false; - } - - if (status != DAWN_BUILDER_ERROR_STATUS_UNKNOWN) { - //* Unknown is the only status that can be returned without a call to GetResult - //* so we are guaranteed to have created an object. - ASSERT(builder->builtObject.id != 0); - - Return{{Type}}ErrorCallbackCmd cmd; - cmd.builtObject = builder->builtObject; - cmd.status = status; - cmd.message = message; - - size_t requiredSize = cmd.GetRequiredSize(); - char* allocatedBuffer = static_cast(GetCmdSpace(requiredSize)); - cmd.Serialize(allocatedBuffer); - } - } - {% endfor %} - -}} // namespace dawn_wire::server diff --git a/generator/templates/dawn_wire/server/ServerDoers.cpp b/generator/templates/dawn_wire/server/ServerDoers.cpp index 5b7d8c37ea..3ebdedf1fb 100644 --- a/generator/templates/dawn_wire/server/ServerDoers.cpp +++ b/generator/templates/dawn_wire/server/ServerDoers.cpp @@ -53,6 +53,11 @@ namespace dawn_wire { namespace server { {%- if not loop.last -%}, {% endif %} {%- endfor -%} ); + {% if ret|length == 1 %} + //* WebGPU error handling guarantees that no null object can be returned by + //* object creation functions. + ASSERT(*{{as_varName(ret[0].name)}} != nullptr); + {% endif %} return true; } {% endif %} diff --git a/generator/templates/dawn_wire/server/ServerHandlers.cpp b/generator/templates/dawn_wire/server/ServerHandlers.cpp index 6a1d48bbbf..4a99bb66a1 100644 --- a/generator/templates/dawn_wire/server/ServerHandlers.cpp +++ b/generator/templates/dawn_wire/server/ServerHandlers.cpp @@ -43,12 +43,6 @@ namespace dawn_wire { namespace server { } {% endif %} - {% if is_method %} - //* Unpack 'self' - auto* selfData = {{type.name.CamelCase()}}Objects().Get(cmd.selfId); - ASSERT(selfData != nullptr); - {% endif %} - //* Allocate any result objects {%- for member in command.members if member.is_return_value -%} {{ assert(member.handle_type) }} @@ -60,24 +54,8 @@ namespace dawn_wire { namespace server { return false; } {{name}}Data->serial = cmd.{{name}}.serial; - - {% if type.is_builder %} - selfData->builtObject = cmd.{{name}}; - {% endif %} {% endfor %} - //* After the data is allocated, apply the argument error propagation mechanism - if (deserializeResult == DeserializeResult::ErrorObject) { - {% if type.is_builder %} - selfData->valid = false; - //* If we are in GetResult, fake an error callback - {% if returns %} - On{{type.name.CamelCase()}}Error(DAWN_BUILDER_ERROR_STATUS_ERROR, "Maybe monad", cmd.selfId, selfData->serial); - {% endif %} - {% endif %} - return true; - } - //* Do command bool success = Do{{Suffix}}( {%- for member in command.members -%} @@ -94,12 +72,6 @@ namespace dawn_wire { namespace server { {%- endfor -%} ); - //* Mark output object handles as valid/invalid - {%- for member in command.members if member.is_return_value and member.handle_type -%} - {% set name = as_varName(member.name) %} - {{name}}Data->valid = {{name}}Data->handle != nullptr; - {% endfor %} - if (!success) { return false; } @@ -110,19 +82,7 @@ namespace dawn_wire { namespace server { {% if Type in server_reverse_lookup_objects %} //* For created objects, store a mapping from them back to their client IDs - if ({{name}}Data->valid) { - {{Type}}ObjectIdTable().Store({{name}}Data->handle, cmd.{{name}}.id); - } - {% endif %} - - //* builders remember the ID of the object they built so that they can send it - //* in the callback to the client. - {% if member.handle_type.is_builder %} - if ({{name}}Data->valid) { - uint64_t userdata1 = static_cast(reinterpret_cast(this)); - uint64_t userdata2 = (uint64_t({{name}}Data->serial) << uint64_t(32)) + cmd.{{name}}.id; - mProcs.{{as_varName(member.handle_type.name, Name("set error callback"))}}({{name}}Data->handle, Forward{{Type}}, userdata1, userdata2); - } + {{Type}}ObjectIdTable().Store({{name}}Data->handle, cmd.{{name}}.id); {% endif %} {% endfor %} diff --git a/generator/templates/dawn_wire/server/ServerPrototypes.inl b/generator/templates/dawn_wire/server/ServerPrototypes.inl index d5f6a1f87e..03b7b4aa7b 100644 --- a/generator/templates/dawn_wire/server/ServerPrototypes.inl +++ b/generator/templates/dawn_wire/server/ServerPrototypes.inl @@ -12,17 +12,6 @@ //* See the License for the specific language governing permissions and //* limitations under the License. -// Forwarding callbacks -{% for type in by_category["object"] if type.is_builder%} - static void Forward{{type.name.CamelCase()}}(DawnBuilderErrorStatus status, const char* message, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2); -{% endfor %} - -// Error callbacks -{% for type in by_category["object"] if type.is_builder%} - {% set Type = type.name.CamelCase() %} - void On{{Type}}Error(DawnBuilderErrorStatus status, const char* message, uint32_t id, uint32_t serial); -{% endfor %} - // Command handlers & doers {% for command in cmd_records["command"] if command.name.CamelCase() not in client_side_commands %} {% set Suffix = command.name.CamelCase() %} diff --git a/generator/wire_cmd.py b/generator/wire_cmd.py index d7198d5a65..97d3f0ca38 100644 --- a/generator/wire_cmd.py +++ b/generator/wire_cmd.py @@ -57,20 +57,6 @@ def compute_wire_params(api_params, wire_json): command.derived_method = method commands.append(command) - # Create builder return ErrorCallback commands - # This can be removed when WebGPU error handling is implemented - if api_object.is_builder: - command_name = concat_names(api_object.name, Name('error callback')) - built_object = common.RecordMember(Name('built object'), types['ObjectHandle'], 'value', False, False) - built_object.set_handle_type(api_object.built_type) - command = common.Command(command_name, [ - built_object, - callback_status_member, - string_message_member, - ]) - command.derived_object = api_object - return_commands.append(command) - for (name, json_data) in wire_json['commands'].items(): commands.append(common.Command(name, common.linked_record_members(json_data, types))) diff --git a/src/dawn_wire/client/ObjectAllocator.h b/src/dawn_wire/client/ObjectAllocator.h index 3b1b60e6d0..7caacbe527 100644 --- a/src/dawn_wire/client/ObjectAllocator.h +++ b/src/dawn_wire/client/ObjectAllocator.h @@ -25,8 +25,6 @@ namespace dawn_wire { namespace client { class Client; class Device; - // TODO(cwallez@chromium.org): Do something with objects before they are destroyed ? - // - Call still uncalled builder callbacks template class ObjectAllocator { using ObjectOwner = diff --git a/src/dawn_wire/client/ObjectBase.h b/src/dawn_wire/client/ObjectBase.h index 42d0805738..b97b0409a2 100644 --- a/src/dawn_wire/client/ObjectBase.h +++ b/src/dawn_wire/client/ObjectBase.h @@ -21,24 +21,6 @@ namespace dawn_wire { namespace client { class Device; - struct BuilderCallbackData { - bool Call(DawnBuilderErrorStatus status, const char* message) { - if (canCall && callback != nullptr) { - canCall = true; - callback(status, message, userdata1, userdata2); - return true; - } - - return false; - } - - // For help with development, prints all builder errors by default. - DawnBuilderErrorCallback callback = nullptr; - DawnCallbackUserdata userdata1 = 0; - DawnCallbackUserdata userdata2 = 0; - bool canCall = true; - }; - // All non-Device objects of the client side have: // - A pointer to the device to get where to serialize commands // - The external reference count @@ -51,8 +33,6 @@ namespace dawn_wire { namespace client { Device* device; uint32_t refcount; uint32_t id; - - BuilderCallbackData builderCallback; }; }} // namespace dawn_wire::client diff --git a/src/dawn_wire/server/ObjectStorage.h b/src/dawn_wire/server/ObjectStorage.h index 4bfce34447..689dc0b3f7 100644 --- a/src/dawn_wire/server/ObjectStorage.h +++ b/src/dawn_wire/server/ObjectStorage.h @@ -15,7 +15,6 @@ #ifndef DAWNWIRE_SERVER_OBJECTSTORAGE_H_ #define DAWNWIRE_SERVER_OBJECTSTORAGE_H_ -#include "dawn_wire/TypeTraits_autogen.h" #include "dawn_wire/WireCmd_autogen.h" #include @@ -29,26 +28,17 @@ namespace dawn_wire { namespace server { T handle; uint32_t serial = 0; - // Used by the error-propagation mechanism to know if this object is an error. - // TODO(cwallez@chromium.org): this is doubling the memory usage of - // std::vector consider making it a special marker value in handle instead. - bool valid; // Whether this object has been allocated, used by the KnownObjects queries // TODO(cwallez@chromium.org): make this an internal bit vector in KnownObjects. bool allocated; }; // Stores what the backend knows about the type. - template ::value> + template struct ObjectData : public ObjectDataBase {}; - template - struct ObjectData : public ObjectDataBase { - ObjectHandle builtObject = ObjectHandle{0, 0}; - }; - template <> - struct ObjectData : public ObjectDataBase { + struct ObjectData : public ObjectDataBase { void* mappedData = nullptr; size_t mappedDataSize = 0; }; @@ -65,7 +55,6 @@ namespace dawn_wire { namespace server { // KnownObjects for ID 0. Data reservation; reservation.handle = nullptr; - reservation.valid = false; reservation.allocated = false; mKnown.push_back(reservation); } @@ -109,7 +98,6 @@ namespace dawn_wire { namespace server { Data data; data.allocated = true; - data.valid = false; data.handle = nullptr; if (id >= mKnown.size()) { @@ -136,7 +124,6 @@ namespace dawn_wire { namespace server { for (Data& data : mKnown) { if (data.allocated && data.handle != nullptr) { objects.push_back(data.handle); - data.valid = false; data.allocated = false; data.handle = nullptr; } diff --git a/src/dawn_wire/server/Server.cpp b/src/dawn_wire/server/Server.cpp index 6d420baf96..fdb6cec9a5 100644 --- a/src/dawn_wire/server/Server.cpp +++ b/src/dawn_wire/server/Server.cpp @@ -21,7 +21,6 @@ namespace dawn_wire { namespace server { // The client-server knowledge is bootstrapped with device 1. auto* deviceData = DeviceObjects().Allocate(1); deviceData->handle = device; - deviceData->valid = true; auto userdata = static_cast(reinterpret_cast(this)); mProcs.deviceSetErrorCallback(device, ForwardDeviceError, userdata); @@ -43,7 +42,6 @@ namespace dawn_wire { namespace server { data->handle = texture; data->serial = generation; - data->valid = true; data->allocated = true; // The texture is externally owned so it shouldn't be destroyed when we receive a destroy diff --git a/src/dawn_wire/server/ServerBuffer.cpp b/src/dawn_wire/server/ServerBuffer.cpp index cf337bbc13..22fd60e708 100644 --- a/src/dawn_wire/server/ServerBuffer.cpp +++ b/src/dawn_wire/server/ServerBuffer.cpp @@ -52,17 +52,6 @@ namespace dawn_wire { namespace server { auto userdata = static_cast(reinterpret_cast(data)); - if (!buffer->valid) { - // Fake the buffer returning a failure, data will be freed in this call. - if (isWrite) { - ForwardBufferMapWriteAsync(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, - userdata); - } else { - ForwardBufferMapReadAsync(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata); - } - return true; - } - if (isWrite) { mProcs.bufferMapWriteAsync(buffer->handle, ForwardBufferMapWriteAsync, userdata); } else { @@ -79,8 +68,7 @@ namespace dawn_wire { namespace server { } auto* buffer = BufferObjects().Get(bufferId); - if (buffer == nullptr || !buffer->valid || buffer->mappedData == nullptr || - buffer->mappedDataSize != count) { + if (buffer == nullptr || buffer->mappedData == nullptr || buffer->mappedDataSize != count) { return false; } diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp index 10dbd89356..d6c5d73f45 100644 --- a/src/tests/unittests/wire/WireArgumentTests.cpp +++ b/src/tests/unittests/wire/WireArgumentTests.cpp @@ -158,12 +158,14 @@ TEST_F(WireArgumentTests, CStringArgument) { pipelineDescriptor.depthStencilState = &depthStencilState; dawnDeviceCreateRenderPipeline(device, &pipelineDescriptor); + + DawnRenderPipeline apiDummyPipeline = api.GetNewRenderPipeline(); EXPECT_CALL(api, DeviceCreateRenderPipeline( apiDevice, MatchesLambda([](const DawnRenderPipelineDescriptor* desc) -> bool { return desc->vertexStage->entryPoint == std::string("main"); }))) - .WillOnce(Return(nullptr)); + .WillOnce(Return(apiDummyPipeline)); FlushClient(); } @@ -247,6 +249,8 @@ TEST_F(WireArgumentTests, StructureOfValuesArgument) { descriptor.borderColor = DAWN_BORDER_COLOR_TRANSPARENT_BLACK; dawnDeviceCreateSampler(device, &descriptor); + + DawnSampler apiDummySampler = api.GetNewSampler(); EXPECT_CALL(api, DeviceCreateSampler( apiDevice, MatchesLambda([](const DawnSamplerDescriptor* desc) -> bool { return desc->nextInChain == nullptr && @@ -260,7 +264,7 @@ TEST_F(WireArgumentTests, StructureOfValuesArgument) { desc->borderColor == DAWN_BORDER_COLOR_TRANSPARENT_BLACK && desc->lodMinClamp == kLodMin && desc->lodMaxClamp == kLodMax; }))) - .WillOnce(Return(nullptr)); + .WillOnce(Return(apiDummySampler)); FlushClient(); } @@ -281,6 +285,8 @@ TEST_F(WireArgumentTests, StructureOfObjectArrayArgument) { descriptor.bindGroupLayouts = &bgl; dawnDeviceCreatePipelineLayout(device, &descriptor); + + DawnPipelineLayout apiDummyLayout = api.GetNewPipelineLayout(); EXPECT_CALL(api, DeviceCreatePipelineLayout( apiDevice, MatchesLambda([apiBgl](const DawnPipelineLayoutDescriptor* desc) -> bool { @@ -288,7 +294,7 @@ TEST_F(WireArgumentTests, StructureOfObjectArrayArgument) { desc->bindGroupLayoutCount == 1 && desc->bindGroupLayouts[0] == apiBgl; }))) - .WillOnce(Return(nullptr)); + .WillOnce(Return(apiDummyLayout)); FlushClient(); } diff --git a/src/tests/unittests/wire/WireBufferMappingTests.cpp b/src/tests/unittests/wire/WireBufferMappingTests.cpp index e1037ff541..db3c61cae1 100644 --- a/src/tests/unittests/wire/WireBufferMappingTests.cpp +++ b/src/tests/unittests/wire/WireBufferMappingTests.cpp @@ -73,29 +73,16 @@ class WireBufferMappingTests : public WireTest { mockBufferMapReadCallback = std::make_unique>(); mockBufferMapWriteCallback = std::make_unique>(); - { - DawnBufferDescriptor descriptor; - descriptor.nextInChain = nullptr; + DawnBufferDescriptor descriptor; + descriptor.nextInChain = nullptr; - apiBuffer = api.GetNewBuffer(); - buffer = dawnDeviceCreateBuffer(device, &descriptor); + apiBuffer = api.GetNewBuffer(); + buffer = dawnDeviceCreateBuffer(device, &descriptor); - EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _)) - .WillOnce(Return(apiBuffer)) - .RetiresOnSaturation(); - FlushClient(); - } - { - DawnBufferDescriptor descriptor; - descriptor.nextInChain = nullptr; - - errorBuffer = dawnDeviceCreateBuffer(device, &descriptor); - - EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _)) - .WillOnce(Return(nullptr)) - .RetiresOnSaturation(); - FlushClient(); - } + EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _)) + .WillOnce(Return(apiBuffer)) + .RetiresOnSaturation(); + FlushClient(); } void TearDown() override { @@ -117,9 +104,6 @@ class WireBufferMappingTests : public WireTest { // A successfully created buffer DawnBuffer buffer; DawnBuffer apiBuffer; - - // An buffer that wasn't created on the server side - DawnBuffer errorBuffer; }; // MapRead-specific tests @@ -171,35 +155,24 @@ TEST_F(WireBufferMappingTests, ErrorWhileMappingForRead) { FlushServer(); } -// Check mapping for reading a buffer that didn't get created on the server side -TEST_F(WireBufferMappingTests, MappingForReadErrorBuffer) { - DawnCallbackUserdata userdata = 8655; - dawnBufferMapReadAsync(errorBuffer, ToMockBufferMapReadCallback, userdata); - - FlushClient(); - - EXPECT_CALL(*mockBufferMapReadCallback, - Call(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata)) - .Times(1); - - FlushServer(); - - dawnBufferUnmap(errorBuffer); - - FlushClient(); -} - // Check that the map read callback is called with UNKNOWN when the buffer is destroyed before the // request is finished TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) { DawnCallbackUserdata userdata = 8656; - dawnBufferMapReadAsync(errorBuffer, ToMockBufferMapReadCallback, userdata); + dawnBufferMapReadAsync(buffer, ToMockBufferMapReadCallback, userdata); + // Return success + EXPECT_CALL(api, OnBufferMapReadAsyncCallback(apiBuffer, _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapReadCallback(apiBuffer, DAWN_BUFFER_MAP_ASYNC_STATUS_SUCCESS, nullptr, 0); + })); + + // Destroy before the client gets the success, so the callback is called with unknown. EXPECT_CALL(*mockBufferMapReadCallback, Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, 0, userdata)) .Times(1); - - dawnBufferRelease(errorBuffer); + dawnBufferRelease(buffer); + EXPECT_CALL(api, BufferRelease(apiBuffer)); FlushClient(); FlushServer(); @@ -381,35 +354,27 @@ TEST_F(WireBufferMappingTests, ErrorWhileMappingForWrite) { FlushServer(); } -// Check mapping for writing a buffer that didn't get created on the server side -TEST_F(WireBufferMappingTests, MappingForWriteErrorBuffer) { - DawnCallbackUserdata userdata = 8655; - dawnBufferMapWriteAsync(errorBuffer, ToMockBufferMapWriteCallback, userdata); - - FlushClient(); - - EXPECT_CALL(*mockBufferMapWriteCallback, - Call(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata)) - .Times(1); - - FlushServer(); - - dawnBufferUnmap(errorBuffer); - - FlushClient(); -} - // Check that the map write callback is called with UNKNOWN when the buffer is destroyed before the // request is finished TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) { DawnCallbackUserdata userdata = 8656; - dawnBufferMapWriteAsync(errorBuffer, ToMockBufferMapWriteCallback, userdata); + dawnBufferMapWriteAsync(buffer, ToMockBufferMapWriteCallback, userdata); + // Return success + EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, _, _)) + .WillOnce(InvokeWithoutArgs([&]() { + api.CallMapWriteCallback(apiBuffer, DAWN_BUFFER_MAP_ASYNC_STATUS_SUCCESS, nullptr, 0); + })); + + // Destroy before the client gets the success, so the callback is called with unknown. EXPECT_CALL(*mockBufferMapWriteCallback, Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, 0, userdata)) .Times(1); + dawnBufferRelease(buffer); + EXPECT_CALL(api, BufferRelease(apiBuffer)); - dawnBufferRelease(errorBuffer); + FlushClient(); + FlushServer(); } // Check the map read callback is called with UNKNOWN when the map request would have worked, but diff --git a/src/tests/unittests/wire/WireInjectTextureTests.cpp b/src/tests/unittests/wire/WireInjectTextureTests.cpp index b8e0cbe7fd..23da573061 100644 --- a/src/tests/unittests/wire/WireInjectTextureTests.cpp +++ b/src/tests/unittests/wire/WireInjectTextureTests.cpp @@ -37,7 +37,8 @@ TEST_F(WireInjectTextureTests, CallAfterReserveInject) { ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id, reservation.generation)); dawnTextureCreateDefaultTextureView(reservation.texture); - EXPECT_CALL(api, TextureCreateDefaultTextureView(apiTexture)).WillOnce(Return(nullptr)); + DawnTextureView apiDummyView = api.GetNewTextureView(); + EXPECT_CALL(api, TextureCreateDefaultTextureView(apiTexture)).WillOnce(Return(apiDummyView)); FlushClient(); } diff --git a/src/tests/unittests/wire/WireOptionalTests.cpp b/src/tests/unittests/wire/WireOptionalTests.cpp index 55b67ed727..29972b1ed8 100644 --- a/src/tests/unittests/wire/WireOptionalTests.cpp +++ b/src/tests/unittests/wire/WireOptionalTests.cpp @@ -49,6 +49,8 @@ TEST_F(WireOptionalTests, OptionalObjectValue) { bgDesc.bindings = &binding; dawnDeviceCreateBindGroup(device, &bgDesc); + + DawnBindGroup apiDummyBindGroup = api.GetNewBindGroup(); EXPECT_CALL(api, DeviceCreateBindGroup( apiDevice, MatchesLambda([](const DawnBindGroupDescriptor* desc) -> bool { return desc->nextInChain == nullptr && desc->bindingCount == 1 && @@ -57,7 +59,7 @@ TEST_F(WireOptionalTests, OptionalObjectValue) { desc->bindings[0].buffer == nullptr && desc->bindings[0].textureView == nullptr; }))) - .WillOnce(Return(nullptr)); + .WillOnce(Return(apiDummyBindGroup)); FlushClient(); } @@ -147,6 +149,8 @@ TEST_F(WireOptionalTests, OptionalStructPointer) { // First case: depthStencilState is not null. pipelineDescriptor.depthStencilState = &depthStencilState; dawnDeviceCreateRenderPipeline(device, &pipelineDescriptor); + + DawnRenderPipeline apiDummyPipeline = api.GetNewRenderPipeline(); EXPECT_CALL( api, DeviceCreateRenderPipeline( @@ -172,7 +176,7 @@ TEST_F(WireOptionalTests, OptionalStructPointer) { desc->depthStencilState->stencilReadMask == 0xff && desc->depthStencilState->stencilWriteMask == 0xff; }))) - .WillOnce(Return(nullptr)); + .WillOnce(Return(apiDummyPipeline)); FlushClient(); @@ -184,7 +188,7 @@ TEST_F(WireOptionalTests, OptionalStructPointer) { apiDevice, MatchesLambda([](const DawnRenderPipelineDescriptor* desc) -> bool { return desc->depthStencilState == nullptr; }))) - .WillOnce(Return(nullptr)); + .WillOnce(Return(apiDummyPipeline)); FlushClient(); }