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

First attempt GLFW gamepad driver (don't merge yet?) #222

Closed
wants to merge 49 commits into from

Conversation

mcclure
Copy link
Contributor

@mcclure mcclure commented Feb 13, 2020

This is a standalone driver that provides gamepad support on desktop. Gamepad support on oculus mobile would have to be done separately. In my testing this works on Mac and Windows.

This should probably wait until the isTracked changes to go in. Right now isTracked() returns false for all gamepads because they don't have a pose.

@mcclure mcclure changed the title First attempt GLFW gamepad driver First attempt GLFW gamepad driver (don't merge yet?) Feb 13, 2020
@@ -10,6 +10,9 @@
#include <GLFW/glfw3native.h>
#endif

#ifdef OS_GLFW_H
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did something dubious here. Right now os_glfw.h can only be included in one file because it defines non-static symbols. But os_glfw seems(?) to be the only way we have of getting glfw.h. For this reason gamepad.c #defines OS_GLFW_H and then includes os_glfw.h. Maybe this could be done a cleaner way.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os_glfw is designed to only be included in one translation unit right now, the os_*.c translation units (since it provides most of the symbols in os.h).

Joystick input could go through the existing "os" layer or GLFW could be included directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's OK with you I think I'll switch to including GLFW directly.

I wonder if the driver should be named "GLFWGamepad" instead of "Gamepad", or something.

MAX_DEVICES,

DEVICE_GAMEPAD_FIRST = DEVICE_GAMEPAD_1,
DEVICE_GAMEPAD_LAST = DEVICE_GAMEPAD_1,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current gamepad driver accepts 1 gamepad at a time. If you have 2 plugged in and unplug 1, it promotes the other plugged-in one.

But notice, the way gamepad.c is written it can handle an arbitrary number of gamepad devices. The GLFW maximum number of gamepads is, I think, 14. I think having 4 gamepad devices would make a lot of sense. In my testing I had a problem because "spacemouse emulator" was showing up as a joystick device on Windows according to GLFW, so a real gamepad never got a chance to grab the GAMEPAD_1 slot (I uninstalled spacemouse for testing to work around this). If we supported 4 gamepads this would not have been a problem.


void discoverGamepads() {
for(int jid = 0; jid < GLFW_JOYSTICK_LAST; jid++) {
if (glfwJoystickPresent(jid)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use glfwJoystickPresent(), but maybe we should use glfwJoystickIsGamepad(). It is not clear to me exactly which devices say yes to "present" but not to "isGamepad".

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLFW docs say if you're only interested in gamepads, you can use isGamepad as a replacement for isPresent

@bjornbytes
Copy link
Owner

bjornbytes commented Feb 14, 2020

Currently it isn't possible to access the separate left/right triggers, left/right joysticks, or access the dpad. These features seem important for gamepad input.

Also I think mapping both shoulder buttons to the grip button/axis is confusing, if I saw a grip input on a gamepad I would think it was force-sensitive. Maybe there could be buttons for shoulder/left and shoulder/right.

@mcclure
Copy link
Contributor Author

mcclure commented Feb 14, 2020

Agreed, those were stopgap behaviors.
I can think of two approaches for the final behavior:

  1. Add Trigger/left and Trigger/right as you suggest— this does mean cluttering up the enums a lot for a driver we're not even sure we need

  2. Allow you to say something like getStatus("gamepad/1", "thumbstick", 2). So getStatus("gamepad/1", "thumbstick", 1) or getStatus("gamepad/1", "thumbstick") would map to left thumbstick and getStatus("gamepad/1", "thumbstick", 2) would map to right thumbstick. That would complicate the code for getStatus/glob a little but would make talking to a gamepad very much like talking to a handset.

What do you think of possibility 2?

I have no idea what to do with the D-pad.

@bjornbytes
Copy link
Owner

bjornbytes commented Feb 14, 2020

Option 2 sounds like "arrayed buttons and axes" are being proposed. So anywhere a DeviceButton or DeviceAxis is used, another index is used to specify which index of the specified button/axis type is being requested. This would apply to isDown, isTouched, getAxis, and getStatus, and I'm guessing the index would default to 1.

@bjornbytes
Copy link
Owner

I think the dpad could be 4 new buttons.

@mcclure
Copy link
Contributor Author

mcclure commented Feb 14, 2020

@bjornbytes What you say in both of your new comments sounds good to me.

@bjornbytes
Copy link
Owner

bjornbytes commented Feb 15, 2020

Ok. I can think of some downsides to adopting the system of arrayed buttons and axes.

  • It could be confusing or inconsistent to have 'gamepad/1' and 'button', 1 as 2 different ways of representing arrayed things.
  • Since the full representation of a button/axis consists of 2 values instead of 1 now, it makes it slightly more difficult to add additional arguments to any of the functions that take buttons/axes, and there could be problems with Lua's multiple args handling which wouldn't happen if it was a single value.
  • Loss of semantics around left/right

There are some benefits though like being able to use numbers instead of format strings if you wanted to programmatically generate buttons or something. So the benefits would need to be weighed against the drawbacks.

bjornbytes and others added 24 commits February 26, 2020 14:40
Instead it returns a boolean indicating if the send worked.
Instead of anisotropic being its own filter, it is now removed and
anisotropy settings can be used with any of the other filter modes.
Surprisingly, this appears to be the only place reading out of enum
arrays like this, so there shouldn't be any other places to fix.
as mentioned on slack.

there are some situations you can get into (high load in some place or other) where the newer frame submission api will behave much more consistently, and I've noticed no negative effects.

besides, the other one is deprecated as best i can tell.
update ovr driver to newer frame submission api
seems a little strange that vibrating head or something would affect the right controller.
implement haptics on oculus desktop
@mcclure
Copy link
Contributor Author

mcclure commented Apr 15, 2020

Merged with master and fixed some bugs.

Outstanding questions before this can be merged:

  • How do you loop over controllers? getHands returns only hand/left and hand/right.
  • How do you detect if a controller is present? isTracked will always be false.
  • How do we handle multiple triggers / multiple thumbstick axes? The two options I see here are introduce separate THUMBSTICK_LEFT and THUMBSTICK_RIGHT or add an, index parameter to isDown and getAxis, from the above I can't tell if bjorn signed on to a particular one.

@bjornbytes
Copy link
Owner

  • I think for the looping, it would be okay to delegate this to Lua where you can do a for i = 1, 4 and local device = 'gamepad/' .. i thing. Should try to improve the convenience of that at some point though.
  • It does seem like there needs to be a new "isPresent" concept for the gamepads. I guess headset drivers that don't have connectivity info for some devices could return either nil or true if they're not sure.
  • I'd prefer separate enumerants like thumbstick/left and thumbstick/right for the multiple buttons/axes. I want it to be easier in the future to do button mappings (started thoughts in Action-based Input System #245), but I think it's too restrictive to implement indexed buttons here just so the left trigger/thumbstick match up with the one on VR controllers.

@mcclure
Copy link
Contributor Author

mcclure commented Jul 10, 2020

"it would be okay to delegate this to Lua"

if we include the isPresent.

"I'd prefer separate enumerants like thumbstick/left and thumbstick/right for the multiple buttons/axes"

Alright, but long term I'd still like some sort of method (or easy means to build a method) for merging "thumbstick", "thumbstick/left" and "touchpad" into one input in apps that need that.

Sorry I haven't worked on this in a while…

@bjornbytes
Copy link
Owner

I wonder if getPose could return true and all zeroes if the gamepad is connected. This causes an issue with PS move controllers because with those you would no longer be able to tell if tracking is lost for those. But you would at least be able to do lovr.headset.isTracked('gamepad') for detection. It's a hacky compromise -- I do have some ideas for improving this but it'd be nice to get gamepad support in the meantime.

@bjornbytes
Copy link
Owner

I'm...not sure what to do about this, but I think about it often. There should definitely be joystick input, eventually. But the current structure of the headset module, and how it handles input, makes integrating joysticks challenging. The design of OpenXR's action system, which lovr.headset sort of followed, is not really conducive to joysticks (can't have 'numbered' devices, can't detect whether a device is present/connected).

Thinking super highlevel/longterm I think aligning with OpenXR actions was a mistake, and an engine needs to have its own input abstraction, so that it can integrate with keyboard/mice/joysticks/spacethings. Maybe that input abstraction is still action-based, but it has to be on top of OpenXR, treating it like a lower-level driver. Otherwise it simply can't integrate with the other stuff cleanly. So, maybe someday it will look like lovr.input and lovr.headset? Who knows.

Plugins are a thing now. Maybe starting there would be more successful, free from the confines of lovr.headset? There's even https://github.com/Rabios/lovr-joystick which is an ffi-based solution.

It also seems like this PR has fallen victim to gitrot.

I think it's best to close backs away slowly

@bjornbytes bjornbytes closed this Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants