From 5d94b2a05441c89a80c5fdcb956cfa6cb1dbd94c Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 18 Dec 2023 03:04:59 +0000 Subject: [PATCH 01/13] config: add a debug environment variable This is where we keep temporary, short living, private debug options. Adding and removing command line and config file options are troublesome, and we don't want people adding these to their config files. Signed-off-by: Yuxuan Shui --- src/config.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/config.h | 7 ++++++ 2 files changed, 75 insertions(+) diff --git a/src/config.c b/src/config.c index ec39aa4390..efbf279f7d 100644 --- a/src/config.c +++ b/src/config.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -615,6 +616,72 @@ char *locate_auxiliary_file(const char *scope, const char *path, const char *inc return ret; } +struct debug_options_entry { + const char *name; + const char **choices; + size_t offset; +}; + +static const struct debug_options_entry debug_options_entries[] = { + +}; + +void parse_debug_option_single(char *setting, struct debug_options *debug_options) { + char *equal = strchr(setting, '='); + size_t name_len = equal ? (size_t)(equal - setting) : strlen(setting); + for (size_t i = 0; i < ARR_SIZE(debug_options_entries); i++) { + if (strncmp(setting, debug_options_entries[i].name, name_len) != 0) { + continue; + } + if (debug_options_entries[i].name[name_len] != '\0') { + continue; + } + auto value = (int *)((void *)debug_options + debug_options_entries[i].offset); + if (equal) { + const char *const arg = equal + 1; + if (debug_options_entries[i].choices != NULL) { + for (size_t j = 0; debug_options_entries[i].choices[j]; j++) { + if (strcmp(arg, debug_options_entries[i].choices[j]) == + 0) { + *value = (int)j; + return; + } + } + } + if (!parse_int(arg, value)) { + log_error("Invalid value for debug option %s: %s, it " + "will be ignored.", + debug_options_entries[i].name, arg); + } + } else if (debug_options_entries[i].choices == NULL) { + *value = 1; + } else { + log_error( + "Missing value for debug option %s, it will be ignored.", setting); + } + return; + } + log_error("Invalid debug option: %s", setting); +} + +/// Parse debug options from environment variable `PICOM_DEBUG`. +void parse_debug_options(struct debug_options *debug_options) { + const char *debug = getenv("PICOM_DEBUG"); + const struct debug_options default_debug_options = {}; + + *debug_options = default_debug_options; + if (!debug) { + return; + } + + scoped_charp debug_copy = strdup(debug); + char *tmp, *needle = strtok_r(debug_copy, ";", &tmp); + while (needle) { + parse_debug_option_single(needle, debug_options); + needle = strtok_r(NULL, ";", &tmp); + } +} + /** * Parse a list of window shader rules. */ @@ -817,5 +884,6 @@ char *parse_config(options_t *opt, const char *config_file, bool *shadow_enable, (void)hasneg; (void)winopt_mask; #endif + parse_debug_options(&opt->debug_options); return ret; } diff --git a/src/config.h b/src/config.h index 31e6774c8f..93bede8420 100644 --- a/src/config.h +++ b/src/config.h @@ -73,6 +73,11 @@ enum blur_method { typedef struct _c2_lptr c2_lptr_t; +/// Internal, private options for debugging and development use. +struct debug_options { + +}; + /// Structure representing all options. typedef struct options { // === Debugging === @@ -262,6 +267,8 @@ typedef struct options { c2_lptr_t *transparent_clipping_blacklist; bool dithered_present; + + struct debug_options debug_options; } options_t; extern const char *const BACKEND_STRS[NUM_BKEND + 1]; From 5bcd34449cff61adfa223bfd2ab13a6c81fa2b45 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 9 Jul 2023 16:39:44 +0100 Subject: [PATCH 02/13] core: refactor frame pacing Factored out vblank event generation, add abstraction over how vblank events are generated. The goal is so we can request vblank events in different ways based on the driver we are running on. Tried to simplify the frame scheduling logic, we will see if I succeeded or not. Also, the logic to exclude vblank events for vblank interval estimation when the screen off is dropped. It's too hard to get right, we need to find something robust. Signed-off-by: Yuxuan Shui --- src/common.h | 41 ++--- src/config.h | 10 ++ src/meson.build | 3 +- src/picom.c | 454 ++++++++++++++++++++---------------------------- src/vblank.c | 246 ++++++++++++++++++++++++++ src/vblank.h | 37 ++++ src/x.c | 24 ++- src/x.h | 8 +- 8 files changed, 514 insertions(+), 309 deletions(-) create mode 100644 src/vblank.c create mode 100644 src/vblank.h diff --git a/src/common.h b/src/common.h index 889ca88328..52967e600d 100644 --- a/src/common.h +++ b/src/common.h @@ -134,17 +134,6 @@ struct shader_info { UT_hash_handle hh; }; -enum render_progress { - /// Render is finished and presented to the screen. - RENDER_IDLE = 0, - /// Rendering is queued, but not started yet. - RENDER_QUEUED, - /// Backend has been called, render commands have been issued. - RENDER_STARTED, - /// Backend reported render commands have been finished. (not actually used). - RENDER_FINISHED, -}; - /// Structure containing all necessary data for a session. typedef struct session { // === Event handlers === @@ -156,8 +145,6 @@ 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 @@ -225,12 +212,6 @@ typedef struct session { bool first_frame; /// Whether screen has been turned off bool screen_is_off; - /// We asked X server to send us a event for the end of a vblank, and we haven't - /// received one yet. - bool vblank_event_requested; - /// Event context for X Present extension. - uint32_t present_event_id; - xcb_special_event_t *present_event; /// When last MSC event happened, in useconds. uint64_t last_msc_instant; /// The last MSC number @@ -242,6 +223,8 @@ typedef struct session { uint64_t next_render; /// Whether we can perform frame pacing. bool frame_pacing; + /// Vblank event scheduler + struct vblank_scheduler *vblank_scheduler; /// Render statistics struct render_statistics render_stats; @@ -253,14 +236,18 @@ typedef struct session { options_t o; /// Whether we have hit unredirection timeout. bool tmout_unredir_hit; - /// Rendering is currently in progress. This means we are in any stage of - /// rendering a frame. The render could be queued but not yet started, or it could - /// have finished but not yet presented. - enum render_progress render_in_progress; - /// Whether there are changes pending for the next render. A render is currently - /// in progress, otherwise we would have started a new render instead of setting - /// this flag. - bool redraw_needed; + /// If the backend is busy. This means two things: + /// Either the backend is currently rendering a frame, or a frame has been + /// rendered but has yet to be presented. In either case, we should not start + /// another render right now. As if we start issuing rendering commands now, we + /// will have to wait for either the the current render to finish, or the current + /// back buffer to be become available again. In either case, we will be wasting + /// time. + bool backend_busy; + /// Whether a render is queued. This generally means there are pending updates + /// to the screen that's neither included in the current render, nor on the + /// screen. + bool render_queued; /// Cache a xfixes region so we don't need to allocate it every time. /// A workaround for yshui/picom#301 diff --git a/src/config.h b/src/config.h index 93bede8420..bdf384aca7 100644 --- a/src/config.h +++ b/src/config.h @@ -73,6 +73,16 @@ enum blur_method { typedef struct _c2_lptr c2_lptr_t; +enum vblank_scheduler_type { + /// X Present extension based vblank events + VBLANK_SCHEDULER_PRESENT, + /// GLX_SGI_video_sync based vblank events + VBLANK_SCHEDULER_SGI_VIDEO_SYNC, + /// An invalid scheduler, served as a scheduler count, and + /// as a sentinel value. + LAST_VBLANK_SCHEDULER, +}; + /// Internal, private options for debugging and development use. struct debug_options { diff --git a/src/meson.build b/src/meson.build index a608c57538..51f96488cd 100644 --- a/src/meson.build +++ b/src/meson.build @@ -9,7 +9,8 @@ base_deps = [ srcs = [ files('picom.c', 'win.c', 'c2.c', 'x.c', 'config.c', 'vsync.c', 'utils.c', 'diagnostic.c', 'string_utils.c', 'render.c', 'kernel.c', 'log.c', - 'options.c', 'event.c', 'cache.c', 'atom.c', 'file_watch.c', 'statistics.c') ] + 'options.c', 'event.c', 'cache.c', 'atom.c', 'file_watch.c', 'statistics.c', + 'vblank.c') ] picom_inc = include_directories('.') cflags = [] diff --git a/src/picom.c b/src/picom.c index 2cfa08ecd7..57ee4f0a47 100644 --- a/src/picom.c +++ b/src/picom.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,7 @@ #include "options.h" #include "statistics.h" #include "uthash_extra.h" +#include "vblank.h" /// Get session_t pointer from a pointer to a member of session_t #define session_ptr(ptr, member) \ @@ -142,43 +144,150 @@ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t return w; } +void collect_vblank_interval_statistics(struct vblank_event *e, void *ud) { + auto ps = (session_t *)ud; + assert(ps->frame_pacing); + assert(ps->vblank_scheduler); + + if (ps->last_msc == e->msc) { + // Already collected statistics for this vblank + return; + } + + // TODO(yshui): this naive method of estimating vblank interval does not handle + // the variable refresh rate case very well. This includes the case + // of a VRR enabled monitor; or a monitor that's turned off, in which + // case the vblank events might slow down or stop all together. + // I tried using DPMS to detect monitor power state, and stop adding + // samples when the monitor is off, but I had a hard time to get it + // working reliably, there are just too many corner cases. + + if (ps->last_msc_instant != 0) { + auto frame_count = e->msc - ps->last_msc; + int frame_time = (int)((e->ust - ps->last_msc_instant) / frame_count); + if (frame_count == 1) { + render_statistics_add_vblank_time_sample(&ps->render_stats, frame_time); + log_trace("Frame count %lu, frame time: %d us, ust: %" PRIu64 "", + frame_count, frame_time, e->ust); + } else { + log_trace("Frame count %lu, frame time: %d us, msc: %" PRIu64 + ", not adding sample.", + frame_count, frame_time, e->ust); + } + } + ps->last_msc_instant = e->ust; + ps->last_msc = e->msc; + double vblank_interval = render_statistics_get_vblank_time(&ps->render_stats); + log_trace("Vblank interval estimate: %f us", vblank_interval); + if (vblank_interval == 0) { + // We don't have enough data for vblank interval estimate, schedule + // another vblank event. + vblank_scheduler_schedule(ps->vblank_scheduler, + collect_vblank_interval_statistics, ud); + } +} +/// vblank callback scheduled by schedule_render. +/// +/// Check if previously queued render has finished, and record the time it took. +void schedule_render_at_vblank(struct vblank_event *e, void *ud) { + auto ps = (session_t *)ud; + assert(ps->frame_pacing); + assert(ps->backend_busy); + assert(ps->render_queued); + assert(ps->vblank_scheduler); + + collect_vblank_interval_statistics(e, ud); + + struct timespec render_time; + bool completed = + ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); + if (!completed) { + // Render hasn't completed yet, we can't start another render. + // Check again at the next vblank. + log_debug("Last render did not complete during vblank, msc: " + "%" PRIu64, + ps->last_msc); + vblank_scheduler_schedule(ps->vblank_scheduler, schedule_render_at_vblank, ud); + return; + } + + // The frame has been finished and presented, record its render time. + int render_time_us = + (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); + render_statistics_add_render_time_sample( + &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); + log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " + "last_msc: %" PRIu64, + render_time_us, (int)ps->last_schedule_delay, ps->last_msc); + ps->last_schedule_delay = 0; + ps->backend_busy = false; + + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + auto now_us = (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_nsec / 1000; + double delay_s = 0; + if (ps->next_render > now_us) { + delay_s = (double)(ps->next_render - now_us) / 1000000.0; + } + log_verbose("Prepare to start rendering: delay: %f s, next_render: %" PRIu64 + ", now_us: %" PRIu64, + delay_s, ps->next_render, now_us); + assert(!ev_is_active(&ps->draw_timer)); + ev_timer_set(&ps->draw_timer, delay_s, 0); + ev_timer_start(ps->loop, &ps->draw_timer); +} + /// How many seconds into the future should we start rendering the next frame. /// /// Renders are scheduled like this: /// -/// 1. queue_redraw() queues a new render by calling schedule_render, if there is no -/// render currently scheduled. i.e. render_in_progress == RENDER_IDLE. -/// 2. then, we need to figure out the best time to start rendering. first, we need to -/// know when the current vblank will end. we have this information from the Present -/// extension: we know when was the end of last vblank, and we know the refresh rate. -/// so we can calculate the end of the current vblank. if our render time estimation -/// shows we could miss that target, we push the target back an integer number of -/// frames. and we calculate the end of the target vblank similarly. -/// 3. We schedule a render for that target. we use past statistics about how long our -/// renders took to figure out when to start rendering. we start rendering as late as -/// possible, but not too late that we miss the target vblank. render_in_progress is -/// set to RENDER_QUEUED. -/// 4. draw_callback() is called at the schedule time. Backend APIs are called to issue -/// render commands. render_in_progress is set to RENDER_STARTED. -/// 5. PresentCompleteNotify is received, which gives us the actual time when the current -/// vblank will end/ended. We schedule a call to handle_end_of_vblank at the -/// appropriate time. -/// 6. in handle_end_of_vblank, we check the backend to see if the render has finished. if -/// not, render_in_progress is unchanged; otherwise, render_in_progress is set to -/// RENDER_IDLE, and the next frame can be scheduled. +/// 1. queue_redraw() queues a new render by calling schedule_render, if there +/// is no render currently scheduled. i.e. render_queued == false. +/// 2. then, we need to figure out the best time to start rendering. we need to +/// at least know when the current vblank will start, as we can't start render +/// before the current rendered frame is diplayed on screen. we have this +/// information from the vblank scheduler, it will notify us when that happens. +/// we might also want to delay the rendering even further to reduce latency, +/// this is discussed below, in FUTURE WORKS. +/// 3. we schedule a render for that target point in time. +/// 4. draw_callback() is called at the schedule time (i.e. when scheduled +/// vblank event is delivered). Backend APIs are called to issue render +/// commands. render_queued is set to false, and backend_busy is set to true. /// -/// This is what happens when frame_pacing is true. Otherwise render_in_progress is -/// either QUEUED or IDLE, and queue_redraw will always schedule a render to be started -/// immediately. PresentCompleteNotify will not be received, and handle_end_of_vblank will -/// not be called. +/// There is a small caveat in step 2. As a vblank event being delivered +/// doesn't necessarily mean the frame has been displayed on screen. If a frame +/// takes too long to render, it might miss the current vblank, and will be +/// displayed on screen during one of the subsequent vblanks. So in +/// schedule_render_at_vblank, we ask the backend to see if it has finished +/// rendering. if not, render_queued is unchanged, and another vblank is +/// scheduled; otherwise, draw_callback_impl will be scheduled to be call at +/// an appropriate time. /// -/// The `triggered_by_timer` parameter is used to indicate whether this function is -/// triggered by a steady timer, i.e. we are rendering for each vblank. The other case is -/// when we stop rendering for a while because there is no changes on screen, then -/// something changed and schedule_render is triggered by a DamageNotify. The idea is that -/// when the schedule is triggered by a steady timer, schedule_render will be called at a -/// predictable offset into each vblank. - +/// All of the above is what happens when frame_pacing is true. Otherwise +/// render_in_progress is either QUEUED or IDLE, and queue_redraw will always +/// schedule a render to be started immediately. PresentCompleteNotify will not +/// be received, and handle_end_of_vblank will not be called. +/// +/// The `triggered_by_timer` parameter is used to indicate whether this function +/// is triggered by a steady timer, i.e. we are rendering for each vblank. The +/// other case is when we stop rendering for a while because there is no changes +/// on screen, then something changed and schedule_render is triggered by a +/// DamageNotify. The idea is that when the schedule is triggered by a steady +/// timer, schedule_render will be called at a predictable offset into each +/// vblank. +/// +/// # FUTURE WORKS +/// +/// As discussed in step 2 above, we might want to delay the rendering even +/// further. If we know the time it takes to render a frame, and the interval +/// between vblanks, we can try to schedule the render to start at a point in +/// time that's closer to the next vblank. We should be able to get this +/// information by doing statistics on the render time of previous frames, which +/// is available from the backends; and the interval between vblank events, +/// which is available from the vblank scheduler. +/// +/// The code that does this is already implemented below, but disabled by +/// default. There are several problems with it, see bug #1072. void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { // By default, we want to schedule render immediately, later in this function we // might adjust that and move the render later, based on render timing statistics. @@ -228,97 +337,37 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { delay_s, render_budget, frame_time, now_us, deadline); } - log_trace("Delay: %.6lf s, last_msc: %" PRIu64 ", render_budget: %d, frame_time: " + log_verbose("Delay: %.6lf s, last_msc: %" PRIu64 ", render_budget: %d, frame_time: " "%" PRIu32 ", now_us: %" PRIu64 ", next_msc: %" PRIu64 ", " "divisor: %d", delay_s, ps->last_msc_instant, render_budget, frame_time, now_us, deadline, divisor); schedule: - ps->render_in_progress = RENDER_QUEUED; - ps->redraw_needed = false; - - x_request_vblank_event(ps, ps->last_msc + 1); - - assert(!ev_is_active(&ps->draw_timer)); - ev_timer_set(&ps->draw_timer, delay_s, 0); - ev_timer_start(ps->loop, &ps->draw_timer); -} - -/// Called after a vblank has ended -/// -/// Check if previously queued render has finished, and record the time it took. -void handle_end_of_vblank(session_t *ps) { - if (ps->render_in_progress == RENDER_IDLE) { - // We didn't start rendering for this vblank, no render time to record. - // But if we don't have a vblank estimate, we will ask for one more vblank - // event, so we can collect more data and get an estimate sooner. - if (render_statistics_get_vblank_time(&ps->render_stats) == 0) { - x_request_vblank_event(ps, ps->last_msc + 1); - } - return; - } - - // render_in_progress is either RENDER_STARTED or RENDER_QUEUED - struct timespec render_time; - bool completed; - if (ps->render_in_progress == RENDER_STARTED) { - completed = - ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); + ps->render_queued = true; + // If the backend is not busy, we just need to schedule the render at the + // specified time; otherwise we need to wait for vblank events. + if (!ps->backend_busy) { + assert(!ev_is_active(&ps->draw_timer)); + ev_timer_set(&ps->draw_timer, delay_s, 0); + ev_timer_start(ps->loop, &ps->draw_timer); } else { - completed = false; - } - - // Do we want to be notified when the next vblank comes? First, if frame_pacing is - // disabled, we don't need vblank events; or if the screen is off, we cannot - // request vblank events. Otherwise, we need vblank events in these cases: - // 1) if we know we need to redraw for the next vblank. - // 2) previous render hasn't completed yet, so it will be presented during the - // next vblank. we need to ask for an event for that. - // 3) if we don't have enough data for a vblank interval estimate, see above. - bool need_vblank_events = - ps->frame_pacing && (ps->redraw_needed || !completed || - render_statistics_get_vblank_time(&ps->render_stats) == 0); - - if (need_vblank_events) { - x_request_vblank_event(ps, ps->last_msc + 1); - } - - if (!completed) { - // Render hasn't completed yet, keep render_in_progress unchanged. - log_debug("Last render did not complete during vblank, msc: %" PRIu64, - ps->last_msc); - return; - } - - int render_time_us = - (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); - // The frame has been finished and presented, record its render time. - log_trace("Last render call took: %d (gpu) + %d (cpu) us, " - "last_msc: %" PRIu64, - render_time_us, (int)ps->last_schedule_delay, ps->last_msc); - render_statistics_add_render_time_sample( - &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); - ps->last_schedule_delay = 0; - ps->render_in_progress = RENDER_IDLE; - - if (ps->redraw_needed) { - schedule_render(ps, true); + // We should never set backend_busy to true unless frame_pacing is + // enabled. + assert(ps->vblank_scheduler); + if (!vblank_scheduler_schedule(ps->vblank_scheduler, + schedule_render_at_vblank, ps)) { + // TODO(yshui): handle error here + abort(); + } } } void queue_redraw(session_t *ps) { - // Whether we have already rendered for the current frame. - // If frame pacing is not enabled, pretend this is false. - // If --benchmark is used, redraw is always queued - if (ps->render_in_progress == RENDER_IDLE && !ps->o.benchmark) { - schedule_render(ps, false); - } else if (ps->render_in_progress > RENDER_QUEUED) { - // render_in_progress > RENDER_QUEUED means we have already issued the - // render commands, so a new render must be scheduled to reflect new - // changes. Otherwise the queued render will include1 the new changes. - ps->redraw_needed = true; + if (ps->render_queued) { + return; } + schedule_render(ps, false); } /** @@ -1395,23 +1444,19 @@ static bool redirect_start(session_t *ps) { } if (ps->present_exists && ps->frame_pacing) { - ps->present_event_id = x_new_id(&ps->c); - auto select_input = xcb_present_select_input( - ps->c.c, ps->present_event_id, session_get_target_window(ps), - XCB_PRESENT_EVENT_MASK_COMPLETE_NOTIFY); - auto notify_msc = xcb_present_notify_msc( - ps->c.c, session_get_target_window(ps), 0, 0, 1, 0); - set_cant_fail_cookie(&ps->c, select_input); - set_cant_fail_cookie(&ps->c, notify_msc); - ps->present_event = xcb_register_for_special_xge( - ps->c.c, &xcb_present_id, ps->present_event_id, NULL); - // Initialize rendering and frame timing statistics, and frame pacing // states. ps->last_msc_instant = 0; ps->last_msc = 0; ps->last_schedule_delay = 0; render_statistics_reset(&ps->render_stats); + ps->vblank_scheduler = + vblank_scheduler_new(ps->loop, &ps->c, session_get_target_window(ps)); + if (!ps->vblank_scheduler) { + return false; + } + vblank_scheduler_schedule(ps->vblank_scheduler, + collect_vblank_interval_statistics, ps); } else if (ps->frame_pacing) { log_error("Present extension is not supported, frame pacing disabled."); ps->frame_pacing = false; @@ -1459,12 +1504,9 @@ static void unredirect(session_t *ps) { free(ps->damage_ring); ps->damage_ring = ps->damage = NULL; - if (ps->present_event_id) { - xcb_present_select_input(ps->c.c, ps->present_event_id, - session_get_target_window(ps), 0); - ps->present_event_id = XCB_NONE; - xcb_unregister_for_special_event(ps->c.c, ps->present_event); - ps->present_event = NULL; + if (ps->vblank_scheduler) { + vblank_scheduler_free(ps->vblank_scheduler); + ps->vblank_scheduler = NULL; } // Must call XSync() here @@ -1474,122 +1516,12 @@ static void unredirect(session_t *ps) { log_debug("Screen unredirected."); } -/// Handle PresentCompleteNotify events -/// -/// Record the MSC value and their timestamps, and schedule handle_end_of_vblank() at the -/// correct time. -static void -handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_t *cne) { - if (cne->kind != XCB_PRESENT_COMPLETE_KIND_NOTIFY_MSC) { - return; - } - - assert(ps->frame_pacing); - assert(ps->vblank_event_requested); - ps->vblank_event_requested = false; - - // X sometimes sends duplicate/bogus MSC events, when screen has just been turned - // off. Don't use the msc value in these events. We treat this as not receiving a - // vblank event at all, and try to get a new one. - // - // See: - // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 - bool event_is_invalid = cne->msc <= ps->last_msc || cne->ust == 0; - if (event_is_invalid) { - log_debug("Invalid PresentCompleteNotify event, %" PRIu64 " %" PRIu64, - cne->msc, cne->ust); - x_request_vblank_event(ps, ps->last_msc + 1); - return; - } - - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - 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 && drift > 1000000) { - // This is the first MSC event we receive, let's check if the timestamps - // 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?), %" PRIi64 " %" PRIu64, - now_us, ps->last_msc_instant); - if (ps->render_in_progress == RENDER_STARTED) { - // When frame_pacing is off, render_in_progress can't be - // RENDER_STARTED. See the comment on schedule_render(). - ps->render_in_progress = RENDER_IDLE; - } - ps->frame_pacing = false; - return; - } - - x_check_dpms_status(ps); - - if (ps->last_msc_instant != 0) { - auto frame_count = cne->msc - ps->last_msc; - int frame_time = (int)((cne->ust - ps->last_msc_instant) / frame_count); - if (frame_count == 1 && !ps->screen_is_off) { - render_statistics_add_vblank_time_sample(&ps->render_stats, frame_time); - log_trace("Frame count %lu, frame time: %d us, rolling average: " - "%u us, " - "msc: %" PRIu64 ", offset: %d us", - frame_count, frame_time, - render_statistics_get_vblank_time(&ps->render_stats), - cne->ust, (int)drift); - } else { - log_trace("Frame count %lu, frame time: %d us, msc: %" PRIu64 - ", offset: %d us, not adding sample.", - frame_count, frame_time, cne->ust, (int)drift); - } - } - ps->last_msc_instant = cne->ust; - ps->last_msc = cne->msc; - // Note we can't update ps->render_in_progress here because of this present - // complete notify, as we don't know if the render finished before the end of - // vblank or not. We schedule a call to handle_end_of_vblank() to figure out if we - // are still rendering, and update ps->render_in_progress accordingly. - if (now_us > (int64_t)cne->ust) { - handle_end_of_vblank(ps); - } else { - // Wait until the end of the current vblank to call - // handle_end_of_vblank. If we call it 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); - } -} - -static void handle_present_events(session_t *ps) { - if (!ps->present_event) { - // Screen not redirected - return; - } - xcb_present_generic_event_t *ev; - while ((ev = (void *)xcb_poll_for_special_event(ps->c.c, ps->present_event))) { - if (ev->event != ps->present_event_id) { - // This event doesn't have the right event context, it's not meant - // for us. - goto next; - } - - // We only subscribed to the complete notify event. - assert(ev->evtype == XCB_PRESENT_EVENT_COMPLETE_NOTIFY); - handle_present_complete_notify(ps, (void *)ev); - next: - free(ev); - } -} - // Handle queued events before we go to sleep static void handle_queued_x_events(EV_P attr_unused, ev_prepare *w, int revents attr_unused) { session_t *ps = session_ptr(w, event_check); - handle_present_events(ps); + if (ps->vblank_scheduler) { + vblank_handle_x_events(ps->vblank_scheduler); + } xcb_generic_event_t *ev; while ((ev = xcb_poll_for_queued_event(ps->c.c))) { @@ -1711,6 +1643,9 @@ static void handle_pending_updates(EV_P_ struct session *ps) { } static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { + assert(!ps->backend_busy); + assert(ps->render_queued); + struct timespec now; int64_t draw_callback_enter_us; clock_gettime(CLOCK_MONOTONIC, &now); @@ -1801,13 +1736,13 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { if (ps->redirected && ps->o.stoppaint_force != ON) { static int paint = 0; - log_trace("Render start, frame %d", paint); + log_verbose("Render start, frame %d", paint); if (!ps->o.legacy_backends) { did_render = paint_all_new(ps, bottom); } else { paint_all(ps, bottom); } - log_trace("Render end"); + log_verbose("Render end"); ps->first_frame = false; paint++; @@ -1816,30 +1751,30 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { } } - if (ps->frame_pacing && did_render) { - ps->render_in_progress = RENDER_STARTED; - } else { - // With frame pacing, we set render_in_progress to RENDER_IDLE after the - // end of vblank. Without frame pacing, we won't be receiving vblank - // events, so we set render_in_progress to RENDER_IDLE here, right after - // we issue the render commands. - // The other case is if we decided there is no change to render, in that - // case no render command is issued, so we also set render_in_progress to - // RENDER_IDLE. - ps->render_in_progress = RENDER_IDLE; - } + // With frame pacing, we set backend_busy to true after the end of + // vblank. Without frame pacing, we won't be receiving vblank events, so + // we set backend_busy to false here, right after we issue the render + // commands. + // The other case is if we decided there is no change to render, in that + // case no render command is issued, so we also set backend_busy to + // false. + ps->backend_busy = (ps->frame_pacing && did_render); ps->next_render = 0; if (!fade_running) { ps->fade_time = 0L; } + ps->render_queued = false; + // TODO(yshui) Investigate how big the X critical section needs to be. There are // suggestions that rendering should be in the critical section as well. // Queue redraw if animation is running. This should be picked up by next present // event. - ps->redraw_needed = animation; + if (animation) { + queue_redraw(ps); + } } static void draw_callback(EV_P_ ev_timer *w, int revents) { @@ -1855,13 +1790,6 @@ static void draw_callback(EV_P_ ev_timer *w, int revents) { } } -static void vblank_callback(EV_P_ ev_timer *w, int revents attr_unused) { - session_t *ps = session_ptr(w, vblank_timer); - ev_timer_stop(EV_A_ w); - - handle_end_of_vblank(ps); -} - 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.c); @@ -2013,7 +1941,6 @@ static session_t *session_init(int argc, char **argv, Display *dpy, .randr_exists = 0, .randr_event = 0, .randr_error = 0, - .present_event_id = XCB_NONE, .glx_exists = false, .glx_event = 0, .glx_error = 0, @@ -2450,7 +2377,6 @@ 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, vblank_callback); ev_init(&ps->fade_timer, fade_timer_callback); diff --git a/src/vblank.c b/src/vblank.c new file mode 100644 index 0000000000..219e2cb1fb --- /dev/null +++ b/src/vblank.c @@ -0,0 +1,246 @@ +#include + +#include +#include +#include +#include +#include + +#include "compiler.h" +#include "config.h" +#include "list.h" // for container_of +#include "log.h" +#include "vblank.h" +#include "x.h" + +struct vblank_callback { + vblank_callback_t fn; + void *user_data; +}; + +struct vblank_scheduler { + struct x_connection *c; + size_t callback_capacity, callback_count; + struct vblank_callback *callbacks; + struct ev_loop *loop; + xcb_window_t target_window; + enum vblank_scheduler_type type; + bool vblank_event_requested; +}; + +struct present_vblank_scheduler { + struct vblank_scheduler base; + + uint64_t last_msc; + /// The timestamp for the end of last vblank. + uint64_t last_ust; + ev_timer callback_timer; + xcb_present_event_t event_id; + xcb_special_event_t *event; +}; + +struct vblank_scheduler_ops { + void (*init)(struct vblank_scheduler *self); + void (*deinit)(struct vblank_scheduler *self); + void (*schedule)(struct vblank_scheduler *self); + bool (*handle_x_events)(struct vblank_scheduler *self); +}; + +static void +vblank_scheduler_invoke_callbacks(struct vblank_scheduler *self, struct vblank_event *event); + +static void present_vblank_scheduler_schedule(struct vblank_scheduler *base) { + auto self = (struct present_vblank_scheduler *)base; + log_verbose("Requesting vblank event for window 0x%08x, msc %" PRIu64, + base->target_window, self->last_msc + 1); + assert(!base->vblank_event_requested); + x_request_vblank_event(base->c, base->target_window, self->last_msc + 1); + base->vblank_event_requested = true; +} + +static void present_vblank_callback(EV_P attr_unused, ev_timer *w, int attr_unused revents) { + auto sched = container_of(w, struct present_vblank_scheduler, callback_timer); + auto event = (struct vblank_event){ + .msc = sched->last_msc, + .ust = sched->last_ust, + }; + sched->base.vblank_event_requested = false; + vblank_scheduler_invoke_callbacks(&sched->base, &event); +} + +static void present_vblank_scheduler_init(struct vblank_scheduler *base) { + auto self = (struct present_vblank_scheduler *)base; + base->type = VBLANK_SCHEDULER_PRESENT; + ev_timer_init(&self->callback_timer, present_vblank_callback, 0, 0); + + self->event_id = x_new_id(base->c); + auto select_input = + xcb_present_select_input(base->c->c, self->event_id, base->target_window, + XCB_PRESENT_EVENT_MASK_COMPLETE_NOTIFY); + set_cant_fail_cookie(base->c, select_input); + self->event = + xcb_register_for_special_xge(base->c->c, &xcb_present_id, self->event_id, NULL); +} + +static void present_vblank_scheduler_deinit(struct vblank_scheduler *base) { + auto self = (struct present_vblank_scheduler *)base; + ev_timer_stop(base->loop, &self->callback_timer); + auto select_input = + xcb_present_select_input(base->c->c, self->event_id, base->target_window, 0); + set_cant_fail_cookie(base->c, select_input); + xcb_unregister_for_special_event(base->c->c, self->event); +} + +/// Handle PresentCompleteNotify events +/// +/// Schedule the registered callback to be called when the current vblank ends. +static void handle_present_complete_notify(struct present_vblank_scheduler *self, + xcb_present_complete_notify_event_t *cne) { + assert(self->base.type == VBLANK_SCHEDULER_PRESENT); + + if (cne->kind != XCB_PRESENT_COMPLETE_KIND_NOTIFY_MSC) { + return; + } + + assert(self->base.vblank_event_requested); + + // X sometimes sends duplicate/bogus MSC events, when screen has just been turned + // off. Don't use the msc value in these events. We treat this as not receiving a + // vblank event at all, and try to get a new one. + // + // See: + // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 + bool event_is_invalid = cne->msc <= self->last_msc || cne->ust == 0; + if (event_is_invalid) { + log_debug("Invalid PresentCompleteNotify event, %" PRIu64 " %" PRIu64, + cne->msc, cne->ust); + x_request_vblank_event(self->base.c, cne->window, self->last_msc + 1); + return; + } + + self->last_ust = cne->ust; + self->last_msc = cne->msc; + + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + auto now_us = (unsigned long)(now.tv_sec * 1000000L + now.tv_nsec / 1000); + double delay_sec = 0.0; + if (now_us < cne->ust) { + log_trace("The end of this vblank is %lu us into the " + "future", + cne->ust - now_us); + delay_sec = (double)(cne->ust - now_us) / 1000000.0; + } + // Wait until the end of the current vblank to invoke callbacks. If we + // call it too early, it can mistakenly think the render missed the + // vblank, and doesn't schedule render for the next vblank, causing frame + // drops. + assert(!ev_is_active(&self->callback_timer)); + ev_timer_set(&self->callback_timer, delay_sec, 0); + ev_timer_start(self->base.loop, &self->callback_timer); +} + +static bool handle_present_events(struct vblank_scheduler *base) { + auto self = (struct present_vblank_scheduler *)base; + xcb_present_generic_event_t *ev; + while ((ev = (void *)xcb_poll_for_special_event(base->c->c, self->event))) { + if (ev->event != self->event_id) { + // This event doesn't have the right event context, it's not meant + // for us. + goto next; + } + + // We only subscribed to the complete notify event. + assert(ev->evtype == XCB_PRESENT_EVENT_COMPLETE_NOTIFY); + handle_present_complete_notify(self, (void *)ev); + next: + free(ev); + } + return true; +} + +static const struct vblank_scheduler_ops vblank_scheduler_ops[LAST_VBLANK_SCHEDULER] = { + [VBLANK_SCHEDULER_PRESENT] = + { + .init = present_vblank_scheduler_init, + .deinit = present_vblank_scheduler_deinit, + .schedule = present_vblank_scheduler_schedule, + .handle_x_events = handle_present_events, + }, +}; + +static void vblank_scheduler_schedule_internal(struct vblank_scheduler *self) { + assert(self->type < LAST_VBLANK_SCHEDULER); + auto fn = vblank_scheduler_ops[self->type].schedule; + assert(fn != NULL); + fn(self); +} + +bool vblank_scheduler_schedule(struct vblank_scheduler *self, + vblank_callback_t vblank_callback, void *user_data) { + if (self->callback_count == 0) { + vblank_scheduler_schedule_internal(self); + } + if (self->callback_count == self->callback_capacity) { + size_t new_capacity = + self->callback_capacity ? self->callback_capacity * 2 : 1; + void *new_buffer = + realloc(self->callbacks, new_capacity * sizeof(*self->callbacks)); + if (!new_buffer) { + return false; + } + self->callbacks = new_buffer; + self->callback_capacity = new_capacity; + } + self->callbacks[self->callback_count++] = (struct vblank_callback){ + .fn = vblank_callback, + .user_data = user_data, + }; + return true; +} + +static void +vblank_scheduler_invoke_callbacks(struct vblank_scheduler *self, struct vblank_event *event) { + // callbacks might be added during callback invocation, so we need to + // copy the callback_count. + size_t count = self->callback_count; + for (size_t i = 0; i < count; i++) { + self->callbacks[i].fn(event, self->callbacks[i].user_data); + } + // remove the callbacks that we have called, keep the newly added ones. + memmove(self->callbacks, self->callbacks + count, + (self->callback_count - count) * sizeof(*self->callbacks)); + self->callback_count -= count; + if (self->callback_count) { + vblank_scheduler_schedule_internal(self); + } +} + +void vblank_scheduler_free(struct vblank_scheduler *self) { + assert(self->type < LAST_VBLANK_SCHEDULER); + auto fn = vblank_scheduler_ops[self->type].deinit; + if (fn != NULL) { + fn(self); + } + free(self->callbacks); + free(self); +} + +struct vblank_scheduler *vblank_scheduler_new(struct ev_loop *loop, struct x_connection *c, + xcb_window_t target_window) { + struct vblank_scheduler *self = calloc(1, sizeof(struct present_vblank_scheduler)); + self->target_window = target_window; + self->c = c; + self->loop = loop; + vblank_scheduler_ops[VBLANK_SCHEDULER_PRESENT].init(self); + return self; +} + +bool vblank_handle_x_events(struct vblank_scheduler *self) { + assert(self->type < LAST_VBLANK_SCHEDULER); + auto fn = vblank_scheduler_ops[self->type].handle_x_events; + if (fn != NULL) { + return fn(self); + } + return true; +} \ No newline at end of file diff --git a/src/vblank.h b/src/vblank.h new file mode 100644 index 0000000000..3f5c7d1af6 --- /dev/null +++ b/src/vblank.h @@ -0,0 +1,37 @@ +#pragma once + +#include +#include +#include +#include + +#include +#include + +#include "x.h" + +/// An object that schedule vblank events. +struct vblank_scheduler; + +struct vblank_event { + uint64_t msc; + uint64_t ust; +}; + +typedef void (*vblank_callback_t)(struct vblank_event *event, void *user_data); + +/// Schedule a vblank event. +/// +/// Schedule for `cb` to be called when the current vblank ends. If this is called +/// from a callback function for the current vblank, the newly scheduled callback +/// will be called in the next vblank. +/// +/// Returns whether the scheduling is successful. Scheduling can fail if there +/// is not enough memory. +bool vblank_scheduler_schedule(struct vblank_scheduler *self, vblank_callback_t cb, + void *user_data); +struct vblank_scheduler * +vblank_scheduler_new(struct ev_loop *loop, struct x_connection *c, xcb_window_t target_window); +void vblank_scheduler_free(struct vblank_scheduler *); + +bool vblank_handle_x_events(struct vblank_scheduler *self); diff --git a/src/x.c b/src/x.c index 89786e18e3..45abe31197 100644 --- a/src/x.c +++ b/src/x.c @@ -779,15 +779,10 @@ bool x_fence_sync(struct x_connection *c, xcb_sync_fence_t f) { return false; } -void x_request_vblank_event(session_t *ps, uint64_t msc) { - if (ps->vblank_event_requested) { - return; - } - +void x_request_vblank_event(struct x_connection *c, xcb_window_t window, uint64_t msc) { auto cookie = - xcb_present_notify_msc(ps->c.c, session_get_target_window(ps), 0, msc, 0, 0); - set_cant_fail_cookie(&ps->c, cookie); - ps->vblank_event_requested = true; + xcb_present_notify_msc(c->c, window, 0, msc, 0, 0); + set_cant_fail_cookie(c, cookie); } static inline bool dpms_screen_is_off(xcb_dpms_info_reply_t *info) { @@ -795,18 +790,19 @@ static inline bool dpms_screen_is_off(xcb_dpms_info_reply_t *info) { return info->state && (info->power_level != XCB_DPMS_DPMS_MODE_ON); } -void x_check_dpms_status(session_t *ps) { - auto r = xcb_dpms_info_reply(ps->c.c, xcb_dpms_info(ps->c.c), NULL); +bool x_check_dpms_status(struct x_connection *c, bool *screen_is_off) { + auto r = xcb_dpms_info_reply(c->c, xcb_dpms_info(c->c), NULL); if (!r) { - log_fatal("Failed to query DPMS status."); - abort(); + log_error("Failed to query DPMS status."); + return false; } auto now_screen_is_off = dpms_screen_is_off(r); - if (ps->screen_is_off != now_screen_is_off) { + if (*screen_is_off != now_screen_is_off) { log_debug("Screen is now %s", now_screen_is_off ? "off" : "on"); - ps->screen_is_off = now_screen_is_off; + *screen_is_off = now_screen_is_off; } free(r); + return true; } /** diff --git a/src/x.h b/src/x.h index 60bcfef492..df45b5cca7 100644 --- a/src/x.h +++ b/src/x.h @@ -421,7 +421,9 @@ void x_free_monitor_info(struct x_monitors *); uint32_t attr_deprecated xcb_generate_id(xcb_connection_t *c); /// Ask X server to send us a notification for the next end of vblank. -void x_request_vblank_event(session_t *ps, uint64_t msc); +void x_request_vblank_event(struct x_connection *c, xcb_window_t window, uint64_t msc); -/// Update ps->screen_is_off to reflect the current DPMS state. -void x_check_dpms_status(session_t *ps); +/// Update screen_is_off to reflect the current DPMS state. +/// +/// Returns true if the DPMS state was successfully queried, false otherwise. +bool x_check_dpms_status(struct x_connection *c, bool *screen_is_off); From 8e35b334584fab1f4ecd8765e185b9e46f98fffc Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 18 Dec 2023 04:34:25 +0000 Subject: [PATCH 03/13] config: add debug options to enable timing based pacing Disable timing estimation based pacing by default, as it might not work well across drivers, and might have subtle bugs. You can try setting `PICOM_DEBUG=smart_frame_pacing` if you want to try it out. Signed-off-by: Yuxuan Shui --- src/config.c | 5 ++++- src/config.h | 4 +++- src/picom.c | 40 ++++++++++++++++++++++++++-------------- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/config.c b/src/config.c index efbf279f7d..8c1ca91d94 100644 --- a/src/config.c +++ b/src/config.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -623,7 +624,9 @@ struct debug_options_entry { }; static const struct debug_options_entry debug_options_entries[] = { - + "smart_frame_pacing", + NULL, + offsetof(struct debug_options, smart_frame_pacing), }; void parse_debug_option_single(char *setting, struct debug_options *debug_options) { diff --git a/src/config.h b/src/config.h index bdf384aca7..ac3b6ad2cd 100644 --- a/src/config.h +++ b/src/config.h @@ -85,7 +85,9 @@ enum vblank_scheduler_type { /// Internal, private options for debugging and development use. struct debug_options { - + /// Try to reduce frame latency by using vblank interval and render time + /// estimates. Right now it's not working well across drivers. + int smart_frame_pacing; }; /// Structure representing all options. diff --git a/src/picom.c b/src/picom.c index 57ee4f0a47..f24f18fc5e 100644 --- a/src/picom.c +++ b/src/picom.c @@ -154,6 +154,12 @@ void collect_vblank_interval_statistics(struct vblank_event *e, void *ud) { return; } + if (!ps->o.debug_options.smart_frame_pacing) { + // We don't need to collect statistics if we are not doing smart frame + // pacing. + return; + } + // TODO(yshui): this naive method of estimating vblank interval does not handle // the variable refresh rate case very well. This includes the case // of a VRR enabled monitor; or a monitor that's turned off, in which @@ -212,13 +218,15 @@ void schedule_render_at_vblank(struct vblank_event *e, void *ud) { } // The frame has been finished and presented, record its render time. - int render_time_us = - (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); - render_statistics_add_render_time_sample( - &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); - log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " - "last_msc: %" PRIu64, - render_time_us, (int)ps->last_schedule_delay, ps->last_msc); + if (ps->o.debug_options.smart_frame_pacing) { + int render_time_us = + (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); + render_statistics_add_render_time_sample( + &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); + log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " + "last_msc: %" PRIu64, + render_time_us, (int)ps->last_schedule_delay, ps->last_msc); + } ps->last_schedule_delay = 0; ps->backend_busy = false; @@ -230,8 +238,8 @@ void schedule_render_at_vblank(struct vblank_event *e, void *ud) { delay_s = (double)(ps->next_render - now_us) / 1000000.0; } log_verbose("Prepare to start rendering: delay: %f s, next_render: %" PRIu64 - ", now_us: %" PRIu64, - delay_s, ps->next_render, now_us); + ", now_us: %" PRIu64, + delay_s, ps->next_render, now_us); assert(!ev_is_active(&ps->draw_timer)); ev_timer_set(&ps->draw_timer, delay_s, 0); ev_timer_start(ps->loop, &ps->draw_timer); @@ -310,6 +318,9 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { return; } + // if ps->o.debug_options.smart_frame_pacing is false, we won't have any render + // time or vblank interval estimates, so we would naturally fallback to schedule + // render immediately. auto render_budget = render_statistics_get_budget(&ps->render_stats, &divisor); auto frame_time = render_statistics_get_vblank_time(&ps->render_stats); if (frame_time == 0) { @@ -337,11 +348,12 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { delay_s, render_budget, frame_time, now_us, deadline); } - log_verbose("Delay: %.6lf s, last_msc: %" PRIu64 ", render_budget: %d, frame_time: " - "%" PRIu32 ", now_us: %" PRIu64 ", next_msc: %" PRIu64 ", " - "divisor: %d", - delay_s, ps->last_msc_instant, render_budget, frame_time, now_us, - deadline, divisor); + log_verbose("Delay: %.6lf s, last_msc: %" PRIu64 ", render_budget: %d, " + "frame_time: " + "%" PRIu32 ", now_us: %" PRIu64 ", next_msc: %" PRIu64 ", " + "divisor: %d", + delay_s, ps->last_msc_instant, render_budget, frame_time, now_us, + deadline, divisor); schedule: ps->render_queued = true; From 91a0ccc39132adc27113c853b5ad8148e124c229 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 19 Dec 2023 10:52:26 +0000 Subject: [PATCH 04/13] core: simplify pacing logic a bit more Also, vblank event callback should call schedule_render to queue renders instead of starting the draw timer directly, so that the CPU time calculation will be correct. Signed-off-by: Yuxuan Shui --- src/backend/backend.c | 8 +-- src/picom.c | 150 ++++++++++++++++++++++++------------------ 2 files changed, 89 insertions(+), 69 deletions(-) diff --git a/src/backend/backend.c b/src/backend/backend.c index dd3366d66a..5120672bc5 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -194,10 +194,10 @@ bool paint_all_new(session_t *ps, struct managed_win *const t) { auto after_damage_us = (uint64_t)now.tv_sec * 1000000UL + (uint64_t)now.tv_nsec / 1000; log_trace("Getting damage took %" PRIu64 " us", after_damage_us - after_sync_fence_us); if (ps->next_render > 0) { - log_trace("Render schedule deviation: %ld us (%s) %" PRIu64 " %ld", - labs((long)after_damage_us - (long)ps->next_render), - after_damage_us < ps->next_render ? "early" : "late", - after_damage_us, ps->next_render); + log_verbose("Render schedule deviation: %ld us (%s) %" PRIu64 " %ld", + labs((long)after_damage_us - (long)ps->next_render), + after_damage_us < ps->next_render ? "early" : "late", + after_damage_us, ps->next_render); ps->last_schedule_delay = 0; if (after_damage_us > ps->next_render) { ps->last_schedule_delay = after_damage_us - ps->next_render; diff --git a/src/picom.c b/src/picom.c index f24f18fc5e..4d9ae5f87c 100644 --- a/src/picom.c +++ b/src/picom.c @@ -192,57 +192,52 @@ void collect_vblank_interval_statistics(struct vblank_event *e, void *ud) { collect_vblank_interval_statistics, ud); } } -/// vblank callback scheduled by schedule_render. + +void schedule_render(session_t *ps, bool triggered_by_vblank); + +/// vblank callback scheduled by schedule_render, when a render is ongoing. /// -/// Check if previously queued render has finished, and record the time it took. -void schedule_render_at_vblank(struct vblank_event *e, void *ud) { +/// Check if previously queued render has finished, and reschedule render if it has. +void reschedule_render_at_vblank(struct vblank_event *e, void *ud) { auto ps = (session_t *)ud; assert(ps->frame_pacing); - assert(ps->backend_busy); assert(ps->render_queued); assert(ps->vblank_scheduler); - collect_vblank_interval_statistics(e, ud); + log_verbose("Rescheduling render at vblank, msc: %" PRIu64, e->msc); - struct timespec render_time; - bool completed = - ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); - if (!completed) { - // Render hasn't completed yet, we can't start another render. - // Check again at the next vblank. - log_debug("Last render did not complete during vblank, msc: " - "%" PRIu64, - ps->last_msc); - vblank_scheduler_schedule(ps->vblank_scheduler, schedule_render_at_vblank, ud); - return; - } + collect_vblank_interval_statistics(e, ud); - // The frame has been finished and presented, record its render time. - if (ps->o.debug_options.smart_frame_pacing) { - int render_time_us = - (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); - render_statistics_add_render_time_sample( - &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); - log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " - "last_msc: %" PRIu64, - render_time_us, (int)ps->last_schedule_delay, ps->last_msc); + if (ps->backend_busy) { + struct timespec render_time; + bool completed = + ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); + if (!completed) { + // Render hasn't completed yet, we can't start another render. + // Check again at the next vblank. + log_debug("Last render did not complete during vblank, msc: " + "%" PRIu64, + ps->last_msc); + vblank_scheduler_schedule(ps->vblank_scheduler, + reschedule_render_at_vblank, ud); + return; + } + + // The frame has been finished and presented, record its render time. + if (ps->o.debug_options.smart_frame_pacing) { + int render_time_us = (int)(render_time.tv_sec * 1000000L + + render_time.tv_nsec / 1000L); + render_statistics_add_render_time_sample( + &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); + log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " + "last_msc: %" PRIu64, + render_time_us, (int)ps->last_schedule_delay, + ps->last_msc); + } + ps->backend_busy = false; } - ps->last_schedule_delay = 0; - ps->backend_busy = false; - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - auto now_us = (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_nsec / 1000; - double delay_s = 0; - if (ps->next_render > now_us) { - delay_s = (double)(ps->next_render - now_us) / 1000000.0; - } - log_verbose("Prepare to start rendering: delay: %f s, next_render: %" PRIu64 - ", now_us: %" PRIu64, - delay_s, ps->next_render, now_us); - assert(!ev_is_active(&ps->draw_timer)); - ev_timer_set(&ps->draw_timer, delay_s, 0); - ev_timer_start(ps->loop, &ps->draw_timer); + schedule_render(ps, false); } /// How many seconds into the future should we start rendering the next frame. @@ -252,7 +247,7 @@ void schedule_render_at_vblank(struct vblank_event *e, void *ud) { /// 1. queue_redraw() queues a new render by calling schedule_render, if there /// is no render currently scheduled. i.e. render_queued == false. /// 2. then, we need to figure out the best time to start rendering. we need to -/// at least know when the current vblank will start, as we can't start render +/// at least know when the next vblank will start, as we can't start render /// before the current rendered frame is diplayed on screen. we have this /// information from the vblank scheduler, it will notify us when that happens. /// we might also want to delay the rendering even further to reduce latency, @@ -262,14 +257,20 @@ void schedule_render_at_vblank(struct vblank_event *e, void *ud) { /// vblank event is delivered). Backend APIs are called to issue render /// commands. render_queued is set to false, and backend_busy is set to true. /// -/// There is a small caveat in step 2. As a vblank event being delivered +/// There are some considerations in step 2: +/// +/// First of all, a vblank event being delivered /// doesn't necessarily mean the frame has been displayed on screen. If a frame /// takes too long to render, it might miss the current vblank, and will be /// displayed on screen during one of the subsequent vblanks. So in /// schedule_render_at_vblank, we ask the backend to see if it has finished /// rendering. if not, render_queued is unchanged, and another vblank is /// scheduled; otherwise, draw_callback_impl will be scheduled to be call at -/// an appropriate time. +/// an appropriate time. Second, we might not have rendered for the previous vblank, +/// in which case the last vblank event we received could be many frames in the past, +/// so we can't make scheduling decisions based on that. So we always schedule +/// a vblank event when render is queued, and make scheduling decisions when the +/// event is delivered. /// /// All of the above is what happens when frame_pacing is true. Otherwise /// render_in_progress is either QUEUED or IDLE, and queue_redraw will always @@ -297,6 +298,21 @@ void schedule_render_at_vblank(struct vblank_event *e, void *ud) { /// The code that does this is already implemented below, but disabled by /// default. There are several problems with it, see bug #1072. void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { + // If the backend is busy, we will try again at the next vblank. + if (ps->backend_busy) { + // We should never have set backend_busy to true unless frame_pacing is + // enabled. + assert(ps->vblank_scheduler); + assert(ps->frame_pacing); + log_verbose("Backend busy, will reschedule render at next vblank."); + if (!vblank_scheduler_schedule(ps->vblank_scheduler, + reschedule_render_at_vblank, ps)) { + // TODO(yshui): handle error here + abort(); + } + return; + } + // By default, we want to schedule render immediately, later in this function we // might adjust that and move the render later, based on render timing statistics. double delay_s = 0; @@ -349,37 +365,41 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { } log_verbose("Delay: %.6lf s, last_msc: %" PRIu64 ", render_budget: %d, " - "frame_time: " - "%" PRIu32 ", now_us: %" PRIu64 ", next_msc: %" PRIu64 ", " - "divisor: %d", + "frame_time: %" PRIu32 ", now_us: %" PRIu64 ", next_render: %" PRIu64 + ", next_msc: %" PRIu64 ", divisor: " + "%d", delay_s, ps->last_msc_instant, render_budget, frame_time, now_us, - deadline, divisor); + ps->next_render, deadline, divisor); schedule: - ps->render_queued = true; // If the backend is not busy, we just need to schedule the render at the - // specified time; otherwise we need to wait for vblank events. - if (!ps->backend_busy) { - assert(!ev_is_active(&ps->draw_timer)); - ev_timer_set(&ps->draw_timer, delay_s, 0); - ev_timer_start(ps->loop, &ps->draw_timer); - } else { - // We should never set backend_busy to true unless frame_pacing is - // enabled. - assert(ps->vblank_scheduler); - if (!vblank_scheduler_schedule(ps->vblank_scheduler, - schedule_render_at_vblank, ps)) { - // TODO(yshui): handle error here - abort(); - } - } + // specified time; otherwise we need to wait for the next vblank event and + // reschedule. + ps->last_schedule_delay = 0; + assert(!ev_is_active(&ps->draw_timer)); + ev_timer_set(&ps->draw_timer, delay_s, 0); + ev_timer_start(ps->loop, &ps->draw_timer); } void queue_redraw(session_t *ps) { + log_verbose("Queue redraw, render_queued: %d, backend_busy: %d", + ps->render_queued, ps->backend_busy); + if (ps->render_queued) { return; } - schedule_render(ps, false); + ps->render_queued = true; + if (ps->o.debug_options.smart_frame_pacing && ps->vblank_scheduler) { + // Make we schedule_render call is synced with vblank events. + // See the comment on schedule_render for more details. + if (!vblank_scheduler_schedule(ps->vblank_scheduler, + reschedule_render_at_vblank, ps)) { + // TODO(yshui): handle error here + abort(); + } + } else { + schedule_render(ps, false); + } } /** From 58150dfe552ce94909efdff19d79ade07c3ea45e Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 18 Dec 2023 09:33:17 +0000 Subject: [PATCH 05/13] x: fix x_request_vblank_event present_notify_msc with divisor == 0 has undocumented special meaning, it means async present notify, which means we could receive MSC notifications from the past. Signed-off-by: Yuxuan Shui --- src/x.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/x.c b/src/x.c index 45abe31197..06c8b939d6 100644 --- a/src/x.c +++ b/src/x.c @@ -780,8 +780,7 @@ bool x_fence_sync(struct x_connection *c, xcb_sync_fence_t f) { } void x_request_vblank_event(struct x_connection *c, xcb_window_t window, uint64_t msc) { - auto cookie = - xcb_present_notify_msc(c->c, window, 0, msc, 0, 0); + auto cookie = xcb_present_notify_msc(c->c, window, 0, msc, 1, 0); set_cant_fail_cookie(c, cookie); } From 4574977287a5f534504dfbe1fc1b5e4a5f2c4842 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 19 Dec 2023 10:55:03 +0000 Subject: [PATCH 06/13] vblank: winding down vblank events instead of stopping immediately I noticed sometimes full frame rate video is rendered at half frame rate sometimes. That is because the damage notify is sent very close to vblank, and since we request vblank events when we get the damage, we miss the vblank event immediately after, despite the damage happening before the vblank. request next ...... next next damage vblank vblank vblank | | | ...... | v v v v ---------------------->>>>>>--------- `request vblank` is triggered by `damage`, but because it's too close to `next vblank`, that vblank is missed despite we requested before it happening, and we only get `next next vblank`. The result is we will drop every other frame. The solution in this commit is that we will keep requesting vblank events, right after we received a vblank event, even when nobody is asking for them. We would do that for a set number of vblanks before stopping (currently 4). Signed-off-by: Yuxuan Shui --- src/vblank.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/vblank.c b/src/vblank.c index 219e2cb1fb..35f8389376 100644 --- a/src/vblank.c +++ b/src/vblank.c @@ -18,11 +18,19 @@ struct vblank_callback { void *user_data; }; +#define VBLANK_WIND_DOWN 4 + struct vblank_scheduler { struct x_connection *c; size_t callback_capacity, callback_count; struct vblank_callback *callbacks; struct ev_loop *loop; + /// Request extra vblank events even when no callbacks are scheduled. + /// This is because when callbacks are scheduled too close to a vblank, + /// we might send PresentNotifyMsc request too late and miss the vblank event. + /// So we request extra vblank events right after the last vblank event + /// to make sure this doesn't happen. + unsigned int wind_down; xcb_window_t target_window; enum vblank_scheduler_type type; bool vblank_event_requested; @@ -178,7 +186,7 @@ static void vblank_scheduler_schedule_internal(struct vblank_scheduler *self) { bool vblank_scheduler_schedule(struct vblank_scheduler *self, vblank_callback_t vblank_callback, void *user_data) { - if (self->callback_count == 0) { + if (self->callback_count == 0 && self->wind_down == 0) { vblank_scheduler_schedule_internal(self); } if (self->callback_count == self->callback_capacity) { @@ -204,6 +212,11 @@ vblank_scheduler_invoke_callbacks(struct vblank_scheduler *self, struct vblank_e // callbacks might be added during callback invocation, so we need to // copy the callback_count. size_t count = self->callback_count; + if (count == 0) { + self->wind_down--; + } else { + self->wind_down = VBLANK_WIND_DOWN; + } for (size_t i = 0; i < count; i++) { self->callbacks[i].fn(event, self->callbacks[i].user_data); } @@ -211,7 +224,7 @@ vblank_scheduler_invoke_callbacks(struct vblank_scheduler *self, struct vblank_e memmove(self->callbacks, self->callbacks + count, (self->callback_count - count) * sizeof(*self->callbacks)); self->callback_count -= count; - if (self->callback_count) { + if (self->callback_count || self->wind_down) { vblank_scheduler_schedule_internal(self); } } From 057c8265b0df37f0348e3ecaabf08fa59020a586 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 18 Dec 2023 20:12:14 +0000 Subject: [PATCH 07/13] core: always check if render is finished at next vblank Right now we rely on `reschedule_render_at_vblank` being scheduled for the render finish check. Which means we will only know if a frame has finished rendering the next time we queue a render. And we only check at vblank boundary, which means when a render is queued we have to wait for the next vblank, when had we checked earlier, we will be able to start rendering immediately Signed-off-by: Yuxuan Shui --- src/picom.c | 68 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/src/picom.c b/src/picom.c index 4d9ae5f87c..920eb071f8 100644 --- a/src/picom.c +++ b/src/picom.c @@ -144,6 +144,38 @@ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t return w; } +void check_render_finish(struct vblank_event *e attr_unused, void *ud) { + auto ps = (session_t *)ud; + if (!ps->backend_busy) { + return; + } + + struct timespec render_time; + bool completed = + ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); + if (!completed) { + // Render hasn't completed yet, we can't start another render. + // Check again at the next vblank. + log_debug("Last render did not complete during vblank, msc: " + "%" PRIu64, + ps->last_msc); + vblank_scheduler_schedule(ps->vblank_scheduler, check_render_finish, ud); + return; + } + + // The frame has been finished and presented, record its render time. + if (ps->o.debug_options.smart_frame_pacing) { + int render_time_us = + (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); + render_statistics_add_render_time_sample( + &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); + log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " + "last_msc: %" PRIu64, + render_time_us, (int)ps->last_schedule_delay, ps->last_msc); + } + ps->backend_busy = false; +} + void collect_vblank_interval_statistics(struct vblank_event *e, void *ud) { auto ps = (session_t *)ud; assert(ps->frame_pacing); @@ -207,34 +239,12 @@ void reschedule_render_at_vblank(struct vblank_event *e, void *ud) { log_verbose("Rescheduling render at vblank, msc: %" PRIu64, e->msc); collect_vblank_interval_statistics(e, ud); + check_render_finish(e, ud); if (ps->backend_busy) { - struct timespec render_time; - bool completed = - ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); - if (!completed) { - // Render hasn't completed yet, we can't start another render. - // Check again at the next vblank. - log_debug("Last render did not complete during vblank, msc: " - "%" PRIu64, - ps->last_msc); - vblank_scheduler_schedule(ps->vblank_scheduler, - reschedule_render_at_vblank, ud); - return; - } - - // The frame has been finished and presented, record its render time. - if (ps->o.debug_options.smart_frame_pacing) { - int render_time_us = (int)(render_time.tv_sec * 1000000L + - render_time.tv_nsec / 1000L); - render_statistics_add_render_time_sample( - &ps->render_stats, render_time_us + (int)ps->last_schedule_delay); - log_verbose("Last render call took: %d (gpu) + %d (cpu) us, " - "last_msc: %" PRIu64, - render_time_us, (int)ps->last_schedule_delay, - ps->last_msc); - } - ps->backend_busy = false; + vblank_scheduler_schedule(ps->vblank_scheduler, + reschedule_render_at_vblank, ud); + return; } schedule_render(ps, false); @@ -1807,6 +1817,12 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { if (animation) { queue_redraw(ps); } + if (ps->vblank_scheduler) { + // Even if we might not want to render during next vblank, we want to keep + // `backend_busy` up to date, so when the next render comes, we can + // immediately know if we can render. + vblank_scheduler_schedule(ps->vblank_scheduler, check_render_finish, ps); + } } static void draw_callback(EV_P_ ev_timer *w, int revents) { From 75f89caf0443a9e4a156b41a9ca1ab9cad2fa317 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 18 Dec 2023 20:32:22 +0000 Subject: [PATCH 08/13] core: don't always delay schedule_render to vblank It's kind of dumb anyway. If we get damage event right after a vblank event, we would waste the whole vblank. Instead improve the frame scheduling logic to target the right vblank interval. This only affects smart_frame_pacing anyway. Signed-off-by: Yuxuan Shui --- src/picom.c | 40 ++++++++++++++++++++-------------------- src/statistics.c | 13 +------------ src/statistics.h | 7 +------ 3 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/picom.c b/src/picom.c index 920eb071f8..3fca519d00 100644 --- a/src/picom.c +++ b/src/picom.c @@ -334,29 +334,39 @@ void schedule_render(session_t *ps, bool triggered_by_vblank attr_unused) { ps->next_render = now_us; if (!ps->frame_pacing || !ps->redirected) { - // If not doing frame pacing, schedule a render immediately unless it's - // already scheduled; if not redirected, we schedule immediately to have a - // chance to redirect. We won't have frame or render timing information + // If not doing frame pacing, schedule a render immediately; if + // not redirected, we schedule immediately to have a chance to + // redirect. We won't have frame or render timing information // anyway. - if (!ev_is_active(&ps->draw_timer)) { - goto schedule; - } - return; + assert(!ev_is_active(&ps->draw_timer)); + goto schedule; } // if ps->o.debug_options.smart_frame_pacing is false, we won't have any render // time or vblank interval estimates, so we would naturally fallback to schedule // render immediately. - auto render_budget = render_statistics_get_budget(&ps->render_stats, &divisor); + auto render_budget = render_statistics_get_budget(&ps->render_stats); auto frame_time = render_statistics_get_vblank_time(&ps->render_stats); if (frame_time == 0) { // We don't have enough data for render time estimates, maybe there's // no frame rendered yet, or the backend doesn't support render timing // information, schedule render immediately. + log_verbose("Not enough data for render time estimates."); goto schedule; } - auto const deadline = ps->last_msc_instant + (unsigned long)divisor * frame_time; + if (render_budget >= frame_time) { + // If the estimated render time is already longer than the estimated + // vblank interval, there is no way we can make it. Instead of always + // dropping frames, we try desperately to catch up and schedule a + // render immediately. + log_verbose("Render budget: %u us >= frame time: %" PRIu32 " us", + render_budget, frame_time); + goto schedule; + } + + auto target_frame = (now_us + render_budget - ps->last_msc_instant) / frame_time + 1; + auto const deadline = ps->last_msc_instant + target_frame * frame_time; unsigned int available = 0; if (deadline > now_us) { available = (unsigned int)(deadline - now_us); @@ -399,17 +409,7 @@ void queue_redraw(session_t *ps) { return; } ps->render_queued = true; - if (ps->o.debug_options.smart_frame_pacing && ps->vblank_scheduler) { - // Make we schedule_render call is synced with vblank events. - // See the comment on schedule_render for more details. - if (!vblank_scheduler_schedule(ps->vblank_scheduler, - reschedule_render_at_vblank, ps)) { - // TODO(yshui): handle error here - abort(); - } - } else { - schedule_render(ps, false); - } + schedule_render(ps, false); } /** diff --git a/src/statistics.c b/src/statistics.c index 5e3d92491d..11c7466ef6 100644 --- a/src/statistics.c +++ b/src/statistics.c @@ -55,26 +55,15 @@ void render_statistics_add_render_time_sample(struct render_statistics *rs, int /// A `divisor` is also returned, indicating the target framerate. The divisor is /// the number of vblanks we should wait between each frame. A divisor of 1 means /// full framerate, 2 means half framerate, etc. -unsigned int -render_statistics_get_budget(struct render_statistics *rs, unsigned int *divisor) { +unsigned int render_statistics_get_budget(struct render_statistics *rs) { if (rs->render_times.nelem < rs->render_times.window_size) { // No valid render time estimates yet. Assume maximum budget. - *divisor = 1; return UINT_MAX; } // N-th percentile of render times, see render_statistics_init for N. auto render_time_percentile = rolling_quantile_estimate(&rs->render_time_quantile, &rs->render_times); - auto vblank_time_us = render_statistics_get_vblank_time(rs); - if (vblank_time_us == 0) { - // We don't have a good estimate of the vblank time yet, so we - // assume we can finish in one vblank. - *divisor = 1; - } else { - *divisor = - (unsigned int)(render_time_percentile / rs->vblank_time_us.mean + 1); - } return (unsigned int)render_time_percentile; } diff --git a/src/statistics.h b/src/statistics.h index 42d7bc2d91..a111486111 100644 --- a/src/statistics.h +++ b/src/statistics.h @@ -23,12 +23,7 @@ void render_statistics_add_vblank_time_sample(struct render_statistics *rs, int void render_statistics_add_render_time_sample(struct render_statistics *rs, int time_us); /// How much time budget we should give to the backend for rendering, in microseconds. -/// -/// A `divisor` is also returned, indicating the target framerate. The divisor is -/// the number of vblanks we should wait between each frame. A divisor of 1 means -/// full framerate, 2 means half framerate, etc. -unsigned int -render_statistics_get_budget(struct render_statistics *rs, unsigned int *divisor); +unsigned int render_statistics_get_budget(struct render_statistics *rs); /// Return the measured vblank interval in microseconds. Returns 0 if not enough /// samples have been collected yet. From 5ee7b09542fade7f78aab371dcb4ea0b0fd5e319 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 18 Dec 2023 21:08:44 +0000 Subject: [PATCH 09/13] vblank: add GLX_SGI_video_sync based scheduler Present extension based scheduler doesn't work well on NVIDIA drivers. GLX_SGI_video_sync is less accurate, but is rumoured to work. See [kwin's usage](https://invent.kde.org/plasma/kwin/-/blob/master/src/ backends/x11/standalone/x11_standalone_sgivideosyncvsyncmonitor.cpp) Signed-off-by: Yuxuan Shui --- src/meson.build | 2 +- src/picom.c | 3 +- src/vblank.c | 281 +++++++++++++++++++++++++++++++++++++++++++++++- src/vblank.h | 4 +- 4 files changed, 282 insertions(+), 8 deletions(-) diff --git a/src/meson.build b/src/meson.build index 51f96488cd..a2f9c36c73 100644 --- a/src/meson.build +++ b/src/meson.build @@ -59,7 +59,7 @@ endif if get_option('opengl') cflags += ['-DCONFIG_OPENGL', '-DGL_GLEXT_PROTOTYPES'] - deps += [dependency('gl', required: true), dependency('egl', required: true)] + deps += [dependency('gl', required: true), dependency('egl', required: true), dependency('threads', required:true)] srcs += [ 'opengl.c' ] endif diff --git a/src/picom.c b/src/picom.c index 3fca519d00..e797f5fc82 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1493,7 +1493,8 @@ static bool redirect_start(session_t *ps) { ps->last_schedule_delay = 0; render_statistics_reset(&ps->render_stats); ps->vblank_scheduler = - vblank_scheduler_new(ps->loop, &ps->c, session_get_target_window(ps)); + vblank_scheduler_new(ps->loop, &ps->c, session_get_target_window(ps), + VBLANK_SCHEDULER_PRESENT); if (!ps->vblank_scheduler) { return false; } diff --git a/src/vblank.c b/src/vblank.c index 35f8389376..6c07619e9c 100644 --- a/src/vblank.c +++ b/src/vblank.c @@ -2,12 +2,26 @@ #include #include +#include #include +#include #include #include +#include "config.h" + +#ifdef CONFIG_OPENGL +// Enable sgi_video_sync_vblank_scheduler +#include +#include +#include +#include +#include +#include + +#include "backend/gl/glx.h" +#endif #include "compiler.h" -#include "config.h" #include "list.h" // for container_of #include "log.h" #include "vblank.h" @@ -48,6 +62,7 @@ struct present_vblank_scheduler { }; struct vblank_scheduler_ops { + size_t size; void (*init)(struct vblank_scheduler *self); void (*deinit)(struct vblank_scheduler *self); void (*schedule)(struct vblank_scheduler *self); @@ -57,6 +72,242 @@ struct vblank_scheduler_ops { static void vblank_scheduler_invoke_callbacks(struct vblank_scheduler *self, struct vblank_event *event); +#ifdef CONFIG_OPENGL +struct sgi_video_sync_vblank_scheduler { + struct vblank_scheduler base; + + // Since glXWaitVideoSyncSGI blocks, we need to run it in a separate thread. + // ... and all the thread shenanigans that come with it. + _Atomic unsigned int last_msc; + _Atomic uint64_t last_ust; + ev_async notify; + pthread_t sync_thread; + bool running, error; + + /// Protects `running`, `error` and `base.vblank_event_requested` + pthread_mutex_t vblank_requested_mtx; + pthread_cond_t vblank_requested_cnd; +}; + +struct sgi_video_sync_thread_args { + struct sgi_video_sync_vblank_scheduler *self; + int start_status; + pthread_mutex_t start_mtx; + pthread_cond_t start_cnd; +}; + +static bool check_sgi_video_sync_extension(Display *dpy, int screen) { + const char *glx_ext = glXQueryExtensionsString(dpy, screen); + const char *needle = "GLX_SGI_video_sync"; + char *found = strstr(glx_ext, needle); + if (!found) { + return false; + } + if (found != glx_ext && found[-1] != ' ') { + return false; + } + if (found[strlen(needle)] != ' ' && found[strlen(needle)] != '\0') { + return false; + } + + glXWaitVideoSyncSGI = (PFNGLXWAITVIDEOSYNCSGIPROC)(void *)glXGetProcAddress( + (const GLubyte *)"glXWaitVideoSyncSGI"); + if (!glXWaitVideoSyncSGI) { + return false; + } + return true; +} + +static void *sgi_video_sync_thread(void *data) { + auto args = (struct sgi_video_sync_thread_args *)data; + auto self = args->self; + Display *dpy = XOpenDisplay(NULL); + int error_code = 0; + if (!dpy) { + error_code = 1; + goto start_failed; + } + Window root = DefaultRootWindow(dpy), dummy = None; + int screen = DefaultScreen(dpy); + int ncfg = 0; + GLXFBConfig *cfg_ = glXChooseFBConfig( + dpy, screen, + (int[]){GLX_RENDER_TYPE, GLX_RGBA_BIT, GLX_DRAWABLE_TYPE, GLX_WINDOW_BIT, 0}, + &ncfg); + GLXContext ctx = NULL; + GLXDrawable drawable = None; + + if (!cfg_) { + error_code = 2; + goto start_failed; + } + GLXFBConfig cfg = cfg_[0]; + XFree(cfg_); + + XVisualInfo *vi = glXGetVisualFromFBConfig(dpy, cfg); + if (!vi) { + error_code = 3; + goto start_failed; + } + + Visual *visual = vi->visual; + const int depth = vi->depth; + XFree(vi); + + Colormap colormap = XCreateColormap(dpy, root, visual, AllocNone); + XSetWindowAttributes attributes; + attributes.colormap = colormap; + + dummy = XCreateWindow(dpy, root, 0, 0, 1, 1, 0, depth, InputOutput, visual, + CWColormap, &attributes); + XFreeColormap(dpy, colormap); + if (dummy == None) { + error_code = 4; + goto start_failed; + } + + drawable = glXCreateWindow(dpy, cfg, dummy, NULL); + if (drawable == None) { + error_code = 5; + goto start_failed; + } + + ctx = glXCreateNewContext(dpy, cfg, GLX_RGBA_TYPE, 0, true); + if (ctx == NULL) { + error_code = 6; + goto start_failed; + } + + if (!glXMakeContextCurrent(dpy, drawable, drawable, ctx)) { + error_code = 7; + goto start_failed; + } + + if (!check_sgi_video_sync_extension(dpy, screen)) { + error_code = 8; + goto start_failed; + } + + pthread_mutex_lock(&args->start_mtx); + args->start_status = 0; + pthread_cond_signal(&args->start_cnd); + pthread_mutex_unlock(&args->start_mtx); + + pthread_mutex_lock(&self->vblank_requested_mtx); + while (self->running) { + if (!self->base.vblank_event_requested) { + pthread_cond_wait(&self->vblank_requested_cnd, + &self->vblank_requested_mtx); + continue; + } + pthread_mutex_unlock(&self->vblank_requested_mtx); + + unsigned int last_msc; + glXWaitVideoSyncSGI(1, 0, &last_msc); + + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + atomic_store(&self->last_msc, last_msc); + atomic_store(&self->last_ust, + (uint64_t)(now.tv_sec * 1000000 + now.tv_nsec / 1000)); + ev_async_send(self->base.loop, &self->notify); + pthread_mutex_lock(&self->vblank_requested_mtx); + } + pthread_mutex_unlock(&self->vblank_requested_mtx); + goto cleanup; + +start_failed: + pthread_mutex_lock(&args->start_mtx); + args->start_status = error_code; + pthread_cond_signal(&args->start_cnd); + pthread_mutex_unlock(&args->start_mtx); + +cleanup: + if (dpy) { + glXMakeCurrent(dpy, None, NULL); + if (ctx) { + glXDestroyContext(dpy, ctx); + } + if (drawable) { + glXDestroyWindow(dpy, drawable); + } + if (dummy) { + XDestroyWindow(dpy, dummy); + } + XCloseDisplay(dpy); + } + return NULL; +} + +static void sgi_video_sync_scheduler_schedule(struct vblank_scheduler *base) { + auto self = (struct sgi_video_sync_vblank_scheduler *)base; + log_verbose("Requesting vblank event for msc %d", self->last_msc + 1); + pthread_mutex_lock(&self->vblank_requested_mtx); + assert(!base->vblank_event_requested); + base->vblank_event_requested = true; + pthread_cond_signal(&self->vblank_requested_cnd); + pthread_mutex_unlock(&self->vblank_requested_mtx); +} + +static void +sgi_video_sync_scheduler_callback(EV_P attr_unused, ev_async *w, int attr_unused revents) { + auto sched = container_of(w, struct sgi_video_sync_vblank_scheduler, notify); + auto event = (struct vblank_event){ + .msc = atomic_load(&sched->last_msc), + .ust = atomic_load(&sched->last_ust), + }; + sched->base.vblank_event_requested = false; + log_verbose("Received vblank event for msc %lu", event.msc); + vblank_scheduler_invoke_callbacks(&sched->base, &event); +} + +static void sgi_video_sync_scheduler_init(struct vblank_scheduler *base) { + auto self = (struct sgi_video_sync_vblank_scheduler *)base; + auto args = (struct sgi_video_sync_thread_args){ + .self = self, + .start_status = -1, + }; + pthread_mutex_init(&args.start_mtx, NULL); + pthread_cond_init(&args.start_cnd, NULL); + + base->type = VBLANK_SCHEDULER_SGI_VIDEO_SYNC; + ev_async_init(&self->notify, sgi_video_sync_scheduler_callback); + ev_async_start(base->loop, &self->notify); + pthread_mutex_init(&self->vblank_requested_mtx, NULL); + pthread_cond_init(&self->vblank_requested_cnd, NULL); + + self->running = true; + pthread_create(&self->sync_thread, NULL, sgi_video_sync_thread, &args); + + pthread_mutex_lock(&args.start_mtx); + while (args.start_status == -1) { + pthread_cond_wait(&args.start_cnd, &args.start_mtx); + } + if (args.start_status != 0) { + log_fatal("Failed to start sgi_video_sync_thread, error code: %d", + args.start_status); + abort(); + } + pthread_mutex_destroy(&args.start_mtx); + pthread_cond_destroy(&args.start_cnd); + log_info("Started sgi_video_sync_thread"); +} + +static void sgi_video_sync_scheduler_deinit(struct vblank_scheduler *base) { + auto self = (struct sgi_video_sync_vblank_scheduler *)base; + ev_async_stop(base->loop, &self->notify); + pthread_mutex_lock(&self->vblank_requested_mtx); + self->running = false; + pthread_cond_signal(&self->vblank_requested_cnd); + pthread_mutex_unlock(&self->vblank_requested_mtx); + + pthread_join(self->sync_thread, NULL); + + pthread_mutex_destroy(&self->vblank_requested_mtx); + pthread_cond_destroy(&self->vblank_requested_cnd); +} +#endif + static void present_vblank_scheduler_schedule(struct vblank_scheduler *base) { auto self = (struct present_vblank_scheduler *)base; log_verbose("Requesting vblank event for window 0x%08x, msc %" PRIu64, @@ -170,11 +421,22 @@ static bool handle_present_events(struct vblank_scheduler *base) { static const struct vblank_scheduler_ops vblank_scheduler_ops[LAST_VBLANK_SCHEDULER] = { [VBLANK_SCHEDULER_PRESENT] = { + .size = sizeof(struct present_vblank_scheduler), .init = present_vblank_scheduler_init, .deinit = present_vblank_scheduler_deinit, .schedule = present_vblank_scheduler_schedule, .handle_x_events = handle_present_events, }, +#ifdef CONFIG_OPENGL + [VBLANK_SCHEDULER_SGI_VIDEO_SYNC] = + { + .size = sizeof(struct sgi_video_sync_vblank_scheduler), + .init = sgi_video_sync_scheduler_init, + .deinit = sgi_video_sync_scheduler_deinit, + .schedule = sgi_video_sync_scheduler_schedule, + .handle_x_events = NULL, + }, +#endif }; static void vblank_scheduler_schedule_internal(struct vblank_scheduler *self) { @@ -239,13 +501,22 @@ void vblank_scheduler_free(struct vblank_scheduler *self) { free(self); } -struct vblank_scheduler *vblank_scheduler_new(struct ev_loop *loop, struct x_connection *c, - xcb_window_t target_window) { - struct vblank_scheduler *self = calloc(1, sizeof(struct present_vblank_scheduler)); +struct vblank_scheduler * +vblank_scheduler_new(struct ev_loop *loop, struct x_connection *c, + xcb_window_t target_window, enum vblank_scheduler_type type) { + size_t object_size = vblank_scheduler_ops[type].size; + auto init_fn = vblank_scheduler_ops[type].init; + if (!object_size || !init_fn) { + log_error("Unsupported or invalid vblank scheduler type: %d", type); + return NULL; + } + + assert(object_size >= sizeof(struct vblank_scheduler)); + struct vblank_scheduler *self = calloc(1, object_size); self->target_window = target_window; self->c = c; self->loop = loop; - vblank_scheduler_ops[VBLANK_SCHEDULER_PRESENT].init(self); + init_fn(self); return self; } diff --git a/src/vblank.h b/src/vblank.h index 3f5c7d1af6..392b5ae972 100644 --- a/src/vblank.h +++ b/src/vblank.h @@ -8,6 +8,7 @@ #include #include +#include "config.h" #include "x.h" /// An object that schedule vblank events. @@ -31,7 +32,8 @@ typedef void (*vblank_callback_t)(struct vblank_event *event, void *user_data); bool vblank_scheduler_schedule(struct vblank_scheduler *self, vblank_callback_t cb, void *user_data); struct vblank_scheduler * -vblank_scheduler_new(struct ev_loop *loop, struct x_connection *c, xcb_window_t target_window); +vblank_scheduler_new(struct ev_loop *loop, struct x_connection *c, + xcb_window_t target_window, enum vblank_scheduler_type type); void vblank_scheduler_free(struct vblank_scheduler *); bool vblank_handle_x_events(struct vblank_scheduler *self); From 4080a8d25478fe6cdb90359aaa610f42c411f80d Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 19 Dec 2023 10:29:31 +0000 Subject: [PATCH 10/13] core: add debug options to override the vblank scheduler Useful for debugging. Signed-off-by: Yuxuan Shui --- src/config.c | 16 ++++++++++++---- src/config.h | 4 ++++ src/picom.c | 10 +++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/config.c b/src/config.c index 8c1ca91d94..7dc28146c5 100644 --- a/src/config.c +++ b/src/config.c @@ -623,11 +623,17 @@ struct debug_options_entry { size_t offset; }; +// clang-format off +const char *vblank_scheduler_str[] = { + [VBLANK_SCHEDULER_PRESENT] = "present", + [VBLANK_SCHEDULER_SGI_VIDEO_SYNC] = "sgi_video_sync", + [LAST_VBLANK_SCHEDULER] = NULL +}; static const struct debug_options_entry debug_options_entries[] = { - "smart_frame_pacing", - NULL, - offsetof(struct debug_options, smart_frame_pacing), + {"smart_frame_pacing", NULL, offsetof(struct debug_options, smart_frame_pacing)}, + {"force_vblank_sched", vblank_scheduler_str, offsetof(struct debug_options, force_vblank_scheduler)}, }; +// clang-format on void parse_debug_option_single(char *setting, struct debug_options *debug_options) { char *equal = strchr(setting, '='); @@ -670,7 +676,9 @@ void parse_debug_option_single(char *setting, struct debug_options *debug_option /// Parse debug options from environment variable `PICOM_DEBUG`. void parse_debug_options(struct debug_options *debug_options) { const char *debug = getenv("PICOM_DEBUG"); - const struct debug_options default_debug_options = {}; + const struct debug_options default_debug_options = { + .force_vblank_scheduler = LAST_VBLANK_SCHEDULER, + }; *debug_options = default_debug_options; if (!debug) { diff --git a/src/config.h b/src/config.h index ac3b6ad2cd..d91dbb4563 100644 --- a/src/config.h +++ b/src/config.h @@ -83,11 +83,15 @@ enum vblank_scheduler_type { LAST_VBLANK_SCHEDULER, }; +extern const char *vblank_scheduler_str[]; + /// Internal, private options for debugging and development use. struct debug_options { /// Try to reduce frame latency by using vblank interval and render time /// estimates. Right now it's not working well across drivers. int smart_frame_pacing; + /// Override the vblank scheduler chosen by the compositor. + int force_vblank_scheduler; }; /// Structure representing all options. diff --git a/src/picom.c b/src/picom.c index e797f5fc82..02aaf6f97b 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1492,9 +1492,13 @@ static bool redirect_start(session_t *ps) { ps->last_msc = 0; ps->last_schedule_delay = 0; render_statistics_reset(&ps->render_stats); - ps->vblank_scheduler = - vblank_scheduler_new(ps->loop, &ps->c, session_get_target_window(ps), - VBLANK_SCHEDULER_PRESENT); + enum vblank_scheduler_type scheduler_type = VBLANK_SCHEDULER_PRESENT; + if (ps->o.debug_options.force_vblank_scheduler != LAST_VBLANK_SCHEDULER) { + scheduler_type = + (enum vblank_scheduler_type)ps->o.debug_options.force_vblank_scheduler; + } + ps->vblank_scheduler = vblank_scheduler_new( + ps->loop, &ps->c, session_get_target_window(ps), scheduler_type); if (!ps->vblank_scheduler) { return false; } From bd1c6e600e6abc3331a8aeea3e32470a6fac8bc0 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 19 Dec 2023 10:29:59 +0000 Subject: [PATCH 11/13] driver: choose sgi_video_sync scheduler for NVIDIA Signed-off-by: Yuxuan Shui --- src/backend/driver.c | 7 +++++++ src/backend/driver.h | 3 +++ src/picom.c | 12 +++++++----- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/backend/driver.c b/src/backend/driver.c index b909a62272..f17743a4aa 100644 --- a/src/backend/driver.c +++ b/src/backend/driver.c @@ -19,6 +19,13 @@ void apply_driver_workarounds(struct session *ps, enum driver driver) { } } +enum vblank_scheduler_type choose_vblank_scheduler(enum driver driver) { + if (driver & DRIVER_NVIDIA) { + return VBLANK_SCHEDULER_SGI_VIDEO_SYNC; + } + return VBLANK_SCHEDULER_PRESENT; +} + enum driver detect_driver(xcb_connection_t *c, backend_t *backend_data, xcb_window_t window) { enum driver ret = 0; // First we try doing backend agnostic detection using RANDR diff --git a/src/backend/driver.h b/src/backend/driver.h index e4cc3981f1..1b0877c14b 100644 --- a/src/backend/driver.h +++ b/src/backend/driver.h @@ -7,6 +7,7 @@ #include #include +#include "config.h" #include "utils.h" struct session; @@ -41,6 +42,8 @@ enum driver detect_driver(xcb_connection_t *, struct backend_base *, xcb_window_ /// Apply driver specified global workarounds. It's safe to call this multiple times. void apply_driver_workarounds(struct session *ps, enum driver); +/// Choose a vblank scheduler based on the driver. +enum vblank_scheduler_type choose_vblank_scheduler(enum driver driver); // Print driver names to stdout, for diagnostics static inline void print_drivers(enum driver drivers) { diff --git a/src/picom.c b/src/picom.c index 02aaf6f97b..a008f9f1b8 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1485,6 +1485,10 @@ static bool redirect_start(session_t *ps) { ps->frame_pacing = false; } + // Re-detect driver since we now have a backend + ps->drivers = detect_driver(ps->c.c, ps->backend_data, ps->c.screen_info->root); + apply_driver_workarounds(ps, ps->drivers); + if (ps->present_exists && ps->frame_pacing) { // Initialize rendering and frame timing statistics, and frame pacing // states. @@ -1492,11 +1496,13 @@ static bool redirect_start(session_t *ps) { ps->last_msc = 0; ps->last_schedule_delay = 0; render_statistics_reset(&ps->render_stats); - enum vblank_scheduler_type scheduler_type = VBLANK_SCHEDULER_PRESENT; + enum vblank_scheduler_type scheduler_type = + choose_vblank_scheduler(ps->drivers); if (ps->o.debug_options.force_vblank_scheduler != LAST_VBLANK_SCHEDULER) { scheduler_type = (enum vblank_scheduler_type)ps->o.debug_options.force_vblank_scheduler; } + log_info("Using vblank scheduler: %s.", vblank_scheduler_str[scheduler_type]); ps->vblank_scheduler = vblank_scheduler_new( ps->loop, &ps->c, session_get_target_window(ps), scheduler_type); if (!ps->vblank_scheduler) { @@ -1515,10 +1521,6 @@ static bool redirect_start(session_t *ps) { ps->redirected = true; ps->first_frame = true; - // Re-detect driver since we now have a backend - ps->drivers = detect_driver(ps->c.c, ps->backend_data, ps->c.screen_info->root); - apply_driver_workarounds(ps, ps->drivers); - root_damaged(ps); // Repaint the whole screen From c9840c1a7bd2d5196be7d689fadc5f490b85c5fb Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 19 Dec 2023 09:53:57 +0000 Subject: [PATCH 12/13] core: make missing dpms extension non-fatal We are not using it for anything at the moment, and it is breaking CI. Signed-off-by: Yuxuan Shui --- src/picom.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/picom.c b/src/picom.c index a008f9f1b8..552cc1f9aa 100644 --- a/src/picom.c +++ b/src/picom.c @@ -2131,8 +2131,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ext_info = xcb_get_extension_data(ps->c.c, &xcb_dpms_id); ps->dpms_exists = ext_info && ext_info->present; if (!ps->dpms_exists) { - log_fatal("No DPMS extension"); - exit(1); + log_warn("No DPMS extension"); } // Parse configuration file From a5b2ceb55ce3874800387fcb2c6f121da5f66965 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 19 Dec 2023 11:07:55 +0000 Subject: [PATCH 13/13] core: disable frame pacing when vsync is disabled Signed-off-by: Yuxuan Shui --- src/picom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/picom.c b/src/picom.c index 552cc1f9aa..56fd63c712 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1476,7 +1476,7 @@ static bool redirect_start(session_t *ps) { pixman_region32_init(&ps->damage_ring[i]); } - ps->frame_pacing = !ps->o.no_frame_pacing; + ps->frame_pacing = !ps->o.no_frame_pacing && ps->o.vsync; if ((ps->o.legacy_backends || ps->o.benchmark || !ps->backend_data->ops->last_render_time) && ps->frame_pacing) { // Disable frame pacing if we are using a legacy backend or if we are in