Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it smooth-scrolls
trackpad events inconsistently.

BUG=575401, 545234
Committed: https://crrev.com/b1e03f8f3ced70e1182401196683d34512238c9b
Cr-Commit-Position: refs/heads/master@{#369610}

[skobes, 2016-01-11 19:33:22]

Description was changed from

==========
Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it causes trackpad
events to be smooth-scrolled inconsistently.

BUG=575401,545234
==========

to

==========
Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it smooth-scrolls
trackpad events inconsistently.

BUG=575401,545234
==========

[skobes, 2016-01-11 19:40:56]

skobes@chromium.org changed reviewers:
+ rbyers@chromium.org, ymalik@chromium.org

[ymalik, 2016-01-11 19:51:04]

lgtm with question

I believe that the purpose of that check was to disable smooth scrolling for
high precision trackpads. What is the
effect of always having smooth scrolling on?

[skobes, 2016-01-11 20:41:21]

On 2016/01/11 19:51:04, ymalik1 wrote:
> lgtm with question
> 
> I believe that the purpose of that check was to disable smooth scrolling for
> high precision trackpads. What is the
> effect of always having smooth scrolling on?

The effect will be that we always smooth-scroll trackpads on Windows.

[ymalik, 2016-01-11 20:43:15]

On 2016/01/11 20:41:21, skobes wrote:
> On 2016/01/11 19:51:04, ymalik1 wrote:
> > lgtm with question
> > 
> > I believe that the purpose of that check was to disable smooth scrolling for
> > high precision trackpads. What is the
> > effect of always having smooth scrolling on?
> 
> The effect will be that we always smooth-scroll trackpads on Windows.

Yes. I meant is that bad for high-precision trackpads?

[skobes, 2016-01-11 20:48:24]

On 2016/01/11 20:43:15, ymalik1 wrote:
> On 2016/01/11 20:41:21, skobes wrote:
> > On 2016/01/11 19:51:04, ymalik1 wrote:
> > > lgtm with question
> > > 
> > > I believe that the purpose of that check was to disable smooth scrolling
for
> > > high precision trackpads. What is the
> > > effect of always having smooth scrolling on?
> > 
> > The effect will be that we always smooth-scroll trackpads on Windows.
> 
> Yes. I meant is that bad for high-precision trackpads?

That's been an area of active discussion on http://crbug.com/545234.  My
experience has been that smooth scrolling actually improves the experience on
trackpads.  I think if we don't have a good way to distinguish wheel vs trackpad
then maybe we should just have it for trackpads, though that is something to
discuss with rbyers also.

(BTW is there some distinction between "high-precision" and "not-high-precision"
trackpads?)

[ymalik, 2016-01-11 21:01:02]

On 2016/01/11 20:48:24, skobes wrote:
> On 2016/01/11 20:43:15, ymalik1 wrote:
> > On 2016/01/11 20:41:21, skobes wrote:
> > > On 2016/01/11 19:51:04, ymalik1 wrote:
> > > > lgtm with question
> > > > 
> > > > I believe that the purpose of that check was to disable smooth scrolling
> for
> > > > high precision trackpads. What is the
> > > > effect of always having smooth scrolling on?
> > > 
> > > The effect will be that we always smooth-scroll trackpads on Windows.
> > 
> > Yes. I meant is that bad for high-precision trackpads?
> 
> That's been an area of active discussion on http://crbug.com/545234.  My
> experience has been that smooth scrolling actually improves the experience on
> trackpads.  I think if we don't have a good way to distinguish wheel vs
trackpad
> then maybe we should just have it for trackpads, though that is something to
> discuss with rbyers also.
> 
> (BTW is there some distinction between "high-precision" and
"not-high-precision"
> trackpads?)

I see, I so presumably not having a set solution to http://crbug.com/545234 for
smooth scroll launch in M49 is fine.

I don't know if there is a distinction per say, but I think high-precision
generally
refers to the new windows trackpads that should presumably generate small scroll
deltas at
a higher frequency.

This has some other explanation:
https://www.reddit.com/r/windows/comments/1hky29/precision_touchpads_the_futu...

[skobes, 2016-01-11 21:11:48]

On 2016/01/11 21:01:02, ymalik1 wrote:
> On 2016/01/11 20:48:24, skobes wrote:
> > On 2016/01/11 20:43:15, ymalik1 wrote:
> > > On 2016/01/11 20:41:21, skobes wrote:
> > > > On 2016/01/11 19:51:04, ymalik1 wrote:
> > > > > lgtm with question
> > > > > 
> > > > > I believe that the purpose of that check was to disable smooth
scrolling
> > for
> > > > > high precision trackpads. What is the
> > > > > effect of always having smooth scrolling on?
> > > > 
> > > > The effect will be that we always smooth-scroll trackpads on Windows.
> > > 
> > > Yes. I meant is that bad for high-precision trackpads?
> > 
> > That's been an area of active discussion on http://crbug.com/545234.  My
> > experience has been that smooth scrolling actually improves the experience
on
> > trackpads.  I think if we don't have a good way to distinguish wheel vs
> trackpad
> > then maybe we should just have it for trackpads, though that is something to
> > discuss with rbyers also.
> > 
> > (BTW is there some distinction between "high-precision" and
> "not-high-precision"
> > trackpads?)
> 
> I see, I so presumably not having a set solution to http://crbug.com/545234
for
> smooth scroll launch in M49 is fine.
> 
> I don't know if there is a distinction per say, but I think high-precision
> generally
> refers to the new windows trackpads that should presumably generate small
scroll
> deltas at
> a higher frequency.
> 
> This has some other explanation:
>
https://www.reddit.com/r/windows/comments/1hky29/precision_touchpads_the_futu...

Interesting.  There are some comments in ui/gfx/win/direct_manipulation.h about
Chrome registering itself as a Direct Manipulation consumer.  I don't think we
can tell the difference in the WM_MOUSEWHEEL handler though...

[Rick Byers, 2016-01-11 21:20:50]

On 2016/01/11 21:11:48, skobes wrote:
> On 2016/01/11 21:01:02, ymalik1 wrote:
> > On 2016/01/11 20:48:24, skobes wrote:
> > > On 2016/01/11 20:43:15, ymalik1 wrote:
> > > > On 2016/01/11 20:41:21, skobes wrote:
> > > > > On 2016/01/11 19:51:04, ymalik1 wrote:
> > > > > > lgtm with question
> > > > > > 
> > > > > > I believe that the purpose of that check was to disable smooth
> scrolling
> > > for
> > > > > > high precision trackpads. What is the
> > > > > > effect of always having smooth scrolling on?
> > > > > 
> > > > > The effect will be that we always smooth-scroll trackpads on Windows.
> > > > 
> > > > Yes. I meant is that bad for high-precision trackpads?
> > > 
> > > That's been an area of active discussion on http://crbug.com/545234.  My
> > > experience has been that smooth scrolling actually improves the experience
> on
> > > trackpads.  I think if we don't have a good way to distinguish wheel vs
> > trackpad
> > > then maybe we should just have it for trackpads, though that is something
to
> > > discuss with rbyers also.
> > > 
> > > (BTW is there some distinction between "high-precision" and
> > "not-high-precision"
> > > trackpads?)
> > 
> > I see, I so presumably not having a set solution to http://crbug.com/545234
> for
> > smooth scroll launch in M49 is fine.
> > 
> > I don't know if there is a distinction per say, but I think high-precision
> > generally
> > refers to the new windows trackpads that should presumably generate small
> scroll
> > deltas at
> > a higher frequency.
> > 
> > This has some other explanation:
> >
>
https://www.reddit.com/r/windows/comments/1hky29/precision_touchpads_the_futu...
> 
> Interesting.  There are some comments in ui/gfx/win/direct_manipulation.h
about
> Chrome registering itself as a Direct Manipulation consumer.  I don't think we
> can tell the difference in the WM_MOUSEWHEEL handler though...

The scenario we were concerned with was it feeling "squishy" if you do a precise
movement and stop.  But I guess you're unlikely to be able to stop your finger
completely within 16ms, so maybe the curves can respond to the slow velocity
quickly enough not to notice.  Anyway as long as that scenario feels just as
good on Chrome as it does on Edge, then I'm fine with it being enabled all the
time.  If it doesn't then you might be able to tweak the algorithm to help (eg.
if the velocity of the most recent event is below some threshold perhaps we
shouldn't animate at all).

We still honor the "has precise details" signal on ChromeOS, right?  I think we
reliably know the difference there.

[skobes, 2016-01-11 21:26:03]

On 2016/01/11 21:20:50, Rick Byers wrote:
> On 2016/01/11 21:11:48, skobes wrote:
> > On 2016/01/11 21:01:02, ymalik1 wrote:
> > > On 2016/01/11 20:48:24, skobes wrote:
> > > > On 2016/01/11 20:43:15, ymalik1 wrote:
> > > > > On 2016/01/11 20:41:21, skobes wrote:
> > > > > > On 2016/01/11 19:51:04, ymalik1 wrote:
> > > > > > > lgtm with question
> > > > > > > 
> > > > > > > I believe that the purpose of that check was to disable smooth
> > scrolling
> > > > for
> > > > > > > high precision trackpads. What is the
> > > > > > > effect of always having smooth scrolling on?
> > > > > > 
> > > > > > The effect will be that we always smooth-scroll trackpads on
Windows.
> > > > > 
> > > > > Yes. I meant is that bad for high-precision trackpads?
> > > > 
> > > > That's been an area of active discussion on http://crbug.com/545234.  My
> > > > experience has been that smooth scrolling actually improves the
experience
> > on
> > > > trackpads.  I think if we don't have a good way to distinguish wheel vs
> > > trackpad
> > > > then maybe we should just have it for trackpads, though that is
something
> to
> > > > discuss with rbyers also.
> > > > 
> > > > (BTW is there some distinction between "high-precision" and
> > > "not-high-precision"
> > > > trackpads?)
> > > 
> > > I see, I so presumably not having a set solution to
http://crbug.com/545234
> > for
> > > smooth scroll launch in M49 is fine.
> > > 
> > > I don't know if there is a distinction per say, but I think high-precision
> > > generally
> > > refers to the new windows trackpads that should presumably generate small
> > scroll
> > > deltas at
> > > a higher frequency.
> > > 
> > > This has some other explanation:
> > >
> >
>
https://www.reddit.com/r/windows/comments/1hky29/precision_touchpads_the_futu...
> > 
> > Interesting.  There are some comments in ui/gfx/win/direct_manipulation.h
> about
> > Chrome registering itself as a Direct Manipulation consumer.  I don't think
we
> > can tell the difference in the WM_MOUSEWHEEL handler though...
> 
> The scenario we were concerned with was it feeling "squishy" if you do a
precise
> movement and stop.  But I guess you're unlikely to be able to stop your finger
> completely within 16ms, so maybe the curves can respond to the slow velocity
> quickly enough not to notice.  Anyway as long as that scenario feels just as
> good on Chrome as it does on Edge, then I'm fine with it being enabled all the
> time.  If it doesn't then you might be able to tweak the algorithm to help
(eg.
> if the velocity of the most recent event is below some threshold perhaps we
> shouldn't animate at all).
> 
> We still honor the "has precise details" signal on ChromeOS, right?  I think
we
> reliably know the difference there.

I think the changes we're making for http://crbug.com/575409 will improve the
"squishiness".

This change is Windows specific and the last time I checked it looked like
ChromeOS was setting hasPreciseScrollingDeltas correctly.  Although there is
something weird going on with PDFs in ChromeOS in http://crbug.com/575648 that I
don't understand.

[Rick Byers, 2016-01-12 14:12:21]

https://codereview.chromium.org/1574013002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_builders_win.cc
(right):

https://codereview.chromium.org/1574013002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_builders_win.cc:291:
wheel_delta = static_cast<float>(GET_WHEEL_DELTA_WPARAM(wparam));
nit: maybe worth a comment saying that we err on the side of
hasPreciseScrollingDeltas=false because we don't see any way to know for sure on
Windows.

[Rick Byers, 2016-01-12 14:53:58]

On 2016/01/11 21:26:03, skobes wrote:
> On 2016/01/11 21:20:50, Rick Byers wrote:
> > On 2016/01/11 21:11:48, skobes wrote:
> > > On 2016/01/11 21:01:02, ymalik1 wrote:
> > > > On 2016/01/11 20:48:24, skobes wrote:
> > > > > On 2016/01/11 20:43:15, ymalik1 wrote:
> > > > > > On 2016/01/11 20:41:21, skobes wrote:
> > > > > > > On 2016/01/11 19:51:04, ymalik1 wrote:
> > > > > > > > lgtm with question
> > > > > > > > 
> > > > > > > > I believe that the purpose of that check was to disable smooth
> > > scrolling
> > > > > for
> > > > > > > > high precision trackpads. What is the
> > > > > > > > effect of always having smooth scrolling on?
> > > > > > > 
> > > > > > > The effect will be that we always smooth-scroll trackpads on
> Windows.
> > > > > > 
> > > > > > Yes. I meant is that bad for high-precision trackpads?
> > > > > 
> > > > > That's been an area of active discussion on http://crbug.com/545234. 
My
> > > > > experience has been that smooth scrolling actually improves the
> experience
> > > on
> > > > > trackpads.  I think if we don't have a good way to distinguish wheel
vs
> > > > trackpad
> > > > > then maybe we should just have it for trackpads, though that is
> something
> > to
> > > > > discuss with rbyers also.
> > > > > 
> > > > > (BTW is there some distinction between "high-precision" and
> > > > "not-high-precision"
> > > > > trackpads?)
> > > > 
> > > > I see, I so presumably not having a set solution to
> http://crbug.com/545234
> > > for
> > > > smooth scroll launch in M49 is fine.
> > > > 
> > > > I don't know if there is a distinction per say, but I think
high-precision
> > > > generally
> > > > refers to the new windows trackpads that should presumably generate
small
> > > scroll
> > > > deltas at
> > > > a higher frequency.
> > > > 
> > > > This has some other explanation:
> > > >
> > >
> >
>
https://www.reddit.com/r/windows/comments/1hky29/precision_touchpads_the_futu...
> > > 
> > > Interesting.  There are some comments in ui/gfx/win/direct_manipulation.h
> > about
> > > Chrome registering itself as a Direct Manipulation consumer.  I don't
think
> we
> > > can tell the difference in the WM_MOUSEWHEEL handler though...
> > 
> > The scenario we were concerned with was it feeling "squishy" if you do a
> precise
> > movement and stop.  But I guess you're unlikely to be able to stop your
finger
> > completely within 16ms, so maybe the curves can respond to the slow velocity
> > quickly enough not to notice.  Anyway as long as that scenario feels just as
> > good on Chrome as it does on Edge, then I'm fine with it being enabled all
the
> > time.  If it doesn't then you might be able to tweak the algorithm to help
> (eg.
> > if the velocity of the most recent event is below some threshold perhaps we
> > shouldn't animate at all).
> > 
> > We still honor the "has precise details" signal on ChromeOS, right?  I think
> we
> > reliably know the difference there.
> 
> I think the changes we're making for http://crbug.com/575409 will improve the
> "squishiness".

Have you been able to see this change feels on (eg.) a surfacebook?  I'd hate to
sacrifice the user experience on top-end hardware in order to compensate for
crappy hardware.

The smooth scroll generation exists in the renderer, downstream of wheel event
coalescing (which occurs in the browser's InputRouter), right?  That seems
unfortunate because it means a janky main thread will reduce the frequency of
the input available.  Eg. imagine I smoothly move then stop and hold on a high
precision touchpad (I do this all the time myself for a relatively precise "next
page" action) right when there's some long running JS.  Rather than a nice
stream of events ending in low velocity (which you could presumably use to halt
the animation), event coalescing will reduce the stream to one or two wheel
events with a large delta (looking like a very high velocity flick) which will
presumably trigger a long smooth-scroll animation.  Without smooth scroll the
motion will be delayed but it'll accurately reflect my intention.  With smooth
scroll the page will coast much further than I told it to move.

There's probably things we can do to mitigate this though.  So we probably just
need to play with how this feels in practice (eg. using a page with synthetic
jank like http://rbyers.github.io/janky-touch-scroll.html).  So anyway this
change LGTM if you want to give it a try, but we might want to tweak some things
or (worst case) even revert this if we can't make it feel good.

> This change is Windows specific and the last time I checked it looked like
> ChromeOS was setting hasPreciseScrollingDeltas correctly.  Although there is
> something weird going on with PDFs in ChromeOS in http://crbug.com/575648 that
I
> don't understand.

[skobes, 2016-01-14 04:35:37]

On 2016/01/12 14:53:58, Rick Byers wrote:
> Have you been able to see this change feels on (eg.) a surfacebook?  I'd hate
to
> sacrifice the user experience on top-end hardware in order to compensate for
> crappy hardware.
> 
> The smooth scroll generation exists in the renderer, downstream of wheel event
> coalescing (which occurs in the browser's InputRouter), right?  That seems
> unfortunate because it means a janky main thread will reduce the frequency of
> the input available.  Eg. imagine I smoothly move then stop and hold on a high
> precision touchpad (I do this all the time myself for a relatively precise
"next
> page" action) right when there's some long running JS.  Rather than a nice
> stream of events ending in low velocity (which you could presumably use to
halt
> the animation), event coalescing will reduce the stream to one or two wheel
> events with a large delta (looking like a very high velocity flick) which will
> presumably trigger a long smooth-scroll animation.  Without smooth scroll the
> motion will be delayed but it'll accurately reflect my intention.  With smooth
> scroll the page will coast much further than I told it to move.

Yes, the animation is done in the renderer while wheel event coalescing is done
by InputRouterImpl in the browser process.  However your concern about "high
velocity" producing a longer animation should be mitigated by
http://crrev.com/1575803002 which does the opposite (higher deltas produce a
shorter animation).

With the combination of this change and http://crrev.com/1575803002 I think the
Dell XPS trackpad feels about equally good with and without smooth scrolling. 
The scroll takes slightly longer to "settle", but is noticeably smoother in the
"intertia" phase.  The X230 trackpad is dramatically better with smooth
scrolling.  I haven't tried a Surfacebook, if you have one handy I can point you
to a build to test.

[Rick Byers, 2016-01-14 20:57:49]

On 2016/01/14 04:35:37, skobes wrote:
> On 2016/01/12 14:53:58, Rick Byers wrote:
> > Have you been able to see this change feels on (eg.) a surfacebook?  I'd
hate
> to
> > sacrifice the user experience on top-end hardware in order to compensate for
> > crappy hardware.
> > 
> > The smooth scroll generation exists in the renderer, downstream of wheel
event
> > coalescing (which occurs in the browser's InputRouter), right?  That seems
> > unfortunate because it means a janky main thread will reduce the frequency
of
> > the input available.  Eg. imagine I smoothly move then stop and hold on a
high
> > precision touchpad (I do this all the time myself for a relatively precise
> "next
> > page" action) right when there's some long running JS.  Rather than a nice
> > stream of events ending in low velocity (which you could presumably use to
> halt
> > the animation), event coalescing will reduce the stream to one or two wheel
> > events with a large delta (looking like a very high velocity flick) which
will
> > presumably trigger a long smooth-scroll animation.  Without smooth scroll
the
> > motion will be delayed but it'll accurately reflect my intention.  With
smooth
> > scroll the page will coast much further than I told it to move.
> 
> Yes, the animation is done in the renderer while wheel event coalescing is
done
> by InputRouterImpl in the browser process.  However your concern about "high
> velocity" producing a longer animation should be mitigated by
> http://crrev.com/1575803002 which does the opposite (higher deltas produce a
> shorter animation).
> 
> With the combination of this change and http://crrev.com/1575803002 I think
the
> Dell XPS trackpad feels about equally good with and without smooth scrolling. 
> The scroll takes slightly longer to "settle", but is noticeably smoother in
the
> "intertia" phase.  The X230 trackpad is dramatically better with smooth
> scrolling.  I haven't tried a Surfacebook, if you have one handy I can point
you
> to a build to test.

Great thanks.  If you think XPS feels just as good with this change, then that's
good enough for me.  We can double-check our surface book after this CL makes it
to canary.  Still LGTM

[skobes, 2016-01-14 21:03:20]

skobes@chromium.org changed reviewers:
+ aelias@chromium.org

[skobes, 2016-01-14 21:03:20]

+aelias for OWNERS

[aelias_OOO_until_Jul13, 2016-01-14 21:43:01]

lgtm

[skobes, 2016-01-14 22:11:09]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2016-01-14 22:12:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1574013002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1574013002/20001

[skobes, 2016-01-14 22:35:08]

The CQ bit was unchecked by skobes@chromium.org

[skobes, 2016-01-14 22:36:07]

https://codereview.chromium.org/1574013002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_builders_win.cc
(right):

https://codereview.chromium.org/1574013002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_builders_win.cc:291:
wheel_delta = static_cast<float>(GET_WHEEL_DELTA_WPARAM(wparam));
On 2016/01/12 14:12:21, Rick Byers wrote:
> nit: maybe worth a comment saying that we err on the side of
> hasPreciseScrollingDeltas=false because we don't see any way to know for sure
on
> Windows.

Done.

[skobes, 2016-01-14 22:36:13]

The CQ bit was checked by skobes@chromium.org

[skobes, 2016-01-14 22:36:13]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
ymalik@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1574013002/#ps40001
(title: "forgot rbyers nit")

[commit-bot: I haz the power, 2016-01-14 22:37:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1574013002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1574013002/40001

[commit-bot: I haz the power, 2016-01-14 23:39:53]

Description was changed from

==========
Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it smooth-scrolls
trackpad events inconsistently.

BUG=575401,545234
==========

to

==========
Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it smooth-scrolls
trackpad events inconsistently.

BUG=575401,545234
==========

[commit-bot: I haz the power, 2016-01-14 23:39:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-14 23:40:43]

Description was changed from

==========
Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it smooth-scrolls
trackpad events inconsistently.

BUG=575401,545234
==========

to

==========
Remove the % WHEEL_DELTA check in WebMouseWheelEventBuilder.

We should investigate if there is a good way to distinguish wheel from trackpad
scrolls, but this check is doing more harm than good since it smooth-scrolls
trackpad events inconsistently.

BUG=575401,545234

Committed: https://crrev.com/b1e03f8f3ced70e1182401196683d34512238c9b
Cr-Commit-Position: refs/heads/master@{#369610}
==========

[commit-bot: I haz the power, 2016-01-14 23:40:44]

Patchset 3 (id:??) landed as
https://crrev.com/b1e03f8f3ced70e1182401196683d34512238c9b
Cr-Commit-Position: refs/heads/master@{#369610}