Re: [Qemu-devel] [PATCH v3] ui/cocoa.m: replace scrollingDeltaY with deltaY

[G 3]

On Jul 11, 2018, at 5:18 PM, Peter Maydell wrote:

> On 9 July 2018 at 16:02, John Arbuckle <address@hidden>  
> wrote:
> > The NSEvent class method scrollingDeltaY is available
> > for Mac OS 10.7 and newer. Since QEMU supports Mac OS
> > 10.5 and up, we need to be using a method that is
> > available on these version of Mac OS X. The deltaY
> > method is a method that does the same thing as
> > scrollingDeltaY and is available on Mac OS 10.5 and
> > up. So we replace scrollingDeltaY with deltaY.
> >
> > We only check deltaY's value if it is not zero
> > because zero means no scrolling took place.
>
> Not quite -- it means that the scrolling was fine enough
> that it doesn't add up to a big movement. If you use a
> fine-scrolling input device (I used the trackpad on my
> macbook air), then you will get a sequence of events
> like this for a very slow trackpad scroll:
>
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -3
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -3
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -3
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -3
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY 0 scrollingDeltaY -4
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY -2
> scroll: deltaY 0 scrollingDeltaY 0
>
> (interestingly it does give events with both deltas 0; maybe
> that event had a horizontal scroll component to it).
>
> Larger scroll gestures get you this sort of thing:
>
> scroll: deltaY 0 scrollingDeltaY 0
> scroll: deltaY 0 scrollingDeltaY -1
> scroll: deltaY -3 scrollingDeltaY -31
> scroll: deltaY -3 scrollingDeltaY -36
> scroll: deltaY -2 scrollingDeltaY -30
> scroll: deltaY 0 scrollingDeltaY 0
> scroll: deltaY -1 scrollingDeltaY -16
> scroll: deltaY -3 scrollingDeltaY -32
> scroll: deltaY -3 scrollingDeltaY -30
> scroll: deltaY -2 scrollingDeltaY -28
> scroll: deltaY -2 scrollingDeltaY -25
> scroll: deltaY -2 scrollingDeltaY -23
> scroll: deltaY -1 scrollingDeltaY -19
> scroll: deltaY -1 scrollingDeltaY -15
> scroll: deltaY -1 scrollingDeltaY -14
> scroll: deltaY 0 scrollingDeltaY -12
>
> where OSX has decided that there's enough movement to
> report a deltaY change, not just the fine-grained
> scrollingDeltaY.
>
> Anyway, since the QEMU input layer doesn't provide a mechanism
> for reporting fine-scrolling I guess this patch is OK.
> If we get complaints about the trackpad no longer being
> as responsive as it used to be to scrolls then we can
> look at doing something more complicated then.
>
> I'll tweak the comments and put this in for 3.0 rc1.
>
> > Signed-off-by: John Arbuckle <address@hidden>
> > ---
> > v3 changes:
> > - Added a comment explaining why we drop scrolling events in both  
> > the code and
> > the patch comment.
> >
> > v2 changes:
> > - Added a condition that drops scroll events that have a deltaY  
> > value of zero.
> >
> >  ui/cocoa.m | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > index 2991ed4f19..3bae090101 100644
> > --- a/ui/cocoa.m
> > +++ b/ui/cocoa.m
> > @@ -802,14 +802,19 @@ - (void) handleEvent:(NSEvent *)event
> >               * This is in-line with standard Mac OS X UI behaviour.
> >               */
> >
> > +            /*
> > +             * When deltaY is zero, it means the scrolling device  
> > did not move
> > +             * for this event. So we drop the event.
> > +             */
> > +            if ([event deltaY] != 0) {
> >              /* Determine if this is a scroll up or scroll down  
> > event */
> > -            buttons = ([event scrollingDeltaY] > 0) ?
> > -                INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN;
> > -            qemu_input_queue_btn(dcl->con, buttons, true);
> > -            qemu_input_event_sync();
> > -            qemu_input_queue_btn(dcl->con, buttons, false);
> > -            qemu_input_event_sync();
> > -
> > +                buttons = ([event deltaY] > 0) ?
> > +                    INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN;
> > +                qemu_input_queue_btn(dcl->con, buttons, true);
> > +                qemu_input_event_sync();
> > +                qemu_input_queue_btn(dcl->con, buttons, false);
> > +                qemu_input_event_sync();
> > +            }
> >              /*
> >               * Since deltaY also reports scroll wheel events we  
> > prevent mouse
> >               * movement code from executing.
> > --
> > 2.14.3 (Apple Git-98)
>
> thanks
> -- PMM

Excellent job investing.