Sideways 'wobble' when scrolling with trackpad on Mavericks

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

Scrolling Safari on Mavericks is wobbly. We are receiving scroll events with a lot more horizontal deltas (when the user is trying to primarily scroll vertically).

<rdar://problem/14137306>

[Beth Dakin]

Created attachment 222042 [details]
Patch

[Sam Weinig]

Comment on attachment 222042 [details]
Patch

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

> Source/WebCore/platform/PlatformWheelEvent.h:129
> +        void ignoreHorizontalDelta() { m_deltaX = 0; }
> +        void ignoreVerticalDelta() { m_deltaY = 0; }

Is there a way to do this without mutating the platform event? Over the years, I have been trying to make these immutable.

[Beth Dakin]

(In reply to comment #2)
> (From update of attachment 222042 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222042&action=review
> 
> > Source/WebCore/platform/PlatformWheelEvent.h:129
> > +        void ignoreHorizontalDelta() { m_deltaX = 0; }
> > +        void ignoreVerticalDelta() { m_deltaY = 0; }
> 
> Is there a way to do this without mutating the platform event? Over the years, I have been trying to make these immutable.

We might be able to put the tracker down in the ElasticityController instead when we are handing the wheel event there, and that might allow us to avoid mutating the event.

[Simon Fraser (smfr)]

Or we can just clone the event to avoid mutation.

[Beth Dakin]

Created attachment 222754 [details]
Patch

Okay, scrollperf is wonky, and this does not appear to be a regression after all.

[WebKit Commit Bot]

Attachment 222754 [details] did not pass style-queue:


ERROR: Source/WebCore/page/WheelEventDeltaTracker.cpp:28:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 222754 [details]
Patch

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

> Source/WebCore/page/WheelEventDeltaTracker.h:57
> +    Deque<FloatSize> m_recentWheelEventDeltas;

This should probably have some inline capacity, matching recentEventCount.

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.cpp:115
> +        // This is a little crazy because it is working around <rdar://problem/14758615>, which is a bug in a
> +        // lower-level component. While that bug is still unresolved, this will help to reduce some of the noise
> +        // we are seeing with lots of small horizontal deltas when a user is scrolling in primarily vertical
> +        // direction and vice versa.

I think just // Work around <rdar://problem/14758615>
would be sufficient.

[Beth Dakin]

Committed with http://trac.webkit.org/changeset/163180

I added the new files to all of the other projects which will hopeful fix all of the builds.

[Darin Adler]

Comment on attachment 222754 [details]
Patch

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

> Source/WebCore/page/EventHandler.h:517
> +    OwnPtr<WheelEventDeltaTracker> m_recentWheelEventDeltaTracker;

Why not std::unique_ptr instead of OwnPtr?

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.h:81
> +    OwnPtr<WebCore::WheelEventDeltaTracker> m_recentWheelEventDeltaTracker;

Why not std::unique_ptr instead of OwnPtr?