Changeset 95452 in webkit


Ignore:
Timestamp:
Sep 19, 2011 12:13:04 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

[chromium] Gesture recognizer fires taptype only every other touch
down/up sequence ​https://bugs.webkit.org/show_bug.cgi?id=68368

Patch by Robert Kroeger <​rjkroege@chromium.org> on 2011-09-19
Reviewed by Adam Barth.

Source/WebCore:

The addition of doubletap detection to the gesture recognizer missed some of the
outgoing edges in the recognizer state machine. This change simplifies the logic
and handles all outgoing edges.

  • platform/chromium/GestureRecognizerChromium.cpp:

(WebCore::GestureRecognizerChromium::GestureRecognizerChromium):
(WebCore::GestureRecognizerChromium::isInSecondClickTimeWindow):
(WebCore::GestureRecognizerChromium::updateValues):
(WebCore::GestureRecognizerChromium::touchDown):
(WebCore::GestureRecognizerChromium::click):

  • platform/chromium/GestureRecognizerChromium.h:

Source/WebKit/chromium:

The addition of doubletap detection to the gesture recognizer missed some of the
outgoing edges in the recognizer state machine. Thsi change simplifies the logic
and handles all outgoing edges.

  • tests/InnerGestureRecognizerTest.cpp:

Additional tests added to ensure that all edges in the state
machine are accounted for.
(BuildablePlatformTouchEvent::BuildablePlatformTouchEvent):
(SimulateAndTestFirstClick):
(TEST_F):

