Patch that "fixes" compositor-x11

[Bill Spitzak]

Kristian Høgsberg wrote:

> What do you mean by "produces a composite" and how do you know?  The
> x11 backend renders every 10ms when it's rendering continuously, not
> after every X event.  At least that's how it's supposed to work (and
> how it works here), if that doesn't happen for you, it would be nice
> if you could get some more details on what actually happens and why.

If weston compositor-x11 is doing some heavy work (for instance when it 
is starting up, or if you launch about 10 applications) and then move 
the mouse quickly in circles, it will move the mouse cursor (and any 
other changes) much more slowly at a smooth even speed, falling way 
behind my hand movements. It can take tens of seconds after I stop 
moving the mouse for it to track through all the events.

I placed a print statement in window.c in the run-loop function after it 
gets a block of events to print how many events were read. It never 
printed any number other than 1 or 0, confirming my suspicion of what 
was wrong.

I then moved the print statement to compositor-x11 to where it calls 
xcb_poll functions, with the intention of finding out if the bug was in 
the x11 portion, or in wayland itself. I was surprised to discover the 
print statement *fixed* the problem. Now the window.c function gets 6-15 
events at a time, and the cursor now keeps up with mouse movements, 
jumping over intermediate positions if weston gets slow.

The clickdot motion trail is now also straight line segments, since it 
apparently does not record all the motion events it gets, but instead 
draws a straight line to the current position when redraw is called.

I do not have an explanation as to why the print statement makes a 
difference. My best guess is that it produces a blocking i/o call, 
during which time the x server changes it's state in such a way that 
xcb_poll works a second time. But it is also very sensitive to the 
location of the print statment, as otherwise-identical code in window.c 
did not fix it.

>> This patch also makes x11_compositor_handle_event return non-zero,
>> which I believe was intended. But that does not fix the bug.
> 
> Is there a case where we return 0 but actually handled events in the
> loop?  I don't see the code path where that may happen, if we handle
> an event, 'event' will be non-NULL and we'll return 1.  The mainloop
> only cares whether the return values is 0 or positive, the actual
> number of events processed doesn't matter.

The previous version of x11_compositor_handle_event always returned 0. 
This is because it did this (changing the out parameter to a return value):

     while (event = getEvent()) { ... }
     return event != 0;

Since the while loop does not exit until getEvent() returns 0, p will 
always be 0 and this function always returns 0.

My patch changes it to return how many times p was set to non-zero.

It is also possible that x11_compositor_next_event is supposed to not 
write over the event parameter when it returns false, but that is not 
how it is written.

>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>> index d23553e..f329cc4 100644
>> --- a/src/compositor-x11.c
>> +++ b/src/compositor-x11.c
>> @@ -619,9 +619,11 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
>>  	uint32_t *k;
>>  	uint32_t i, set;
>>  	wl_fixed_t x, y;
>> +	int count = 0;
>>  
>>  	prev = NULL;
>>  	while (x11_compositor_next_event(c, &event, mask)) {
>> +		++count;
>>  		switch (prev ? prev->response_type & ~0x80 : 0x80) {
>>  		case XCB_KEY_RELEASE:
>>  			key_release = (xcb_key_press_event_t *) prev;
>> @@ -773,8 +775,8 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
>>  	default:
>>  		break;
>>  	}
>> -
>> -	return event != NULL;
>> +	if (count) printf("count = %d\n", count);
>> +	return count;
>>  }
>>  
>>  #define F(field) offsetof(struct x11_compositor, field)
> 
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>