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

[bugzilla-daemon at webkit.org]

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

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #281977|                            |review+, commit-queue-
              Flags|                            |

--- Comment #34 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 281977
  --> https://bugs.webkit.org/attachment.cgi?id=281977
[GTK] Add kinetic scrolling

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

Looks good to me, thanks! We just need a wk2 owner to approve the changes in WebPageProxy

> Source/WebCore/platform/ScrollAnimationKinetic.h:74
> +    Optional<PerAxisData> m_horizontalData { Nullopt };
> +    Optional<PerAxisData> m_verticalData { Nullopt };

You don't need to initialize these, the constructor already sets is to Nullopt.

> Source/WebCore/platform/gtk/ScrollAnimatorGtk.cpp:60
> +        updatePosition(WTFMove(position));
> +#if ENABLE(SMOOTH_SCROLLING)
> +        if (m_smoothAnimation)
> +            m_smoothAnimation->setCurrentPosition(position);
> +#endif

hmm, this is a problem now that I see it, this is using position after being moved. You can probably just change the order, and call updatePosition after setCurrentPosition

> Source/WebCore/platform/gtk/ScrollAnimatorGtk.cpp:107
> +#if ENABLE(SMOOTH_SCROLLING)
> +    if (m_smoothAnimation)
> +        m_smoothAnimation->setCurrentPosition(position);
> +#endif
> +
> +    updatePosition(WTFMove(position));

like you do here

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1893
> +bool WebPageProxy::shouldProcessWheelEventNow(const WebWheelEvent& event)
> +{
> +#if PLATFORM(GTK)
> +    // Don't queue events representing a non-trivial scrolling phase to
> +    // avoid having them trapped in the queue, potentially preventing a
> +    // scrolling session to beginning or end correctly.
> +    // This is only needed by platforms whose WebWheelEvent has this phase
> +    // information (Cocoa and GTK+) but Cocoa was fine without it.
> +    if (event.phase() == WebWheelEvent::Phase::PhaseNone
> +        || event.phase() == WebWheelEvent::Phase::PhaseChanged
> +        || event.momentumPhase() == WebWheelEvent::Phase::PhaseNone
> +        || event.momentumPhase() == WebWheelEvent::Phase::PhaseChanged)
> +        return true;
> +#else
> +    UNUSED_PARAM(event);
> +#endif
> +    if (m_wheelEventQueue.size() >= wheelEventQueueSizeThreshold)
> +        return true;
> +    return false;
> +}

This is in cross-platform code, we need approval from a WebKit2 owner

> Source/WebKit2/UIProcess/WebPageProxy.h:1469
> +    bool shouldProcessWheelEventNow(const WebWheelEvent&);

This could probably be const

-- 
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/20160627/e1693c42/attachment.html>