Bug 235436 – [GTK] REGRESSION: Touch scrolling is broken

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alice Mikhaylenko]

There are 2 issues, I'm not sure whether they are symptoms of the same thing or not.

1. Scrolling is very very slow and delayed - I've investigated this and it's caused by unwanted smooth scroll animations
2. Kinetic scrolling is too fast

With a quick hack:

```diff
diff --git a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
index 559a89bfc027..c940e565bea8 100644
--- a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
+++ b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
@@ -3017,7 +3017,7 @@ void webkitWebViewBaseSynthesizeWheelEvent(WebKitWebViewBase* webViewBase, const
         delta.scale(static_cast<float>(Scrollbar::pixelsPerLineStep()));
 
     priv->pageProxy->handleWheelEvent(NativeWebWheelEvent(const_cast<GdkEvent*>(event), { x, y }, widgetRootCoords(GTK_WIDGET(webViewBase), x, y),
-        delta, wheelTicks, toWebKitWheelEventPhase(phase), toWebKitWheelEventPhase(momentumPhase), priv->wheelHasPreciseDeltas));
+        delta, wheelTicks, toWebKitWheelEventPhase(phase), toWebKitWheelEventPhase(momentumPhase), priv->wheelHasPreciseDeltas || TRUE));
 }
 
 void webkitWebViewBaseSetWheelHasPreciseDeltas(WebKitWebViewBase* webViewBase, bool hasPreciseDeltas)
```

the lag indeed goes away, but kinetic scrolling just stops working altogether. Without that hack and with smooth scrolling disabled, kinetic scrolling seems like it immediately jumps at the position where it would end up if it was animating. With it enabled I can't tell if it's smooth or kinetic scrolling animating.

[Alice Mikhaylenko]

Clarification: this is all only on touchscreen; touchpad scrolling works as expected. Seems to happen with both GTK3 and GTK4 builds too.

[Alice Mikhaylenko]

Ok, and no kinetic scrolling boils down to simply:

```diff
--- a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
+++ b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
@@ -2011,7 +2011,7 @@ static void webkitWebViewBaseTouchSwipe(WebKitWebViewBase* webViewBase, gdouble
                 return;
         }
 
-        webkitWebViewBaseSynthesizeWheelEvent(webViewBase, event, -velocityX / Scrollbar::pixelsPerLineStep(), velocityY / Scrollbar::pixelsPerLineStep(), x, y, WheelEventPhase::NoPhase, WheelEventPhase::Began);
+        webkitWebViewBaseSynthesizeWheelEvent(webViewBase, event, 0, 0, x, y, WheelEventPhase::Ended, WheelEventPhase::NoPhase);
     }
 }
 

```

Since the kinetic scroll handling currently handles event history and everything, we don't care about the velocity we get from GtkGestureSwipe. On the contrary, it messes things up as the event is no longer a scroll stop event as the deltas are non-0. Makign the event look same as the equivalent touchpad event fixes everything.

[Alice Mikhaylenko]

Created attachment 449789 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Chris Lord]

Comment on attachment 449789 [details]
Patch

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

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2014
> +        webkitWebViewBaseSynthesizeWheelEvent(webViewBase, event, 0, 0, x, y, WheelEventPhase::Ended, WheelEventPhase::NoPhase);

Great that this is so simple - I don't know if this will affect test results anywhere, but EWS ought to tell us soon I suppose. Looks fine to me though.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:3020
> +        delta, wheelTicks, toWebKitWheelEventPhase(phase), toWebKitWheelEventPhase(momentumPhase), true));

This will definitely cause many tests to fail - precise deltas will need to be conditionally enabled for wheel events synthesised by touch events.

[Alice Mikhaylenko]

That codepath is only used for touch events though. :)

[Chris Lord]

(In reply to Alexander Mikhaylenko from comment #6)
> That codepath is only used for touch events though. :)

Is the patch view annotation tripping me up here? webkitWebViewBaseSynthesizeWheelEvent is used by WebKitTestRunner/gtk/EventSenderProxyGtk.cpp for synthesising wheel events in layout tests.

[Alice Mikhaylenko]

Ah ok, nevermind then. Once again I've only looked in Source/ but not Tools/.

[Alice Mikhaylenko]

Actually I have another idea.

[Alice Mikhaylenko]

Created attachment 449799 [details]
Patch

[Chris Lord]

Comment on attachment 449799 [details]
Patch

LGTM :)

[Alice Mikhaylenko]

Created attachment 449804 [details]
Patch

[Chris Lord]

Comment on attachment 449804 [details]
Patch

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

LGTM. Wondering about that sign, but I assume you've tested it :)

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2013
> +        webkitWebViewBaseSynthesizeWheelEvent(webViewBase, event, -velocityX, -velocityY, x, y, WheelEventPhase::NoPhase, WheelEventPhase::Began, true);

Is flipping the sign of velocityY here intentional?

[Alice Mikhaylenko]

> but I assume you've tested it :)

I had. The path that broke was tests, not touch scrolling - hence adjustments in the new patch.

> Is flipping the sign of velocityY here intentional?

Yes, it was wrong.

[EWS]

Committed r288609 (246428@main): <https://commits.webkit.org/246428@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449804 [details].