| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
android: Let Servo handle touch events #40240
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
Conversation
This change makes it so that Servo handles touch events on Android. Flinging becomes a bit less native, but with two big benefits: 1. Before touch event handling on Android wasn't standards compliant, because web content didn't get a chance to call `preventDefault()` on touch events, whcih important for proper site behavior. 2. This unifies all touch event handling across Servo platforms. This means that improvements to things like fling will benefit all platforms equally. In addition, fling on Android can be integrated into Servo's animation handler. Generally, this also just makes things much simpler to reason about on Android. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
servo-highfive
added
the
S-awaiting-review
|
🔨 Triggering try run (#18870213659) for Android |
| case (MotionEvent.ACTION_CANCEL): | ||
| mServo.touchCancel(x, y, pointerId); | ||
| break; | ||
| default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other cases? Perhaps we could add a log / warning here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are the other action constants: https://developer.android.com/reference/android/view/MotionEvent#constants_1
I don't think any of the others are interesting or will be triggered in this method. Is there something in particular that you are worried about missing? I was hoping to actually have Servo print fewer log statements, because already floods the logs with many events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so in particular mouse events are not part of this motion event, right?
Nothing else in that list looks particularly interesting, so I'm okay with not logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, though there are mouse events as part of the motion events, I think that we'd need to handle it via an implementation of onGenericMotionEvent(MotionEvent event). I suspect that makes sense to handle mice, but maybe in a followup.
|
✨ Try run (#18870213659) succeeded. |
jschwe
left a comment
jschwe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on android. Scrolling still works, but indeed noticably different. I guess we might want to look into vsync and improving the scroll handling in servo later.
servo-highfive
removed
the
S-awaiting-review
Yep. My next steps here are to add a vsync implementation for Android and then to add an easing function for fling animations. Hopefully that makes things seem a bit more natural. Thanks for the review! |
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Labels
3 participants
This change makes it so that Servo handles touch events on Android.
Flinging becomes a bit less native, but with two big benefits:
because web content didn't get a chance to call
preventDefault()ontouch events, whcih important for proper site behavior.
means that improvements to things like fling will benefit all
platforms equally. In addition, fling on Android can be integrated
into Servo's animation handler.
Generally, this also just makes things much simpler to reason about on
Android.
Testing: This kind of input handling isn't tested in Servo yet.