dawn.json: Use void* instead of uint8_t* for data pointers

This requires some changes in the wire because WireCmd doesn't know the
size of void. This also adds a handwritten implementation of the Wire
commands for SetSubData that internally converts to uint8_t so that
WireCmd can generate the de/serialization.

BUG=dawn:160

Change-Id: Icbf0fd7dd841639ee6f67333844e027b27a8afcc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/7780
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

dawn.json

@@ -135,7 +135,7 @@
                 "args": [
                     {"name": "start", "type": "uint64_t"},
                     {"name": "count", "type": "uint64_t"},
-                    {"name": "data", "type": "uint8_t", "annotation": "const*", "length": "count"}
+                    {"name": "data", "type": "void", "annotation": "const*", "length": "count"}
                 ]
             },
             {
@@ -215,7 +215,7 @@
         "members": [
             {"name": "buffer", "type": "buffer"},
             {"name": "data length", "type": "uint64_t"},
-            {"name": "data", "type": "uint8_t", "annotation": "*", "length": "data length"}
+            {"name": "data", "type": "void", "annotation": "*", "length": "data length"}
         ]
     },
     "color": {

dawn_wire.json

@@ -20,6 +20,12 @@
             { "name": "request serial", "type": "uint32_t" },
             { "name": "is write", "type": "bool" }
         ],
