[PATCH weston v2 09/11] Switch to global output repaint timer

[Daniel Stone]

Hi,

On 8 March 2017 at 14:05, Pekka Paalanen <pekka.paalanen at collabora.co.uk> wrote:
> On Wed,  1 Mar 2017 11:34:08 +0000 Daniel Stone <daniels at collabora.com> wrote:
>> +     /* XXX: This can keep postponing forever, maybe? */
>> +     if (msec_to_next < 1)
>> +             msec_to_next = 1;
>
> I think there is no longer a risk of postponing indefinitely, because
> output_repaint_timer_arm() is no longer called from
> weston_output_schedule_repaint(). The other call sites are well
> throttled.

Yeah, as detailed; that was just a placeholder until you'd fully
reviewed the new code. If you're happy to merge, then do take it out.

> I'm curious about the reason to have at least 1 ms delay, even if
> weston_output_finish_frame() deemed that we should paint ASAP. Is it to
> allow coalescing several outputs' finish_frame needing immediate
> repaint into single call through output_repaint_timer_handler()?
>
> Would be really nice to see some explanation in a comment here. Not
> that it actually matters much, since we are talking about the time
> instant to start painting, which is not that crucial to begin with.

Well, wl_event_source_timer_update() only takes milliseconds, and a
value of 0 milliseconds disables the timer entirely (cf. man
timerfd_settime). So, it's either a minimum 1ms delay, or bypassing
the timer entirely. Calling repaint directly means we lose the ability
to coalesce all the output repaints together, which was the entire
point of this series! So, our two options are to take the 1ms delay as
a given, or to have two timers: a delay timer armed for >= 1ms, or an
idle timer for < 1ms. Which would be nice, but seems like the kind of
thing that wl_event_source's timer should be doing for us, rather than
having to juggle these through the repaint loop.

> Although, I suppose if one constructed a 16-head monster, each head
> adding another 1 ms delay could ruin everything depending on how
> finish_frame calls get timed.

It'd have to be pretty special, in terms of very slightly staggered
clock offsets.

Cheers,
Daniel