SDL on Linux needs a way to turn deadzones off

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• SDL deadzones patch (deadzones-2.patch, text/plain, 2020-07-22 05:02:33 +0000, 2589 bytes)
• only disable deadzone and handle SYN_DROPPED event (deadzone_hint.patch, text/plain, 2020-08-25 10:59:53 +0000, 1601 bytes)
• Comments out the code that applies the dead zone in Linux. (SDL2_remove_dead_zone_patch.txt, text/plain, 2020-08-26 21:48:19 +0000, 600 bytes)
• Removes dead zone and outputs axis values in correct range. (SDL2_remove_dead_zone_correctly_patch.txt, text/plain, 2020-08-27 04:00:57 +0000, 4267 bytes)
• Same patch as previous, but for SDL version 1. (SDL1_remove_dead_zone_correctly_patch.txt, text/plain, 2020-08-27 04:02:29 +0000, 3792 bytes)
• Patch for SDL1 with the changes made to the SDL2 patch. (SDL1_remove_dead_zone_like_in_SDL2.patch, text/plain, 2020-12-30 21:15:43 +0000, 2519 bytes)
• SDL-1.2 patch rebased to current branch (5241-SDL12.patch, text/plain, 2020-12-30 21:50:54 +0000, 2770 bytes)

Reported in version: HG 2.0
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2020-07-22 05:02:33 +0000, Kai Krakow wrote:

Created attachment 4423
SDL deadzones patch

We are currently facing the following problem with SDL on Linux:

1. SDL forces deadzones on joystick devices with no option of preventing that
2. SDL doesn't do this in Windows (there's even no code path that implements deadzones)
3. For reason 1, Wine/Proton has no way to expose a joystick on xinput the same way as Windows does

Background:

Microsoft explicitly documents in the xinput API that xinput devices will have no deadzones and you must implement deadzones yourself in applications using xinput. Wine/Proton use SDL as the lower layer for exposing game controllers as xinput devices for games. Because SDL forces deadzones on us in Linux, there's no way for Wine/Proton to do the correct thing for xinput emulation.

Because xinput explicitly requires devices exposed through the API to have no deadzones but return unfiltered values, game developers have to and do implement their own deadzones, often even depending dynamically on the situation in the game, e.g. during weapon zoom.

Impact:

Users see a - sometimes dramatically - reduced precision in games compared to running under native Windows when playing games in Proton. This is because there are two deadzones applied on top of each other: One from SDL because Wine uses SDL for xinput devices, and one from the game itself - ultimately resulting in a huge deadzone with all precision gone we can expect from a modern game controller.

Temporary work-around:

Users can set deadzones to 0 in their joystick drivers, SDL will then apply no deadzone. The effect of this is that they instantly see a vastly improved behavior and precision that matches native Windows behavior. But this has the downside that it disables deadzones in joydev devices, too, which native Linux games may not expect / handle well. This work-around will no longer work with the new HIDAPI implementation, as those seem to bypass Linux drivers.

Proposed solution:

Since SDL reads joystick values from evdev, it already reads unfiltered values from the kernel. We just need a way for users / applications to let SDL know that we want these values unfiltered, without adding a deadzone. I'm attaching a proposed patch which implements this as an SDL hint. When not setting the hint, the default behavior doesn't change and SDL provides filtered values with deadzone applied.

Downstream impact:

As soon as this patch landed, Proton can add the new hint to their launcher or winebus.sys code itself, and it works as soon as the new SDL version landed in the Steam distribution. It is an easy change that can be easily backported to older versions of Proton. It has no impact if SDL does not yet handle the hint, and it has thus no compatibility problems. Users will just see improved behavior when requirements are fulfilled, or otherwise nothing changes.

See also:
ValveSoftware/wine#94

Developer note:

Please review, I'm absolutely unfamiliar with how SDL works internally. Also, I do not know how the proposed patch work flow is to be done with hg, it is very limited compared to what I know from git. I found no contribution guide.

On 2020-08-10 15:46:23 +0000, Sam Lantinga wrote:

Looks good, thanks!
https://hg.libsdl.org/SDL/rev/369fea168258

On 2020-08-25 10:59:53 +0000, Mathieu Eyraud wrote:

Created attachment 4446
only disable deadzone and handle SYN_DROPPED event

There are two issues with this hint:

• This hint does not work with Dualshock4 controller because its axis report value between 0 and 255. It needs absinfo.maximum and absinfo.minimum to be applied.

• Axis value are not reinitialized after SYN_DROPPED event.

I attached a patch that only disables absinfo.flat and modifies
PollAllValues to check if axis exit instead of checking if correction coefficient are used.

On 2020-08-25 11:03:41 +0000, Mathieu Eyraud wrote:

Reopen: see my previous comment and patch.

On 2020-08-26 21:48:19 +0000, wrote:

Created attachment 4450
Comments out the code that applies the dead zone in Linux.

May I suggest just removing the dead zone entirely? Any application that wants a dead zone can implement one itself, but there's nothing that can be done to remove a dead zone after it has been created.

The first comment on this bug report, and my own experience comparing crrcsim on Windows vs. crrcsim on Linux, tells me that SDL doesn't do this in Windows. So why should it do it on one platform and not another?

While I can't find any documentation about the ioctl that's called from which the information about what dead zone to apply is pulled, I do wonder why any API should even know what is appropriate. If it were indeed appropriate, as in it's some kind of joystick that by design should always have a dead zone, then I would expect the joystick to be implementing that dead zone itself. So what can this ioctl be providing other than someone's recommendation, with that someone being some random programmer somewhere rather than someone who actually knows what is appropriate? Indeed, if the returned recommendations were appropriate, then bug 5269 wouldn't exist.

I was trying to play crrcsim (an RC plane flight simulator) and having the controls in the simulator have these enormous dead zones when in real life there is no dead zone at all wasn't making for a very good simulation. There was even one in the middle of the throttle range which doesn't make any kind of sense. As the controller I was using an FS-i6X RC transmitter, which has a USB cable to attach it to a computer and use it as a joystick for RC flight simulators. I suppose anything is possible, but I doubt it's communicating to the PC that it would be appropriate to have an enormous dead zone in the middle of each axis, given that when used to fly a plane there is no dead zone at all.

Since the code that implements the dead zone is also in SDL1 (which crrcsim uses), I assume it's also the reason I could never get into the Linux port of Descent. I could never find a joystick that didn't have a huge dead zone in the middle of each axis. When I would try to make a small movement, nothing would happen, so I'd move the stick more, but still nothing would happen, so I'd move it a lot more, since the previous lack of reaction told my brain it totally wasn't moving the stick enough, but then I'd be past the dead zone and the ship would turn wildly fast. There just wasn't any way my brain was getting used to "when you want to make a small movement, first make this much larger one very precisely to pass over the exact width of the dead zone, then make the small movement," and so it was just impossible to make fine adjustments, and thus impossible to aim at anything far away.

Just get rid of the dead zones. It'll be a massively better playing experience for any game that actually requires analog controls, and those that don't can apply their own dead zones of whatever size they deem appropriate to turn the analog stick into a digital stick.

I attached my own patch which comments out the code which implements the dead zone while continuing to do whatever else that ioctl is telling it to do.

On 2020-08-27 04:00:57 +0000, wrote:

Created attachment 4451
Removes dead zone and outputs axis values in correct range.

It occurred to me that my simple patch that comments out a few lines of code does not correctly remove the dead zone since the calculation presumably assumes the dead zone has been cut out of the range. Then, while looking into how to make it output the correct range of values, I realized SDL wasn't returning the correct range of values to begin with.

This line of code was already present:

printf("Values = { %d, %d, %d, %d, %d }\n", absinfo.value, absinfo.minimum, absinfo.maximum, absinfo.fuzz, absinfo.flat);

For my joystick this yeilds:

Values = { 0, -127, 127, 0, 15 }

Then this code calculates the coefficients:

In SDL1:
joystick->hwdata->abs_correct[i].coef[0] = (absinfo.maximum + absinfo.minimum) / 2 - absinfo.flat;
joystick->hwdata->abs_correct[i].coef[1] = (absinfo.maximum + absinfo.minimum) / 2 + absinfo.flat;
t = ((absinfo.maximum - absinfo.minimum) / 2 - 2 * absinfo.flat);
if ( t != 0 ) {
joystick->hwdata->abs_correct[i].coef[2] = (1 << 29) / t;
} else {
joystick->hwdata->abs_correct[i].coef[2] = 0;
}