+        "buffer set sub data internal": [
+            {"name": "buffer id", "type": "ObjectId" },
+            {"name": "start", "type": "uint64_t"},
+            {"name": "count", "type": "uint64_t"},
+            {"name": "data", "type": "uint8_t", "annotation": "const*", "length": "count"}
+        ],
         "buffer update mapped data": [
             { "name": "buffer id", "type": "ObjectId" },
             { "name": "data length", "type": "uint32_t" },
@@ -58,10 +64,15 @@
         ]
     },
     "special items": {
+        "client_side_structures": [
+            "CreateBufferMappedResult"
+        ],
         "client_side_commands": [
+            "BufferSetSubData",
             "FenceGetCompletedValue"
         ],
         "client_handwritten_commands": [
+            "BufferSetSubData",
             "BufferUnmap",
             "DeviceCreateBufferMapped",
             "QueueCreateFence",

examples/Animometer.cpp

@@ -148,8 +148,7 @@ void frame() {
     for (auto& data : shaderData) {
         data.time = f / 60.0f;
     }
-    ubo.SetSubData(0, kNumTriangles * sizeof(ShaderData),
+    ubo.SetSubData(0, kNumTriangles * sizeof(ShaderData), shaderData.data());
-                   reinterpret_cast<const uint8_t*>(shaderData.data()));
 
     utils::ComboRenderPassDescriptor renderPass({backbuffer.CreateDefaultView()});
     dawn::CommandEncoder encoder = device.CreateCommandEncoder();

examples/CubeReflection.cpp

@@ -257,7 +257,7 @@ void frame() {
         glm::vec3(0.0f, -1.0f, 0.0f)
     );
 
-    cameraBuffer.SetSubData(0, sizeof(CameraData), reinterpret_cast<uint8_t*>(&cameraData));
+    cameraBuffer.SetSubData(0, sizeof(CameraData), &cameraData);
 
     dawn::Texture backbuffer = swapchain.GetNextTexture();
     utils::ComboRenderPassDescriptor renderPass({backbuffer.CreateDefaultView()},

generator/templates/dawn_wire/WireCmd.cpp

@@ -383,8 +383,10 @@ namespace dawn_wire {
         //* Output structure [de]serialization first because it is used by commands.
         {% for type in by_category["structure"] %}
             {% set name = as_cType(type.name) %}
-            {{write_record_serialization_helpers(type, name, type.members,
+            {% if type.name.CamelCase() not in client_side_structures %}
-              is_cmd=False)}}
+                {{write_record_serialization_helpers(type, name, type.members,
+                  is_cmd=False)}}
+            {% endif %}
         {% endfor %}
 
         //* Output [de]serialization helpers for commands

generator/wire_cmd.py

@@ -27,21 +27,22 @@ def compute_wire_params(api_params, wire_json):
     commands = []
     return_commands = []
 
-    string_message_member = common.RecordMember(Name('message'), types['char'], 'const*', False, False)
-    string_message_member.length = 'strlen'
-
-    callback_status_member = common.RecordMember(Name('status'), types['uint32_t'], 'value', False, False)
-
     # Generate commands from object methods
     for api_object in wire_params['by_category']['object']:
         for method in api_object.methods:
+            command_name = concat_names(api_object.name, method.name)
+            command_suffix = Name(command_name).CamelCase()
+
+            # Only object return values or void are supported. Other methods must be handwritten.
             if method.return_type.category != 'object' and method.return_type.name.canonical_case() != 'void':
-                # No other return types supported
+                assert(command_suffix in wire_json['special items']['client_handwritten_commands'])
+                continue
+
+            if command_suffix in wire_json['special items']['client_side_commands']:
                 continue
 
             # Create object method commands by prepending "self"
             members = [common.RecordMember(Name('self'), types[api_object.dict_name], 'value', False, False)]
-
             members += method.arguments
 
             # Client->Server commands that return an object return the result object handle
@@ -50,9 +51,7 @@ def compute_wire_params(api_params, wire_json):
                 result.set_handle_type(method.return_type)
                 members.append(result)
 
-            command_name = concat_names(api_object.name, method.name)
             command = common.Command(command_name, members)
-
             command.derived_object = api_object
             command.derived_method = method
             commands.append(command)

src/dawn_native/Buffer.cpp

@@ -58,9 +58,7 @@ namespace dawn_native {
                 return {};
             }
 
-            MaybeError SetSubDataImpl(uint32_t start,
+            MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const void* data) override {
-                                      uint32_t count,
-                                      const uint8_t* data) override {
                 UNREACHABLE();
                 return {};
             }
@@ -218,7 +216,7 @@ namespace dawn_native {
         }
     }
 
-    void BufferBase::SetSubData(uint32_t start, uint32_t count, const uint8_t* data) {
+    void BufferBase::SetSubData(uint32_t start, uint32_t count, const void* data) {
         if (GetDevice()->ConsumedError(ValidateSetSubData(start, count))) {
             return;
         }
@@ -247,7 +245,7 @@ namespace dawn_native {
         MapReadAsyncImpl(mMapSerial);
     }
 
-    MaybeError BufferBase::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
+    MaybeError BufferBase::SetSubDataImpl(uint32_t start, uint32_t count, const void* data) {
         DynamicUploader* uploader = nullptr;
         DAWN_TRY_ASSIGN(uploader, GetDevice()->GetDynamicUploader());
 

src/dawn_native/Buffer.h

@@ -57,7 +57,7 @@ namespace dawn_native {
         MaybeError ValidateCanUseInSubmitNow() const;
 
         // Dawn API
-        void SetSubData(uint32_t start, uint32_t count, const uint8_t* data);
+        void SetSubData(uint32_t start, uint32_t count, const void* data);
         void MapReadAsync(DawnBufferMapReadCallback callback, void* userdata);
         void MapWriteAsync(DawnBufferMapWriteCallback callback, void* userdata);
         void Unmap();
@@ -79,7 +79,7 @@ namespace dawn_native {
 
       private:
         virtual MaybeError MapAtCreationImpl(uint8_t** mappedPointer) = 0;
-        virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data);
+        virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const void* data);
         virtual void MapReadAsyncImpl(uint32_t serial) = 0;
         virtual void MapWriteAsyncImpl(uint32_t serial) = 0;
         virtual void UnmapImpl() = 0;

src/dawn_native/null/DeviceNull.cpp

@@ -259,7 +259,7 @@ namespace dawn_native { namespace null {
         memcpy(mBackingData.get() + destinationOffset, ptr + sourceOffset, size);
     }
 
-    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
+    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const void* data) {
         ASSERT(start + count <= GetSize());
         ASSERT(mBackingData);
         memcpy(mBackingData.get() + start, data, count);

src/dawn_native/null/DeviceNull.h

@@ -149,7 +149,7 @@ namespace dawn_native { namespace null {
 
       private:
         // Dawn API
-        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
+        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const void* data) override;
         void MapReadAsyncImpl(uint32_t serial) override;
         void MapWriteAsyncImpl(uint32_t serial) override;
         void UnmapImpl() override;

src/dawn_native/opengl/BufferGL.cpp

@@ -48,7 +48,7 @@ namespace dawn_native { namespace opengl {
         return {};
     }
 
-    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
+    MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const void* data) {
         glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
         glBufferSubData(GL_ARRAY_BUFFER, start, count, data);
         return {};

src/dawn_native/opengl/BufferGL.h

@@ -32,7 +32,7 @@ namespace dawn_native { namespace opengl {
 
       private:
         // Dawn API
-        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
+        MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const void* data) override;
         void MapReadAsyncImpl(uint32_t serial) override;
         void MapWriteAsyncImpl(uint32_t serial) override;
         void UnmapImpl() override;

src/dawn_wire/client/ApiProcs.cpp

@@ -128,6 +128,24 @@ namespace dawn_wire { namespace client {
         fence->requests.Enqueue(std::move(request), value);
     }
 
+    void ClientBufferSetSubData(DawnBuffer cBuffer,
+                                uint64_t start,
+                                uint64_t count,
+                                const void* data) {
+        Buffer* buffer = reinterpret_cast<Buffer*>(cBuffer);
+
+        BufferSetSubDataInternalCmd cmd;
+        cmd.bufferId = buffer->id;
+        cmd.start = start;
+        cmd.count = count;
+        cmd.data = static_cast<const uint8_t*>(data);
+
+        Client* wireClient = buffer->device->GetClient();
+        size_t requiredSize = cmd.GetRequiredSize();
+        char* allocatedBuffer = static_cast<char*>(wireClient->GetCmdSpace(requiredSize));
+        cmd.Serialize(allocatedBuffer);
+    }
+
     void ClientBufferUnmap(DawnBuffer cBuffer) {
         Buffer* buffer = reinterpret_cast<Buffer*>(cBuffer);
 

src/dawn_wire/server/ServerBuffer.cpp

@@ -78,6 +78,24 @@ namespace dawn_wire { namespace server {
         return true;
     }
 
+    bool Server::DoBufferSetSubDataInternal(ObjectId bufferId,
+                                            uint64_t start,
+                                            uint64_t offset,
+                                            const uint8_t* data) {
+        // The null object isn't valid as `self`
+        if (bufferId == 0) {
+            return false;
+        }
+
+        auto* buffer = BufferObjects().Get(bufferId);
+        if (buffer == nullptr) {
+            return false;
+        }
+
+        mProcs.bufferSetSubData(buffer->handle, start, offset, data);
+        return true;
+    }
+
     bool Server::DoBufferUpdateMappedData(ObjectId bufferId, uint32_t count, const uint8_t* data) {
         // The null object isn't valid as `self`
         if (bufferId == 0) {
@@ -89,10 +107,7 @@ namespace dawn_wire { namespace server {
             return false;
         }
 
-        if (data == nullptr) {
+        ASSERT(data != nullptr);
-            return false;
-        }
-
         memcpy(buffer->mappedData, data, count);
 
         return true;

src/tests/end2end/BasicTests.cpp

@@ -28,7 +28,7 @@ TEST_P(BasicTests, BufferSetSubData) {
     dawn::Buffer buffer = device.CreateBuffer(&descriptor);
 
     uint32_t value = 0x01020304;
-    buffer.SetSubData(0, sizeof(value), reinterpret_cast<uint8_t*>(&value));
+    buffer.SetSubData(0, sizeof(value), &value);
 
     EXPECT_BUFFER_U32_EQ(value, buffer, 0);
 }

src/tests/end2end/BufferTests.cpp

@@ -50,7 +50,7 @@ TEST_P(BufferMapReadTests, SmallReadAtZero) {
     dawn::Buffer buffer = device.CreateBuffer(&descriptor);
 
     uint32_t myData = 0x01020304;
-    buffer.SetSubData(0, sizeof(myData), reinterpret_cast<uint8_t*>(&myData));
+    buffer.SetSubData(0, sizeof(myData), &myData);
 
     const void* mappedData = MapReadAsyncAndWait(buffer);
     ASSERT_EQ(myData, *reinterpret_cast<const uint32_t*>(mappedData));
@@ -71,7 +71,7 @@ TEST_P(BufferMapReadTests, LargeRead) {
     descriptor.usage = dawn::BufferUsageBit::MapRead | dawn::BufferUsageBit::TransferDst;
     dawn::Buffer buffer = device.CreateBuffer(&descriptor);
 
-    buffer.SetSubData(0, kDataSize * sizeof(uint32_t), reinterpret_cast<uint8_t*>(myData.data()));
+    buffer.SetSubData(0, kDataSize * sizeof(uint32_t), myData.data());
 
     const void* mappedData = MapReadAsyncAndWait(buffer);
     ASSERT_EQ(0, memcmp(mappedData, myData.data(), kDataSize * sizeof(uint32_t)));
@@ -155,7 +155,7 @@ TEST_P(BufferSetSubDataTests, SmallDataAtZero) {
     dawn::Buffer buffer = device.CreateBuffer(&descriptor);
 
     uint32_t value = 0x01020304;
-    buffer.SetSubData(0, sizeof(value), reinterpret_cast<uint8_t*>(&value));
+    buffer.SetSubData(0, sizeof(value), &value);
 
     EXPECT_BUFFER_U32_EQ(value, buffer, 0);
 }
@@ -169,7 +169,7 @@ TEST_P(BufferSetSubDataTests, SmallDataAtOffset) {
 
     constexpr uint64_t kOffset = 2000;
     uint32_t value = 0x01020304;
-    buffer.SetSubData(kOffset, sizeof(value), reinterpret_cast<uint8_t*>(&value));
+    buffer.SetSubData(kOffset, sizeof(value), &value);
 
     EXPECT_BUFFER_U32_EQ(value, buffer, kOffset);
 }
@@ -194,7 +194,7 @@ TEST_P(BufferSetSubDataTests, ManySetSubData) {
 
     std::vector<uint32_t> expectedData;
     for (uint32_t i = 0; i < kElements; ++i) {
-        buffer.SetSubData(i * sizeof(uint32_t), sizeof(i), reinterpret_cast<uint8_t*>(&i));
+        buffer.SetSubData(i * sizeof(uint32_t), sizeof(i), &i);
         expectedData.push_back(i);
     }
 
@@ -215,7 +215,7 @@ TEST_P(BufferSetSubDataTests, LargeSetSubData) {
         expectedData.push_back(i);
     }
 
-    buffer.SetSubData(0, kElements * sizeof(uint32_t), reinterpret_cast<uint8_t*>(expectedData.data()));
+    buffer.SetSubData(0, kElements * sizeof(uint32_t), expectedData.data());
 
     EXPECT_BUFFER_U32_RANGE_EQ(expectedData.data(), buffer, 0, kElements);
 }
@@ -377,7 +377,7 @@ TEST_P(CreateBufferMappedTests, MappableZeroInitialized) {
 
     dawn::CreateBufferMappedResult result = device.CreateBufferMapped(&descriptor);
     ASSERT_EQ(result.dataLength, descriptor.size);
-    ASSERT_EQ(*result.data, 0);
+    ASSERT_EQ(*reinterpret_cast<uint8_t*>(result.data), 0);
     result.buffer.Unmap();
 }
 
@@ -391,7 +391,7 @@ TEST_P(CreateBufferMappedTests, NonMappableZeroInitialized) {
 
     dawn::CreateBufferMappedResult result = device.CreateBufferMapped(&descriptor);
     ASSERT_EQ(result.dataLength, descriptor.size);
-    ASSERT_EQ(*result.data, 0);
+    ASSERT_EQ(*reinterpret_cast<uint8_t*>(result.data), 0);
     result.buffer.Unmap();
 }
 

src/tests/end2end/ComputeCopyStorageBufferTests.cpp

@@ -59,7 +59,7 @@ void ComputeCopyStorageBufferTests::BasicTest(const char* shader) {
     for (uint32_t i = 0; i < kNumUints; ++i) {
         expected[i] = (i + 1u) * 0x11111111u;
     }
-    src.SetSubData(0, sizeof(expected), reinterpret_cast<const uint8_t*>(expected.data()));
+    src.SetSubData(0, sizeof(expected), expected.data());
     EXPECT_BUFFER_U32_RANGE_EQ(expected.data(), src, 0, kNumUints);
 
     // Set up dst storage buffer
@@ -70,7 +70,7 @@ void ComputeCopyStorageBufferTests::BasicTest(const char* shader) {
     dawn::Buffer dst = device.CreateBuffer(&dstDesc);
 
     std::array<uint32_t, kNumUints> zero{};
-    dst.SetSubData(0, sizeof(zero), reinterpret_cast<const uint8_t*>(zero.data()));
+    dst.SetSubData(0, sizeof(zero), zero.data());
 
     // Set up bind group and issue dispatch
     dawn::BindGroup bindGroup = utils::MakeBindGroup(device, bgl, {

src/tests/end2end/ComputeSharedMemoryTests.cpp

@@ -53,7 +53,7 @@ void ComputeSharedMemoryTests::BasicTest(const char* shader) {
     dawn::Buffer dst = device.CreateBuffer(&dstDesc);
 
     const uint32_t zero = 0;
-    dst.SetSubData(0, sizeof(zero), reinterpret_cast<const uint8_t*>(&zero));
+    dst.SetSubData(0, sizeof(zero), &zero);
 
     // Set up bind group and issue dispatch
     dawn::BindGroup bindGroup = utils::MakeBindGroup(device, bgl, {

src/tests/end2end/CopyTests.cpp

@@ -121,7 +121,8 @@ class CopyTests_T2B : public CopyTests {
             bufDescriptor.usage = dawn::BufferUsageBit::TransferSrc | dawn::BufferUsageBit::TransferDst;
             dawn::Buffer buffer = device.CreateBuffer(&bufDescriptor);
             std::vector<RGBA8> emptyData(bufferSpec.size / kBytesPerTexel * textureSpec.arraySize);
-            buffer.SetSubData(0, static_cast<uint32_t>(emptyData.size() * sizeof(RGBA8)), reinterpret_cast<const uint8_t*>(emptyData.data()));
+            buffer.SetSubData(0, static_cast<uint32_t>(emptyData.size() * sizeof(RGBA8)),
+                              emptyData.data());
 
             uint64_t bufferOffset = bufferSpec.offset;
             for (uint32_t slice = 0; slice < textureSpec.arraySize; ++slice) {
@@ -184,7 +185,8 @@ protected:
 
         std::vector<RGBA8> bufferData(bufferSpec.size / kBytesPerTexel);
         FillBufferData(bufferData.data(), bufferData.size());
-        buffer.SetSubData(0, static_cast<uint32_t>(bufferData.size() * sizeof(RGBA8)), reinterpret_cast<const uint8_t*>(bufferData.data()));
+        buffer.SetSubData(0, static_cast<uint32_t>(bufferData.size() * sizeof(RGBA8)),
+                          bufferData.data());
 
         // Create a texture that is `width` x `height` with (`level` + 1) mip levels.
         dawn::TextureDescriptor descriptor;
@@ -730,4 +732,4 @@ TEST_P(CopyTests_T2T, TextureMip) {
 
 // TODO(brandon1.jones@intel.com) Add test for ensuring blitCommandEncoder on Metal.
 
-DAWN_INSTANTIATE_TEST(CopyTests_T2T, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend);
+DAWN_INSTANTIATE_TEST(CopyTests_T2T, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend);

src/tests/unittests/validation/BufferValidationTests.cpp

@@ -440,7 +440,7 @@ TEST_F(BufferValidationTest, SetSubDataSuccess) {
     dawn::Buffer buf = CreateSetSubDataBuffer(4);
 
     uint32_t foo = 0x01020304;
-    buf.SetSubData(0, sizeof(foo), reinterpret_cast<uint8_t*>(&foo));
+    buf.SetSubData(0, sizeof(foo), &foo);
 }
 
 // Test error case for SetSubData out of bounds
@@ -498,7 +498,7 @@ TEST_F(BufferValidationTest, SetSubDataWithUnalignedOffset) {
 
     uint64_t kOffset = 2999;
     uint32_t value = 0x01020304;
-    ASSERT_DEVICE_ERROR(buf.SetSubData(kOffset, sizeof(value), reinterpret_cast<uint8_t*>(&value)));
+    ASSERT_DEVICE_ERROR(buf.SetSubData(kOffset, sizeof(value), &value));
 }
 
 // Test that it is valid to destroy an unmapped buffer

src/utils/DawnHelpers.cpp

@@ -122,7 +122,7 @@ namespace utils {
         descriptor.usage = usage | dawn::BufferUsageBit::TransferDst;
 
         dawn::Buffer buffer = device.CreateBuffer(&descriptor);
-        buffer.SetSubData(0, size, reinterpret_cast<const uint8_t*>(data));
+        buffer.SetSubData(0, size, data);
         return buffer;
     }