[PATCH v4 0/4] MR6289: dsound: Some 3D fixes.
-- v4: dsound: Add non-NaN value tests for SetOrientation(). dsound/tests: Add NaN tests for floating-point 3D functions. dsound: Add an angle check for SetOrientation(). dsound: Handle NaN values in the 3D code. https://gitlab.winehq.org/wine/wine/-/merge_requests/6289
From: Aida Jonikienė <aidas957(a)gmail.com> GTA San Andreas calls floating-point 3D functions with NaN values under certain circumstances which eventually results in dwTotalAmpFactor value being really high (and causing extremely loud audio output). --- dlls/dsound/sound3d.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/dlls/dsound/sound3d.c b/dlls/dsound/sound3d.c index 3fa9ff6801f..8fe10788235 100644 --- a/dlls/dsound/sound3d.c +++ b/dlls/dsound/sound3d.c @@ -65,7 +65,7 @@ static inline D3DVALUE ScalarProduct (const D3DVECTOR *a, const D3DVECTOR *b) c = (a->x*b->x) + (a->y*b->y) + (a->z*b->z); TRACE("(%f,%f,%f) * (%f,%f,%f) = %f)\n", a->x, a->y, a->z, b->x, b->y, b->z, c); - return c; + return !isnan(c) ? c : 0; } /* vector product (I believe it's called cross product in English */ @@ -77,6 +77,12 @@ static inline D3DVECTOR VectorProduct (const D3DVECTOR *a, const D3DVECTOR *b) c.z = (a->x*b->y) - (a->y*b->x); TRACE("(%f,%f,%f) x (%f,%f,%f) = (%f,%f,%f)\n", a->x, a->y, a->z, b->x, b->y, b->z, c.x, c.y, c.z); + if (isnan(c.x) || isnan(c.y) || isnan(c.z)) + { + c.x = 0; + c.y = 0; + c.z = 0; + } return c; } @@ -136,6 +142,12 @@ static inline D3DVECTOR VectorBetweenTwoPoints (const D3DVECTOR *a, const D3DVEC c.z = b->z - a->z; TRACE("A (%f,%f,%f), B (%f,%f,%f), AB = (%f,%f,%f)\n", a->x, a->y, a->z, b->x, b->y, b->z, c.x, c.y, c.z); + if (isnan(c.x) || isnan(c.y) || isnan(c.z)) + { + c.x = 0; + c.y = 0; + c.z = 0; + } return c; } @@ -144,7 +156,7 @@ static inline D3DVALUE ProjectVector (const D3DVECTOR *a, const D3DVECTOR *p) { D3DVALUE prod, result; prod = ScalarProduct(a, p); - result = prod/VectorMagnitude(p); + result = (VectorMagnitude(p) != 0) ? prod / VectorMagnitude(p) : 0; TRACE("length projection of (%f,%f,%f) on (%f,%f,%f) = %f\n", a->x, a->y, a->z, p->x, p->y, p->z, result); return result; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6289
From: Aida Jonikienė <aidas957(a)gmail.com> The subsequent tests reveal the angle between front and top vectors must be higher than 0 degrees. --- dlls/dsound/sound3d.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/dlls/dsound/sound3d.c b/dlls/dsound/sound3d.c index 8fe10788235..554d1de3b8f 100644 --- a/dlls/dsound/sound3d.c +++ b/dlls/dsound/sound3d.c @@ -969,16 +969,22 @@ static HRESULT WINAPI IDirectSound3DListenerImpl_SetOrientation(IDirectSound3DLi D3DVALUE xFront, D3DVALUE yFront, D3DVALUE zFront, D3DVALUE xTop, D3DVALUE yTop, D3DVALUE zTop, DWORD dwApply) { - IDirectSoundBufferImpl *This = impl_from_IDirectSound3DListener(iface); + IDirectSoundBufferImpl *This = impl_from_IDirectSound3DListener(iface); + D3DVECTOR front = {.x = xFront, .y = yFront, .z = zFront}; + D3DVECTOR top = {.x = xTop, .y = yTop, .z = zTop}; + D3DVALUE angle = 0; TRACE("setting: Front vector = (%f,%f,%f); Top vector = (%f,%f,%f); dwApply = %ld\n", xFront, yFront, zFront, xTop, yTop, zTop, dwApply); - This->device->ds3dl.vOrientFront.x = xFront; - This->device->ds3dl.vOrientFront.y = yFront; - This->device->ds3dl.vOrientFront.z = zFront; - This->device->ds3dl.vOrientTop.x = xTop; - This->device->ds3dl.vOrientTop.y = yTop; - This->device->ds3dl.vOrientTop.z = zTop; + + if ((angle = AngleBetweenVectorsDeg(&front, &top)) == 0.0f) + { + WARN("Angle %f is 0 degrees\n", angle); + return DSERR_INVALIDPARAM; + } + + This->device->ds3dl.vOrientFront = front; + This->device->ds3dl.vOrientTop = top; if (dwApply == DS3D_IMMEDIATE) { This->device->ds3dl_need_recalc = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6289
From: Aida Jonikienė <aidas957(a)gmail.com> --- dlls/dsound/tests/ds3d.c | 46 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/dlls/dsound/tests/ds3d.c b/dlls/dsound/tests/ds3d.c index 9c65f674b3b..33273b8a6e5 100644 --- a/dlls/dsound/tests/ds3d.c +++ b/dlls/dsound/tests/ds3d.c @@ -669,6 +669,7 @@ void test_buffer(LPDIRECTSOUND dso, LPDIRECTSOUNDBUFFER *dsbo, buffer_param.vPosition.x,buffer_param.vPosition.y, buffer_param.vPosition.z,DS3D_IMMEDIATE); ok(rc==DS_OK,"IDirectSound3dBuffer_SetPosition() failed: %08lx\n", rc); + } } /* Check the sound duration was within 10% of the expected value */ @@ -1256,6 +1257,7 @@ static void check_doppler(IDirectSound *dsound, IDirectSound3DListener *listener IDirectSound3DBuffer *buffer_3d; IDirectSoundBuffer *ref_buffer; IDirectSoundBuffer *buffer; + D3DVECTOR ds_vector1 = {0}; WAVEFORMATEX format; DSBUFFERDESC desc; DWORD locked_size; @@ -1345,6 +1347,50 @@ static void check_doppler(IDirectSound *dsound, IDirectSound3DListener *listener HeapFree(GetProcessHeap(), 0, data); + /* Test the NaN value behavior. */ + hr = IDirectSound3DBuffer_SetPosition(buffer_3d, NAN, 0, 0, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DBuffer_SetPosition(buffer_3d, 0, NAN, 0, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DBuffer_SetPosition(buffer_3d, 0, 0, NAN, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DBuffer_SetPosition(listener, NAN, NAN, NAN, DS3D_IMMEDIATE); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DBuffer_GetPosition(listener, &ds_vector1); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(isnan(ds_vector1.x), "Expected NaN, got %f.\n", ds_vector1.x); + ok(isnan(ds_vector1.y), "Expected NaN, got %f.\n", ds_vector1.y); + ok(isnan(ds_vector1.z), "Expected NaN, got %f.\n", ds_vector1.z); + hr = IDirectSound3DListener_SetPosition(listener, NAN, 0, 0, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetPosition(listener, 0, NAN, 0, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetPosition(listener, 0, 0, NAN, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetPosition(listener, NAN, NAN, NAN, DS3D_IMMEDIATE); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_GetPosition(listener, &ds_vector1); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(isnan(ds_vector1.x), "Expected NaN, got %f.\n", ds_vector1.x); + ok(isnan(ds_vector1.y), "Expected NaN, got %f.\n", ds_vector1.y); + ok(isnan(ds_vector1.z), "Expected NaN, got %f.\n", ds_vector1.z); + hr = IDirectSound3DListener_SetOrientation(listener, NAN, 0, 0, 0, 0, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, NAN, 0, 0, 0, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, NAN, 0, 0, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, 0, NAN, 0, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, 0, 0, NAN, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, 0, 0, 0, NAN, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, NAN, 0, NAN, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, NAN, NAN, NAN, NAN, NAN, NAN, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + /* Set to different values first to test that the frequency is updated. */ hr = IDirectSound3DListener_SetPosition(listener, 0, 0, 0, DS3D_DEFERRED); ok(hr == S_OK, "Got hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6289
From: Aida Jonikienė <aidas957(a)gmail.com> --- dlls/dsound/tests/ds3d.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/dlls/dsound/tests/ds3d.c b/dlls/dsound/tests/ds3d.c index 33273b8a6e5..3e68fe7e8ed 100644 --- a/dlls/dsound/tests/ds3d.c +++ b/dlls/dsound/tests/ds3d.c @@ -1254,10 +1254,10 @@ static void check_doppler(IDirectSound *dsound, IDirectSound3DListener *listener BOOL play, DWORD mode, float listener_pos, float listener_velocity, float buffer_pos, float buffer_velocity, DWORD set_freq, DWORD expected_freq) { + D3DVECTOR ds_vector1 = {0}, ds_vector2 = {0}; IDirectSound3DBuffer *buffer_3d; IDirectSoundBuffer *ref_buffer; IDirectSoundBuffer *buffer; - D3DVECTOR ds_vector1 = {0}; WAVEFORMATEX format; DSBUFFERDESC desc; DWORD locked_size; @@ -1391,6 +1391,39 @@ static void check_doppler(IDirectSound *dsound, IDirectSound3DListener *listener hr = IDirectSound3DListener_SetOrientation(listener, NAN, NAN, NAN, NAN, NAN, NAN, DS3D_DEFERRED); ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + /* Test the orientation functions. */ + memset(&ds_vector1, 0, sizeof(ds_vector1)); + hr = IDirectSound3DListener_GetOrientation(listener, &ds_vector1, &ds_vector2); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(ds_vector1.x == 0, "Expected 0.0, got %f.\n", ds_vector1.x); + ok(ds_vector1.y == 0, "Expected 0.0, got %f.\n", ds_vector1.y); + ok(ds_vector1.z == 1, "Expected 1.0, got %f.\n", ds_vector1.z); + ok(ds_vector2.x == 0, "Expected 0.0, got %f.\n", ds_vector2.x); + ok(ds_vector2.y == 1, "Expected 1.0, got %f.\n", ds_vector2.y); + ok(ds_vector2.z == 0, "Expected 0.0, got %f.\n", ds_vector2.z); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, 0, 0, 0, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 1, 1, 1, 1, 1, 1, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 1, 0, 0, 1, 0, DS3D_DEFERRED); + ok(hr == DSERR_INVALIDPARAM, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 1, 0, 0, 2, 1, 1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, -1, -1, 0, -1, 0, -1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 2, 0, 0, 1, 105, 1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 2, 0, 0, 1, 150, 1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 2, 0, 0, 1, 1200, 1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 2, 0, 0, 1, 1800, 1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, -1, 0, 0, 0, -1, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectSound3DListener_SetOrientation(listener, 0, 0, 1, 0, 1, 0, DS3D_DEFERRED); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + /* Set to different values first to test that the frequency is updated. */ hr = IDirectSound3DListener_SetPosition(listener, 0, 0, 0, DS3D_DEFERRED); ok(hr == S_OK, "Got hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6289
On Sun Nov 24 09:59:24 2024 +0000, Aida Jonikienė wrote:
The TestBot tests reveal some interesting results (so I should find a better approach instead of returning failure) I think I pushed the better approach
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6289#note_88761
The proposed NaN handling sounds like it's hiding another bug. Are we sure that these NaNs aren't coming from an earlier failure? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6289#note_88885
On Tue Nov 26 06:23:25 2024 +0000, Huw Davies wrote:
The proposed NaN handling sounds like it's hiding another bug. Are we sure that these NaNs aren't coming from an earlier failure? Those NaNs are primarily coming from some game bugs (which happen on Windows too) which cause a NaN position teleport (the game passes these values to the dsound calls)
It's a pretty severe issue but it shouldn't cause extremely loud audio output (that's why native dsound mutes the extremely loud audio completely while keeping the normal menu sounds audible) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6289#note_88887
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6289
On Tue Nov 26 06:23:25 2024 +0000, Aida Jonikienė wrote:
Those NaNs are primarily coming from some game bugs (which happen on Windows too) which cause a NaN position teleport (the game passes these values to the dsound calls) It's a pretty severe issue but it shouldn't cause extremely loud audio output (that's why native dsound mutes the extremely loud audio completely while keeping the normal menu sounds audible) I'd have probably tested the inputs, rather than the outputs, for NaN, but I've approved it anyway.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6289#note_88918
participants (2)
-
Aida Jonikienė -
Huw Davies (＠huw)