In SDL2:
joystick->hwdata->abs_correct[i].coef[0] = (absinfo.maximum + absinfo.minimum) - 2 * absinfo.flat;
joystick->hwdata->abs_correct[i].coef[1] = (absinfo.maximum + absinfo.minimum) + 2 * absinfo.flat;
t = ((absinfo.maximum - absinfo.minimum) - 4 * absinfo.flat);
if (t != 0) {
joystick->hwdata->abs_correct[i].coef[2] = (1 << 28) / t;
} else {
joystick->hwdata->abs_correct[i].coef[2] = 0;
}

Neither calculates the correct coefficients for the code in the AxisCorrect function.

In SDL1:
if ( value > correct->coef[0] ) {
if ( value < correct->coef[1] ) {
return 0;
}
value -= correct->coef[1];
} else {
value -= correct->coef[0];
}
value *= correct->coef[2];
value >>= 14;

In SDL2:
value *= 2;
if (value > correct->coef[0]) {
if (value < correct->coef[1]) {
return 0;
}
value -= correct->coef[1];
} else {
value -= correct->coef[0];
}

In SDL1, the calculated coefficients are coef[0]=15, coef[1]=-15 and coef[2]=5534751. So with a full-scale input of 127, it calculates an output value of 37835, which is considerably out of range.

In SDL2, the calculated coefficients are coef[0]=30, coef[1]=-30, and coef[2]=1383687. So with a full-scale input of 127, it calculates the same output value of 37835.

I tested it with the 3 joysticks I have, and it produces out-of-range values for all of them.

Anyway, since dead zones are garbage, I just deleted all of that junk and wrote some code that takes the absinfo.minimum and absinfo.maximum values and uses them to scale the axis range to -32767 through +32767.

I also made it detect when a range doesn't have an integer center point, e.g. the center of -128 to + 127 is -0.5. In such cases, if either value to the side of the center is provided, it zeros it, but it otherwise doesn't implement any kind of dead zone. This seemed important with my gamepad which provides only the values of 0, 127, and 255, since without this hack it would never be centered.

Also, the previous minimum output value was -32768, but as that creates an output range that has no true center, I changed the minimum value to -32767.

I tested it with the 3 joystick devices I have and it seems to create correct values for all of them.

On 2020-08-27 04:02:29 +0000, wrote:

Created attachment 4452
Same patch as previous, but for SDL version 1.

IDK if anyone is updating SDL version 1 but here's a patch for it as well.

On 2020-12-09 15:05:37 +0000, Sam Lantinga wrote:

Kai, can you comment on pj5085's patch? Does it have good behavior for you? I'm a little concerned about joysticks which may not provide correct minimum and maximum axis values.

On 2020-12-10 12:01:12 +0000, Kai Krakow wrote:

@sam

I'd rather continue working on this using the Github mirror. Will you accept PRs over there?

For pj5058's patches, I'm not sure if we should apply them. I'm currently preferring the patch that simply adjusts "flat" to zero. I'm currently using that for testing.

I'd also like to add another function that allows games to enable or disable deadzones per device. But I'm not sure how to do that in a non-breaking way. Can we make hints per device? Do I had a new API function? If yes, is it okay to implement it in Linux only and make it a no-op in Windows?

On 2020-12-10 12:05:38 +0000, Kai Krakow wrote:

