[PATCH v3 1/3] ddraw: Fix ddraw[1-4] light activation / deactivation.
Related to bug https://bugs.winehq.org/show_bug.cgi?id=42572, while the bug depends on 2 more unrelated issues. Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- v2: Deactivate light if setting inactive light in d3d_light_SetLight(). v3: - Don't trace iface pointer in d3d_device3_SetCurrentViewport(); - Check if viewport is current in light_[de]activate(); - Also call viewport_deactivate() in d3d_device3_DeleteViewport() when deleting current viewport; - Do not check for active device in d3d_viewport_AddLight() (light_activate() checks that anyway). dlls/ddraw/ddraw_private.h | 1 + dlls/ddraw/device.c | 38 +++++++++++++++++--------------------- dlls/ddraw/light.c | 27 ++++++++++++--------------- dlls/ddraw/tests/ddraw4.c | 11 ++++++++++- dlls/ddraw/viewport.c | 19 +++++++++++-------- 5 files changed, 51 insertions(+), 45 deletions(-) diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h index 087c7092f9..2e58123463 100644 --- a/dlls/ddraw/ddraw_private.h +++ b/dlls/ddraw/ddraw_private.h @@ -519,6 +519,7 @@ struct d3d_viewport *unsafe_impl_from_IDirect3DViewport(IDirect3DViewport *iface /* Helper functions */ void viewport_activate(struct d3d_viewport *viewport, BOOL ignore_lights) DECLSPEC_HIDDEN; +void viewport_deactivate(struct d3d_viewport *viewport) DECLSPEC_HIDDEN; void d3d_viewport_init(struct d3d_viewport *viewport, struct ddraw *ddraw) DECLSPEC_HIDDEN; /***************************************************************************** diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index e80599da0f..047cfae0eb 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -819,7 +819,9 @@ static HRESULT WINAPI d3d_device3_DeleteViewport(IDirect3DDevice3 *iface, IDirec if (device->current_viewport == vp) { - TRACE("Deleting current viewport, unsetting and releasing\n"); + TRACE("Deleting current viewport, unsetting and releasing.\n"); + + viewport_deactivate(vp); IDirect3DViewport3_Release(viewport); device->current_viewport = NULL; } @@ -1689,48 +1691,42 @@ static HRESULT WINAPI d3d_device1_GetDirect3D(IDirect3DDevice *iface, IDirect3D * (Is a NULL viewport valid?) * *****************************************************************************/ -static HRESULT WINAPI d3d_device3_SetCurrentViewport(IDirect3DDevice3 *iface, IDirect3DViewport3 *Direct3DViewport3) +static HRESULT WINAPI d3d_device3_SetCurrentViewport(IDirect3DDevice3 *iface, IDirect3DViewport3 *viewport) { - struct d3d_device *This = impl_from_IDirect3DDevice3(iface); - struct d3d_viewport *vp = unsafe_impl_from_IDirect3DViewport3(Direct3DViewport3); + struct d3d_viewport *vp = unsafe_impl_from_IDirect3DViewport3(viewport); + struct d3d_device *device = impl_from_IDirect3DDevice3(iface); - TRACE("iface %p, viewport %p.\n", iface, Direct3DViewport3); + TRACE("iface %p, viewport %p, current_viewport %p.\n", iface, viewport, device->current_viewport); if (!vp) { - WARN("Direct3DViewport3 is NULL, returning DDERR_INVALIDPARAMS\n"); + WARN("Direct3DViewport3 is NULL.\n"); return DDERR_INVALIDPARAMS; } wined3d_mutex_lock(); /* Do nothing if the specified viewport is the same as the current one */ - if (This->current_viewport == vp) + if (device->current_viewport == vp) { wined3d_mutex_unlock(); return D3D_OK; } - if (vp->active_device != This) + if (vp->active_device != device) { - WARN("Viewport %p active device is %p.\n", vp, vp->active_device); + WARN("Viewport %p, active device %p.\n", vp, vp->active_device); wined3d_mutex_unlock(); return DDERR_INVALIDPARAMS; } - /* Release previous viewport and AddRef the new one */ - if (This->current_viewport) + IDirect3DViewport3_AddRef(viewport); + if (device->current_viewport) { - TRACE("ViewportImpl is at %p, interface is at %p\n", This->current_viewport, - &This->current_viewport->IDirect3DViewport3_iface); - IDirect3DViewport3_Release(&This->current_viewport->IDirect3DViewport3_iface); + viewport_deactivate(device->current_viewport); + IDirect3DViewport3_Release(&device->current_viewport->IDirect3DViewport3_iface); } - IDirect3DViewport3_AddRef(Direct3DViewport3); - - /* Set this viewport as the current viewport */ - This->current_viewport = vp; - - /* Activate this viewport */ - viewport_activate(This->current_viewport, FALSE); + device->current_viewport = vp; + viewport_activate(device->current_viewport, FALSE); wined3d_mutex_unlock(); diff --git a/dlls/ddraw/light.c b/dlls/ddraw/light.c index 9d0bb6028f..c19e34cb70 100644 --- a/dlls/ddraw/light.c +++ b/dlls/ddraw/light.c @@ -54,15 +54,14 @@ void light_activate(struct d3d_light *light) TRACE("light %p.\n", light); - if (!light->active_viewport || !light->active_viewport->active_device) return; + if (!light->active_viewport || !light->active_viewport->active_device + || light->active_viewport->active_device->current_viewport != light->active_viewport) + return; device = light->active_viewport->active_device; light_update(light); - if (!(light->light.dwFlags & D3DLIGHT_ACTIVE)) - { + if (light->light.dwFlags & D3DLIGHT_ACTIVE) IDirect3DDevice7_LightEnable(&device->IDirect3DDevice7_iface, light->dwLightIndex, TRUE); - light->light.dwFlags |= D3DLIGHT_ACTIVE; - } } /***************************************************************************** @@ -78,14 +77,13 @@ void light_deactivate(struct d3d_light *light) TRACE("light %p.\n", light); - if (!light->active_viewport || !light->active_viewport->active_device) return; - device = light->active_viewport->active_device; + if (!light->active_viewport || !light->active_viewport->active_device + || light->active_viewport->active_device->current_viewport != light->active_viewport) + return; + device = light->active_viewport->active_device; if (light->light.dwFlags & D3DLIGHT_ACTIVE) - { IDirect3DDevice7_LightEnable(&device->IDirect3DDevice7_iface, light->dwLightIndex, FALSE); - light->light.dwFlags &= ~D3DLIGHT_ACTIVE; - } } static inline struct d3d_light *impl_from_IDirect3DLight(IDirect3DLight *iface) @@ -195,13 +193,12 @@ static HRESULT WINAPI d3d_light_SetLight(IDirect3DLight *iface, D3DLIGHT *data) wined3d_mutex_lock(); memcpy(&light->light, data, sizeof(*data)); - if (!(light->light.dwFlags & D3DLIGHT_ACTIVE) && flags & D3DLIGHT_ACTIVE) - light_activate(light); - else if (light->light.dwFlags & D3DLIGHT_ACTIVE && !(flags & D3DLIGHT_ACTIVE)) + + if (!(flags & D3DLIGHT_ACTIVE)) light_deactivate(light); - else if (flags & D3DLIGHT_ACTIVE) - light_update(light); + light->light.dwFlags = flags; + light_activate(light); wined3d_mutex_unlock(); return D3D_OK; diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 56ff8a66bc..8403c3d754 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -4189,7 +4189,16 @@ static void test_lighting(void) light_desc.dwFlags = D3DLIGHT_ACTIVE; hr = IDirect3DLight_SetLight(light, (D3DLIGHT *)&light_desc); - ok(SUCCEEDED(hr), "Failed to set light, hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DViewport3_DeleteLight(viewport, light); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + light_desc.dwFlags = 0; + hr = IDirect3DLight_GetLight(light, (D3DLIGHT *)&light_desc); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + ok(light_desc.dwFlags == D3DLIGHT_ACTIVE, "Got unexpected flags %#x.\n", light_desc.dwFlags); + + hr = IDirect3DViewport3_AddLight(viewport, light); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); for (i = 0; i < ARRAY_SIZE(tests); ++i) { diff --git a/dlls/ddraw/viewport.c b/dlls/ddraw/viewport.c index e7e4463511..daa5ac8d5a 100644 --- a/dlls/ddraw/viewport.c +++ b/dlls/ddraw/viewport.c @@ -112,6 +112,16 @@ void viewport_activate(struct d3d_viewport *This, BOOL ignore_lights) IDirect3DDevice7_SetViewport(&This->active_device->IDirect3DDevice7_iface, &vp); } +void viewport_deactivate(struct d3d_viewport *viewport) +{ + struct d3d_light *light; + + LIST_FOR_EACH_ENTRY(light, &viewport->light_list, struct d3d_light, entry) + { + light_deactivate(light); + } +} + /***************************************************************************** * _dump_D3DVIEWPORT, _dump_D3DVIEWPORT2 * @@ -783,14 +793,7 @@ static HRESULT WINAPI d3d_viewport_AddLight(IDirect3DViewport3 *iface, IDirect3D /* Attach the light to the viewport */ light_impl->active_viewport = This; - - /* If active, activate the light */ - if (This->active_device && light_impl->light.dwFlags & D3DLIGHT_ACTIVE) - { - /* Disable the flag so that light_activate actually does its job. */ - light_impl->light.dwFlags &= ~D3DLIGHT_ACTIVE; - light_activate(light_impl); - } + light_activate(light_impl); wined3d_mutex_unlock(); -- 2.20.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=50102 Your paranoid android. === debian9 (build log) === X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) === debian9 (build log) === X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig)
participants (3)
-
Henri Verbeet -
Marvin -
Paul Gofman