Bug 81144 - [Qt][WK2] Fix bounce-back behaviour for panning
Summary: [Qt][WK2] Fix bounce-back behaviour for panning
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P3 Normal
Assignee: Andras Becsi
URL:
Keywords: Qt
: 81382 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-03-14 12:39 PDT by Andras Becsi
Modified: 2012-03-16 12:58 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch (2.77 KB, patch)
2012-03-14 13:03 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff
proposed patch (4.38 KB, patch)
2012-03-16 07:53 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Becsi 2012-03-14 12:39:18 PDT
The pan gesture should be cancelled and the content should be moved to the end position immediately when tapping during the bounce-back animation.
Comment 1 Andras Becsi 2012-03-14 13:03:27 PDT
Created attachment 131904 [details]
proposed patch
Comment 2 Kenneth Rohde Christiansen 2012-03-15 02:48:31 PDT
Comment on attachment 131904 [details]
proposed patch

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

> Source/WebKit2/UIProcess/qt/QtPanGestureRecognizer.cpp:70
>          interactionEngine()->panGestureCancelled();
> +        interactionEngine()->ensureContentWithinViewportBoundary(/*immediate*/ true);
>          return false;

hmm, yes you move it back immediately, but isn't the other animation still running?
Comment 3 Andras Becsi 2012-03-15 06:59:42 PDT
(In reply to comment #2)
> (From update of attachment 131904 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131904&action=review
> 
> > Source/WebKit2/UIProcess/qt/QtPanGestureRecognizer.cpp:70
> >          interactionEngine()->panGestureCancelled();
> > +        interactionEngine()->ensureContentWithinViewportBoundary(/*immediate*/ true);
> >          return false;
> 
> hmm, yes you move it back immediately, but isn't the other animation still running?

No, it is stopped by the interactionEngine()->panGestureCancelled() call.
This results in a cancelFlick() call to the Flickable, which just stops the animation where it was without returning into valid boundaries.
Since Flickable has no public API for immediate position correction, we need to use our own implementation here.
Comment 4 Andras Becsi 2012-03-15 08:59:01 PDT
Comment on attachment 131904 [details]
proposed patch

Clearing flags on attachment: 131904

Committed r110850: <http://trac.webkit.org/changeset/110850>
Comment 5 Andras Becsi 2012-03-15 08:59:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Andras Becsi 2012-03-16 07:41:26 PDT
This causes an assert in the interaction engine, reopening and preparing a new patch.
Comment 7 Andras Becsi 2012-03-16 07:53:12 PDT
Created attachment 132283 [details]
proposed patch
Comment 8 Kenneth Rohde Christiansen 2012-03-16 08:11:37 PDT
Comment on attachment 132283 [details]
proposed patch

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

> Source/WebKit2/ChangeLog:10
> +        ongoing kinetic scroll or bounce-back animation from a previous pan
> +        gesture the animation is stopped and the content is positioned back

weird english

> Source/WebKit2/ChangeLog:11
> +        to valid boundaries immediately.

I would use immediately earlier

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:498
> +    // ongoing kinetic scroll or bounce-back animation from a previous pan
> +    // gesture the animation is stopped and the content is positioned back

We only want that for kinetic scroll. Bounce back should be handled differently. ie. if there is a bounce back or other kind of scale animation (tap to zoom) then it should be completed before other events can be accepted
Comment 9 Philippe Normand 2012-03-16 08:33:54 PDT
Comment on attachment 132283 [details]
proposed patch

Attachment 132283 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11962348
Comment 10 Andras Becsi 2012-03-16 09:37:12 PDT
(In reply to comment #9)
> (From update of attachment 132283 [details])
> Attachment 132283 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/11962348

This looks bogus to me, the ews seem busted.
Comment 11 Philippe Normand 2012-03-16 09:42:24 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 132283 [details] [details])
> > Attachment 132283 [details] [details] did not pass gtk-ews (gtk):
> > Output: http://queues.webkit.org/results/11962348
> 
> This looks bogus to me, the ews seem busted.

It was indeed. I've fixed this since then! Sorry for the noise
Comment 12 Andras Becsi 2012-03-16 09:43:55 PDT
Committed r111015: <http://trac.webkit.org/changeset/111015>
Comment 13 Andras Becsi 2012-03-16 09:44:46 PDT
Comment on attachment 132283 [details]
proposed patch

Clearing flags from attachment.
Comment 14 Dinu Jacob 2012-03-16 12:58:09 PDT
*** Bug 81382 has been marked as a duplicate of this bug. ***