[Webkit-unassigned] [Bug 147320] Use _NSScrollingPredominantAxisFilter for wheel event filtering on Mac

[bugzilla-daemon at webkit.org]

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

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #259483|review?                     |review-
              Flags|                            |

--- Comment #15 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 259483
  --> https://bugs.webkit.org/attachment.cgi?id=259483
Patch

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

> Source/WebCore/page/WheelEventDeltaFilter.cpp:34
> +    : m_currentFilteredDelta(0, 0)

No need to initialize FloatSize.

> Source/WebCore/page/WheelEventDeltaFilter.cpp:44
> +    , m_deltaTracker(WheelEventDeltaTracker())

No need to explicitly call zero-argument constructor.

> Source/WebCore/page/WheelEventDeltaFilter.h:50
> +class WheelEventDeltaFilterPlatformInvariant final : public WheelEventDeltaFilter {

"PlatformInvariant" seems like naming overkill. Maybe BasicWheelEventDeltaFilter

> Source/WebCore/page/WheelEventDeltaFilter.h:60
> +    WheelEventDeltaTracker m_deltaTracker;

Why does WheelEventDeltaTracker exist independently from this class? Can it just be merged into this?

> Source/WebCore/page/mac/WheelEventDeltaFilterMac.mm:31
> +#import <AppKit/NSScrollingInputFilter_Private.h>

This will need some SPI header stuff.

> Source/WebCore/page/mac/WheelEventDeltaFilterMac.mm:83
> +bool WheelEventDeltaFilterMac::isFilteringDeltas() const
> +{
> +    return m_isFilteringDeltas;
> +}
> +
> +FloatSize WheelEventDeltaFilterMac::filteredDelta() const
> +{
> +    return m_currentFilteredDelta;
> +}
> +    
> +FloatSize WheelEventDeltaFilterMac::filteredVelocity() const
> +{
> +    return m_currentFilteredVelocity;
> +}

These could all be inline.

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.cpp:66
> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100
> +    , m_recentWheelEventDeltaFilter(std::make_unique<WheelEventDeltaFilterMac>())
> +#else
> +    , m_recentWheelEventDeltaFilter(std::make_unique<WheelEventDeltaFilterPlatformInvariant>())
> +#endif

Shame to see all this here. Maybe add static WheelEventDeltaFilter::create() which is implemented per-platform.

-- 
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/20150820/a6784f07/attachment.html>