[Webkit-unassigned] [Bug 155750] [GTK] Add kinetic scrolling

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=155750

--- Comment #16 from Carlos Garnacho <carlosg at gnome.org> ---
Comment on attachment 281091
  --> https://bugs.webkit.org/attachment.cgi?id=281091
Add inertial scrolling to WebKitGTK+

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

> Source/WebCore/platform/ScrollAnimationKinetic.cpp:71
> +    return (x > high) ? high : ((x < low) ? low : x);

I'd suggest here an assert for low <= high. Shouldn't happen from reading the code, but IMO better to protect against values that'd make this function not work as it'd be expected.

> Source/WebCore/platform/graphics/FloatPoint.h:124
> +    bool isOrigin() const { return !m_x && !m_y; }

I'm usually a bit wary to use ! to check for floats/doubles being 0. Perhaps C++ will do the right thing here, but '!' suggests binary-level operation to me, which might not turn out as expected even if the number is smaller than FLT_EPSILON.

> Source/WebCore/platform/gtk/PlatformWheelEventGtk.cpp:93
> +    m_phase = event->direction == GDK_SCROLL_SMOOTH && !m_deltaX && !m_deltaY ?

same comment here about checking for doubles being equal to 0.0.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:154
> +    WebWheelEvent::Phase phase = event->scroll.direction == GDK_SCROLL_SMOOTH && !deltaX && !deltaY ?

same here with deltaX/Y :)

> Source/WebKit2/UIProcess/gtk/GestureController.cpp:80
> +void GestureController::DragGesture::startDrag(const GdkEvent* event)

It seems this file has now three functions that create a scroll event out of a touch event: this one, startMomentumScroll() a bit further in this patch, and handleDrag() already in code. Maybe a common helper function could be done for these 3?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160613/e7acb6cc/attachment-0001.html>