Mac: Support Sierra's broken MouseWheel events.

Mac: Support Sierra's broken MouseWheel events.

Since 10.12.1, Sierra has started sending MouseWheel events (from
physical, clicky mouse wheels) with a single "tick" (or repeated slow
ticks) all with -[NSEvent deltaY] equal to zero ([NSEveevent
scrollingDeltaY] is also zero. This affects all applications on macOS
including Safari, Console, and any NSScrollView.

After poking around the events generated by my "Logitech B100 model
M-U0026" optical mouse, I discovered we can use
kCGScrollWheelEventPointDeltaAxis1 to get an appropriate value to use
for these events. A single, "slow" tick has
kCGScrollWheelEventPointDeltaAxis1 equal to +/- 1, and they go up by
whole integers (e.g. when using CGEventGetDoubleValueField rather than
CGEventGetIntegerValueField).

When scrolling "fast' the value "is about a tenth" of what starts to
appear in -[NSEvent deltaY]. There doesn't really appear to be a linear
relationship between these (there's probably some internal state
applying momentum in a different way), but 10 happens to be what
CGEventSourceGetPixelsPerLine() reports.

So to fix, detect an event that would have zero scroll deltas when it
shouldn't, and replace the zero delta with the value from
kCGScrollWheelEventPointDeltaAxis1 multiplied by some constant instead.

BUG=660773
TEST=On macOS Sierra 10.12.1, scroll a web page with a physical "clicky"
wheel mouse very slowly, by single ticks. The page should always scroll
a little bit for each click.

[tapted, 2016-10-31 05:05:13]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 05:05:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 05:32:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-31 05:32:08]

Dry run: This issue passed the CQ dry run.

[tapted, 2016-10-31 06:10:26]

Description was changed from

==========
Mac: Support Sierra's broken MouseWheel events

BUG=
==========

to

==========
Mac: Support Sierra's broken MouseWheel events.

Since 10.12.1, Sierra has started sending MouseWheel events (from
physical, clicky mouse wheels) with a single "tick" (or repeated slow
ticks) all with -[NSEvent deltaY] equal to zero ([NSEveevent
scrollingDeltaY] is also zero. This affects all applications on macOS
including Safari, Console, and any NSScrollView.

After poking around the events generated by my "Logitech B100 model
M-U0026" optical mouse, I discovered we can use
kCGScrollWheelEventPointDeltaAxis1 to get an appropriate value to use
for these events. A single, "slow" tick has
kCGScrollWheelEventPointDeltaAxis1 equal to +/- 1, and they go up by
whole integers (e.g. when using CGEventGetDoubleValueField rather than
CGEventGetIntegerValueField).

When scrolling "fast' the value "is about a tenth" of what starts to
appear in -[NSEvent deltaY]. There doesn't really appear to be a linear
relationship between these (there's probably some internal state
applying momentum in a different way), but 10 happens to be what
CGEventSourceGetPixelsPerLine() reports.

So to fix, detect an event that would have zero scroll deltas when it
shouldn't, and replace the zero delta with the value from
kCGScrollWheelEventPointDeltaAxis1 multiplied by some constant instead.

BUG=660773
TEST=On macOS Sierra 10.12.1, scroll a web page with a physical "clicky"
wheel mouse very slowly, by single ticks. The page should always scroll
a little bit for each click.
==========

[tapted, 2016-10-31 06:10:26]

tapted@chromium.org changed reviewers:
+ avi@chromium.org

[tapted, 2016-10-31 06:10:37]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 06:10:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 06:30:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-31 06:30:48]

Dry run: This issue passed the CQ dry run.

[tapted, 2016-10-31 06:32:16]

Hi Avi.... WDYT? Obviously Apple need to fix this, but this is driving me
insane. It's so bad I feel like they _have_ to push out a fix before 10.11.2,
but I don't know how we can get info on that. Even ~December feels too long
(assuming we merge this to m55).

[Avi (use Gerrit), 2016-10-31 13:32:59]

Massive :/

I know what you mean with "There doesn't really appear to be a linear
relationship between these (there's probably some internal state
applying momentum in a different way)". That huge comment titled "Of Mice and
Men on line 441" is mine; you're looking at an unaccelerated value.

Does Apple know about this? This seems like something super broken, and I'm
surprised they shipped it, but how much do we know if this is on their radar?

https://codereview.chromium.org/2461323002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_builders_mac.mm
(right):

https://codereview.chromium.org/2461323002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_builders_mac.mm:571: //
cases. First assume the CGEventSource has the default value of 10, rather
"First" assume? I'm not sure what you mean by that comment.

I'm not sure how CGEventSourceGetPixelsPerLine() works. Does it tend to stay the
same for multiple calls? Why not cache it?

[tapted, 2016-11-01 01:56:39]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 01:57:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2016-11-01 02:29:01]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 02:29:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2016-11-01 02:29:46]

