[PATCH v2 0/3] MR2833: wined3d: Optimize lights handling.
This fixes a performance regression introduced during state redesign shortly after Wine 5.0. Some games create a great amount of lights (around 1000 and growing) and touch them quite often. That results in each light being relayed to CS thread before each draw which is taking a great amount of time (performance drop in Nosferatu: The Wrath of Malachi from ~150 fps to ~10-15 at start location; that's with another unrelated regression related to streaming buffer management sorted out). It is also possible to use bitmasks like for other states, but then with these lights amount even iterating over the full set of lights during stateblock apply leads to noticable performance drop. With this patchset Nosferatu reaches ~180 fps on the same place (~115 on Windows on the same machine, with the difference being probably due to SWVP). -- v2: wined3d: Use RB tree for storing lights. https://gitlab.winehq.org/wine/wine/-/merge_requests/2833
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/wined3d/cs.c | 3 +-- dlls/wined3d/device.c | 4 ++-- dlls/wined3d/stateblock.c | 17 ++++++++++------- dlls/wined3d/wined3d_private.h | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index bca0b017c1f..f790e15cef0 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -2057,8 +2057,7 @@ static void wined3d_cs_exec_set_light_enable(struct wined3d_cs *cs, const void * } prev_idx = light_info->glIndex; - wined3d_light_state_enable_light(&cs->state.light_state, &device->adapter->d3d_info, light_info, op->enable); - if (light_info->glIndex != prev_idx) + if (wined3d_light_state_enable_light(&cs->state.light_state, &device->adapter->d3d_info, light_info, op->enable)) { device_invalidate_state(device, STATE_LIGHT_TYPE); device_invalidate_state(device, STATE_ACTIVELIGHT(op->enable ? light_info->glIndex : prev_idx)); diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index df186d9ffa2..775da5bac1f 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -1695,8 +1695,8 @@ static void wined3d_device_set_light_enable(struct wined3d_device *device, UINT } } - wined3d_light_state_enable_light(light_state, &device->adapter->d3d_info, light_info, enable); - wined3d_device_context_emit_set_light_enable(&device->cs->c, light_idx, enable); + if (wined3d_light_state_enable_light(light_state, &device->adapter->d3d_info, light_info, enable)) + wined3d_device_context_emit_set_light_enable(&device->cs->c, light_idx, enable); } static HRESULT wined3d_device_set_clip_plane(struct wined3d_device *device, diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c index 16f58b726d0..5f806e93908 100644 --- a/dlls/wined3d/stateblock.c +++ b/dlls/wined3d/stateblock.c @@ -634,7 +634,7 @@ HRESULT wined3d_light_state_set_light(struct wined3d_light_state *state, DWORD l return WINED3D_OK; } -void wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, +BOOL wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, struct wined3d_light_info *light_info, BOOL enable) { unsigned int light_count, i; @@ -644,18 +644,18 @@ void wined3d_light_state_enable_light(struct wined3d_light_state *state, const s if (light_info->glIndex == -1) { TRACE("Light already disabled, nothing to do.\n"); - return; + return FALSE; } state->lights[light_info->glIndex] = NULL; light_info->glIndex = -1; - return; + return TRUE; } if (light_info->glIndex != -1) { TRACE("Light already enabled, nothing to do.\n"); - return; + return FALSE; } /* Find a free light. */ @@ -667,7 +667,7 @@ void wined3d_light_state_enable_light(struct wined3d_light_state *state, const s state->lights[i] = light_info; light_info->glIndex = i; - return; + return TRUE; } /* Our tests show that Windows returns D3D_OK in this situation, even with @@ -677,6 +677,7 @@ void wined3d_light_state_enable_light(struct wined3d_light_state *state, const s * * TODO: Test how this affects rendering. */ WARN("Too many concurrently active lights.\n"); + return FALSE; } static void wined3d_state_record_lights(struct wined3d_light_state *dst_state, @@ -1616,8 +1617,10 @@ HRESULT CDECL wined3d_stateblock_set_light_enable(struct wined3d_stateblock *sta if (FAILED(hr = wined3d_light_state_set_light(light_state, light_idx, &WINED3D_default_light, &light_info))) return hr; } - wined3d_light_state_enable_light(light_state, &stateblock->device->adapter->d3d_info, light_info, enable); - stateblock->changed.lights = 1; + + if (wined3d_light_state_enable_light(light_state, &stateblock->device->adapter->d3d_info, light_info, enable)) + stateblock->changed.lights = 1; + return S_OK; } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index d28a86f77c2..56c933511dd 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -5049,7 +5049,7 @@ void wined3d_stateblock_state_init(struct wined3d_stateblock_state *state, const struct wined3d_device *device, uint32_t flags) DECLSPEC_HIDDEN; void wined3d_stateblock_state_cleanup(struct wined3d_stateblock_state *state) DECLSPEC_HIDDEN; -void wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, +BOOL wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, struct wined3d_light_info *light_info, BOOL enable) DECLSPEC_HIDDEN; struct wined3d_light_info *wined3d_light_state_get_light(const struct wined3d_light_state *state, unsigned int idx) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2833
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/wined3d/device.c | 17 ++++++------- dlls/wined3d/stateblock.c | 44 +++++++++++++++++++++------------- dlls/wined3d/wined3d_private.h | 3 +++ 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index 775da5bac1f..adbfffd4e7d 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -3965,15 +3965,14 @@ void CDECL wined3d_device_apply_stateblock(struct wined3d_device *device, if (changed->lights) { - for (i = 0; i < ARRAY_SIZE(state->light_state->light_map); ++i) - { - const struct wined3d_light_info *light; + struct wined3d_light_info *light, *cursor; - LIST_FOR_EACH_ENTRY(light, &state->light_state->light_map[i], struct wined3d_light_info, entry) - { - wined3d_device_context_set_light(context, light->OriginalIndex, &light->OriginalParms); - wined3d_device_set_light_enable(device, light->OriginalIndex, light->glIndex != -1); - } + LIST_FOR_EACH_ENTRY_SAFE(light, cursor, &changed->changed_lights, struct wined3d_light_info, changed_entry) + { + wined3d_device_context_set_light(context, light->OriginalIndex, &light->OriginalParms); + wined3d_device_set_light_enable(device, light->OriginalIndex, light->glIndex != -1); + list_remove(&light->changed_entry); + light->changed_entry.prev = light->changed_entry.next = NULL; } } @@ -4499,7 +4498,9 @@ void CDECL wined3d_device_apply_stateblock(struct wined3d_device *device, wined3d_device_set_clip_plane(device, i, &state->clip_planes[i]); } + assert(list_empty(&stateblock->changed.changed_lights)); memset(&stateblock->changed, 0, sizeof(stateblock->changed)); + list_init(&stateblock->changed.changed_lights); TRACE("Applied stateblock %p.\n", stateblock); } diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c index 5f806e93908..f63b2989c0d 100644 --- a/dlls/wined3d/stateblock.c +++ b/dlls/wined3d/stateblock.c @@ -340,8 +340,9 @@ void CDECL wined3d_stateblock_init_contained_states(struct wined3d_stateblock *s } } -static void stateblock_init_lights(struct list *dst_map, const struct list *src_map) +static void stateblock_init_lights(struct wined3d_stateblock *stateblock, const struct list *src_map) { + struct list *dst_map = stateblock->stateblock_state.light_state->light_map; unsigned int i; for (i = 0; i < LIGHTMAP_SIZE; ++i) @@ -354,6 +355,7 @@ static void stateblock_init_lights(struct list *dst_map, const struct list *src_ *dst_light = *src_light; list_add_tail(&dst_map[i], &dst_light->entry); + list_add_tail(&stateblock->changed.changed_lights, &dst_light->changed_entry); } } } @@ -545,6 +547,8 @@ void wined3d_stateblock_state_cleanup(struct wined3d_stateblock_state *state) { LIST_FOR_EACH_ENTRY_SAFE(light, cursor, &state->light_state->light_map[i], struct wined3d_light_info, entry) { + if (light->changed_entry.prev) + list_remove(&light->changed_entry); list_remove(&light->entry); heap_free(light); } @@ -570,6 +574,8 @@ void state_cleanup(struct wined3d_state *state) { struct wined3d_light_info *light = LIST_ENTRY(e1, struct wined3d_light_info, entry); list_remove(&light->entry); + if (light->changed_entry.prev) + list_remove(&light->changed_entry); heap_free(light); } } @@ -607,6 +613,13 @@ struct wined3d_light_info *wined3d_light_state_get_light(const struct wined3d_li return NULL; } +static void set_light_changed(struct wined3d_stateblock *stateblock, struct wined3d_light_info *light_info) +{ + if (!light_info->changed_entry.prev) + list_add_tail(&stateblock->changed.changed_lights, &light_info->changed_entry); + stateblock->changed.lights = 1; +} + HRESULT wined3d_light_state_set_light(struct wined3d_light_state *state, DWORD light_idx, const struct wined3d_light *params, struct wined3d_light_info **light_info) { @@ -1060,15 +1073,12 @@ void CDECL wined3d_stateblock_apply(const struct wined3d_stateblock *stateblock, if (stateblock->changed.lights) { - for (i = 0; i < ARRAY_SIZE(state->light_state->light_map); ++i) - { - const struct wined3d_light_info *light; + const struct wined3d_light_info *light; - LIST_FOR_EACH_ENTRY(light, &state->light_state->light_map[i], struct wined3d_light_info, entry) - { - wined3d_stateblock_set_light(device_state, light->OriginalIndex, &light->OriginalParms); - wined3d_stateblock_set_light_enable(device_state, light->OriginalIndex, light->glIndex != -1); - } + LIST_FOR_EACH_ENTRY(light, &stateblock->changed.changed_lights, struct wined3d_light_info, changed_entry) + { + wined3d_stateblock_set_light(device_state, light->OriginalIndex, &light->OriginalParms); + wined3d_stateblock_set_light_enable(device_state, light->OriginalIndex, light->glIndex != -1); } } @@ -1568,6 +1578,7 @@ HRESULT CDECL wined3d_stateblock_set_light(struct wined3d_stateblock *stateblock UINT light_idx, const struct wined3d_light *light) { struct wined3d_light_info *object = NULL; + HRESULT hr; TRACE("stateblock %p, light_idx %u, light %p.\n", stateblock, light_idx, light); @@ -1600,8 +1611,9 @@ HRESULT CDECL wined3d_stateblock_set_light(struct wined3d_stateblock *stateblock return WINED3DERR_INVALIDCALL; } - stateblock->changed.lights = 1; - return wined3d_light_state_set_light(stateblock->stateblock_state.light_state, light_idx, light, &object); + if (SUCCEEDED(hr = wined3d_light_state_set_light(stateblock->stateblock_state.light_state, light_idx, light, &object))) + set_light_changed(stateblock, object); + return hr; } HRESULT CDECL wined3d_stateblock_set_light_enable(struct wined3d_stateblock *stateblock, UINT light_idx, BOOL enable) @@ -1616,10 +1628,11 @@ HRESULT CDECL wined3d_stateblock_set_light_enable(struct wined3d_stateblock *sta { if (FAILED(hr = wined3d_light_state_set_light(light_state, light_idx, &WINED3D_default_light, &light_info))) return hr; + set_light_changed(stateblock, light_info); } if (wined3d_light_state_enable_light(light_state, &stateblock->device->adapter->d3d_info, light_info, enable)) - stateblock->changed.lights = 1; + set_light_changed(stateblock, light_info); return S_OK; } @@ -2008,6 +2021,7 @@ static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, const stru type == WINED3D_SBT_PRIMARY ? WINED3D_STATE_INIT_DEFAULT : 0); stateblock->changed.store_stream_offset = 1; + list_init(&stateblock->changed.changed_lights); if (type == WINED3D_SBT_RECORDED || type == WINED3D_SBT_PRIMARY) return WINED3D_OK; @@ -2017,8 +2031,7 @@ static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, const stru switch (type) { case WINED3D_SBT_ALL: - stateblock_init_lights(stateblock->stateblock_state.light_state->light_map, - device_state->stateblock_state.light_state->light_map); + stateblock_init_lights(stateblock, device_state->stateblock_state.light_state->light_map); stateblock_savedstates_set_all(&stateblock->changed, d3d_info->limits.vs_uniform_count, d3d_info->limits.ps_uniform_count); break; @@ -2029,8 +2042,7 @@ static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, const stru break; case WINED3D_SBT_VERTEX_STATE: - stateblock_init_lights(stateblock->stateblock_state.light_state->light_map, - device_state->stateblock_state.light_state->light_map); + stateblock_init_lights(stateblock, device_state->stateblock_state.light_state->light_map); stateblock_savedstates_set_vertex(&stateblock->changed, d3d_info->limits.vs_uniform_count); break; diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 56c933511dd..5c3b45fcc02 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2992,6 +2992,7 @@ struct wined3d_light_info float cutoff; struct list entry; + struct list changed_entry; }; /* The default light parameters */ @@ -5016,6 +5017,8 @@ struct wined3d_saved_states DWORD lights : 1; DWORD transforms : 1; DWORD padding : 1; + + struct list changed_lights; }; struct StageState { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2833
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/wined3d/cs.c | 5 +- dlls/wined3d/device.c | 62 +++++++------ dlls/wined3d/stateblock.c | 153 ++++++++++++++------------------- dlls/wined3d/wined3d_private.h | 8 +- 4 files changed, 97 insertions(+), 131 deletions(-) diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index f790e15cef0..6a09ec2a35a 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -1998,7 +1998,7 @@ static void wined3d_cs_exec_set_light(struct wined3d_cs *cs, const void *data) { const struct wined3d_cs_set_light *op = data; struct wined3d_light_info *light_info; - unsigned int light_idx, hash_idx; + unsigned int light_idx; light_idx = op->light.OriginalIndex; @@ -2011,10 +2011,9 @@ static void wined3d_cs_exec_set_light(struct wined3d_cs *cs, const void *data) return; } - hash_idx = LIGHTMAP_HASHFUNC(light_idx); - list_add_head(&cs->state.light_state.light_map[hash_idx], &light_info->entry); light_info->glIndex = -1; light_info->OriginalIndex = light_idx; + rb_put(&cs->state.light_state.lights_tree, (void *)(ULONG_PTR)light_idx, &light_info->entry); } if (light_info->glIndex != -1) diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index adbfffd4e7d..560c9013bd8 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -1907,7 +1907,7 @@ void CDECL wined3d_device_context_reset_state(struct wined3d_device_context *con void CDECL wined3d_device_context_set_state(struct wined3d_device_context *context, struct wined3d_state *state) { - const struct wined3d_light_info *light; + struct wined3d_light_info *light; unsigned int i, j; TRACE("context %p, state %p.\n", context, state); @@ -1991,13 +1991,10 @@ void CDECL wined3d_device_context_set_state(struct wined3d_device_context *conte wined3d_device_context_emit_set_viewports(context, state->viewport_count, state->viewports); wined3d_device_context_emit_set_scissor_rects(context, state->scissor_rect_count, state->scissor_rects); - for (i = 0; i < LIGHTMAP_SIZE; ++i) + RB_FOR_EACH_ENTRY(light, &state->light_state.lights_tree, struct wined3d_light_info, entry) { - LIST_FOR_EACH_ENTRY(light, &state->light_state.light_map[i], struct wined3d_light_info, entry) - { - wined3d_device_context_set_light(context, light->OriginalIndex, &light->OriginalParms); - wined3d_device_context_emit_set_light_enable(context, light->OriginalIndex, light->glIndex != -1); - } + wined3d_device_context_set_light(context, light->OriginalIndex, &light->OriginalParms); + wined3d_device_context_emit_set_light_enable(context, light->OriginalIndex, light->glIndex != -1); } for (i = 0; i < WINEHIGHEST_RENDER_STATE + 1; ++i) @@ -3074,6 +3071,7 @@ static void init_transformed_lights(struct lights_settings *ls, { const struct wined3d_light_info *lights[WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS]; const struct wined3d_light_info *light_info; + struct wined3d_light_info *light_iter; struct light_transformed *light; struct wined3d_vec4 vec4; unsigned int light_count; @@ -3105,39 +3103,37 @@ static void init_transformed_lights(struct lights_settings *ls, ls->normalise = !!state->render_states[WINED3D_RS_NORMALIZENORMALS]; ls->localviewer = !!state->render_states[WINED3D_RS_LOCALVIEWER]; - for (i = 0, index = 0; i < LIGHTMAP_SIZE && index < ARRAY_SIZE(lights); ++i) + index = 0; + RB_FOR_EACH_ENTRY(light_iter, &state->light_state.lights_tree, struct wined3d_light_info, entry) { - LIST_FOR_EACH_ENTRY(light_info, &state->light_state.light_map[i], struct wined3d_light_info, entry) - { - if (!light_info->enabled) - continue; - - switch (light_info->OriginalParms.type) - { - case WINED3D_LIGHT_DIRECTIONAL: - ++ls->directional_light_count; - break; + if (!light_iter->enabled) + continue; - case WINED3D_LIGHT_POINT: - ++ls->point_light_count; - break; + switch (light_iter->OriginalParms.type) + { + case WINED3D_LIGHT_DIRECTIONAL: + ++ls->directional_light_count; + break; - case WINED3D_LIGHT_SPOT: - ++ls->spot_light_count; - break; + case WINED3D_LIGHT_POINT: + ++ls->point_light_count; + break; - case WINED3D_LIGHT_PARALLELPOINT: - ++ls->parallel_point_light_count; - break; + case WINED3D_LIGHT_SPOT: + ++ls->spot_light_count; + break; - default: - FIXME("Unhandled light type %#x.\n", light_info->OriginalParms.type); - continue; - } - lights[index++] = light_info; - if (index == WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS) + case WINED3D_LIGHT_PARALLELPOINT: + ++ls->parallel_point_light_count; break; + + default: + FIXME("Unhandled light type %#x.\n", light_iter->OriginalParms.type); + continue; } + lights[index++] = light_iter; + if (index == WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS) + break; } light_count = index; diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c index f63b2989c0d..8dace902562 100644 --- a/dlls/wined3d/stateblock.c +++ b/dlls/wined3d/stateblock.c @@ -340,23 +340,18 @@ void CDECL wined3d_stateblock_init_contained_states(struct wined3d_stateblock *s } } -static void stateblock_init_lights(struct wined3d_stateblock *stateblock, const struct list *src_map) +static void stateblock_init_lights(struct wined3d_stateblock *stateblock, const struct rb_tree *src_tree) { - struct list *dst_map = stateblock->stateblock_state.light_state->light_map; - unsigned int i; + struct rb_tree *dst_tree = &stateblock->stateblock_state.light_state->lights_tree; + struct wined3d_light_info *src_light; - for (i = 0; i < LIGHTMAP_SIZE; ++i) + RB_FOR_EACH_ENTRY(src_light, src_tree, struct wined3d_light_info, entry) { - const struct wined3d_light_info *src_light; - - LIST_FOR_EACH_ENTRY(src_light, &src_map[i], struct wined3d_light_info, entry) - { - struct wined3d_light_info *dst_light = heap_alloc(sizeof(*dst_light)); + struct wined3d_light_info *dst_light = heap_alloc(sizeof(*dst_light)); - *dst_light = *src_light; - list_add_tail(&dst_map[i], &dst_light->entry); - list_add_tail(&stateblock->changed.changed_lights, &dst_light->changed_entry); - } + *dst_light = *src_light; + rb_put(dst_tree, (void *)(ULONG_PTR)dst_light->OriginalIndex, &dst_light->entry); + list_add_tail(&stateblock->changed.changed_lights, &dst_light->changed_entry); } } @@ -543,41 +538,34 @@ void wined3d_stateblock_state_cleanup(struct wined3d_stateblock_state *state) } } - for (i = 0; i < LIGHTMAP_SIZE; ++i) + RB_FOR_EACH_ENTRY_DESTRUCTOR(light, cursor, &state->light_state->lights_tree, struct wined3d_light_info, entry) { - LIST_FOR_EACH_ENTRY_SAFE(light, cursor, &state->light_state->light_map[i], struct wined3d_light_info, entry) - { - if (light->changed_entry.prev) - list_remove(&light->changed_entry); - list_remove(&light->entry); - heap_free(light); - } + if (light->changed_entry.prev) + list_remove(&light->changed_entry); + rb_remove(&state->light_state->lights_tree, &light->entry); + heap_free(light); } } void state_cleanup(struct wined3d_state *state) { - unsigned int counter; + struct wined3d_light_info *light, *cursor; + unsigned int i; if (!(state->flags & WINED3D_STATE_NO_REF)) state_unbind_resources(state); - for (counter = 0; counter < WINED3D_MAX_ACTIVE_LIGHTS; ++counter) + for (i = 0; i < WINED3D_MAX_ACTIVE_LIGHTS; ++i) { - state->light_state.lights[counter] = NULL; + state->light_state.lights[i] = NULL; } - for (counter = 0; counter < LIGHTMAP_SIZE; ++counter) + RB_FOR_EACH_ENTRY_DESTRUCTOR(light, cursor, &state->light_state.lights_tree, struct wined3d_light_info, entry) { - struct list *e1, *e2; - LIST_FOR_EACH_SAFE(e1, e2, &state->light_state.light_map[counter]) - { - struct wined3d_light_info *light = LIST_ENTRY(e1, struct wined3d_light_info, entry); - list_remove(&light->entry); - if (light->changed_entry.prev) - list_remove(&light->changed_entry); - heap_free(light); - } + if (light->changed_entry.prev) + list_remove(&light->changed_entry); + rb_remove(&state->light_state.lights_tree, &light->entry); + heap_free(light); } } @@ -600,17 +588,12 @@ ULONG CDECL wined3d_stateblock_decref(struct wined3d_stateblock *stateblock) struct wined3d_light_info *wined3d_light_state_get_light(const struct wined3d_light_state *state, unsigned int idx) { - struct wined3d_light_info *light_info; - unsigned int hash_idx; + struct rb_entry *entry; - hash_idx = LIGHTMAP_HASHFUNC(idx); - LIST_FOR_EACH_ENTRY(light_info, &state->light_map[hash_idx], struct wined3d_light_info, entry) - { - if (light_info->OriginalIndex == idx) - return light_info; - } + if (!(entry = rb_get(&state->lights_tree, (void *)(ULONG_PTR)idx))) + return NULL; - return NULL; + return RB_ENTRY_VALUE(entry, struct wined3d_light_info, entry); } static void set_light_changed(struct wined3d_stateblock *stateblock, struct wined3d_light_info *light_info) @@ -624,7 +607,6 @@ HRESULT wined3d_light_state_set_light(struct wined3d_light_state *state, DWORD l const struct wined3d_light *params, struct wined3d_light_info **light_info) { struct wined3d_light_info *object; - unsigned int hash_idx; if (!(object = wined3d_light_state_get_light(state, light_idx))) { @@ -635,10 +617,9 @@ HRESULT wined3d_light_state_set_light(struct wined3d_light_state *state, DWORD l return E_OUTOFMEMORY; } - hash_idx = LIGHTMAP_HASHFUNC(light_idx); - list_add_head(&state->light_map[hash_idx], &object->entry); object->glIndex = -1; object->OriginalIndex = light_idx; + rb_put(&state->lights_tree, (void *)(ULONG_PTR)light_idx, &object->entry); } object->OriginalParms = *params; @@ -698,44 +679,40 @@ static void wined3d_state_record_lights(struct wined3d_light_state *dst_state, { const struct wined3d_light_info *src; struct wined3d_light_info *dst; - UINT i; /* Lights... For a recorded state block, we just had a chain of actions * to perform, so we need to walk that chain and update any actions which * differ. */ - for (i = 0; i < LIGHTMAP_SIZE; ++i) + RB_FOR_EACH_ENTRY(dst, &dst_state->lights_tree, struct wined3d_light_info, entry) { - LIST_FOR_EACH_ENTRY(dst, &dst_state->light_map[i], struct wined3d_light_info, entry) + if ((src = wined3d_light_state_get_light(src_state, dst->OriginalIndex))) { - if ((src = wined3d_light_state_get_light(src_state, dst->OriginalIndex))) + dst->OriginalParms = src->OriginalParms; + + if (src->glIndex == -1 && dst->glIndex != -1) { - dst->OriginalParms = src->OriginalParms; - - if (src->glIndex == -1 && dst->glIndex != -1) - { - /* Light disabled. */ - dst_state->lights[dst->glIndex] = NULL; - } - else if (src->glIndex != -1 && dst->glIndex == -1) - { - /* Light enabled. */ - dst_state->lights[src->glIndex] = dst; - } - dst->glIndex = src->glIndex; + /* Light disabled. */ + dst_state->lights[dst->glIndex] = NULL; } - else + else if (src->glIndex != -1 && dst->glIndex == -1) { - /* This can happen if the light was originally created as a - * default light for SetLightEnable() while recording. */ - WARN("Light %u in dst_state %p does not exist in src_state %p.\n", - dst->OriginalIndex, dst_state, src_state); - - dst->OriginalParms = WINED3D_default_light; - if (dst->glIndex != -1) - { - dst_state->lights[dst->glIndex] = NULL; - dst->glIndex = -1; - } + /* Light enabled. */ + dst_state->lights[src->glIndex] = dst; + } + dst->glIndex = src->glIndex; + } + else + { + /* This can happen if the light was originally created as a + * default light for SetLightEnable() while recording. */ + WARN("Light %u in dst_state %p does not exist in src_state %p.\n", + dst->OriginalIndex, dst_state, src_state); + + dst->OriginalParms = WINED3D_default_light; + if (dst->glIndex != -1) + { + dst_state->lights[dst->glIndex] = NULL; + dst->glIndex = -1; } } } @@ -1891,18 +1868,21 @@ static void state_init_default(struct wined3d_state *state, const struct wined3d } } +static int lights_compare(const void *key, const struct rb_entry *entry) +{ + const struct wined3d_light_info *light = RB_ENTRY_VALUE(entry, struct wined3d_light_info, entry); + unsigned int original_index = (ULONG_PTR)key; + + return light->OriginalIndex - original_index; +} + void state_init(struct wined3d_state *state, const struct wined3d_d3d_info *d3d_info, uint32_t flags, enum wined3d_feature_level feature_level) { - unsigned int i; - state->feature_level = feature_level; state->flags = flags; - for (i = 0; i < LIGHTMAP_SIZE; i++) - { - list_init(&state->light_state.light_map[i]); - } + rb_init(&state->light_state.lights_tree, lights_compare); if (flags & WINED3D_STATE_INIT_DEFAULT) state_init_default(state, d3d_info); @@ -1997,12 +1977,7 @@ static void stateblock_state_init_default(struct wined3d_stateblock_state *state void wined3d_stateblock_state_init(struct wined3d_stateblock_state *state, const struct wined3d_device *device, uint32_t flags) { - unsigned int i; - - for (i = 0; i < ARRAY_SIZE(state->light_state->light_map); i++) - { - list_init(&state->light_state->light_map[i]); - } + rb_init(&state->light_state->lights_tree, lights_compare); if (flags & WINED3D_STATE_INIT_DEFAULT) stateblock_state_init_default(state, &device->adapter->d3d_info); @@ -2031,7 +2006,7 @@ static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, const stru switch (type) { case WINED3D_SBT_ALL: - stateblock_init_lights(stateblock, device_state->stateblock_state.light_state->light_map); + stateblock_init_lights(stateblock, &device_state->stateblock_state.light_state->lights_tree); stateblock_savedstates_set_all(&stateblock->changed, d3d_info->limits.vs_uniform_count, d3d_info->limits.ps_uniform_count); break; @@ -2042,7 +2017,7 @@ static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, const stru break; case WINED3D_SBT_VERTEX_STATE: - stateblock_init_lights(stateblock, device_state->stateblock_state.light_state->light_map); + stateblock_init_lights(stateblock, &device_state->stateblock_state.light_state->lights_tree); stateblock_savedstates_set_vertex(&stateblock->changed, d3d_info->limits.vs_uniform_count); break; diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 5c3b45fcc02..41492f7b595 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2991,7 +2991,7 @@ struct wined3d_light_info float exponent; float cutoff; - struct list entry; + struct rb_entry entry; struct list changed_entry; }; @@ -3879,13 +3879,9 @@ struct wined3d_rasterizer_state struct wine_rb_entry entry; }; -#define LIGHTMAP_SIZE 43 -#define LIGHTMAP_HASHFUNC(x) ((x) % LIGHTMAP_SIZE) - struct wined3d_light_state { - /* Light hashmap. Collisions are handled using linked lists. */ - struct list light_map[LIGHTMAP_SIZE]; + struct rb_tree lights_tree; const struct wined3d_light_info *lights[WINED3D_MAX_ACTIVE_LIGHTS]; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2833
```diff @@ -634,7 +634,7 @@ HRESULT wined3d_light_state_set_light(struct wined3d_light_state *state, DWORD l return WINED3D_OK; }
-void wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, +BOOL wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, struct wined3d_light_info *light_info, BOOL enable) { unsigned int light_count, i; ```
We probably prefer bool over BOOL here.
```diff +static int lights_compare(const void *key, const struct rb_entry *entry) +{ + const struct wined3d_light_info *light = RB_ENTRY_VALUE(entry, struct wined3d_light_info, entry); + unsigned int original_index = (ULONG_PTR)key; + + return light->OriginalIndex - original_index; +} ```
Subtraction is not generally a safe way to implement comparison functions. It'll probably be fine in practice because the indices will be relatively small, but I think we'll want to use wined3d_uint32_compare() here instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2833#note_33369
participants (3)
-
Henri Verbeet (＠hverbeet) -
Paul Gofman
-
Paul Gofman (＠gofman)