On 5 Oct 2021, at 15:33, Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Wed, 29 Sept 2021 at 17:40, Jan Sikorski <jsikorski(a)codeweavers.com> wrote:
--- dlls/wined3d/cs.c | 56 +++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 31 deletions(-)
Conceptually I think this is fine, although traditionally we'd use something like "wined3d: Use a single allocation for command list data." for the subject line.
@@ -3735,45 +3730,56 @@ HRESULT CDECL wined3d_deferred_context_record_command_list(struct wined3d_device { struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); struct wined3d_command_list *object; + size_t offset; + BYTE *memory;
We'd prefer uint8_t over BYTE here. (More broadly, we prefer standard types over Windows types inside wined3d.) You probably need neither "memory" nor "offset" though.
+ memory = heap_alloc(sizeof(*object) + deferred->resource_count * sizeof(*object->resources) + + deferred->upload_count * sizeof(*object->uploads) + + deferred->command_list_count * sizeof(*object->command_lists) + + deferred->query_count * sizeof(*object->queries) + + deferred->data_size); + + if (!memory) return E_OUTOFMEMORY;
+ object = (void *)memory; + offset = sizeof(*object); + memset(object, 0, sizeof(*object)); + object->refcount = 1; object->device = deferred->c.device;
- if (!(object->data = heap_alloc(deferred->data_size))) - goto out_free_list; - object->data_size = deferred->data_size; - memcpy(object->data, deferred->data, deferred->data_size); - - if (!(object->resources = heap_alloc(deferred->resource_count * sizeof(*object->resources)))) - goto out_free_data; + object->resources = (void *)(memory + offset); + offset += deferred->resource_count * sizeof(*object->resources); object->resource_count = deferred->resource_count; memcpy(object->resources, deferred->resources, deferred->resource_count * sizeof(*object->resources)); /* Transfer our references to the resources to the command list. */
- if (!(object->uploads = heap_alloc(deferred->upload_count * sizeof(*object->uploads)))) - goto out_free_resources; + object->uploads = (void *)(memory + offset); + offset += deferred->upload_count * sizeof(*object->uploads); object->upload_count = deferred->upload_count; memcpy(object->uploads, deferred->uploads, deferred->upload_count * sizeof(*object->uploads)); /* Transfer our references to the resources to the command list. */
We'd typically write that like this:
object = heap_alloc(...); ... object->resources = (struct wined3d_resource *)&object[1]; ... object->uploads = (struct wined3d_deferred_upload *)&object->resources[object->resource_count];
etc.
I’ve seen that elsewhere in the codebase, but I think it has some downsides. As is, each block of code only deals with a single field, and you can reorder these blocks around freely and add/remove without changing the ones next to it. Not that it’s a huge problem to do it, but are there reasons for this?