(In reply to Kai Krakow from comment # 8)

Do I had

*add

On 2020-12-11 07:59:42 +0000, wrote:

For pj5058's patches, I'm not sure if we should apply them.

Why not? They work correctly, whereas the existing code (and I assume the other patches, because they utilize the existing code) does not.

I'm currently preferring the patch that simply adjusts "flat" to zero. I'm currently using that for testing.

The result of the existing code calculating out-of-range values (as I showed in a previous comment) is that it accidentally creates a dead zone at each end of each axis in addition to the dead zone it intentionally creates in the middle of each axis. They're rather significant dead zones too, about the size of the dead zone in the middle of each axis, but I guess no one really knows where their joystick should stop responding and so no one ever noticed that SDL was making their joystick unresponsive at each end of each axis as well.

I'd also like to add another function that allows games to enable or disable deadzones per device. But I'm not sure how to do that in a non-breaking way. Can we make hints per device? Do I had a new API function? If yes, is it okay to implement it in Linux only and make it a no-op in Windows?

Why not just use my code and let any developer who wants deadzones just do this?

for (i = 0; i < num_inputs; i++) {
if (input[i] >= -4096 && input[i] <= +4096) input[i] = 0;
if (input[i] < -4096) input[i] += 4096;
if (input[i] > +4096) input[i] -= 4096;
input[i] = input[i] * 8 / 7;
}

Does anyone need an API function (which only works on one platform) to save them from writing the five lines of code that they'll have to write for the other platforms anyway?

Also, there's more than one way to implement a deadzone, and with each implementation, there are configurable parameters. It seems silly to have SDL implementing one type of deadzone with non-configurable parameters for only one platform when it is so easy for developers to write a few lines of code to get exactly the type of deadzone they want with configurable parameters.

On 2020-12-13 07:57:06 +0000, Sam Lantinga wrote:

I think what pj5085 implemented is what we want when dead zones are turned off.

I went ahead and took a stab at adding this, leaving the minimum axis value at -32768, and leaving the hint off, so now the joystick axis values should be consistent across all platforms by default.

I can't compile on Linux right now, so can you guys check this out and see how it works?
https://hg.libsdl.org/SDL/rev/bcbacded3580
https://hg.libsdl.org/SDL/rev/061500d75312

On 2020-12-13 12:43:45 +0000, Ozkan Sezer wrote:

If things settle and pj5085 patches are in 2.0, I'd be interested in
applying the SDL-1.2 version after any possible updates to it are done.

On 2020-12-13 14:32:27 +0000, Kai Krakow wrote:

After fiddling around with unpatched SDL2 for a while, running different games in Proton, I'm all in for disabling deadzones by default to make it the same behavior on all platforms.

One additional problem code could have is that Proton Wine pulls values from SDL2, emulates Xinput2 and other gamepad APIs with it, and those games in turn may also use Windows-SDL2 to process these values. Having any sort of deadzone calculation in SDL2 inself only created unwanted behavior.

As pj5058 points out, game devs want to use their own deadzone implementation anyways, and most games probably do, even native Linux games. The implementation provided by SDL2 can only be wrong, and I think it's an artifact of making porting away from using the Linux joydev interface (which has deadzones, and the coef code looks exactly the same as in the Linux kernel). This is an old interface anyways, and games using modern interfaces will use either evdev directly (which provides unfiltered values and only provides a deadzone hint per axis), or they use SDL2 (which should do the same).

When I was denying pj5058's patch, I was probably looking at an older version or confused it with another patch. The one here looks good:
https://hg.libsdl.org/SDL/rev/bcbacded3580

pj5058, apologies for that.

Modern devices may have all sorts of deadzones, and evdev calculates a center for all of them although no such center may exist, e.g. I have multiple dials and throttles on my HOTAS set and none of them has a center point - it's up to the game how to interpret such axes. The old joydev interface doesn't know the purpose of such axes, evdev provides a HID symbol for those, so games can decide how to make use of the deadzone hint. As such, SDL2 should never do that unless asked for. And that's why I'm also fine with changing the default to "no deadzone" as in:
https://hg.libsdl.org/SDL/rev/061500d75312

Currently, I'm setting flatness to 0 via udev to make SDL ignore the deadzones and have proper flight controls in games (Elite Dangerous especially where SDL-deadzones disabled are essential for precise flight-control-off steering).

If anyone needs to run an old Linux game which was ported from joydev to SDL, one would need to turn on deadzones. So maybe for the SDL1.2 branch, it should be considered to keep deadzones on. But I'm not sure, I don't know of any games I play that still use SDL1.2.

On 2020-12-13 14:47:19 +0000, Kai Krakow wrote:

(In reply to Sam Lantinga from comment # 11)

I went ahead and took a stab at adding this, leaving the minimum axis value
at -32768

Is this good? This makes games see no clearly defined center value. What does Windows SDL provide as the min value? I think the Linux code path should use the same values.

Also, gamepads that show an axis range of 0-255 in the HID report may just be lazy sometimes. If you look at the raw HID reports, you either never get 0 or never get 255 as the extreme value, so actually the HID report should say 1-255 or 0-254 but it doesn't. That should be fixed at the Linux driver level by patching the HID report before passing it to the input layer, I'm not sure if SDL should take responsibility for flawed HID descriptors. I'm doing a similar thing for the xpadneo driver although the Xbox controller HID descriptor is at least correct in reporting 0-65534 as the values range (tho, it should be -32767-32767 for properly passing Linux gamepad specs).

I've seen the Stadia controller reporting range 0-255, and it seems to use some sort of hardware-implemented deadzone to settle at 127 for the idle position.

On 2020-12-13 19:45:04 +0000, Sam Lantinga wrote:

Windows SDL has the minimum value at -32768 and maximum value at 32767, the full range of signed 16-bit value.

Kai, pj5085, can you try the current implementation and see if it works and matches your expectations?
http://www.libsdl.org/tmp/SDL-2.0.zip

@pj5085, thanks for your patch!

I think this behavior is now consistent across supported platforms.

On 2020-12-14 06:02:53 +0000, wrote:

I added some printf to verify the math being done. Of the three joysticks I have, it works correctly for at least two, and seems to work correctly for the third. I say "seems to" because, for the third joystick, the values never go through the AxisCorrect function, and thus never hit my printf statements, even though they did in the version I wrote my patch against. I'm not sure what's going on there, but it at least seems to be working correctly in as much as I can tell.

I note this result in particular, for an SNES Gamepad (min=0, max=255):

Joystick value 0 becomes -32768
Joystick value 127 becomes 0
Joystick value 255 becomes 32767

Without the code that forces a zero point, the 127 input value would become -129, so I think you see why I added that code to turn it into zero. However, I think Kai Krakow has a point about how SDL shouldn't assume that there should be a center.

Obviously in the majority of cases there actually should be a center, and the code that turns that 127 into an actual 0 is creating only a 0.2% error over 0.4% of this joystick's range. However, what if there is an axis that is some kind of special control, like a 4-position switch, and, for whatever reason, the joystick reports it as an axis with 4 possible values, 0 to 3? In that case, mutilating the two center values to the same value is much more of an error and and turns that 4-position switch into a 3-position switch. If any joystick does this with a 2-position switch, then this code would render that control entirely useless as it would report the same value with the switch in either position. Obviously the code could require that there be at least N possible values, to guess whether something is a proper axis or just some kind of switch, but the choice of N would be arbitrary and that's ugly.

I guess the real problem here is that my gamepad is just kind of broken. It should be reporting a range of -1 to +1 since that's what it actually does. Also, as Kai Krakow points out, it's probably not SDL's place to fix broken hardware. I'll add that, if SDL does fix broken hardware, it should probably actually know that it's broken rather than be merely guessing that it is.

So, to the extent that SDL is able to do stuff like this, perhaps it's something better left for the user to configure in some kind of config file.

Anyway, I noticed that the old broken code appears to still be in this version, but only if deadzones are enabled. Not knowing how to enable them, I just modified the code so that they're always enabled, to test if the code has been fixed. With the same gamepad as above, the full scale input values of 0 and 255 become -37810 and +37809 before being clipped at the end, and thus the code has not been fixed and still creates significant (15% of the full range) dead zones at each end of each axis.

On 2020-12-14 17:17:20 +0000, Sam Lantinga wrote:

Okay, I removed the code that forces the zero point:
https://hg.libsdl.org/SDL/rev/95f648e6e819

The codepath with deadzones enabled shouldn't have changed, that is there for games that rely on the old default behavior.

Okay, thanks for the help everyone, I think this is ready to ship. Please let me know if there's something we need to revisit here.

On 2020-12-29 12:37:30 +0000, Ozkan Sezer wrote:

pj5085: Does the SDL-1.2 patch need updating after the latest
state of the changes went into SDL2-2.0.14?

On 2020-12-30 21:15:43 +0000, wrote:

Created attachment 4619
Patch for SDL1 with the changes made to the SDL2 patch.

Ozkan Sezer: Here you go. I removed the code that creates the center point and set the minimum value to -32768, like we decided to do with the SDL2 patch.

On 2020-12-30 21:50:54 +0000, Ozkan Sezer wrote:

Created attachment 4620
SDL-1.2 patch rebased to current branch

pj5085: The patch does not apply to current SDL-1.2 branch because of
a conflict due to commit https://hg.libsdl.org/SDL/rev/58f64ff3011e --

I rebased your patch to current SDL-1.2 branch, which reverts the above-
mentioned commit, and does some constifications and whitespace fixes.

If you confirm, I will apply it.

On 2020-12-30 21:56:53 +0000, Ozkan Sezer wrote:

(In reply to Ozkan Sezer from comment # 20)

Created attachment 4620 [details]
SDL-1.2 patch rebased to current branch

Paul Cercueil: This suggested patch removes your change in
1.2 branch, i.e. https://hg.libsdl.org/SDL/rev/58f64ff3011e
Can you confirm?

On 2020-12-30 22:11:43 +0000, Paul Cercueil wrote:

Paul Cercueil: This suggested patch removes your change in
1.2 branch, i.e. https://hg.libsdl.org/SDL/rev/58f64ff3011e
Can you confirm?

I can confirm that you should not just revert the patch, if I took the effort to upstream it, that's for a reason. Unless the new code also takes care of the (absinfo.maximum < absinfo.minimum) case, then it's fine.

On 2021-01-01 23:16:31 +0000, wrote:

Unless the new code also takes care of the (absinfo.maximum < absinfo.minimum) case, then it's fine.

It does in that the code results in a functional axis which is inverted relative to the raw values. In other words, if absinfo.minimum = 4 and absinfo.maximum = 0, then it does this:

0 becomes 32767
1 becomes 16383
2 becomes -1
3 becomes -16384
4 becomes -32768

I don't know how to get the code with Paul Cercueil's patch included in order to compare how it functions, but just looking at the patch itself, I think it also results in an axis that is inverted relative to the raw values, because it doesn't swap absinfo.maximum and absinfo.minimum but instead just changes the sign of absinfo.flat. In any event, unless we're just assuming that the ioctl() is giving us bad information, then I think inverting the axis makes sense.

On 2021-01-01 23:32:12 +0000, Ozkan Sezer wrote:

(In reply to pj5085 from comment # 23)

Unless the new code also takes care of the (absinfo.maximum < absinfo.minimum) case, then it's fine.

It does in that the code results in a functional axis which is inverted
relative to the raw values. In other words, if absinfo.minimum = 4 and
absinfo.maximum = 0, then it does this:

0 becomes 32767
1 becomes 16383
2 becomes -1
3 becomes -16384
4 becomes -32768

I don't know how to get the code with Paul Cercueil's patch included in
order to compare how it functions, but just looking at the patch itself, I
think it also results in an axis that is inverted relative to the raw
values, because it doesn't swap absinfo.maximum and absinfo.minimum but
instead just changes the sign of absinfo.flat. In any event, unless we're
just assuming that the ioctl() is giving us bad information, then I think
inverting the axis makes sense.

Paul?

On 2021-01-02 16:09:33 +0000, Paul Cercueil wrote:

(In reply to pj5085 from comment # 23)

Unless the new code also takes care of the (absinfo.maximum < absinfo.minimum) case, then it's fine.

It does in that the code results in a functional axis which is inverted
relative to the raw values. In other words, if absinfo.minimum = 4 and
absinfo.maximum = 0, then it does this:

0 becomes 32767
1 becomes 16383
2 becomes -1
3 becomes -16384
4 becomes -32768

I don't know how to get the code with Paul Cercueil's patch included in
order to compare how it functions, but just looking at the patch itself, I
think it also results in an axis that is inverted relative to the raw
values, because it doesn't swap absinfo.maximum and absinfo.minimum but
instead just changes the sign of absinfo.flat. In any event, unless we're
just assuming that the ioctl() is giving us bad information, then I think
inverting the axis makes sense.

Ok, then I guess it's fine.

On 2021-01-02 17:37:06 +0000, Ozkan Sezer wrote:

(In reply to Paul Cercueil from comment # 25)

Ok, then I guess it's fine.

OK, patch applied to SDL-1.2 as https://hg.libsdl.org/SDL/rev/79147eb8d317
Thanks everyone.