[webkit-reviews] review granted: [Bug 209230] [GTK][WPE] Enable kinetic scrolling with async scrolling : [Attachment 394478] Patch

[bugzilla-daemon at webkit.org]

Zan Dobersek <zan at falconsigh.net> has granted Chris Lord <clord at igalia.com>'s
request for review:
Bug 209230: [GTK][WPE] Enable kinetic scrolling with async scrolling
https://bugs.webkit.org/show_bug.cgi?id=209230

Attachment 394478: Patch

https://bugs.webkit.org/attachment.cgi?id=394478&action=review




--- Comment #10 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 394478
  --> https://bugs.webkit.org/attachment.cgi?id=394478
Patch

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

> Source/WebCore/platform/ScrollAnimationKinetic.cpp:146
> +    auto first = m_scrollHistory[0].timestamp();
> +    auto last = m_scrollHistory.rbegin()->timestamp();

These Vector lookups should use first() and last().

> Source/WebCore/platform/ScrollAnimationKinetic.cpp:149
> +    if (last == first)
> +	   return { };

Furthermore, the `last - first` difference can be computed early, tested here
to be above-zero, and then used in the return statement.

> Source/WebCore/platform/ScrollAnimationKinetic.cpp:157
> +    return FloatPoint(accumDelta.x() * -1 / (last - first).value(),
accumDelta.y() * -1 / (last - first).value());

The -1 can be incorporated as a sign into the accumDelta value lookup.

Given this is all existing code being moved around, you could take this
polishing up in a different patch.

> Source/WebKit/UIProcess/API/wpe/ScrollGestureController.cpp:92
>  #if WPE_CHECK_VERSION(1, 5, 0)
>	       m_axisEvent.base = {
> -		   wpe_input_axis_event_type_null,
> -		   0, 0, 0, 0, 0, 0
> +		   m_axisEvent.base.type,
> +		   touchPoint->time, m_start.x, m_start.y,
> +		   0, 0, 0
>	       };
>	       m_axisEvent.x_axis = m_axisEvent.y_axis = 0;
>  #else
>	       m_axisEvent = {
> -		   wpe_input_axis_event_type_null,
> -		   0, 0, 0, 0, 0, 0
> +		   wpe_input_axis_event_type_motion,
> +		   touchPoint->time, m_start.x, m_start.y,
> +		   0, 0, 0
>	       };
>  #endif

These two changes don't seem aligned. The first one is maintaining the event
type, the second one is overriding it.