Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ControllerEmu: Add support for setting the center of a ReshapableInput #7992

Open
wants to merge 1 commit into
base: master
from

Conversation

3 participants
@artemist
Copy link

artemist commented Apr 12, 2019

This is useful in far out-of-calibration controllers, such as the
Switch Pro controller. This also adds support for configuring the center
in the Mapping widget.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 12, 2019

@Techjar

This comment has been minimized.

Copy link
Contributor

Techjar commented Apr 12, 2019

Great work, it works quite well, though I found a couple issues.

It doesn't affect the rendering of some wiimote widgets:

If I set the center after calibrating, the stick is miscalibrated:

I'm guessing for the latter issue that the reshapeable input vectors need to be offset by the difference between the old and new center values. The former issue is probably those widgets using their own render code.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 12, 2019

In addition to what Techjar said, the logic that tests when calibration seems sane (to make "Finish" the default action) fails with an off-center center.

I think the calibration values should be interpreted as raw values so they do not need to be altered after a center change.
And my memory of the code makes me think It might be a doozie to work the center into the "reshaping" function.

Yeah.. It might be easier to offset the calibration data on center change..

And yes, unfortunately some of the other widgets have separate draw functions that will need some duplicate code to fix the center dot drawing.

@Techjar

This comment has been minimized.

Copy link
Contributor

Techjar commented Apr 12, 2019

Yeah I agree, in fact center should just be applied for deadzone calculation (in ReshapableInput::Reshape). The center is only relevant for the placement of the deadzone, and shouldn't have any effect on the actual stick value.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 12, 2019

Question? Can we just set the center to the average of calibration values or are there dumb gamepads that this wouldn't help?

@Techjar

This comment has been minimized.

Copy link
Contributor

Techjar commented Apr 12, 2019

I'd say to keep it fully configurable, as someone could have a really shitty gamepad with more deadzone on one side than the other.

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

My main motivation is that, when accessed through evdev, the Nintendo Switch Pro Controller has a non-linear vertical access on the left stick. I've also used other gamepads which have bad centering.

In terms of adjusting the calibration data, it is possible (calculate coordinates of each outer value, move then based on the center, and then calculate the intersection with the vectors), but IMHO it's awkward and will bring in quantization error. I also feel weird about storing based on the center (at least with the current system) because that's going to mean a rather complex transformation function run every frame.

@Techjar

This comment has been minimized.

Copy link
Contributor

Techjar commented Apr 13, 2019

Yeah we thought about that, and offsetting the calibration data won't work well because it's stored as distances. As for offsetting the deadzone itself, I've tried to work that out and the math is unreasonably complicated.

We've decided the best option would be to just force the user to recalibrate the stick when changing the center. Rename the option to "Center and Calibrate" to reflect the change.

Also IsCalibrationDataSensible needs to be fixed somehow, as mentioned.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

👍

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

A demonstration of how nonlinear my controller is. This is with my patch, the controller is outputting 0,0 because it's disconnected. The blue dot is its normal center:
Pro Controller Outline

I can try to fix IsCalibrationDataSensible. I looked at it while I was working on my patch, but never got around to it (I thought it was just my controller being bad). Center and Calibrate is doable, I can add it and fix formatting soon.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

@artemist That oblong shape makes me think you've mapped your joystick incorrectly. e.g. Axis "-+" instead of "+". Can you please show your Up/Down/Left/Right mappings?

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

@jordan-woyak Oh, I'm kind of embarrassed, I actually did map it wrong (that was hard to see). However, the center is still far enough off that it would require a 20%-25% deadzone without the patch and makes the controller asymmetric.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

Yeah, it is too hard to see the difference between the "-" and "-+". PR #7923 alleviates the issue. I'll add your screenshot to the collection. :P

@artemist artemist force-pushed the artemist:centering branch from 81c3d4b to 051d757 Apr 13, 2019

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

I don't believe that there is a problem in IsCalibrationDataSensible, if your controller is reasonable and you haven't mapped your controller incorrectly (like I did), then it should say "Finish calibration"

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

Yeah. I think you're right. The "problem" was IsCalibrationDataSensible assumes there isn't much deviation, which is not the case with the absurd center value that I tested.

@Techjar

This comment has been minimized.

Copy link
Contributor

Techjar commented Apr 13, 2019

It's a problem under slightly more extreme circumstances. Here I've faked a rather far off center:

And it won't show finish:

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

You can select finish from the dropdown. The current values do need sensible to me, but we may want to change them to be more forgiving

@Techjar

This comment has been minimized.

Copy link
Contributor

Techjar commented Apr 13, 2019

Yeah but it's not intuitive that you would need to manually select finish from the menu for what looks like (and is) a reasonable calibration. The current threshold is based on the worst case we've found for analog stick range (a GC controller, apparently), so it seems pretty sensible to me.

All that needs to be done is to offset the calibration data by the center when averaging it. How exactly that could be done I'm not entirely sure of yet. I think it will require converting radius to vector and back.

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

I just noticed I didn't deal with the edge case where you cancel calibration after pressing center and calibrate (the center is still changed). I'll go fix that.

@artemist artemist force-pushed the artemist:centering branch from 051d757 to eeb1d5a Apr 13, 2019

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

The blue dot still needs to be added to DrawForce and DrawCursor.

@artemist artemist force-pushed the artemist:centering branch from eeb1d5a to d01d5ad Apr 13, 2019

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

Ugh, I guess clang-format 8 and clang-format 5 have different lint standards. I'll fix that manually.

ControllerEmu: Add support for setting the center of a ReshapableInput
This is useful in far out-of-calibration controllers, such as the
Switch Pro controller. This also adds support for configuring the center
in the Mapping widget.

@artemist artemist force-pushed the artemist:centering branch from d01d5ad to 1f83997 Apr 13, 2019

@artemist

This comment has been minimized.

Copy link
Author

artemist commented Apr 13, 2019

I'm not sure what to do about the new Shake class, but I'm not sure if it needs to be in this PR.

@jordan-woyak

This comment has been minimized.

Copy link
Member

jordan-woyak commented Apr 13, 2019

No reshaping is done in shake. Nothing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.