On 2016/10/31 13:32:59, Avi wrote:
> Massive :/
> 
> I know what you mean with "There doesn't really appear to be a linear
> relationship between these (there's probably some internal state
> applying momentum in a different way)". That huge comment titled "Of Mice and
> Men on line 441" is mine; you're looking at an unaccelerated value.

This bug manages to mix things up a bit :/. There doesn't seem to be un
unaccelerated value any more.

But also the big comment says kCGScrollWheelEventDeltaAxis1 should be the
unaccelerated value, but kCGScrollWheelEvent*Point*DeltaAxis1 is the only one
that gives non-zero values when this bug is hit and is the one I'm using.

I put some trace data at http://cbrug.com/660773#c3 . See also last comment
here.

It seems to be that for kCGScrollWheelEventDeltaAxis1
 WAS: NO acceleration: each notch gives +/- 1
 NOW: Looks like it's just round_to_int(kCGScrollWheelEventPointDeltaAxis1 /
10). i.e. it's accelerated now.


Eugh - but I just saw your comment about a "nasty bug" when using
kCGScrollWheelEventPointDeltaAxis1 when holding shift. With this workaround in
place, scrolling slowly with shift would scroll vertically, and it would switch
to horizontal when scrolling faster. Gross. Well, everything about this is
gross.

Updated so we only try to fix up vertical scrolling. But I'm still a bit unsure
if we should bother landing this... (seeking your opinion on that too - maybe
notchy mouse users like me are a dying breed on Mac).



> Does Apple know about this? This seems like something super broken, and I'm
> surprised they shipped it, but how much do we know if this is on their radar?

I've filed a rdar via apple-bugs@. No updates on it yet (and no dupe reference).

A user at
http://apple.stackexchange.com/questions/255114/unresponsive-mouse-wheel-on-m...
says "this started happening in the Sierra public beta 2, in July, and I
reported it via the feedback assistant. But even on the latest Sierra 10.12.1
beta 3, the issue still persists." 

Maybe there's a build being fed to the machine we have set up in the beta
program... 

Do you know someone@apple.com we can poke directly?

https://codereview.chromium.org/2461323002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_builders_mac.mm
(right):

https://codereview.chromium.org/2461323002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_builders_mac.mm:571: //
cases. First assume the CGEventSource has the default value of 10, rather
On 2016/10/31 13:32:59, Avi wrote:
> "First" assume? I'm not sure what you mean by that comment.

Ah, there were more steps earlier :). Should be more clear now.

> I'm not sure how CGEventSourceGetPixelsPerLine() works. Does it tend to stay
the
> same for multiple calls? Why not cache it?

Yeah - the documentation suggests that it will be the same unless something
changes it with CGEventSourceSetPixelsPerLine . But the documentation is unclear
about how this may be shared between processes (i.e. Chrome certainly doesn't
call CGEventSourceSetPixelsPerLine).

As for caching it.. One issue is that
CGEventSourceGetPixelsPerLine(CGEventSource) requires a CGEventSource, but
CGEventGetSource(CGEvent) is deprecated -

https://developer.apple.com/reference/coregraphics/1658572-quartz_event_servi...

Instead there is only "CGEventCreateSourceFromEvent" which has a less clear
lifetime, and the documentation says it may also return null (maybe if some
other process is injecting events into Chrome?)

CGEventSourceSetPixelsPerLine says

/* Set the scale of pixels per line in a scrolling event source.

   This function sets the scale of pixels per line in the specified event
   source. For example, if you pass the value 12 as the `pixelsPerLine'
   parameter, the scale of pixels per line in the event source would be
   changed to 12. Every scrolling event can be interpreted to be scrolling
   by pixel or by line. By default, the scale is about ten pixels per
   line. */


So just using the default of 10 seems to give predictable results.

And I'm actually not sure what AppKit does behind the scenes (they're both
accelerated, but "differently") -- the actual relationship can be as low as 5x
instead of 10x when momentum ramps up, and the same value of
kCGScrollWheelEventPointDeltaAxisX (e.g. -11) doesn't always give the same value
of `[event deltaX]` (e.g. -11 could be -1.01988, or -1.086). So using
CGEventSourceGetPixelsPerLine seems to give more credibility to the calculation
when really it's a big 'ol guesstimate. There's not even consistency with the
cutoff. E.g. Sometimes -4 will have deltaX of zero, sometimes it will be
-0.358704

I've updated the comment about this.

[commit-bot: I haz the power, 2016-11-01 03:51:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-01 03:51:43]

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2016-11-01 04:49:55]

