[PATCH 1/2] wined3d: Acquire sampler references for command lists by inspecting CS packets.
Signed-off-by: Jan Sikorski <jsikorski(a)codeweavers.com> --- This is meant to be followed by more patches that manage references to all the other objects that need to be kept alive for command lists. That includes: all the ones handled similarly to samplers (i.e. the ones that we put into dedicated arrays); wined3d_resources, which are currently acquired on every draw/dispatch call; and views on these resources, which are not handled at all at this point. Apart from simplifying the code to some degree, it will fix the size problem of wined3d_resources arrays, which can get pretty large when the application is doing many repeated draw/dispatch calls. --- dlls/wined3d/cs.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index 9bdd2c8ed98..9c7ec1edbf8 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -103,6 +103,8 @@ ULONG CDECL wined3d_command_list_incref(struct wined3d_command_list *list) return refcount; } +static void wined3d_decref_packet_stream_objects(const uint8_t *data, size_t data_size); + ULONG CDECL wined3d_command_list_decref(struct wined3d_command_list *list) { ULONG refcount = InterlockedDecrement(&list->refcount); @@ -133,6 +135,8 @@ ULONG CDECL wined3d_command_list_decref(struct wined3d_command_list *list) for (i = 0; i < list->sampler_count; ++i) wined3d_sampler_decref(list->samplers[i]); + wined3d_decref_packet_stream_objects(list->data, list->data_size); + wined3d_mutex_lock(); wined3d_cs_destroy_object(device->cs, wined3d_command_list_destroy_object, list); wined3d_mutex_unlock(); @@ -3627,6 +3631,62 @@ void wined3d_cs_destroy(struct wined3d_cs *cs) heap_free(cs); } +static void wined3d_decref_packet_stream_objects(const uint8_t *data, size_t data_size) +{ + size_t start = 0; + unsigned int i; + + while (start < data_size) + { + const struct wined3d_cs_packet *packet = (const struct wined3d_cs_packet *)&data[start]; + enum wined3d_cs_op opcode = *(const enum wined3d_cs_op *)packet->data; + + switch (opcode) + { + case WINED3D_CS_OP_SET_SAMPLERS: + { + struct wined3d_cs_set_samplers *cs_set + = (struct wined3d_cs_set_samplers *)packet->data; + for (i = 0; i < cs_set->count; ++i) + { + if (cs_set->samplers[i]) + wined3d_sampler_decref(cs_set->samplers[i]); + } + break; + } + default: + break; + } + + start += offsetof(struct wined3d_cs_packet, data[packet->size]); + } +} + +static void wined3d_incref_packet_objects(struct wined3d_cs_packet *packet) +{ + enum wined3d_cs_op opcode; + unsigned int i; + + opcode = *(const enum wined3d_cs_op *)packet->data; + + switch (opcode) + { + case WINED3D_CS_OP_SET_SAMPLERS: + { + struct wined3d_cs_set_samplers *cs_set + = (struct wined3d_cs_set_samplers *)packet->data; + for (i = 0; i < cs_set->count; ++i) + { + if (cs_set->samplers[i]) + wined3d_sampler_incref(cs_set->samplers[i]); + } + break; + } + default: + break; + } +} + struct wined3d_deferred_context { struct wined3d_device_context c; @@ -3689,16 +3749,19 @@ static void *wined3d_deferred_context_require_space(struct wined3d_device_contex packet = (struct wined3d_cs_packet *)((BYTE *)deferred->data + deferred->data_size); TRACE("size was %zu, adding %zu\n", (size_t)deferred->data_size, packet_size); - deferred->data_size += packet_size; packet->size = packet_size - header_size; return &packet->data; } static void wined3d_deferred_context_submit(struct wined3d_device_context *context, enum wined3d_cs_queue_id queue_id) { - assert(queue_id == WINED3D_CS_QUEUE_DEFAULT); + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + struct wined3d_cs_packet *packet; - /* Nothing to do. */ + assert(queue_id == WINED3D_CS_QUEUE_DEFAULT); + packet = (struct wined3d_cs_packet *)((BYTE *)deferred->data + deferred->data_size); + deferred->data_size += packet->size + offsetof(struct wined3d_cs_packet, data[0]); + wined3d_incref_packet_objects(packet); } static void wined3d_deferred_context_finish(struct wined3d_device_context *context, enum wined3d_cs_queue_id queue_id) @@ -4023,6 +4086,8 @@ void CDECL wined3d_deferred_context_destroy(struct wined3d_device_context *conte wined3d_sampler_decref(deferred->samplers[i]); heap_free(deferred->samplers); + wined3d_decref_packet_stream_objects(deferred->data, deferred->data_size); + wined3d_state_destroy(deferred->c.state); heap_free(deferred->data); heap_free(deferred); -- 2.32.0
This reverts commit 0056ebf91ef3dd6a903e87adbdc1460bc08b4181. Sampler references in deferred contexts are now managed by inspecting CS packets. Signed-off-by: Jan Sikorski <jsikorski(a)codeweavers.com> --- dlls/wined3d/cs.c | 55 ---------------------------------- dlls/wined3d/wined3d_private.h | 2 -- 2 files changed, 57 deletions(-) diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index 9c7ec1edbf8..8f46d2abf18 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -76,9 +76,6 @@ struct wined3d_command_list SIZE_T shader_count; struct wined3d_shader **shaders; - - SIZE_T sampler_count; - struct wined3d_sampler **samplers; }; static void wined3d_command_list_destroy_object(void *object) @@ -132,8 +129,6 @@ ULONG CDECL wined3d_command_list_decref(struct wined3d_command_list *list) wined3d_depth_stencil_state_decref(list->depth_stencil_states[i]); for (i = 0; i < list->shader_count; ++i) wined3d_shader_decref(list->shaders[i]); - for (i = 0; i < list->sampler_count; ++i) - wined3d_sampler_decref(list->samplers[i]); wined3d_decref_packet_stream_objects(list->data, list->data_size); @@ -642,12 +637,6 @@ static inline void wined3d_device_context_acquire_shader(struct wined3d_device_c context->ops->acquire_shader(context, shader); } -static inline void wined3d_device_context_acquire_samplers(struct wined3d_device_context *context, - struct wined3d_sampler * const *samplers, unsigned int count) -{ - context->ops->acquire_samplers(context, samplers, count); -} - static struct wined3d_cs *wined3d_cs_from_context(struct wined3d_device_context *context) { return CONTAINING_RECORD(context, struct wined3d_cs, c); @@ -1835,7 +1824,6 @@ void wined3d_device_context_emit_set_samplers(struct wined3d_device_context *con op->count = count; memcpy(op->samplers, samplers, count * sizeof(*samplers)); - wined3d_device_context_acquire_samplers(context, samplers, count); wined3d_device_context_submit(context, WINED3D_CS_QUEUE_DEFAULT); } @@ -3045,11 +3033,6 @@ static void wined3d_cs_acquire_shader(struct wined3d_device_context *context, { } -static void wined3d_cs_acquire_samplers(struct wined3d_device_context *context, - struct wined3d_sampler * const *samplers, unsigned int count) -{ -} - static void wined3d_cs_exec_execute_command_list(struct wined3d_cs *cs, const void *data); static void (* const wined3d_cs_op_handlers[])(struct wined3d_cs *cs, const void *data) = @@ -3293,7 +3276,6 @@ static const struct wined3d_device_context_ops wined3d_cs_st_ops = wined3d_cs_acquire_rasterizer_state, wined3d_cs_acquire_depth_stencil_state, wined3d_cs_acquire_shader, - wined3d_cs_acquire_samplers, }; static BOOL wined3d_cs_queue_is_empty(const struct wined3d_cs *cs, const struct wined3d_cs_queue *queue) @@ -3425,7 +3407,6 @@ static const struct wined3d_device_context_ops wined3d_cs_mt_ops = wined3d_cs_acquire_rasterizer_state, wined3d_cs_acquire_depth_stencil_state, wined3d_cs_acquire_shader, - wined3d_cs_acquire_samplers, }; static void poll_queries(struct wined3d_cs *cs) @@ -3720,9 +3701,6 @@ struct wined3d_deferred_context SIZE_T shader_count, shaders_capacity; struct wined3d_shader **shaders; - - SIZE_T sampler_count, samplers_capacity; - struct wined3d_sampler **samplers; }; static struct wined3d_deferred_context *wined3d_deferred_context_from_context(struct wined3d_device_context *context) @@ -3972,26 +3950,6 @@ static void wined3d_deferred_context_acquire_shader(struct wined3d_device_contex wined3d_shader_incref(shader); } -static void wined3d_deferred_context_acquire_samplers(struct wined3d_device_context *context, - struct wined3d_sampler * const *samplers, unsigned int count) -{ - struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); - unsigned int i; - - if (!wined3d_array_reserve((void **)&deferred->samplers, &deferred->samplers_capacity, - deferred->sampler_count + count, sizeof(*deferred->samplers))) - return; - - for (i = 0; i < count; ++i) - { - if (samplers[i]) - { - deferred->samplers[deferred->sampler_count++] = samplers[i]; - wined3d_sampler_incref(samplers[i]); - } - } -} - static const struct wined3d_device_context_ops wined3d_deferred_context_ops = { wined3d_deferred_context_require_space, @@ -4008,7 +3966,6 @@ static const struct wined3d_device_context_ops wined3d_deferred_context_ops = wined3d_deferred_context_acquire_rasterizer_state, wined3d_deferred_context_acquire_depth_stencil_state, wined3d_deferred_context_acquire_shader, - wined3d_deferred_context_acquire_samplers, }; HRESULT CDECL wined3d_deferred_context_create(struct wined3d_device *device, struct wined3d_device_context **context) @@ -4082,10 +4039,6 @@ void CDECL wined3d_deferred_context_destroy(struct wined3d_device_context *conte wined3d_shader_decref(deferred->shaders[i]); heap_free(deferred->shaders); - for (i = 0; i < deferred->sampler_count; ++i) - wined3d_sampler_decref(deferred->samplers[i]); - heap_free(deferred->samplers); - wined3d_decref_packet_stream_objects(deferred->data, deferred->data_size); wined3d_state_destroy(deferred->c.state); @@ -4111,7 +4064,6 @@ HRESULT CDECL wined3d_deferred_context_record_command_list(struct wined3d_device + deferred->rasterizer_state_count * sizeof(*object->rasterizer_states) + deferred->depth_stencil_state_count * sizeof(*object->depth_stencil_states) + deferred->shader_count * sizeof(*object->shaders) - + deferred->sampler_count * sizeof(*object->samplers) + deferred->data_size); if (!memory) @@ -4177,12 +4129,6 @@ HRESULT CDECL wined3d_deferred_context_record_command_list(struct wined3d_device memcpy(object->shaders, deferred->shaders, deferred->shader_count * sizeof(*object->shaders)); /* Transfer our references to the shaders to the command list. */ - object->samplers = memory; - memory = &object->samplers[deferred->sampler_count]; - object->sampler_count = deferred->sampler_count; - memcpy(object->samplers, deferred->samplers, deferred->sampler_count * sizeof(*object->samplers)); - /* Transfer our references to the samplers to the command list. */ - object->data = memory; object->data_size = deferred->data_size; memcpy(object->data, deferred->data, deferred->data_size); @@ -4196,7 +4142,6 @@ HRESULT CDECL wined3d_deferred_context_record_command_list(struct wined3d_device deferred->rasterizer_state_count = 0; deferred->depth_stencil_state_count = 0; deferred->shader_count = 0; - deferred->sampler_count = 0; /* This is in fact recorded into a subsequent command list. */ if (restore) diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 2fce585c6b1..eeefb4f5be6 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4854,8 +4854,6 @@ struct wined3d_device_context_ops void (*acquire_depth_stencil_state)(struct wined3d_device_context *context, struct wined3d_depth_stencil_state *depth_stencil_state); void (*acquire_shader)(struct wined3d_device_context *context, struct wined3d_shader *shader); - void (*acquire_samplers)(struct wined3d_device_context *context, struct wined3d_sampler * const *samplers, - unsigned int count); }; struct wined3d_device_context -- 2.32.0
On Tue, 23 Nov 2021 at 14:26, Jan Sikorski <jsikorski(a)codeweavers.com> wrote:
--- This is meant to be followed by more patches that manage references to all the other objects that need to be kept alive for command lists.
That includes: all the ones handled similarly to samplers (i.e. the ones that we put into dedicated arrays); wined3d_resources, which are currently acquired on every draw/dispatch call; and views on these resources, which are not handled at all at this point.
Apart from simplifying the code to some degree, it will fix the size problem of wined3d_resources arrays, which can get pretty large when the application is doing many repeated draw/dispatch calls. --- dlls/wined3d/cs.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-)
Conceptually, I think this patch is fine.
+static void wined3d_decref_packet_stream_objects(const uint8_t *data, size_t data_size) +{ + size_t start = 0; + unsigned int i; + + while (start < data_size) + { + const struct wined3d_cs_packet *packet = (const struct wined3d_cs_packet *)&data[start]; + enum wined3d_cs_op opcode = *(const enum wined3d_cs_op *)packet->data; + + switch (opcode) + { + case WINED3D_CS_OP_SET_SAMPLERS: + { + struct wined3d_cs_set_samplers *cs_set + = (struct wined3d_cs_set_samplers *)packet->data; + for (i = 0; i < cs_set->count; ++i) + { + if (cs_set->samplers[i]) + wined3d_sampler_decref(cs_set->samplers[i]); + } + break; + } + default: + break; + } + + start += offsetof(struct wined3d_cs_packet, data[packet->size]); + } +} + +static void wined3d_incref_packet_objects(struct wined3d_cs_packet *packet) +{ + enum wined3d_cs_op opcode; + unsigned int i; + + opcode = *(const enum wined3d_cs_op *)packet->data; + + switch (opcode) + { + case WINED3D_CS_OP_SET_SAMPLERS: + { + struct wined3d_cs_set_samplers *cs_set + = (struct wined3d_cs_set_samplers *)packet->data; + for (i = 0; i < cs_set->count; ++i) + { + if (cs_set->samplers[i]) + wined3d_sampler_incref(cs_set->samplers[i]); + } + break; + } + default: + break; + } +} + The naming/structuring of these is a bit awkward, I think. Following the usual scheme, wined3d_incref_packet_objects() would be called wined3d_cs_packet_incref_objects(). It's counterpart would then be wined3d_cs_packet_decref_objects(). The introduction of wined3d_cs_packet_decref_objects() would make wined3d_decref_packet_stream_objects() a fairly straightforward loop; we could just inline it in its callers. A helper to extract a packet from a stream would further simplify it, and we could then use that helper in wined3d_cs_run() as well.
The name "cs_set" is a bit odd, I think. The conventional name for these is just "op". If we wanted to use a more verbose name, "set_samplers" after the operation name would also make sense, but it's not clear to me what "cs_set" refers to.
static void wined3d_deferred_context_submit(struct wined3d_device_context *context, enum wined3d_cs_queue_id queue_id) { - assert(queue_id == WINED3D_CS_QUEUE_DEFAULT); + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + struct wined3d_cs_packet *packet;
- /* Nothing to do. */ + assert(queue_id == WINED3D_CS_QUEUE_DEFAULT); + packet = (struct wined3d_cs_packet *)((BYTE *)deferred->data + deferred->data_size); + deferred->data_size += packet->size + offsetof(struct wined3d_cs_packet, data[0]); + wined3d_incref_packet_objects(packet); }
"packet = (struct wined3d_cs_packet *)&((uint8_t *)deferred->data)[deferred->data_size];", arguably. Not so much an issue with this patch itself, but now that we're doing these kind of size calculations in more places, perhaps it's also worth introducing helper functions for them. (E.g., "offsetof(struct wined3d_cs_packet, data[0])" is the size of the packet header, and that may not be immediately obvious to a casual observer.)
participants (2)
-
Henri Verbeet -
Jan Sikorski