Bug 81457 – [chromium] Make pixel-accurate wheel scrolling be unanimated

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Robert Kroeger]

Make pixel-accurate wheel scrolling not be animated in ScrollAnimatorNone

[Robert Kroeger]

Created attachment 132469 [details]
Patch

[Robert Kroeger]

I think we need this if we're keeping ScrollAnimatorNone in the code path for handling animated wheels.

jamesr@: care to review?

[Scott Byer]

Comment on attachment 132469 [details]
Patch

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

> Source/WebCore/platform/ScrollAnimatorNone.cpp:421
> +        return ScrollAnimator::scroll(orientation, granularity, step, multiplier);

I haven't been able to confirm on Windows yet, but on Linux, mouse wheels seem to be coming through here, so we'd lose smooth scrolling in that case. It's still behind a flag there, so for the short term that might be OK, but this might also affect the GTK WebKit builds as well.

[Robert Kroeger]

(In reply to comment #3)
> (From update of attachment 132469 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=132469&action=review
> 
> > Source/WebCore/platform/ScrollAnimatorNone.cpp:421
> > +        return ScrollAnimator::scroll(orientation, granularity, step, multiplier);
> 
> I haven't been able to confirm on Windows yet, but on Linux, mouse wheels seem to be coming through here, so we'd lose smooth scrolling in that case. It's still behind a flag there, so for the short term that might be OK, but this might also affect the GTK WebKit builds as well.

change could be chromium platform only? trivial to do. smooth-scrolling can be temporarily disabled on Linux chrome.

[Robert Kroeger]

Created attachment 132471 [details]
Patch

[Scott Byer]

Comment on attachment 132471 [details]
Patch

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

> Source/WebCore/platform/ScrollAnimatorNone.cpp:421
> +#if PLATFORM(CHROMIUM)

Yes, I think this is sufficient until we can get the differentiation between the touch and wheel events down into here.

[James Robinson]

Comment on attachment 132471 [details]
Patch

This is the  WebMouseWheelEvent::hasPreciseScrollingDeltas bool, right?  I think this is fine for now.

[Robert Kroeger]

(In reply to comment #7)
> (From update of attachment 132471 [details])
> This is the  WebMouseWheelEvent::hasPreciseScrollingDeltas bool, right?  I think this is fine for now.

yes

[WebKit Review Bot]

Comment on attachment 132471 [details]
Patch

Clearing flags on attachment: 132471

Committed r111128: <http://trac.webkit.org/changeset/111128>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dana Jansens]

Think this broke a unit test.


[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ScrollAnimatorEnabled
[ RUN      ] ScrollAnimatorEnabled.Enabled
../../third_party/WebKit/Source/WebKit/chromium/tests/ScrollAnimatorNoneTest.cpp:128: Failure
Expected: (100) != (scrollAnimatorNone.currentX()), actual: 100 vs 100
[  FAILED  ] ScrollAnimatorEnabled.Enabled (0 ms)
[----------] 1 test from ScrollAnimatorEnabled (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ScrollAnimatorEnabled.Enabled

[Dana Jansens]

Working now!