969860 - Pinch End cause Double tap event fired

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Oleg Romashin (:romaxa)]

I've noticed that in latest trunk if sequence of events is:
1) Pinch started with 2 points
2) Pinch ended because one point was released
3) very quickly released second point right after first

Double Tap get's fired.

This happen because:
1) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l275 - Generate extra Touch Start 
2) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l81 - set state to GESTURE_WAITING_SINGLE_TAP with that
3) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l172 - set state to GESTURE_WAITING_DOUBLE_TAP
4) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l150 - finally handle Double Tap


This does not happen if no extra Touch Start fired or Second point released with delay > MAX_TAP_TIME

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Doug, can you look into this?

Comment 2

[Doug Sherk (:drs) (inactive)]

I'm unable to repro this on a build from an hour ago. Are there any extra steps or anything that you may have missed?

Comment 3

[Oleg Romashin (:romaxa)]

This was reproduced on Qt embedding setup, which may have different timings on Pinch behavior.
Do you hit GestureEventListener.cpp#l275 at all?

Comment 4

[Doug Sherk (:drs) (inactive)]

(In reply to Oleg Romashin (:romaxa) from comment #3)
> This was reproduced on Qt embedding setup, which may have different timings
> on Pinch behavior.
> Do you hit GestureEventListener.cpp#l275 at all?

Yes. I only went as far as to check that, doing the STR, and checking whether or not I hit GestureEventListener.cpp#l150, which I didn't.

Comment 5

[Doug Sherk (:drs) (inactive)]

Attachment: Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

After talking with :romaxa on IRC, I was still unable to repro the original issue. However, I was able to catch a single tap that was being fired at the end of pinches. This patch seems to fix this.

I believe the best way to fix this would be to have a flag on GEL to indicate whether or not it's spoofing events and it should ignore them until that flag is unset. But I don't think that much machinery is necessary for a single case like this. If we add any more spoofing, we might consider switching to that approach.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

Review of attachment 8373452 [details] [diff] [review]:
-----------------------------------------------------------------

Romaxa, does this patch fix the issue you were seeing?

Comment 7

[Oleg Romashin (:romaxa)]

Comment on attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

yep, this works fine

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

Review of attachment 8373452 [details] [diff] [review]:
-----------------------------------------------------------------

Eventually I'd like to change this interaction so that touches don't keep bouncing back and forth between APZC and GEL but for now this is fine.

Comment 9

[dmitry.rojkov]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> Eventually I'd like to change this interaction so that touches don't keep
> bouncing back and forth between APZC and GEL but for now this is fine.

That would be really nice. Mutually recursive functions with side effects is a receipt for good headache.
In addition to that I think GEL would be much easier to debug if it'd been refactored to a Mealy machine.

Comment 10

[Doug Sherk (:drs) (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec8e723e0991

Comment 11

[Doug Sherk (:drs) (inactive)]

(In reply to dmitry.rojkov from comment #9)
> That would be really nice. Mutually recursive functions with side effects is
> a receipt for good headache.
> In addition to that I think GEL would be much easier to debug if it'd been
> refactored to a Mealy machine.

Feel free to do that. We could really use more people working on refactors like this.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ec8e723e0991