Bug 79521 - [Qt][WK2] Use movementStarted/Ended signals instead of movingChanged on QtViewportInterationEngine
Summary: [Qt][WK2] Use movementStarted/Ended signals instead of movingChanged on QtVie...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-24 12:43 PST by Hugo Parente Lima
Modified: 2012-02-28 02:42 PST (History)
4 users (show)

See Also:


Attachments
Patch (5.81 KB, patch)
2012-02-24 12:49 PST, Hugo Parente Lima
no flags Details | Formatted Diff | Diff
Patch (6.38 KB, patch)
2012-02-27 09:52 PST, Hugo Parente Lima
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hugo Parente Lima 2012-02-24 12:43:45 PST
The movingChanged() signal from Flickable QML component is emitted many times but the QtViewportInteractionEngine is only interested about when the pan starts and when it ends, the Flickable also provide signals for that, so would be better to use those signals (movementStarted()/movementEnded()) instead of movingChanged().
Comment 1 Hugo Parente Lima 2012-02-24 12:49:18 PST
Created attachment 128787 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-02-25 05:13:50 PST
Comment on attachment 128787 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128787&action=review

Almost there!

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:129
> +    connect(m_flickProvider, SIGNAL(movementStarted()), SLOT(panMoveStarted()), Qt::DirectConnection);
> +    connect(m_flickProvider, SIGNAL(movementEnded()), SLOT(panMoveEnded()), Qt::DirectConnection);

I think we should rename those methods to flickableMoveStarted/Ended

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:-193
> -    if (m_flickProvider->isMoving()) {
> -        if (m_scrollUpdateDeferrer)

Can be please assert for this inside the panMoveStarted/Ended to ensure that the state is always correct?
Comment 3 Hugo Parente Lima 2012-02-27 09:03:08 PST
Sir, yes sir!
Comment 4 Hugo Parente Lima 2012-02-27 09:52:33 PST
Created attachment 129056 [details]
Patch

Assertions added, slots renamed.
Comment 5 WebKit Review Bot 2012-02-28 02:40:17 PST
The commit-queue encountered the following flaky tests while processing attachment 129056 [details]:

css3/filters/effect-contrast-hw.html bug 79618 (author: cmarrin@apple.com)
The commit-queue is continuing to process your patch.
Comment 6 WebKit Review Bot 2012-02-28 02:42:20 PST
Comment on attachment 129056 [details]
Patch

Clearing flags on attachment: 129056

Committed r109094: <http://trac.webkit.org/changeset/109094>
Comment 7 WebKit Review Bot 2012-02-28 02:42:24 PST
All reviewed patches have been landed.  Closing bug.