On Thu, 19 Jan 2017 14:05:01 +0000 Daniel Stone <[email protected]> wrote:
> repaint_needed / repaint_scheduled are surprisingly subtle. Explode the > conditional with side-effects into more obvious separate calls, and > document what they do. > > Signed-off-by: Daniel Stone <[email protected]> > --- > libweston/compositor.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/libweston/compositor.c b/libweston/compositor.c > index d79122d..61a84a9 100644 > --- a/libweston/compositor.c > +++ b/libweston/compositor.c > @@ -2335,16 +2335,31 @@ output_repaint_timer_handler(void *data) > { > struct weston_output *output = data; > struct weston_compositor *compositor = output->compositor; > + int ret; > > - if (output->repaint_needed && > - compositor->state != WESTON_COMPOSITOR_SLEEPING && > - compositor->state != WESTON_COMPOSITOR_OFFSCREEN && > - weston_output_repaint(output) == 0) > - return 0; > + /* If we're sleeping, drop the repaint machinery entirely; we will > + * explicitly repaint all outputs when we come back. */ > + if (compositor->state == WESTON_COMPOSITOR_SLEEPING || > + compositor->state == WESTON_COMPOSITOR_OFFSCREEN) > + goto err; > > - weston_output_schedule_repaint_reset(output); > + /* We don't actually need to repaint this output; drop it from > + * repaint until something causes damage. */ > + if (!output->repaint_needed) > + goto err; Hi, FYI, if my memory serves right, this path is the exit condition from the continuous repaint loop. We need to wait a bit (the timer) to see if we are going to get something to repaint for the very next vblank. If we didn't, we cancel the repaint and drop out from the continuous repaint. If we dropped out immediately when in weston_output_finish_frame(), we would be dropping out all the time even when app were continuously updating. The cost of dropping out of continuous repaint is the the next time we have to call weston_output::start_repaint_loop() which in the worst case has to wait for a vblank, or at least do kernel roundtrip to ask the timestamp. Just to answer the "why would you ever schedule repaint without damage?"... > + > + /* If repaint fails, we aren't going to get weston_output_finish_frame > + * to trigger a new repaint, so drop it from repaint and hope > + * something later schedules a successful repaint. */ > + ret = weston_output_repaint(output); > + if (ret != 0) > + goto err; > > return 0; > + > +err: > + weston_output_schedule_repaint_reset(output); > + return 0; > } > > WL_EXPORT void Everything is spot on. Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
