-
Notifications
You must be signed in to change notification settings - Fork 77
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
Support cross-browser touch input #527
Conversation
This commit fixes single touch input for a number of mobile browsers. It uses TouchEvents and MouseEvents instead of PointerEvents only, which is necessary in mobile FF, Opera, Safari, and Samsung Internet. Resolves ryanisaacg#526
Codecov Report
@@ Coverage Diff @@
## development #527 +/- ##
============================================
Coverage 29.93% 29.93%
============================================
Files 33 33
Lines 1894 1894
============================================
Hits 567 567
Misses 1327 1327
Continue to review full report at Codecov.
|
c90d0d7
to
9d2d58f
Compare
Earlier today, I added a commit to avoid this warning in the console: My commit did successfully prevent the warning by not calling preventDefault() in the event handler. But that caused some browsers to fire a Mouse event in addition to the Touch event and Quicksilver ended up reporting the button event twice. Now I removed the commit again (with a force push). With this current status, there is no double-reporting in any browser to the best of my knowledge after testing it thoroughly. Is this warning acceptable for now? It is only on mobile and it's currently the best solution I can provide to enable touch input compatible with all mobile browsers. |
Just a heads up, I probably won't have time to look at this until later next week. |
I'm not sure the extra effort to support low-usage browsers is worth it, especially because Quicksilver doesn't really have support for mobile interfaces. Also, Quicksilver's web input code will be moved upstream to winit soon, so I'm reluctant to make short-lived changes to it. |
Thanks for looking into it. I understand your concerns, the long tail of work included to support each and every browser is far too much for Quicksilver and its current plans for the future. However, I think the reported bug is bigger than I might have made it look in my description. Let me try to elaborate a bit more. With way of handling input that seems obvious to me, it is impossible to know the coordinates of a click event caused by a single touch. The move event is produced too late, if at all. This is independent of the browser used, as long as the input-method is touch. In an implementation that uses Quicksilver and just assumes it works as intended, the click position lags always one behind. In other words, the user can tap once at (100,100) and nothing happens. Then, the user tapping again (anywhere, say (300,300)) produces a click event in Quicksilver while the mouse position on the window is at at (100,100). And, if the touch triggers a PointerMove, the move event is produced by Quicksilver right afterwards, so you could theoretically buffer and delay the handling of the event as a user of the library as a workaround. But as far as I remember, this PointerMove is only triggered when the finger position changes slightly during the touch. Therefore, it is crucial that Quicksilver reads the position of the TouchStart event. Or, if you want to keep the PointerEvents, then at least read the position of the PointerDownEvent, which is not done right now. At least this has been my experience. But maybe I have been doing something wrong? Has anyone else actively tested and used the touch input of Quicksilver in a Game? If I am right, it should be a problem for others as well... Anyway, if you don't have the time right now to look into this, it is also fine with me if this is not merged. I am already using my fork of Quicksilver including the fix and there is no pressing reason why I cannot continue to do so. If you want to check the touch on your own phone, you could check it on my demo of my little hobby project at http://demo.paddlers.ch/ (very much WIP, please ignore the ugly UI). I use double-taps to simulate right-clicks, therefore all inputs are a bit delayed. Note also that this is using my fix. With Quicksilver 0.3.18 it would not work as intended. |
That's a good point about PointerDownEvent, I'll make sure that behavior is fixed in the winit web backend. I don't have any touch hardware other than an iPhone, which can't run WebGL2, so I find it hard to test Quicksilver touch input (hence the obvious, breaking but that's gone unfixed.) If you're fine with your fork, is this good to close for now? |
Oh, I see. I didn't know that WebGL2 doesn't run on the iPhone.
Yes, it's fine with me. One further question, only partially related to this issue. Given that you want to move this into winit, does that mean mobile support is off the table within Quicksilver? I will require full mobile support for my game anyway and I would like to know if it is a feasible option that I can contribute this to Quicksilver or if I should put this code elsewhere. For me, it doesn't really matter tbh. |
Mobile support would likely depend on further work on Winit and the underlying graphics libraries, but if they add support I guess Quicksilver would get support as well. |
Just putting this here in the hopes it helps: Firefox is able to emulate touch events. I will admit that I never tried it but being able to simulate it is probably a lot better than not being able to test touch input at all. Open the dev tools -> response design mode -> click the hand at the top of the screen |
@lenscas Thanks! |
Commit Message:
This commit fixes single touch input for a number of mobile browsers.
It uses TouchEvents and MouseEvents instead of PointerEvents only,
which is necessary in mobile FF, Opera, Safari, and Samsung Internet.
Resolves #526
Implementation Details
I have removed dependency on PointerEvents, which are only implemented in some browsers. (also described in more detail in #526)
In particular, these changes were made to the code:
Motivation and Context
Fixes #526
Other than that, I tried to keep everything as it is. (No changes on public API were made.)
Testing
The behavior on browsers that have not been affected by this bug should remain untouched. I have manually click-monkey tested this in Firefox (on Android and Ubuntu), Samsung Internet, and Chrome on Android.
I don't think there is much reason to be concerned that other browsers could be broken now.
Screenshots (if appropriate):
Types of changes
Checks
CONTRIBUTING.md
.CHANGES.md
, with [BREAKING] next to all breaking changes