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

[core] Partial fix gestures for PLATFORM_DRM #3502

Merged
merged 1 commit into from Nov 2, 2023
Merged

[core] Partial fix gestures for PLATFORM_DRM #3502

merged 1 commit into from Nov 2, 2023

Conversation

ghost
Copy link

@ghost ghost commented Nov 2, 2023

Changes

  1. The issue with the gesture system on PLATFORM_DRM is that ProcessGestureEvent(gestureEvent) is called by EventThread() (L1729) that runs on its own thread. By the time PollInputEvents() calls UpdateGestures() (L539) the GESTURE.current has been overwritten. That causes the meaningful gestures to not be detected when calling GetGestureDetected() (L431) or IsGestureDetected() (L258).

  2. This partial fix attempts to address that by moving the ProcessGestureEvent(gestureEvent) call from EventThread() (L1729) to PollInputEvents() (R607), where it runs consistently and in a predictable manner.

  3. It does that by making GestureEvent gestureEvent a global variable (R174) that will hold the data from the gesture generated during EventThread(). That gestureEvent will then be used inside PollInputEvents() when calling ProcessGestureEvent(gestureEvent) (R607).

  4. To only run ProcessGestureEvent(gestureEvent) when there's a new gesture (R605), a new control var newGesture was added (R175) and is reseted after the ProcessGestureEvent(gestureEvent) call (R608).

  5. This fixes the gestures detection for GESTURE_TAP, GESTURE_DOUBLETAP and GESTURE_HOLD.

  6. The GESTURE_DRAG and GESTURE_SWIPE_* work, but are not reliable. There's some other issue with the touch position or reset that is causing them to not be precise as on other platforms. I believe that's related to the way they are pulled from evdev inside EventThread() (L1569-L1706), but I was not able to finish debugging it yet.

Note

  • @raysan5 Unfortunately compiling on the Raspberry Pi is slow and trying to debug with the framebuffer hiding the log/prints makes progress even slower. I may need a lot more time to debug what's causing GESTURE_DRAG and GESTURE_SWIPE_* to malfunction, so I don't think I'll be able to get this fully fixed fast enough for the 5.0 release. This PR, however, should at least restore basic gesture operation for PLATFORM_DRM in the meantime.

Reference

Environment

  • Tested on Raspberry Pi 3 Model B 1.2 with Linux (Raspberry Pi OS 11 Bullseye 32-bit armhf).

Edits

  • 1: added line marks.

@raysan5
Copy link
Owner

raysan5 commented Nov 2, 2023

@ubkp thank you very much for the review and the so detailed explanation!

Reading inputs in separate threads has that kind of downsizes, there should be some synchronization at a fixed rate to get the current state and process it per-frame, similar to other platforms... or just simplify the input and let some inputs run in the main thread.

In any case, that solution looks good to me, but maybe @michaelfiber can take a look before merging, he has more experience than me with PLATFORM_DRM and touch-displays where gestures could apply.

@ghost
Copy link
Author

ghost commented Nov 2, 2023

or just simplify the input and let some inputs run in the main thread.

IMHO, if possible, this would be the best scenario. By the way, now that SDL is an option, couldn't DRM also use SDL2 too? That would make development/maintenance for it a lot easier.

Edit: Looks possible: https://wiki.libsdl.org/SDL2/README/raspberrypi.
Not sure about https://wiki.libsdl.org/SDL2/README/raspberrypi#opengl-problems tho.
What do you think?

@michaelfiber
Copy link
Sponsor Contributor

@ubkp thank you very much for the review and the so detailed explanation!

Reading inputs in separate threads has that kind of downsizes, there should be some synchronization at a fixed rate to get the current state and process it per-frame, similar to other platforms... or just simplify the input and let some inputs run in the main thread.

In any case, that solution looks good to me, but maybe @michaelfiber can take a look before merging, he has more experience than me with PLATFORM_DRM and touch-displays where gestures could apply.

Sorry for my quietness recently. I'm following these but haven't had a chance to chime in.

This fix is very much in line with what I've been working on. I've been trying to find a down side to moving more input processing into the main thread but so far performance has been fantastic.

@raysan5
Copy link
Owner

raysan5 commented Nov 2, 2023

By the way, now that SDL is an option, couldn't DRM also use SDL2 too? That would make development/maintenance for it a lot easier.

@ubkp Yes, it's possible and it also applies to all the other supported platforms... but it's a path I prefer to avoid.

SDL is a great library with support for many platforms, including Android, iOS, Web and even consoles BUT it's a big external dependency. Since the beginning I tried to keep raylib as minimal and self-contained as possible, minimizing external dependencies (and reinventing the wheel when required). I really like that some platforms are completely dependency-free at a platform-level (Android, DRM) and if I got enough resources and time I would implement more custom platform layers but unfortunately resources are limited and GLFW/SDL do an excelent work at that level.

GLFW is quite small and really focused on its task (window and input management), it was easy to integrate and ship with raylib code... but its development has slowdown in the last two years. SDL is bigger and more featured but afaik, more difficult to integrate seamlessly into raylib codebase, it should be shipped as an external dependency.

So, I decided to add the SDL backend as a possible alternative to GLFW, targeting advance users that feel confortable with it and want to target more platform or have some advance needs... BUT I like to keep the custom low-level dependency-free implementations for users that prefer the full raylib self-contained experience.

In any case, it could change in the future, SDL3 is in the works at the moment.

@michaelfiber Ok, I understand is aligned with the new input direction so I'm merging it.

@raysan5 raysan5 merged commit fe34fc7 into raysan5:master Nov 2, 2023
12 checks passed
@ghost
Copy link
Author

ghost commented Nov 2, 2023

@raysan5 Got it! That's very good reasoning. Thank you very much for the explanation! 👍

@ghost ghost deleted the fix/drm-gestures branch November 2, 2023 17:45
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.

2 participants