Re: X11 scroll-wheel improvements

[Eric Wasylishen]

Hi Fred,

On 2012-01-16, at 3:29 AM, Fred Kiefer wrote:

> On 16.01.2012 09:43, Eric Wasylishen wrote:
>> Hey, I just made two commits to gnustep-back (r34553 and r34554)
>> which make a big improvement to scrolling with a mouse scrollwheel or
>> trackpad's scrolling feature.
>> 
>> The first helps avoid the GSDisplayServer event queue from filling up
>> which prevents window autodisplay, and the second implements
>> coalescing for scroll events. The coalescing seems to work very
>> nicely - it means that the rate of scroll events sent to gui depends
>> on how quickly gui can deal with them. As a result, scrolling with
>> the scroll wheel feels just about as responsive as grabbing and
>> moving the scroll bar.
>> 
>> Let me know if it also works well for you, or you have any problems.
>> :-)
> 
> I definitely like the coalescing of scroll events (although this might mess 
> up the order of events, was that intended?), but am unsure about the other 
> change.

You're right, that is a mistake and I should only be reading events from the 
head of the queue.

> If we should not listen for events in NSConnectionReplyMode, we need a 
> similar change for the Windows backend as well. But wont this lead to an 
> application that might be unable to participate in a Drag&Drop interaction? 
> Or anything else that requires a mixture of X events and DO?

I'm not sure… I don't have a great understanding of run loops. It doesn't seem 
to have broken drag & drop.

My understanding is that NSConnectionReplyMode is a run loop mode only used for 
private run loops created by NSConnection for synchronous method calls. If that 
is a correct interpretation than I think "not listening for X events in 
NSConnectionReplyMode" makes sense.

> In you checkin comment you state "In the long run I think we should get rid 
> of the AppKit event queue". What benefit do we get from only getting one 
> event at a time from the X event queue?

The problem is, the run loop doesn't run until the GSDisplayServer queue is 
empty (see -[GSDisplayServer getEventMatchingMask:…]) so you end up with bugs 
like prior to r34553, you can prevent the Ink window from autodisplaying by 
continuously rotating the scroll wheel.

In other words, if there are 3 events in the GSDisplayServer queue, the main 
loop in -[NSApplication run] will run for 3 iterations without running the 
NSRunLoop, and hence no performers fire, and window autodisplay won't happen.

This was the motivation r34553, it only adds one event at a time to the 
GSDisplayServer queue.

For visualizing the queue size you can add this:
      if ([event_queue count] > 1)
        {
          NSLog(@"--queue has >1 ( %d ) objects. last: %@", (int)[event_queue 
count], [event_queue objectAtIndex: [event_queue count] - 1]);
        }
to the start of the do/while loop in -[GSDisplayServer getEventMatchingMask:…].

> We would also loose the ability to post events into the queue from code. Or 
> would you want to generate an X event from the NSEvent and put that in the X 
> queue?


> A separate queue allows us to generate multiple events from one X event, or 
> as seen in your scrollwheel code, to compress events.
> 
> I think that these changes are wrong and that we rather should try to find 
> out how to better manage our event queue to get the benefits from it.

Well, maybe the suggestion to get rid of the GSDisplayServer queue is not a 
good idea. I think r34553 is sensible, though, and it eliminates the bug I 
mentioned in Ink.

Eric