berarma / new-lg4ff

Experimental Logitech force feedback module for Linux
GNU General Public License v2.0
297 stars 18 forks source link

32bit regression #96

Open zakk4223 opened 3 weeks ago

zakk4223 commented 3 weeks ago

I'm using lg4ff on an armv7l system (32bit) and it looks like this commit 2b865afe016b1a25a7a0ff921f94f81f3d1c2969 regressed the previous 32bit fix.

I'm getting what I think is the same behavior that was originally reported in #47. FF_CONSTANT direction is mostly ignored, wheel tends to rotate to the left no matter what. Same with FF_PERIODIC.

I've confirmed that changing the line to parameters[0].level = (long)parameters[0].level * (int)gain / 0xffff; fixes it. Not sure if that re-introduces the clipping problem tho.

berarma commented 3 weeks ago

Are you sure that the issue isn't that the forces are reversed?

Your change casts an unsigned 32bit value to a signed 32bit value, and that wouldn't have any effect in this case.

Unless armv7l is using 16bit values, then your change would produce an inversion of the forces.

zakk4223 commented 3 weeks ago

I think I sorted it out. the (int) cast was just masking the actual issue.

on arm arm7l int and long are 32 bits, long long is 64. The int cast was suppressing the implicit conversion of level to an unsigned value for arithmetic.

Using long long and doing the calculation like this fixes it:

parameters[0].level = div_s64((long long)parameters[0].level * gain, 0xffff);

I think the calculation of k1 and k2 in the loop below this also need this change.

I've verified this works on both 32 bit arm and 64bit x86

berarma commented 3 weeks ago

These operations can be done with 32bit signed integers, the values they use are 16bit. Indeed, the result will be stored in a 32bit signed variable in 32bits systems.

I've created PR #97 to test whether there can be something else going on and avoid using 64bit types if possible.