On 2016/11/01 02:29:46, tapted wrote:
> On 2016/10/31 13:32:59, Avi wrote:
> > Massive :/
> > 
> > I know what you mean with "There doesn't really appear to be a linear
> > relationship between these (there's probably some internal state
> > applying momentum in a different way)". That huge comment titled "Of Mice
and
> > Men on line 441" is mine; you're looking at an unaccelerated value.
> 
> This bug manages to mix things up a bit :/. There doesn't seem to be un
> unaccelerated value any more.
> 
> But also the big comment says kCGScrollWheelEventDeltaAxis1 should be the
> unaccelerated value, but kCGScrollWheelEvent*Point*DeltaAxis1 is the only one
> that gives non-zero values when this bug is hit and is the one I'm using.
> 
> I put some trace data at http://cbrug.com/660773#c3 . See also last comment
> here.
> 
> It seems to be that for kCGScrollWheelEventDeltaAxis1
>  WAS: NO acceleration: each notch gives +/- 1
>  NOW: Looks like it's just round_to_int(kCGScrollWheelEventPointDeltaAxis1 /
> 10). i.e. it's accelerated now.
> 
> 
> Eugh - but I just saw your comment about a "nasty bug" when using
> kCGScrollWheelEventPointDeltaAxis1 when holding shift. With this workaround in
> place, scrolling slowly with shift would scroll vertically, and it would
switch
> to horizontal when scrolling faster. Gross. Well, everything about this is
> gross.
> 
> Updated so we only try to fix up vertical scrolling. But I'm still a bit
unsure
> if we should bother landing this... (seeking your opinion on that too - maybe
> notchy mouse users like me are a dying breed on Mac).
> 
> 
> 
> > Does Apple know about this? This seems like something super broken, and I'm
> > surprised they shipped it, but how much do we know if this is on their
radar?
> 
> I've filed a rdar via apple-bugs@. No updates on it yet (and no dupe
reference).
> 
> A user at
>
http://apple.stackexchange.com/questions/255114/unresponsive-mouse-wheel-on-m...
> says "this started happening in the Sierra public beta 2, in July, and I
> reported it via the feedback assistant. But even on the latest Sierra 10.12.1
> beta 3, the issue still persists." 
> 
> Maybe there's a build being fed to the machine we have set up in the beta
> program... 
> 
> Do you know mailto:someone@apple.com we can poke directly?
> 
>
https://codereview.chromium.org/2461323002/diff/20001/content/browser/rendere...
> File content/browser/renderer_host/input/web_input_event_builders_mac.mm
> (right):
> 
>
https://codereview.chromium.org/2461323002/diff/20001/content/browser/rendere...
> content/browser/renderer_host/input/web_input_event_builders_mac.mm:571: //
> cases. First assume the CGEventSource has the default value of 10, rather
> On 2016/10/31 13:32:59, Avi wrote:
> > "First" assume? I'm not sure what you mean by that comment.
> 
> Ah, there were more steps earlier :). Should be more clear now.
> 
> > I'm not sure how CGEventSourceGetPixelsPerLine() works. Does it tend to stay
> the
> > same for multiple calls? Why not cache it?
> 
> Yeah - the documentation suggests that it will be the same unless something
> changes it with CGEventSourceSetPixelsPerLine . But the documentation is
unclear
> about how this may be shared between processes (i.e. Chrome certainly doesn't
> call CGEventSourceSetPixelsPerLine).
> 
> As for caching it.. One issue is that
> CGEventSourceGetPixelsPerLine(CGEventSource) requires a CGEventSource, but
> CGEventGetSource(CGEvent) is deprecated -
> 
>
https://developer.apple.com/reference/coregraphics/1658572-quartz_event_servi...
> 
> Instead there is only "CGEventCreateSourceFromEvent" which has a less clear
> lifetime, and the documentation says it may also return null (maybe if some
> other process is injecting events into Chrome?)
> 
> CGEventSourceSetPixelsPerLine says
> 
> /* Set the scale of pixels per line in a scrolling event source.
> 
>    This function sets the scale of pixels per line in the specified event
>    source. For example, if you pass the value 12 as the `pixelsPerLine'
>    parameter, the scale of pixels per line in the event source would be
>    changed to 12. Every scrolling event can be interpreted to be scrolling
>    by pixel or by line. By default, the scale is about ten pixels per
>    line. */
> 
> 
> So just using the default of 10 seems to give predictable results.
> 
> And I'm actually not sure what AppKit does behind the scenes (they're both
> accelerated, but "differently") -- the actual relationship can be as low as 5x
> instead of 10x when momentum ramps up, and the same value of
> kCGScrollWheelEventPointDeltaAxisX (e.g. -11) doesn't always give the same
value
> of `[event deltaX]` (e.g. -11 could be -1.01988, or -1.086). So using
> CGEventSourceGetPixelsPerLine seems to give more credibility to the
calculation
> when really it's a big 'ol guesstimate. There's not even consistency with the
> cutoff. E.g. Sometimes -4 will have deltaX of zero, sometimes it will be
> -0.358704
> 
> I've updated the comment about this.

Can you talk to dmac? I'd like to try official channels first.

[tapted, 2016-11-01 06:45:46]

ooh - I just got my hands on the 10.12.2 Developer Seed and it looks like this
is fixed. (at least with the hardware I tried). So, I guess now we just hold our
breath.