From c769988cc2ce6d760fc39b72e7d46a997e45d53a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 18 Jun 2023 17:12:57 +0100 Subject: [PATCH] core: don't call schedule_render too early I mistakenly assumed that PresentCompleteNotify event signifies the end of a vblank (or the start of scanout). But actually this event can in theory in sent at any point during a vblank, with its timestamp pointing to when the end of vblank is. (that's why we often find the timestamp to be in the future). Add a delay so schedule_render is actually called at the end of vblank, so it doesn't mistakenly think the render is too slow to complete. Signed-off-by: Yuxuan Shui --- src/common.h | 2 ++ src/event.c | 3 ++- src/picom.c | 37 +++++++++++++++++++++++++++---------- src/utils.h | 22 ++++++++++++++++------ 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/common.h b/src/common.h index e183a1e6..99da45b9 100644 --- a/src/common.h +++ b/src/common.h @@ -159,6 +159,8 @@ typedef struct session { ev_timer fade_timer; /// Use an ev_timer callback for drawing ev_timer draw_timer; + /// Timer for the end of each vblanks. Used for calling schedule_render. + ev_timer vblank_timer; /// Called every time we have timeouts or new data on socket, /// so we can be sure if xcb read from X socket at anytime during event /// handling, we will not left any event unhandled in the queue diff --git a/src/event.c b/src/event.c index b1400828..e05ee764 100644 --- a/src/event.c +++ b/src/event.c @@ -710,7 +710,8 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { // XXX redraw needs to be more fine grained queue_redraw(ps); - // the events sent from SendEvent will be ignored + // We intentionally ignore events sent via SendEvent. Those events has the 8th bit + // of response_type set, meaning they will match none of the cases below. switch (ev->response_type) { case FocusIn: ev_focus_in(ps, (xcb_focus_in_event_t *)ev); break; case FocusOut: ev_focus_out(ps, (xcb_focus_out_event_t *)ev); break; diff --git a/src/picom.c b/src/picom.c index 8a58fe50..5173d58f 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1562,13 +1562,8 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); - uint64_t now_usec = (uint64_t)(now.tv_sec * 1000000 + now.tv_nsec / 1000); - uint64_t drift; - if (cne->ust > now_usec) { - drift = cne->ust - now_usec; - } else { - drift = now_usec - cne->ust; - } + auto now_us = (int64_t)(now.tv_sec * 1000000L + now.tv_nsec / 1000); + auto drift = iabs((int64_t)cne->ust - now_us); if (ps->last_msc_instant != 0) { auto frame_count = cne->msc - ps->last_msc; @@ -1584,14 +1579,28 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ // align with the monotonic clock. If not, disable frame pacing because we // can't schedule frames reliably. log_error("Temporal anomaly detected, frame pacing disabled. (Are we " - "running inside a time namespace?), %" PRIu64 " %" PRIu64, - now_usec, ps->last_msc_instant); + "running inside a time namespace?), %" PRIi64 " %" PRIu64, + now_us, ps->last_msc_instant); ps->frame_pacing = false; } ps->last_msc_instant = cne->ust; ps->last_msc = cne->msc; if (ps->redraw_needed) { - schedule_render(ps, true); + if (now_us > (int64_t)cne->ust) { + schedule_render(ps, true); + } else { + // Wait until the end of the current vblank to call + // schedule_render. If we call schedule_render too early, it can + // mistakenly think the render missed the vblank, and doesn't + // schedule render for the next vblank, causing frame drops. + log_trace("The end of this vblank is %" PRIi64 " us into the " + "future", + (int64_t)cne->ust - now_us); + assert(!ev_is_active(&ps->vblank_timer)); + ev_timer_set(&ps->vblank_timer, + ((double)cne->ust - (double)now_us) / 1000000.0, 0); + ev_timer_start(ps->loop, &ps->vblank_timer); + } } } @@ -1865,6 +1874,13 @@ static void draw_callback(EV_P_ ev_timer *w, int revents) { } } +static void schedule_render_callback(EV_P_ ev_timer *w, int revents attr_unused) { + session_t *ps = session_ptr(w, vblank_timer); + ev_timer_stop(EV_A_ w); + + schedule_render(ps, true); +} + static void x_event_callback(EV_P attr_unused, ev_io *w, int revents attr_unused) { session_t *ps = (session_t *)w; xcb_generic_event_t *ev = xcb_poll_for_event(ps->c); @@ -2475,6 +2491,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ev_io_start(ps->loop, &ps->xiow); ev_init(&ps->unredir_timer, tmout_unredir_callback); ev_init(&ps->draw_timer, draw_callback); + ev_init(&ps->vblank_timer, schedule_render_callback); ev_init(&ps->fade_timer, fade_timer_callback); diff --git a/src/utils.h b/src/utils.h index fc6dd306..446fba8b 100644 --- a/src/utils.h +++ b/src/utils.h @@ -125,14 +125,22 @@ safe_isnan(double a) { * @param max maximum value * @return normalized value */ -static inline int attr_const normalize_i_range(int i, int min, int max) { - if (i > max) +static inline int attr_const attr_unused normalize_i_range(int i, int min, int max) { + if (i > max) { return max; - if (i < min) + } + if (i < min) { return min; + } return i; } +/// Generic integer abs() +#define iabs(val) \ + ({ \ + __auto_type __tmp = (val); \ + __tmp > 0 ? __tmp : -__tmp; \ + }) #define min2(a, b) ((a) > (b) ? (b) : (a)) #define max2(a, b) ((a) > (b) ? (a) : (b)) #define min3(a, b, c) min2(a, min2(b, c)) @@ -149,10 +157,12 @@ static inline int attr_const normalize_i_range(int i, int min, int max) { * @return normalized value */ static inline double attr_const normalize_d_range(double d, double min, double max) { - if (d > max) + if (d > max) { return max; - if (d < min) + } + if (d < min) { return min; + } return d; } @@ -162,7 +172,7 @@ static inline double attr_const normalize_d_range(double d, double min, double m * @param d double value to normalize * @return normalized value */ -static inline double attr_const normalize_d(double d) { +static inline double attr_const attr_unused normalize_d(double d) { return normalize_d_range(d, 0.0, 1.0); }