[PATCH 1/2] dinput: Fix phase conversion on periodic effects

[Matteo Bruni]

2016-09-14 7:25 GMT+02:00 Bruno Jesus <00cpxxx at gmail.com>:
> Based on idea by Elias Vanderstuyft.
>
> Signed-off-by: Bruno Jesus <00cpxxx at gmail.com>
> ---
>  dlls/dinput/effect_linuxinput.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks for giving dinput and force feedback some love and care lately.
I'm going to be a nag though...

> diff --git a/dlls/dinput/effect_linuxinput.c b/dlls/dinput/effect_linuxinput.c
> index 9205c9c..bb45792 100644
> --- a/dlls/dinput/effect_linuxinput.c
> +++ b/dlls/dinput/effect_linuxinput.c
> @@ -632,7 +632,8 @@ static HRESULT WINAPI LinuxInputEffectImpl_SetParameters(
>
>              This->effect.u.periodic.magnitude = (tsp->dwMagnitude / 10) * 32;
>              This->effect.u.periodic.offset = (tsp->lOffset / 10) * 32;
> -            This->effect.u.periodic.phase = (tsp->dwPhase / 9) * 8; /* == (/ 36 * 32) */
> +            /* phase ranges from 0 - 35999 in dinput and 0 - 65535 on linux */
> +            This->effect.u.periodic.phase = (tsp->dwPhase / 36) * 65;

Probably I'm missing something but... Why don't you actually use 35999
and 65535 in the expression?
Relatedly, why not do the multiplication before the division instead
of throwing bits of precision away? Maybe phase can go over 35999? If
that's the only reason, I'd adjust the phase before doing this
computation instead.
I guess these points apply to the other effect parameters too.

BTW, you didn't change the corresponding piece of code in
GetParameters (which, for some reason, uses 33 and 36 as scaling
constants...) so that's probably broken now. Or at least more broken
than it was before.