Hi, On 8 March 2017 at 14:05, Pekka Paalanen <[email protected]> wrote: > On Wed, 1 Mar 2017 11:34:08 +0000 Daniel Stone <[email protected]> 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 _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