Location:
trunk/Source
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r95451 r95452  
     12011-09-19  Robert Kroeger  <rjkroege@chromium.org>
     2
     3        [chromium] Gesture recognizer fires taptype only every other touch
     4        down/up sequence https://bugs.webkit.org/show_bug.cgi?id=68368
     5 
     6        Reviewed by Adam Barth.
     7
     8        The addition of doubletap detection to the gesture recognizer missed some of the
     9        outgoing edges in the recognizer state machine. This change simplifies the logic
     10        and handles all outgoing edges.
     11
     12        * platform/chromium/GestureRecognizerChromium.cpp:
     13        (WebCore::GestureRecognizerChromium::GestureRecognizerChromium):
     14        (WebCore::GestureRecognizerChromium::isInSecondClickTimeWindow):
     15        (WebCore::GestureRecognizerChromium::updateValues):
     16        (WebCore::GestureRecognizerChromium::touchDown):
     17        (WebCore::GestureRecognizerChromium::click):
     18        * platform/chromium/GestureRecognizerChromium.h:
     19
    1202011-09-19  Tom Sepez  <tsepez@chromium.org>
    221

  • trunk/Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp

    r94797 r95452  
    3737static const double maximumTouchDownDurationInSecondsForClick = 0.8;
    3838static const double minimumTouchDownDurationInSecondsForClick = 0.01;
     39static const double maximumSecondsBetweenDoubleClick = 0.7;
    3940static const int maximumTouchMoveInPixelsForClick = 20;
    4041
    … …  
    6364    addEdgeFunction(Scroll, FirstFinger, Released, false, &GestureRecognizerChromium::scrollEnd);
    6465    addEdgeFunction(Scroll, FirstFinger, Cancelled, false, &GestureRecognizerChromium::scrollEnd);
    65 
    66     addEdgeFunction(FirstClickReceived, FirstFinger, Pressed, false, &GestureRecognizerChromium::touchDown);
    67     addEdgeFunction(PendingDoubleClick, FirstFinger, Cancelled, false, &GestureRecognizerChromium::noGesture);
    68     addEdgeFunction(PendingDoubleClick, FirstFinger, Released, false, &GestureRecognizerChromium::doubleClick);
    69     addEdgeFunction(PendingDoubleClick, FirstFinger, Moved, false, &GestureRecognizerChromium::maybeDoubleClick);
    70     addEdgeFunction(PendingDoubleClick, FirstFinger, Stationary, false, &GestureRecognizerChromium::maybeDoubleClick);
    7166}
    7267
    … …  
    9691{
    9792    double duration(m_lastTouchTime - m_lastClickTime);
    98     return duration >= minimumTouchDownDurationInSecondsForClick && duration < maximumTouchDownDurationInSecondsForClick;
     93    return duration < maximumSecondsBetweenDoubleClick;
    9994}
    10095
    … …  
    160155void GestureRecognizerChromium::updateValues(const double touchTime, const PlatformTouchPoint& touchPoint)
    161156{
    162     if (state() == FirstClickReceived) {
    163         m_firstTouchTime = touchTime;
    164         m_lastClickTime = m_lastTouchTime;
    165     }
    166157    m_lastTouchTime = touchTime;
    167158    if (state() == NoGesture) {
    … …  
    186177bool GestureRecognizerChromium::touchDown(const PlatformTouchPoint& touchPoint, Gestures gestures)
    187178{
    188     ASSERT(state() == NoGesture || state() == FirstClickReceived);
    189179    appendTapDownGestureEvent(touchPoint, gestures);
    190     if (state() == FirstClickReceived && isInSecondClickTimeWindow() && isInsideManhattanSquare(touchPoint)) {
    191         setState(PendingDoubleClick);
    192         return false;
    193     }
    194180    setState(PendingSyntheticClick);
    195181    return false;
    … …  
    204190}
    205191
    206 bool GestureRecognizerChromium::doubleClick(const PlatformTouchPoint& point, Gestures gestures)
    207 {
     192bool GestureRecognizerChromium::noGesture(const PlatformTouchPoint&, Gestures)
     193{
     194    reset();
     195    return false;
     196}
     197
     198bool GestureRecognizerChromium::click(const PlatformTouchPoint& point, Gestures gestures)
     199{
     200    bool gestureAdded = false;
    208201    if (isInClickTimeWindow() && isInsideManhattanSquare(point)) {
    209         setState(NoGesture);
    210         appendDoubleClickGestureEvent(point, gestures);
    211         return true;
    212     }
    213     return noGesture(point, gestures);
    214 }
    215 
    216 bool GestureRecognizerChromium::maybeDoubleClick(const PlatformTouchPoint& point, Gestures gestures)
    217 {
    218     ASSERT(state() == GestureRecognizerChromium::PendingDoubleClick);
    219     if (point.state() == PlatformTouchPoint::TouchMoved && !isInsideManhattanSquare(point))
    220         return noGesture(point, gestures);
    221     return false;
    222 }
    223 
    224 bool GestureRecognizerChromium::noGesture(const PlatformTouchPoint&, Gestures)
    225 {
     202        gestureAdded = true;
     203        appendClickGestureEvent(point, gestures);
     204        if (isInSecondClickTimeWindow())
     205            appendDoubleClickGestureEvent(point, gestures);
     206       m_lastClickTime = m_lastTouchTime;
     207    }
    226208    reset();
    227     return false;
    228 }
    229 
    230 bool GestureRecognizerChromium::click(const PlatformTouchPoint& point, Gestures gestures)
    231 {
    232     if (isInClickTimeWindow() && isInsideManhattanSquare(point)) {
    233         setState(FirstClickReceived);
    234         appendClickGestureEvent(point, gestures);
    235         return true;
    236     }
    237     return noGesture(point, gestures);
     209    return gestureAdded;
    238210}
    239211

  • trunk/Source/WebCore/platform/chromium/GestureRecognizerChromium.h

    r94797 r95452  
    5050        PendingSyntheticClick,
    5151        Scroll,
    52         FirstClickReceived,
    53         PendingDoubleClick,
    5452    };
    5553
    … …  
    8886    bool scrollEnd(const PlatformTouchPoint&, Gestures);
    8987
    90     bool doubleClick(const PlatformTouchPoint&, Gestures);
    91     bool maybeDoubleClick(const PlatformTouchPoint&, Gestures);
    92 
    9388    WTF::HashMap<int, GestureTransitionFunction> m_edgeFunctions;
    9489    IntPoint m_firstTouchPosition;

  • trunk/Source/WebKit/chromium/ChangeLog

    r95449 r95452  
     12011-09-19  Robert Kroeger  <rjkroege@chromium.org>
     2
     3        [chromium] Gesture recognizer fires taptype only every other touch
     4        down/up sequence https://bugs.webkit.org/show_bug.cgi?id=68368
     5 
     6        Reviewed by Adam Barth.
     7
     8        The addition of doubletap detection to the gesture recognizer missed some of the
     9        outgoing edges in the recognizer state machine. Thsi change simplifies the logic
     10        and handles all outgoing edges.
     11
     12
     13        * tests/InnerGestureRecognizerTest.cpp:
     14        Additional tests added to ensure that all edges in the state
     15        machine are accounted for.
     16        (BuildablePlatformTouchEvent::BuildablePlatformTouchEvent):
     17        (SimulateAndTestFirstClick):
     18        (TEST_F):
     19
    1202011-09-19  Peter Rybin  <peter.rybin@gmail.com>
    221

  • trunk/Source/WebKit/chromium/tests/InnerGestureRecognizerTest.cpp

    r94797 r95452  
    141141class BuildablePlatformTouchEvent : public WebCore::PlatformTouchEvent {
    142142public:
    143     BuildablePlatformTouchEvent(WebCore::TouchEventType type, PlatformTouchPoint& point)
     143    BuildablePlatformTouchEvent(WebCore::TouchEventType type, PlatformTouchPoint& point, double time)
    144144    {
    145145        m_type = type;
    146146        m_touchPoints.append(point);
     147        m_timestamp = time;
     148    }
     149
     150    BuildablePlatformTouchEvent(WebCore::TouchEventType type, PlatformTouchPoint& point)
     151    {
     152        m_type = type;
     153        m_touchPoints.append(point);
     154        m_timestamp = 100.;
    147155    }
    148156};
    … …  
    162170
    163171    BuildablePlatformTouchPoint press(10, 15, PlatformTouchPoint::TouchPressed);
    164     BuildablePlatformTouchEvent pressEvent(WebCore::TouchStart, press);
     172    BuildablePlatformTouchEvent pressEvent(WebCore::TouchStart, press, 1000.);
    165173    OwnPtr<Vector<WebCore::PlatformGestureEvent> > gestureStart(gm.processTouchEventForGestures(pressEvent, false));
    166174    ASSERT_EQ((unsigned int)1, gestureStart->size());
    … …  
    169177
    170178    BuildablePlatformTouchPoint release(10, 16, PlatformTouchPoint::TouchReleased);
    171     BuildablePlatformTouchEvent releaseEvent(WebCore::TouchEnd, release);
    172     gm.setFirstTouchTime(gm.firstTouchTime() - 0.01);
     179    BuildablePlatformTouchEvent releaseEvent(WebCore::TouchEnd, release, 1000. + 0.7);
    173180    OwnPtr<Vector<WebCore::PlatformGestureEvent> > gestureEnd(gm.processTouchEventForGestures(releaseEvent, false));
    174181    ASSERT_EQ((unsigned int)1, gestureEnd->size());
    … …  
    307314    InspectableGestureRecognizerChromium gm;
    308315    SimulateAndTestFirstClick(gm);
    309     ASSERT_EQ(GestureRecognizerChromium::FirstClickReceived, gm.state());
     316    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
     317
     318    BuildablePlatformTouchPoint press(10, 15, PlatformTouchPoint::TouchPressed);
     319    BuildablePlatformTouchEvent pressEvent(WebCore::TouchStart, press, 1000 + .5);
     320    Gestures gestureStart(gm.processTouchEventForGestures(pressEvent, false));
     321    ASSERT_EQ((unsigned int)1, gestureStart->size());
     322    ASSERT_EQ(PlatformGestureEvent::TapDownType, (*gestureStart)[0].type());
     323    ASSERT_EQ(GestureRecognizerChromium::PendingSyntheticClick, gm.state());
     324
     325    BuildablePlatformTouchPoint move(10, 16, PlatformTouchPoint::TouchMoved);
     326    BuildablePlatformTouchEvent moveEvent(WebCore::TouchMove, move, 1000 + .5 + .01);
     327    Gestures gestureMove(gm.processTouchEventForGestures(moveEvent, false));
     328    ASSERT_EQ((unsigned int)0, gestureMove->size());
     329    ASSERT_EQ(GestureRecognizerChromium::PendingSyntheticClick, gm.state());
     330
     331    BuildablePlatformTouchPoint release(10, 16, PlatformTouchPoint::TouchReleased);
     332    BuildablePlatformTouchEvent releaseEvent(WebCore::TouchEnd, release, 1000 + .5 + .02);
     333    Gestures gestureEnd(gm.processTouchEventForGestures(releaseEvent, false));
     334    ASSERT_EQ((unsigned int)2, gestureEnd->size());
     335    ASSERT_EQ(PlatformGestureEvent::TapType, (*gestureEnd)[0].type());
     336    ASSERT_EQ(PlatformGestureEvent::DoubleTapType, (*gestureEnd)[1].type());
     337    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
     338}
     339
     340TEST_F(GestureRecognizerTest, doubleTapGestureIncompleteTest)
     341{
     342    InspectableGestureRecognizerChromium gm;
     343    SimulateAndTestFirstClick(gm);
     344    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
    310345
    311346    BuildablePlatformTouchPoint press(10, 15, PlatformTouchPoint::TouchPressed);
    … …  
    315350    ASSERT_EQ((unsigned int)1, gestureStart->size());
    316351    ASSERT_EQ(PlatformGestureEvent::TapDownType, (*gestureStart)[0].type());
    317     ASSERT_EQ(GestureRecognizerChromium::PendingDoubleClick, gm.state());
    318 
    319     BuildablePlatformTouchPoint move(10, 16, PlatformTouchPoint::TouchMoved);
     352    ASSERT_EQ(GestureRecognizerChromium::PendingSyntheticClick, gm.state());
     353
     354    BuildablePlatformTouchPoint move(10, 50, PlatformTouchPoint::TouchMoved);
    320355    BuildablePlatformTouchEvent moveEvent(WebCore::TouchMove, move);
    321356    Gestures gestureMove(gm.processTouchEventForGestures(moveEvent, false));
    322     ASSERT_EQ((unsigned int)0, gestureMove->size());
    323     ASSERT_EQ(GestureRecognizerChromium::PendingDoubleClick, gm.state());
    324 
    325     BuildablePlatformTouchPoint release(10, 16, PlatformTouchPoint::TouchReleased);
     357    ASSERT_EQ((unsigned int)2, gestureMove->size());
     358    ASSERT_EQ(PlatformGestureEvent::ScrollBeginType, (*gestureMove)[0].type());
     359    ASSERT_EQ(PlatformGestureEvent::ScrollUpdateType, (*gestureMove)[1].type());
     360    ASSERT_EQ(GestureRecognizerChromium::Scroll, gm.state());
     361
     362    BuildablePlatformTouchPoint release(10, 50, PlatformTouchPoint::TouchReleased);
    326363    BuildablePlatformTouchEvent releaseEvent(WebCore::TouchEnd, release);
    327364    gm.setFirstTouchTime(gm.firstTouchTime() - 0.01);
    328365    Gestures gestureEnd(gm.processTouchEventForGestures(releaseEvent, false));
    329366    ASSERT_EQ((unsigned int)1, gestureEnd->size());
    330     ASSERT_EQ(PlatformGestureEvent::DoubleTapType, (*gestureEnd)[0].type());
    331     ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
    332 }
    333 
    334 TEST_F(GestureRecognizerTest, doubleTapGestureIncompleteTest)
    335 {
    336     InspectableGestureRecognizerChromium gm;
    337     SimulateAndTestFirstClick(gm);
    338     ASSERT_EQ(GestureRecognizerChromium::FirstClickReceived, gm.state());
    339 
    340     BuildablePlatformTouchPoint press(10, 15, PlatformTouchPoint::TouchPressed);
    341     BuildablePlatformTouchEvent pressEvent(WebCore::TouchStart, press);
    342     gm.setLastTouchTime(gm.lastTouchTime() - 0.01);
    343     Gestures gestureStart(gm.processTouchEventForGestures(pressEvent, false));
    344     ASSERT_EQ((unsigned int)1, gestureStart->size());
    345     ASSERT_EQ(PlatformGestureEvent::TapDownType, (*gestureStart)[0].type());
    346     ASSERT_EQ(GestureRecognizerChromium::PendingDoubleClick, gm.state());
    347 
    348     BuildablePlatformTouchPoint move(10, 50, PlatformTouchPoint::TouchMoved);
    349     BuildablePlatformTouchEvent moveEvent(WebCore::TouchMove, move);
    350     Gestures gestureMove(gm.processTouchEventForGestures(moveEvent, false));
    351     ASSERT_EQ((unsigned int)0, gestureMove->size());
    352     ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
    353 
    354     BuildablePlatformTouchPoint release(10, 50, PlatformTouchPoint::TouchReleased);
    355     BuildablePlatformTouchEvent releaseEvent(WebCore::TouchEnd, release);
    356     gm.setFirstTouchTime(gm.firstTouchTime() - 0.01);
    357     Gestures gestureEnd(gm.processTouchEventForGestures(releaseEvent, false));
    358     ASSERT_EQ((unsigned int)0, gestureEnd->size());
     367    ASSERT_EQ(PlatformGestureEvent::ScrollEndType, (*gestureEnd)[0].type());
    359368    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
    360369}
    … …  
    414423    ASSERT_EQ((unsigned int)1, gestureEnd->size());
    415424    ASSERT_EQ(PlatformGestureEvent::TapType, (*gestureEnd)[0].type());
    416     ASSERT_EQ(GestureRecognizerChromium::FirstClickReceived, gm.state());
    417 }
     425    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
     426}
     427
     428TEST_F(GestureRecognizerTest, noDoubleTapGestureBecauseOfInterTouchIntervalTest)
     429{
     430    InspectableGestureRecognizerChromium gm;
     431    SimulateAndTestFirstClick(gm);
     432    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
     433
     434    BuildablePlatformTouchPoint press(10, 15, PlatformTouchPoint::TouchPressed);
     435    // Values are from GestureRecognizerChromium.cpp
     436    BuildablePlatformTouchEvent pressEvent(WebCore::TouchStart, press, 1000 + .8 + 10 + .1);
     437    Gestures gestureStart(gm.processTouchEventForGestures(pressEvent, false));
     438    ASSERT_EQ((unsigned int)1, gestureStart->size());
     439    ASSERT_EQ(PlatformGestureEvent::TapDownType, (*gestureStart)[0].type());
     440    ASSERT_EQ(GestureRecognizerChromium::PendingSyntheticClick, gm.state());
     441
     442    BuildablePlatformTouchPoint move(10, 16, PlatformTouchPoint::TouchMoved);
     443    BuildablePlatformTouchEvent moveEvent(WebCore::TouchMove, move, 1000 + .8 + 10 + .1 + .05);
     444    Gestures gestureMove(gm.processTouchEventForGestures(moveEvent, false));
     445    ASSERT_EQ((unsigned int)0, gestureMove->size());
     446    ASSERT_EQ(GestureRecognizerChromium::PendingSyntheticClick, gm.state());
     447
     448    BuildablePlatformTouchPoint release(11, 17, PlatformTouchPoint::TouchReleased);
     449    BuildablePlatformTouchEvent releaseEvent(WebCore::TouchEnd, release, 1000 + .8 + 10 + .1 + .02);
     450    Gestures gestureEnd(gm.processTouchEventForGestures(releaseEvent, false));
     451    ASSERT_EQ((unsigned int)1, gestureEnd->size());
     452    ASSERT_EQ(PlatformGestureEvent::TapType, (*gestureEnd)[0].type());
     453    ASSERT_EQ(GestureRecognizerChromium::NoGesture, gm.state());
     454}
     455
     456
    418457
    419458TEST_F(GestureRecognizerTest, gestureScrollEvents)
Note: See TracChangeset for help on using the changeset viewer.