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

On PS4/PS5 Controller SDL_EVENT_GAMEPAD_BUTTON_DOWN can trigger before SDL_EVENT_GAMEPAD_TOUCHPAD_DOWN #11085

Closed
fireph opened this issue Oct 6, 2024 · 10 comments
Assignees
Milestone

Comments

@fireph
Copy link

fireph commented Oct 6, 2024

The button down event can trigger before the touchpad down event which means there is no way to access the x/y coordinates of the touchpad event from the button down (this can be done by just tapping the PS4/5 touchpad very quickly). Ideally the SDL_GamepadTouchpadEvent could be populated even in the case of a button down/up event if the type was SDL_GAMEPAD_BUTTON_TOUCHPAD. The only way to work around this issue would be to wait until the touchpad down event, but I would like to avoid that since it would be very hacky and potentially buggy.

The alternative is to just ensure that the touchpad down always happens first, not sure which solution would be easier inside of SDL.

@slouken
Copy link
Collaborator

slouken commented Oct 6, 2024

It seems like the touchpad event should happen first, which is pretty easy to do.

@slouken
Copy link
Collaborator

slouken commented Oct 6, 2024

Fixed, thanks!

@fireph
Copy link
Author

fireph commented Oct 8, 2024

@slouken unfortunately this has only seemed to alleviate the issue, but not solve it entirely :(

If I tap on the touchpad very fast (on a PS5 controller) I am still able to get the SDL_GAMEPAD_BUTTON_TOUCHPAD to trigger before any SDL_EVENT_GAMEPAD_TOUCHPAD_DOWN or SDL_EVENT_GAMEPAD_TOUCHPAD_MOTION events. Not sure what the right approach would be to entirely solve this issue.

@slouken slouken reopened this Oct 8, 2024
@slouken
Copy link
Collaborator

slouken commented Oct 8, 2024

What platform are you on?

@fireph
Copy link
Author

fireph commented Oct 8, 2024

What platform are you on?

Windows 11 and I am plugging in the PS5 controller using USB (not wireless)

@slouken
Copy link
Collaborator

slouken commented Oct 8, 2024

It looks like this might actually be a bug in the PS5 firmware, or a touch sensor issue.

I modified the code to print out the touch state when the touchpad button is pressed, and occasionally I get this:

Gamepad 2 touchpad 0 finger 0 moved to 0.65, 0.49, 1.00
Gamepad 2 touchpad 0 finger 0 moved to 0.65, 0.49, 1.00
Gamepad 2 touchpad 0 finger 0 released at 0.65, 0.49, 0.00
Touch button down, touch 0,0
Touchpad button down!
Gamepad 2 touchpad 0 finger 0 pressed at 0.63, 0.48, 1.00
Gamepad 2 touchpad 0 finger 0 moved to 0.63, 0.47, 1.00
Gamepad 2 touchpad 0 finger 0 moved to 0.63, 0.48, 1.00

We really do get an update from the controller that has the finger released and then pressed again, even though I definitely was not doing that at the time.

Here's the patch, for reference:

diff --git a/src/joystick/hidapi/SDL_hidapi_ps5.c b/src/joystick/hidapi/SDL_hidapi_ps5.c
index dc35636f8..8fe032b1d 100644
--- a/src/joystick/hidapi/SDL_hidapi_ps5.c
+++ b/src/joystick/hidapi/SDL_hidapi_ps5.c
@@ -1373,15 +1373,20 @@ static void HIDAPI_DriverPS5_HandleStatePacket(SDL_Joystick *joystick, SDL_hid_d
     int touchpad_x, touchpad_y;

     if (ctx->report_touchpad) {
+bool touch1, touch2;
         touchpad_down = ((packet->ucTouchpadCounter1 & 0x80) == 0);
+touch1 = touchpad_down;
         touchpad_x = packet->rgucTouchpadData1[0] | (((int)packet->rgucTouchpadData1[1] & 0x0F) << 8);
         touchpad_y = (packet->rgucTouchpadData1[1] >> 4) | ((int)packet->rgucTouchpadData1[2] << 4);
         SDL_SendJoystickTouchpad(timestamp, joystick, 0, 0, touchpad_down, touchpad_x * TOUCHPAD_SCALEX, touchpad_y * TOUCHPAD_SCALEY, touchpad_down ? 1.0f : 0.0f);

         touchpad_down = ((packet->ucTouchpadCounter2 & 0x80) == 0);
+touch2 = touchpad_down;
         touchpad_x = packet->rgucTouchpadData2[0] | (((int)packet->rgucTouchpadData2[1] & 0x0F) << 8);
         touchpad_y = (packet->rgucTouchpadData2[1] >> 4) | ((int)packet->rgucTouchpadData2[2] << 4);
         SDL_SendJoystickTouchpad(timestamp, joystick, 0, 1, touchpad_down, touchpad_x * TOUCHPAD_SCALEX, touchpad_y * TOUCHPAD_SCALEY, touchpad_down ? 1.0f : 0.0f);
+if (packet->rgucButtonsAndHat[2] & 0x02)
+    SDL_Log("Touch button down, touch %d,%d\n", touch1, touch2);
     }

     if (ctx->report_battery) {
diff --git a/test/testcontroller.c b/test/testcontroller.c
index dc032ff14..e0675c2f1 100644
--- a/test/testcontroller.c
+++ b/test/testcontroller.c
@@ -1656,6 +1656,8 @@ static void loop(void *arg)
             if (display_mode == CONTROLLER_MODE_TESTING) {
                 SetController(event.jbutton.which);
             }
+            if (event.jbutton.button == 11)
+                SDL_Log("Touchpad button down!\n");
             break;

         case SDL_EVENT_JOYSTICK_BUTTON_UP:
@@ -1697,6 +1699,7 @@ static void loop(void *arg)
             RefreshControllerName();
             break;

+#define VERBOSE_TOUCHPAD
 #ifdef VERBOSE_TOUCHPAD
         case SDL_EVENT_GAMEPAD_TOUCHPAD_DOWN:
         case SDL_EVENT_GAMEPAD_TOUCHPAD_MOTION:

@fireph
Copy link
Author

fireph commented Oct 8, 2024

interesting, would it make any sense to put the SDL_GAMEPAD_BUTTON_TOUCHPAD into some sort of wait queue for a small amount of time until a proper touchpad x/y event comes in? This does seem like the controller is sending the events wrong which is very unfortunate

@slouken
Copy link
Collaborator

slouken commented Oct 8, 2024

I don’t think we want to introduce any latency here. Is it possible to dissociate them in your application? The position looks like it’s correct, so you have the position and the knowledge that the button is pressed. The touchpad isn’t designed for frantic activity, so maybe that’s enough?

@fireph
Copy link
Author

fireph commented Oct 8, 2024

Unfortunately not really possible since each side of the touchpad triggers something different so knowing the position of the click is very important. Might just have to introduce a delay in the app to try to work around this.

@slouken
Copy link
Collaborator

slouken commented Oct 8, 2024

Okay, we've made all the changes in SDL that we can to pass through exactly what the controller is telling us. For the normal touchpad use case, where the user swipes and clicks, it seems like it works well. Since you're using it a little more vigorously, it makes sense that you'd have to do more work to handle hardware/firmware bugs.

@slouken slouken closed this as completed Oct 8, 2024
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

No branches or pull requests

2 participants