Fix a gesture failure problem on iOS #6145
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d/Cancelled can't always be called in paris".
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
1 similar comment
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
kangwang1988
changed the title
|
I don't fully follow the issue that you are describing. Can you elaborate on what iOS is sending us and how we are misinterpreting it? |
|
@Hixie In this demo, a listview contains severals images which is clickable and could open a new native Page(ViewController for iOS and Activity for android) on click. My flutter environment is as below: KyleWongdeMacBook-Pro:tap_demo kylewong$ /Users/kylewong/Codes/Flutter/official/flutter/bin/flutter doctor -v [✓] Android toolchain - develop for Android devices (Android SDK 28.0.2) [!] iOS toolchain - develop for iOS devices (Xcode 9.4.1) [✓] Android Studio (version 3.1) [!] VS Code (version 1.25.1) [✓] Connected devices (2 available) ! Doctor found issues in 2 categories. I add some log calls(FlutterViewContoller and gesture/convert.dart) as below:
|
|
I reproduced three problematic cases as below: Case 1: In this case, _trackPointers will always hold 3 and the state will never be restored to ready. Henceforth, the TapGestureRecognizer will fails. This part of the log shows that pointer: 3 is related to device_id:2 and generated from UITouch: 0x10bb019d0 Search 0x10bb019d0 and we'll see: One UITouch refers to two device_ids, and only one is finished. |
|
Case2: In this case, _trackPointers will always hold 4 and the state will never be restored to ready. Henceforth, the TapGestureRecognizer will fails. Related log is as below: Here, pointer: 4 is related to device_id:2 and generated from UITouch: 0x12bd418f0 Search 0x12bd418f0 and we'll see: |
|
case 3: In case 1&2, the problem occurs on TagGestureRecognizer. In this case, _trackPointers will always hold 11 and the state will never be restored to ready. Henceforth, the TapGestureRecognizer will fails. Here, pointer: 11 is related to device_id:2 and generated from UITouch: 0x10e469370 Search 0x10e469370 and we'll see: Similar to case1, One UITouch refers to two device_ids, and only one is finished. |
|
As to the cla, we are "Corporate signers". My email is: kang.wang1988@gmail.com and we have a google group: alibaba-flutter-contributors |
|
I signed it! |
|
cc @xster for review |
|
Ah, I remember having this conversation in person. Can you describe the repro steps a bit more? I tried running https://github.com/FlutterRepo/tap_demo and understand that I should run into issues when 2-finger-tapping images in the list or 2-finger-scrolling the list view. But the demo app is behaving as expected (expect for flutter/flutter#11884 which is indeed a bit ridiculous :D). Are there more steps to run into the problem you're describing? Or is it perhaps device specific? |
|
The current PR seems a bit complex with the touch registering map and timestamp tracking etc. I vaguely remember you were saying that the root problem is that we get calls in iOS where some UITouches began and never ended. Seems like there could be a simpler, less compute/memory intensive solution to it. Also, just looking at the current source code, something suspicious jumps to me at the moment. In FlutterViewController, we seem to treat all UITouches in a UITouches NSSet with the same UITouchPhase whereas it's possible in touchesBegan to receive a set of UITouches with different phases between them (with some beginning and some ending). @chinmaygarde looking at the history, it seems like we always treated all touches with the same phase. Do you agree it could be an issue? |
|
@kangwang1988 Chinmay and I tried to reproduce the issue you were describing but couldn't. Here's a sample project Chinmay created to check the incoming and outgoing touches. Can you describe a bit more the reproduction steps? Also, could there be other native UIGestureRecognizers above the FlutterView (which may need https://developer.apple.com/documentation/uikit/uigesturerecognizer/1624218-cancelstouchesinview?language=objc). |
|
Heuristic fix: #6430 |
|
CLAs look good, thanks! |
1 similar comment
|
CLAs look good, thanks! |
|
@kangwang1988 can you describe a bit more how we can reproduce the touch down/up mismatch issue? Was it on a particular iOS device/version/simulator? We weren't able to reproduce the issue. |
|
@xster I'm using flutter v0.8.2 now. |
kangwang1988
added a commit
to XianyuTech/engine
that referenced
this pull request
Nov 15, 2018
|
@xster (cc: @dnfield) can you follow up on @kangwang1988's latest comments? Once that's done, let's get it rebased. |
|
Thanks for the new demo. I can repro the issue with your demo on iOS now. |
|
I'm definitely getting some weird behaviors out of the touch events too from your demo where from a clean state, one single set of touches would create some touches that began and never ended or canceled. Though I'm not sure a timer is the right way to clean this up. |
|
@kangwang1988 to double check, did this issue ever happen with just a single FlutterViewController and no other native code involved? There may be a few issues mixed together but at least one sub issue I can see is when we push other view controllers on top of the FlutterViewController, we're not flushing a cancel on all current ongoing pointers: Filed flutter/flutter#30342 for that issue (which we should just fix by flushing on viewWillDisappear or something). Not sure yet if there are more issues. |
|
As far as I can tell from testing, I can't reproduce any of the issues described here anymore after #8400. I'm going to close this issue now. Please re-open if there are still uncovered cases. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, our team met a gesture failure problem in our ItemDetail page. The scenario is like below:
i.e., we have a scrollable listview(VerticalDragGestureRecognizer actully) which contains a lot of items. Some items are pure text for display, some items are images which is wrapped in a GestureDetector(TapGestureRecognizer actully) so that a single tap could allow the app open a full-screen image.
However, in some abnormal operations,especially when user touch the screen with multiple fingers. For example, when I touch the screen with two fingers, if I touch one image with two fingers in the same image multiple times, the image will not be clickable.
I reproduced the problem and found that that's because that TapGestureRecognizer's state is "GestureRecognizerState.possible" when a new touchevent comes after some idle time, as below:
So this event will be ignored and the tap will not work.
In some operations with two fingers, like one on the listview(text section) while another one on the Image multiple times, the list will not scroll smoothly.
I also reproduces the problem and found that that's because the VerticalDragGestureRecognizer's trackedPointers will be never be empty again, so the didStopTrackingLastPointer will be not called and it's state will be always _DragState.accepted.
The list stuck like below(no bounce-back effect):
When this happens, the image will also be not clickable because of the corresponding RenderIgnorePointer has a true value for ignoring, as below:
I dug more and found this is a platform-specific problem, i.e., it happens only for iOS.
Currently, gestures in flutter track every UITouch in iOS side. For certain flutter gesture, named FG, it tracks any UITouche (UT) in FG's area. Normally, for UT, there are several states, BEGIN(touchesBegan), UPDATE(touchesMoved), FINISH(touchesEnded/touchesCancelled). When the BEGIN and FINISH are called in pair for UT, the flutter gesture works fine. However, when faced with some unusual operations as mentioned above, I found several typical cases as shown below(for one UITouch object, or UT):
A. touchesBegan called -> touchesBegan called -> touchesEnded called
B. touchesBegan called -> touchesEnded called -> touchesBegan called
C. touchesBegan called
That's to say, in some unusual operations, the logic behind flutter gesture in iOS will not work properly.
I add some track and check code based on a timeout logic in my patch to fix this problem, it works fine for us and those problems mentioned above disappear.