From 11840cb658a20a8ac570351f2eb48ea51c2d5ae6 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Sat, 21 Jan 2023 18:16:52 +0300 Subject: [PATCH 01/37] options: allow use of windows shaders for the egl backend and notify a user better if the shader interface is not available --- src/options.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/options.c b/src/options.c index e69bbb0223..238b636ee6 100644 --- a/src/options.c +++ b/src/options.c @@ -787,14 +787,13 @@ bool get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, } if (opt->window_shader_fg || opt->window_shader_fg_rules) { - if (opt->legacy_backends || opt->backend != BKEND_GLX) { - log_warn("The new window shader interface does not work with the " - "legacy glx backend.%s", - (opt->backend == BKEND_GLX) ? " You may want to use " - "\"--glx-fshader-win\" " - "instead on the legacy " - "glx backend." - : ""); + if (opt->backend == BKEND_XRENDER || opt->legacy_backends) { + log_warn(opt->backend == BKEND_XRENDER + ? "Shader interface is not available for the " + "xrender backend." + : "The new shader interface is not available for " + "the legacy glx backend. You may want to use " + "--glx-fshader-win instead."); opt->window_shader_fg = NULL; c2_list_free(&opt->window_shader_fg_rules, free); } From ce7758a07b93a48fb272f22695fe421b07e70ace Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Mon, 13 Feb 2023 02:23:53 +0300 Subject: [PATCH 02/37] backend: fix leak in default_backend_render_shadow --- src/backend/backend_common.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/backend/backend_common.c b/src/backend/backend_common.c index d6fcce21ce..ecdefa4cf5 100644 --- a/src/backend/backend_common.c +++ b/src/backend/backend_common.c @@ -294,13 +294,14 @@ bool build_shadow(xcb_connection_t *c, xcb_drawable_t d, double opacity, const i void *default_backend_render_shadow(backend_t *backend_data, int width, int height, struct backend_shadow_context *sctx, struct color color) { const conv *kernel = (void *)sctx; - xcb_pixmap_t shadow_pixel = solid_picture(backend_data->c, backend_data->root, true, - 1, color.red, color.green, color.blue), - shadow = XCB_NONE; + xcb_render_picture_t shadow_pixel = solid_picture( + backend_data->c, backend_data->root, true, 1, color.red, color.green, color.blue); + xcb_pixmap_t shadow = XCB_NONE; xcb_render_picture_t pict = XCB_NONE; if (!build_shadow(backend_data->c, backend_data->root, color.alpha, width, height, kernel, shadow_pixel, &shadow, &pict)) { + xcb_render_free_picture(backend_data->c, shadow_pixel); return NULL; } @@ -308,6 +309,7 @@ void *default_backend_render_shadow(backend_t *backend_data, int width, int heig void *ret = backend_data->ops->bind_pixmap( backend_data, shadow, x_get_visual_info(backend_data->c, visual), true); xcb_render_free_picture(backend_data->c, pict); + xcb_render_free_picture(backend_data->c, shadow_pixel); return ret; } From 6d459badbcb6b31ae622df329fa663b5b3293f48 Mon Sep 17 00:00:00 2001 From: oofsauce Date: Wed, 29 Mar 2023 21:18:41 +0100 Subject: [PATCH 03/37] Add corner-radius-rules configuration option This option accepts a list of patterns and overrides the corner radii of matching windows Authored-by: oofsauce Signed-off-by: Yuxuan Shui --- man/picom.1.asciidoc | 3 +++ src/config.c | 23 ++++++++++++++--------- src/config.h | 4 +++- src/config_libconfig.c | 37 +++++++++++++++++++++++++++++++++---- src/options.c | 9 ++++++++- src/win.c | 19 +++++++++++++++---- 6 files changed, 76 insertions(+), 19 deletions(-) diff --git a/man/picom.1.asciidoc b/man/picom.1.asciidoc index e7a91315f2..0a2e3c31a5 100644 --- a/man/picom.1.asciidoc +++ b/man/picom.1.asciidoc @@ -103,6 +103,9 @@ OPTIONS *--corner-radius* 'VALUE':: Sets the radius of rounded window corners. When > 0, the compositor will round the corners of windows. Does not interact well with *--transparent-clipping*. (defaults to 0). +*--corner-radius-rules* 'RADIUS':'CONDITION':: + Specify a list of corner radius rules. Overrides the corner radii of matching windows. This option takes precedence over the *--rounded-corners-exclude* option, and also overrides the default exclusion of fullscreen windows. The condition has the same format as *--opacity-rule*. + *--rounded-corners-exclude* 'CONDITION':: Exclude conditions for rounded corners. diff --git a/src/config.c b/src/config.c index dd231470ee..1013d128fa 100644 --- a/src/config.c +++ b/src/config.c @@ -509,32 +509,37 @@ bool parse_geometry(session_t *ps, const char *src, region_t *dest) { } /** - * Parse a list of opacity rules. + * Parse a list of window rules, prefixed with a number, separated by a ':' */ -bool parse_rule_opacity(c2_lptr_t **res, const char *src) { - // Find opacity value +bool parse_numeric_window_rule(c2_lptr_t **res, const char *src, long min, long max) { + if (!src) { + return false; + } + + // Find numeric value char *endptr = NULL; long val = strtol(src, &endptr, 0); if (!endptr || endptr == src) { - log_error("No opacity specified: %s", src); + log_error("No number specified: %s", src); return false; } - if (val > 100 || val < 0) { - log_error("Opacity %ld invalid: %s", val, src); + + if (val < min || val > max) { + log_error("Number not in range (%ld <= n <= %ld): %s", min, max, src); return false; } // Skip over spaces - while (*endptr && isspace((unsigned char)*endptr)) + while (*endptr && isspace((unsigned char)*endptr)) { ++endptr; + } if (':' != *endptr) { - log_error("Opacity terminator not found: %s", src); + log_error("Number separator (':') not found: %s", src); return false; } ++endptr; // Parse pattern - // I hope 1-100 is acceptable for (void *) return c2_parse(res, endptr, (void *)val); } diff --git a/src/config.h b/src/config.h index 632800c32e..5ce3ba9635 100644 --- a/src/config.h +++ b/src/config.h @@ -228,6 +228,8 @@ typedef struct options { int corner_radius; /// Rounded corners blacklist. A linked list of conditions. c2_lptr_t *rounded_corners_blacklist; + /// Rounded corner rules. A linked list of conditions. + c2_lptr_t *corner_radius_rules; // === Focus related === /// Whether to try to detect WM windows and mark them as focused. @@ -266,7 +268,7 @@ bool must_use parse_long(const char *, long *); bool must_use parse_int(const char *, int *); struct conv **must_use parse_blur_kern_lst(const char *, bool *hasneg, int *count); bool must_use parse_geometry(session_t *, const char *, region_t *); -bool must_use parse_rule_opacity(c2_lptr_t **, const char *); +bool must_use parse_numeric_window_rule(c2_lptr_t **, const char *, long, long); bool must_use parse_rule_window_shader(c2_lptr_t **, const char *, const char *); char *must_use locate_auxiliary_file(const char *scope, const char *path, const char *include_dir); diff --git a/src/config_libconfig.c b/src/config_libconfig.c index 589e139b63..63f05412bf 100644 --- a/src/config_libconfig.c +++ b/src/config_libconfig.c @@ -135,6 +135,32 @@ void parse_cfg_condlst(const config_t *pcfg, c2_lptr_t **pcondlst, const char *n } } +/** + * Parse a window corner radius rule list in configuration file. + */ +static inline void +parse_cfg_condlst_corner(options_t *opt, const config_t *pcfg, const char *name) { + config_setting_t *setting = config_lookup(pcfg, name); + if (setting) { + // Parse an array of options + if (config_setting_is_array(setting)) { + int i = config_setting_length(setting); + while (i--) + if (!parse_numeric_window_rule( + &opt->corner_radius_rules, + config_setting_get_string_elem(setting, i), 0, INT_MAX)) + exit(1); + } + // Treat it as a single pattern if it's a string + else if (config_setting_type(setting) == CONFIG_TYPE_STRING) { + if (!parse_numeric_window_rule(&opt->corner_radius_rules, + config_setting_get_string(setting), + 0, INT_MAX)) + exit(1); + } + } +} + /** * Parse an opacity rule list in configuration file. */ @@ -146,15 +172,15 @@ parse_cfg_condlst_opct(options_t *opt, const config_t *pcfg, const char *name) { if (config_setting_is_array(setting)) { int i = config_setting_length(setting); while (i--) - if (!parse_rule_opacity( + if (!parse_numeric_window_rule( &opt->opacity_rules, - config_setting_get_string_elem(setting, i))) + config_setting_get_string_elem(setting, i), 0, 100)) exit(1); } // Treat it as a single pattern if it's a string else if (config_setting_type(setting) == CONFIG_TYPE_STRING) { - if (!parse_rule_opacity(&opt->opacity_rules, - config_setting_get_string(setting))) + if (!parse_numeric_window_rule( + &opt->opacity_rules, config_setting_get_string(setting), 0, 100)) exit(1); } } @@ -334,6 +360,9 @@ char *parse_config_libconfig(options_t *opt, const char *config_file, bool *shad config_lookup_int(&cfg, "corner-radius", &opt->corner_radius); // --rounded-corners-exclude parse_cfg_condlst(&cfg, &opt->rounded_corners_blacklist, "rounded-corners-exclude"); + // --corner-radius-rules + parse_cfg_condlst_corner(opt, &cfg, "corner-radius-rules"); + // -e (frame_opacity) config_lookup_float(&cfg, "frame-opacity", &opt->frame_opacity); // -c (shadow_enable) diff --git a/src/options.c b/src/options.c index d2020c6ab8..9d500b8e65 100644 --- a/src/options.c +++ b/src/options.c @@ -161,6 +161,7 @@ static const struct picom_option picom_options[] = { {"corner-radius" , required_argument, 333, NULL , "Sets the radius of rounded window corners. When > 0, the compositor will " "round the corners of windows. (defaults to 0)."}, {"rounded-corners-exclude" , required_argument, 334, "COND" , "Exclude conditions for rounded corners."}, + {"corner-radius-rules" , required_argument, 340, "RADIUS:COND" , "Window rules for specific rounded corner radii."}, {"clip-shadow-above" , required_argument, 335, NULL , "Specify a list of conditions of windows to not paint a shadow over, such " "as a dock window."}, {"window-shader-fg" , required_argument, 336, "PATH" , "Specify GLSL fragment shader path for rendering window contents. Does not" @@ -174,6 +175,7 @@ static const struct picom_option picom_options[] = { {"dithered-present" , no_argument , 339, NULL , "Use higher precision during rendering, and apply dither when presenting the " "rendered screen. Reduces banding artifacts, but might cause performance " "degradation. Only works with OpenGL."}, + // 340 is corner-radius-rules {"legacy-backends" , no_argument , 733, NULL , "Use deprecated version of the backends."}, {"monitor-repaint" , no_argument , 800, NULL , "Highlight the updated area of the screen. For debugging."}, {"diagnostics" , no_argument , 801, NULL , "Print diagnostic information"}, @@ -619,7 +621,7 @@ bool get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, break; case 304: // --opacity-rule - if (!parse_rule_opacity(&opt->opacity_rules, optarg)) + if (!parse_numeric_window_rule(&opt->opacity_rules, optarg, 0, 100)) exit(1); break; case 305: @@ -723,6 +725,11 @@ bool get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, // --rounded-corners-exclude condlst_add(&opt->rounded_corners_blacklist, optarg); break; + case 340: + // --corner-radius-rules + if (!parse_numeric_window_rule(&opt->corner_radius_rules, optarg, 0, INT_MAX)) + exit(1); + break; case 335: // --clip-shadow-above condlst_add(&opt->shadow_clip_list, optarg); diff --git a/src/win.c b/src/win.c index a52b4e1a37..a97733dbd4 100644 --- a/src/win.c +++ b/src/win.c @@ -1139,13 +1139,24 @@ static void win_determine_rounded_corners(session_t *ps, struct managed_win *w) return; } - // Don't round full screen windows & excluded windows - if ((w && win_is_fullscreen(ps, w)) || - c2_match(ps, w, ps->o.rounded_corners_blacklist, NULL)) { + void *radius_override = NULL; + if (c2_match(ps, w, ps->o.corner_radius_rules, &radius_override)) { + log_debug("Matched corner rule! %d", w->corner_radius); + } + + // Don't round full screen windows & excluded windows, + // unless we find a corner override in corner_radius_rules + if (!radius_override && ((w && win_is_fullscreen(ps, w)) || + c2_match(ps, w, ps->o.rounded_corners_blacklist, NULL))) { w->corner_radius = 0; log_debug("Not rounding corners for window %#010x", w->base.id); } else { - w->corner_radius = ps->o.corner_radius; + if (radius_override) { + w->corner_radius = (int)(long)radius_override; + } else { + w->corner_radius = ps->o.corner_radius; + } + log_debug("Rounding corners for window %#010x", w->base.id); // Initialize the border color to an invalid value w->border_col[0] = w->border_col[1] = w->border_col[2] = From 379f67f5c289ec84ca712c3ea66a2ccc440bee7f Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 1 Apr 2023 11:27:05 +0100 Subject: [PATCH 04/37] tests: add corner-radius-rules to the parsing test Signed-off-by: Yuxuan Shui --- tests/configs/parsing_test.conf | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/configs/parsing_test.conf b/tests/configs/parsing_test.conf index 2476d49a6c..7b6acd310a 100644 --- a/tests/configs/parsing_test.conf +++ b/tests/configs/parsing_test.conf @@ -155,6 +155,11 @@ rounded-corners-exclude = [ "window_type = 'desktop'" ]; +corner-radius-rules = [ + "10:window_type = 'dock'", + "0x32:window_type = 'desktop'" +]; + ################################# # Background-Blurring # From 3aed5599c3f73cbfa53b0249795e76ab07cf9ecd Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 8 May 2023 14:58:20 +0100 Subject: [PATCH 05/37] glx: calculate residual in both directions We only considered residual in the positive direction, i.e. we would set dithering to zero if the color is (whole number + 0.0001). But we should also set dithering to zero if color is (whole number - 0.0001). Fixes #1064 --- src/backend/gl/shaders.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/gl/shaders.c b/src/backend/gl/shaders.c index 90f636b7e7..96e385b192 100644 --- a/src/backend/gl/shaders.c +++ b/src/backend/gl/shaders.c @@ -202,7 +202,8 @@ const char dither_glsl[] = GLSL(330, } vec4 dither(vec4 c, vec2 coord) { vec4 residual = mod(c, 1.0 / 255.0); - vec4 dithered = vec4(greaterThan(residual, vec4(1e-4))); + residual = min(residual, vec4(1.0 / 255.0) - residual); + vec4 dithered = vec4(greaterThan(residual, vec4(1.0 / 65535.0))); return vec4(c + dithered * bayer(coord) / 255.0); } ); From 62fcfe5d1a67ea88ad7356964153c31218b7c62d Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 12 Dec 2022 10:41:54 +0000 Subject: [PATCH 06/37] backend: remove unused parameter 'ignore_damage' Signed-off-by: Yuxuan Shui --- src/backend/backend.c | 9 ++------- src/backend/backend.h | 3 +-- src/picom.c | 4 ++-- src/render.c | 4 ++-- src/render.h | 2 +- 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/backend/backend.c b/src/backend/backend.c index 59ddcaf2a0..3543ee127e 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -81,7 +81,7 @@ void handle_device_reset(session_t *ps) { } /// paint all windows -void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) { +void paint_all_new(session_t *ps, struct managed_win *t) { if (ps->backend_data->ops->device_status && ps->backend_data->ops->device_status(ps->backend_data) != DEVICE_STATUS_NORMAL) { return handle_device_reset(ps); @@ -100,12 +100,7 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) { // the paints bleed out of the damage region, it will destroy // part of the image we want to reuse region_t reg_damage; - if (!ignore_damage) { - reg_damage = get_damage(ps, ps->o.monitor_repaint || !ps->o.use_damage); - } else { - pixman_region32_init(®_damage); - pixman_region32_copy(®_damage, &ps->screen_reg); - } + reg_damage = get_damage(ps, ps->o.monitor_repaint || !ps->o.use_damage); if (!pixman_region32_not_empty(®_damage)) { pixman_region32_fini(®_damage); diff --git a/src/backend/backend.h b/src/backend/backend.h index b34b96d5d1..ef1bcb8800 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -363,5 +363,4 @@ struct backend_operations { extern struct backend_operations *backend_list[]; -void paint_all_new(session_t *ps, struct managed_win *const t, bool ignore_damage) - attr_nonnull(1); +void paint_all_new(session_t *ps, struct managed_win *const t) attr_nonnull(1); diff --git a/src/picom.c b/src/picom.c index 8a7a967039..d31c6634d1 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1524,9 +1524,9 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { log_trace("Render start, frame %d", paint); if (!ps->o.legacy_backends) { - paint_all_new(ps, bottom, false); + paint_all_new(ps, bottom); } else { - paint_all(ps, bottom, false); + paint_all(ps, bottom); } log_trace("Render end"); diff --git a/src/render.c b/src/render.c index afde7db82b..80cb59a6fa 100644 --- a/src/render.c +++ b/src/render.c @@ -969,7 +969,7 @@ win_blur_background(session_t *ps, struct managed_win *w, xcb_render_picture_t t /// paint all windows /// region = ?? /// region_real = the damage region -void paint_all(session_t *ps, struct managed_win *t, bool ignore_damage) { +void paint_all(session_t *ps, struct managed_win *t) { if (ps->o.xrender_sync_fence || (ps->drivers & DRIVER_NVIDIA)) { if (ps->xsync_exists && !x_fence_sync(ps->c, ps->sync_fence)) { log_error("x_fence_sync failed, xrender-sync-fence will be " @@ -984,7 +984,7 @@ void paint_all(session_t *ps, struct managed_win *t, bool ignore_damage) { region_t region; pixman_region32_init(®ion); int buffer_age = get_buffer_age(ps); - if (buffer_age == -1 || buffer_age > ps->ndamage || ignore_damage) { + if (buffer_age == -1 || buffer_age > ps->ndamage) { pixman_region32_copy(®ion, &ps->screen_reg); } else { for (int i = 0; i < get_buffer_age(ps); i++) { diff --git a/src/render.h b/src/render.h index 95a46dbee0..249f7bf255 100644 --- a/src/render.h +++ b/src/render.h @@ -37,7 +37,7 @@ void render(session_t *ps, int x, int y, int dx, int dy, int w, int h, int fullw const glx_prog_main_t *pprogram, clip_t *clip); void paint_one(session_t *ps, struct managed_win *w, const region_t *reg_paint); -void paint_all(session_t *ps, struct managed_win *const t, bool ignore_damage); +void paint_all(session_t *ps, struct managed_win *const t); void free_picture(xcb_connection_t *c, xcb_render_picture_t *p); From 64a97b57b5c872555c0a8f74f8e83551618c4eb5 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 13 Dec 2022 20:31:32 +0000 Subject: [PATCH 07/37] core: collect frame timing information. Needed to estimate refresh rate, and for us to stay in phase with vblank. Signed-off-by: Yuxuan Shui --- src/common.h | 9 ++++++ src/picom.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils.h | 4 +-- 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/common.h b/src/common.h index 8f307dc9fd..99ccc8e17c 100644 --- a/src/common.h +++ b/src/common.h @@ -240,6 +240,15 @@ typedef struct session { bool first_frame; /// Whether screen has been turned off bool screen_is_off; + /// Event context for X Present extension. + uint32_t present_event_id; + xcb_special_event_t *present_event; + /// Last MSC event, in useconds. + uint64_t last_msc; + /// When did we render our last frame. + uint64_t last_render; + + struct rolling_avg *frame_time; // === Operation related === /// Flags related to the root window diff --git a/src/picom.c b/src/picom.c index d31c6634d1..b37ea54da3 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1291,6 +1291,24 @@ static bool redirect_start(session_t *ps) { pixman_region32_init(&ps->damage_ring[i]); } + if (ps->present_exists) { + ps->present_event_id = x_new_id(ps->c); + auto select_input = xcb_present_select_input( + ps->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, session_get_target_window(ps), 0, 0, 1, 0); + set_cant_fail_cookie(ps, select_input); + set_cant_fail_cookie(ps, notify_msc); + ps->present_event = xcb_register_for_special_xge( + ps->c, &xcb_present_id, ps->present_event_id, NULL); + + // Initialize rendering and frame timing statistics. + ps->last_msc = 0; + ps->last_render = 0; + rolling_avg_reset(ps->frame_time); + } + // Must call XSync() here x_sync(ps->c); @@ -1332,6 +1350,14 @@ 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, ps->present_event_id, + session_get_target_window(ps), 0); + ps->present_event_id = XCB_NONE; + xcb_unregister_for_special_event(ps->c, ps->present_event); + ps->present_event = NULL; + } + // Must call XSync() here x_sync(ps->c); @@ -1339,9 +1365,53 @@ static void unredirect(session_t *ps) { log_debug("Screen unredirected."); } +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; + } + if (cne->ust <= ps->last_msc) { + return; + } + + auto cookie = xcb_present_notify_msc(ps->c, session_get_target_window(ps), 0, + cne->msc + 1, 0, 0); + set_cant_fail_cookie(ps, cookie); + + if (ps->last_msc != 0) { + int frame_time = (int)(cne->ust - ps->last_msc); + rolling_avg_push(ps->frame_time, frame_time); + log_trace("Frame time: %d us, rolling average: %lf us, msc: %" PRIu64, + frame_time, rolling_avg_get_avg(ps->frame_time), cne->ust); + } + ps->last_msc = cne->ust; +} + +static void handle_present_events(session_t *ps) { + if (!ps->present_event) { + return; + } + xcb_present_generic_event_t *ev; + while ((ev = (void *)xcb_poll_for_special_event(ps->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); + xcb_generic_event_t *ev; while ((ev = xcb_poll_for_queued_event(ps->c))) { ev_handle(ps, ev); @@ -1697,6 +1767,8 @@ static session_t *session_init(int argc, char **argv, Display *dpy, .white_picture = XCB_NONE, .shadow_context = NULL, + .last_msc = 0, + #ifdef CONFIG_VSYNC_DRM .drm_fd = -1, #endif @@ -1716,6 +1788,7 @@ 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, @@ -1743,6 +1816,9 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ps->loop = EV_DEFAULT; pixman_region32_init(&ps->screen_reg); + // TODO(yshui) investigate what's the best window size + ps->frame_time = rolling_avg_new(16); + ps->pending_reply_tail = &ps->pending_reply_head; ps->o.show_all_xerrors = all_xerrors; @@ -2378,6 +2454,8 @@ static void session_destroy(session_t *ps) { free(ps->o.glx_fshader_win_str); x_free_randr_info(ps); + rolling_avg_destroy(ps->frame_time); + // Release custom window shaders free(ps->o.window_shader_fg); struct shader_info *shader, *tmp; diff --git a/src/utils.h b/src/utils.h index 5fa92199bb..b7c452912e 100644 --- a/src/utils.h +++ b/src/utils.h @@ -292,14 +292,14 @@ int next_power_of_two(int n); struct rolling_max; struct rolling_max *rolling_max_new(int window_size); -void rolling_max_free(struct rolling_max *rm); +void rolling_max_destroy(struct rolling_max *rm); void rolling_max_reset(struct rolling_max *rm); void rolling_max_push(struct rolling_max *rm, int val); int rolling_max_get_max(struct rolling_max *rm); struct rolling_avg; struct rolling_avg *rolling_avg_new(int window_size); -void rolling_avg_free(struct rolling_avg *ra); +void rolling_avg_destroy(struct rolling_avg *ra); void rolling_avg_reset(struct rolling_avg *ra); void rolling_avg_push(struct rolling_avg *ra, int val); double rolling_avg_get_avg(struct rolling_avg *ra); From 2a4e32babf14564e9b0658be5765dc577a1164ea Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 08:07:36 +0000 Subject: [PATCH 08/37] backend: add last_render_time interface. Used for querying how long render takes to complete, used for frame pacing. Signed-off-by: Yuxuan Shui --- src/backend/backend.h | 8 ++++++++ src/backend/gl/egl.c | 2 ++ src/backend/gl/gl_common.c | 31 +++++++++++++++++++++++++++++++ src/backend/gl/gl_common.h | 4 ++++ src/backend/gl/glx.c | 2 ++ src/common.h | 1 + src/picom.c | 15 +++++++++++++++ 7 files changed, 63 insertions(+) diff --git a/src/backend/backend.h b/src/backend/backend.h index ef1bcb8800..7cd64a082b 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -292,6 +292,14 @@ struct backend_operations { /// Optional int (*buffer_age)(backend_t *backend_data); + /// Get the render time of the last frame. If the render is still in progress, + /// returns false. The time is returned in `ts`. Frames are delimited by the + /// present() calls. i.e. after a present() call, last_render_time() should start + /// reporting the time of the just presen1ted frame. + /// + /// Optional, if not available, the most conservative estimation will be used. + bool (*last_render_time)(backend_t *backend_data, struct timespec *ts); + /// The maximum number buffer_age might return. int max_buffer_age; diff --git a/src/backend/gl/egl.c b/src/backend/gl/egl.c index dd1d4037c3..770bd5d4ab 100644 --- a/src/backend/gl/egl.c +++ b/src/backend/gl/egl.c @@ -372,6 +372,7 @@ struct backend_operations egl_ops = { .deinit = egl_deinit, .bind_pixmap = egl_bind_pixmap, .release_image = gl_release_image, + .prepare = gl_prepare, .compose = gl_compose, .image_op = gl_image_op, .set_image_property = gl_set_image_property, @@ -380,6 +381,7 @@ struct backend_operations egl_ops = { .is_image_transparent = default_is_image_transparent, .present = egl_present, .buffer_age = egl_buffer_age, + .last_render_time = gl_last_render_time, .create_shadow_context = gl_create_shadow_context, .destroy_shadow_context = gl_destroy_shadow_context, .render_shadow = backend_render_shadow_from_mask, diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index 9789eb51cf..b02572dfad 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -22,6 +22,11 @@ #include "backend/backend_common.h" #include "backend/gl/gl_common.h" +void gl_prepare(backend_t *base, const region_t *reg attr_unused) { + auto gd = (struct gl_data *)base; + glBeginQuery(GL_TIME_ELAPSED, gd->frame_timing[gd->current_frame_timing]); +} + GLuint gl_create_shader(GLenum shader_type, const char *shader_str) { log_trace("===\n%s\n===", shader_str); @@ -800,6 +805,9 @@ uint64_t gl_get_shader_attributes(backend_t *backend_data attr_unused, void *sha } bool gl_init(struct gl_data *gd, session_t *ps) { + glGenQueries(2, gd->frame_timing); + gd->current_frame_timing = 0; + // Initialize GLX data structure glDisable(GL_DEPTH_TEST); glDepthMask(GL_FALSE); @@ -1154,10 +1162,33 @@ void gl_present(backend_t *base, const region_t *region) { glDeleteBuffers(2, bo); glDeleteVertexArrays(1, &vao); + glEndQuery(GL_TIME_ELAPSED); + gd->current_frame_timing ^= 1; + + gl_check_err(); + free(coord); free(indices); } +bool gl_last_render_time(backend_t *base, struct timespec *ts) { + auto gd = (struct gl_data *)base; + GLint available = 0; + glGetQueryObjectiv(gd->frame_timing[gd->current_frame_timing ^ 1], + GL_QUERY_RESULT_AVAILABLE, &available); + if (!available) { + return false; + } + + GLuint64 time; + glGetQueryObjectui64v(gd->frame_timing[gd->current_frame_timing ^ 1], + GL_QUERY_RESULT, &time); + ts->tv_sec = (long)(time / 1000000000); + ts->tv_nsec = (long)(time % 1000000000); + gl_check_err(); + return true; +} + bool gl_image_op(backend_t *base, enum image_operations op, void *image_data, const region_t *reg_op, const region_t *reg_visible attr_unused, void *arg) { struct backend_image *tex = image_data; diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index 4aad9c0d50..f7e721c289 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -108,6 +108,8 @@ struct gl_data { gl_shadow_shader_t shadow_shader; GLuint back_texture, back_fbo; GLint back_format; + GLuint frame_timing[2]; + int current_frame_timing; GLuint present_prog; bool dithered_present; @@ -129,6 +131,7 @@ typedef struct session session_t; #define GL_PROG_MAIN_INIT \ { .prog = 0, .unifm_opacity = -1, .unifm_invert_color = -1, .unifm_tex = -1, } +void gl_prepare(backend_t *base, const region_t *reg); void x_rect_to_coords(int nrects, const rect_t *rects, coord_t image_dst, int extent_height, int texture_height, int root_height, bool y_inverted, GLint *coord, GLuint *indices); @@ -142,6 +145,7 @@ void gl_destroy_window_shader(backend_t *backend_data, void *shader); uint64_t gl_get_shader_attributes(backend_t *backend_data, void *shader); bool gl_set_image_property(backend_t *backend_data, enum image_properties prop, void *image_data, void *args); +bool gl_last_render_time(backend_t *backend_data, struct timespec *time); /** * @brief Render a region with texture data. diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index 109bec9429..b4ace13388 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -528,6 +528,7 @@ struct backend_operations glx_ops = { .deinit = glx_deinit, .bind_pixmap = glx_bind_pixmap, .release_image = gl_release_image, + .prepare = gl_prepare, .compose = gl_compose, .image_op = gl_image_op, .set_image_property = gl_set_image_property, @@ -536,6 +537,7 @@ struct backend_operations glx_ops = { .is_image_transparent = default_is_image_transparent, .present = glx_present, .buffer_age = glx_buffer_age, + .last_render_time = gl_last_render_time, .create_shadow_context = gl_create_shadow_context, .destroy_shadow_context = gl_destroy_shadow_context, .render_shadow = backend_render_shadow_from_mask, diff --git a/src/common.h b/src/common.h index 99ccc8e17c..1684b6c27f 100644 --- a/src/common.h +++ b/src/common.h @@ -249,6 +249,7 @@ typedef struct session { uint64_t last_render; struct rolling_avg *frame_time; + struct rolling_max *render_stats; // === Operation related === /// Flags related to the root window diff --git a/src/picom.c b/src/picom.c index b37ea54da3..7d5ab2f2ec 100644 --- a/src/picom.c +++ b/src/picom.c @@ -159,6 +159,19 @@ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t void queue_redraw(session_t *ps) { // If --benchmark is used, redraw is always queued if (!ps->redraw_needed && !ps->o.benchmark) { + if (!ps->first_frame && ps->redirected && + ps->backend_data->ops->last_render_time) { + struct timespec render_time; + if (ps->backend_data->ops->last_render_time(ps->backend_data, + &render_time)) { + int render_time_us = (int)(render_time.tv_sec * 1000000 + + render_time.tv_nsec / 1000); + log_info( + "Last render call took: %d us, rolling_max: %d us", + render_time_us, rolling_max_get_max(ps->render_stats)); + rolling_max_push(ps->render_stats, render_time_us); + } + } ev_idle_start(ps->loop, &ps->draw_idle); } ps->redraw_needed = true; @@ -1817,6 +1830,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, pixman_region32_init(&ps->screen_reg); // TODO(yshui) investigate what's the best window size + ps->render_stats = rolling_max_new(128); ps->frame_time = rolling_avg_new(16); ps->pending_reply_tail = &ps->pending_reply_head; @@ -2455,6 +2469,7 @@ static void session_destroy(session_t *ps) { x_free_randr_info(ps); rolling_avg_destroy(ps->frame_time); + rolling_max_destroy(ps->render_stats); // Release custom window shaders free(ps->o.window_shader_fg); From 86d3739374a9cdc0f81f10ba3a6599fa40747dd5 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 13 Dec 2022 20:56:11 +0000 Subject: [PATCH 09/37] core: frame pacing Use frame timing and render time statistic to pace frames. Right now the criteria are simple: * Don't render multiple frames in one vblank cycle. Otherwise the rendered frame will be delay multiple cycles, which isn't ideal. * Start rendering as late as possible while still hitting vblank. Refresh rate is estimated from a rolling average of frame timing. Render time is predicted from the rolling maximum of past 128 frames. The window size still needs to be investigated. Remove glFinish calls and GL_MaxFramesAllowed=1, frame pacing superseeds them. Professionals might laugh at how rudimentary this is, but hopefully this is better than what we had before. Which is absolutely nothing at all. Signed-off-by: Yuxuan Shui --- src/backend/driver.c | 2 - src/backend/gl/egl.c | 3 - src/backend/gl/gl_common.c | 2 +- src/backend/gl/glx.c | 3 - src/common.h | 7 +- src/picom.c | 178 +++++++++++++++++++++++++++++++------ 6 files changed, 156 insertions(+), 39 deletions(-) diff --git a/src/backend/driver.c b/src/backend/driver.c index a41d2fdc1f..b909a62272 100644 --- a/src/backend/driver.c +++ b/src/backend/driver.c @@ -15,8 +15,6 @@ /// Apply driver specified global workarounds. It's safe to call this multiple times. void apply_driver_workarounds(struct session *ps, enum driver driver) { if (driver & DRIVER_NVIDIA) { - // setenv("__GL_YIELD", "usleep", true); - setenv("__GL_MaxFramesAllowed", "1", true); ps->o.xrender_sync_fence = true; } } diff --git a/src/backend/gl/egl.c b/src/backend/gl/egl.c index 770bd5d4ab..f1f67d91e6 100644 --- a/src/backend/gl/egl.c +++ b/src/backend/gl/egl.c @@ -320,9 +320,6 @@ static void egl_present(backend_t *base, const region_t *region attr_unused) { struct egl_data *gd = (void *)base; gl_present(base, region); eglSwapBuffers(gd->display, gd->target_win); - if (!gd->gl.is_nvidia) { - glFinish(); - } } static int egl_buffer_age(backend_t *base) { diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index b02572dfad..42f7136dc6 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -953,7 +953,7 @@ bool gl_init(struct gl_data *gd, session_t *ps) { const char *vendor = (const char *)glGetString(GL_VENDOR); log_debug("GL_VENDOR = %s", vendor); if (strcmp(vendor, "NVIDIA Corporation") == 0) { - log_info("GL vendor is NVIDIA, don't use glFinish"); + log_info("GL vendor is NVIDIA, enable xrender sync fence."); gd->is_nvidia = true; } else { gd->is_nvidia = false; diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index b4ace13388..80056c6798 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -468,9 +468,6 @@ static void glx_present(backend_t *base, const region_t *region attr_unused) { struct _glx_data *gd = (void *)base; gl_present(base, region); glXSwapBuffers(gd->display, gd->target_win); - if (!gd->gl.is_nvidia) { - glFinish(); - } } static int glx_buffer_age(backend_t *base) { diff --git a/src/common.h b/src/common.h index 1684b6c27f..e6bbff0624 100644 --- a/src/common.h +++ b/src/common.h @@ -156,9 +156,8 @@ typedef struct session { ev_timer unredir_timer; /// Timer for fading ev_timer fade_timer; - /// Use an ev_idle callback for drawing - /// So we only start drawing when events are processed - ev_idle draw_idle; + /// Use an ev_timer callback for drawing + ev_timer draw_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 @@ -247,6 +246,8 @@ typedef struct session { uint64_t last_msc; /// When did we render our last frame. uint64_t last_render; + /// Whether we can perform frame pacing. + bool frame_pacing; struct rolling_avg *frame_time; struct rolling_max *render_stats; diff --git a/src/picom.c b/src/picom.c index 7d5ab2f2ec..b5867413e9 100644 --- a/src/picom.c +++ b/src/picom.c @@ -156,23 +156,87 @@ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t return w; } +/// How many seconds into the future should we start rendering the next frame. +double next_frame_offset(session_t *ps) { + int render_time = rolling_max_get_max(ps->render_stats); + if (render_time < 0) { + // We don't have render time estimates, maybe there's no frame rendered + // yet, or the backend doesn't support render timing information, + // queue render immediately. + return 0; + } + int frame_time = (int)rolling_avg_get_avg(ps->frame_time); + auto next_msc = ps->last_msc + (uint64_t)frame_time; + auto deadline = next_msc - (uint64_t)render_time; + + const uint64_t slack = 1000; + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + auto now_us = (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_nsec / 1000; + if (now_us + slack >= deadline) { + // We are already late, render immediately. + log_trace("Already late, rendering immediately, last_msc: %" PRIu64 + ", render_time: %d, frame_time: %d, now_us: %" PRIu64, + ps->last_msc, render_time, frame_time, now_us); + return 0; + } + log_trace("Delay: %lf s, last_msc: %" PRIu64 ", render_time: %d, frame_time: %d, " + "now_us: %" PRIu64 ", next_msc: %" PRIu64, + (double)(deadline - now_us - slack) / 1000000.0, ps->last_msc, + render_time, frame_time, now_us, next_msc); + return (double)(deadline - now_us - slack) / 1000000.0; +} + +/// Update render stats. Return false if a frame is still being rendered. +static bool update_render_stats(session_t *ps) { + if (!ps->first_frame && ps->redirected && ps->backend_data->ops->last_render_time) { + struct timespec render_time; + if (ps->backend_data->ops->last_render_time(ps->backend_data, &render_time)) { + int render_time_us = + (int)(render_time.tv_sec * 1000000 + render_time.tv_nsec / 1000); + log_trace("Last render call took: %d us, " + "rolling_max: %d us", + render_time_us, rolling_max_get_max(ps->render_stats)); + rolling_max_push(ps->render_stats, render_time_us); + return true; + } + return false; + } + return true; +} + 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. + bool already_rendered = (ps->last_render > ps->last_msc) && ps->frame_pacing; + if (already_rendered) { + log_debug("Already rendered for the current frame, not queuing " + "redraw"); + } // If --benchmark is used, redraw is always queued - if (!ps->redraw_needed && !ps->o.benchmark) { - if (!ps->first_frame && ps->redirected && - ps->backend_data->ops->last_render_time) { - struct timespec render_time; - if (ps->backend_data->ops->last_render_time(ps->backend_data, - &render_time)) { - int render_time_us = (int)(render_time.tv_sec * 1000000 + - render_time.tv_nsec / 1000); - log_info( - "Last render call took: %d us, rolling_max: %d us", - render_time_us, rolling_max_get_max(ps->render_stats)); - rolling_max_push(ps->render_stats, render_time_us); + if (!ps->redraw_needed && !ps->o.benchmark && !already_rendered) { + double delay = 0; + if (ps->frame_pacing) { + if (!update_render_stats(ps)) { + // A frame is still being rendered. This means we missed + // previous vblank. We shouldn't queue a new frame until + // the next vblank. + log_debug("A frame is still being rendered, not queueing " + "redraw"); + ps->redraw_needed = true; + return; + } + // Our loop can be blocked by frame present, which cause ev_now to + // drift away from the real time. We need to correct it. + delay = next_frame_offset(ps) + ev_now(ps->loop) - ev_time(); + if (delay < 0) { + delay = 0; } } - ev_idle_start(ps->loop, &ps->draw_idle); + // Not doing frame pacing, just redraw immediately + ev_timer_set(&ps->draw_timer, delay, 0); + assert(!ev_is_active(&ps->draw_timer)); + ev_timer_start(ps->loop, &ps->draw_timer); } ps->redraw_needed = true; } @@ -1304,7 +1368,16 @@ static bool redirect_start(session_t *ps) { pixman_region32_init(&ps->damage_ring[i]); } - if (ps->present_exists) { + ps->frame_pacing = true; + if (ps->o.legacy_backends || ps->o.benchmark || + !ps->backend_data->ops->last_render_time) { + // Disable frame pacing if we are using a legacy backend or if we are in + // benchmark mode, or if the backend doesn't report render time + log_info("Disabling frame pacing."); + ps->frame_pacing = false; + } + + 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, ps->present_event_id, session_get_target_window(ps), @@ -1316,10 +1389,17 @@ static bool redirect_start(session_t *ps) { ps->present_event = xcb_register_for_special_xge( ps->c, &xcb_present_id, ps->present_event_id, NULL); - // Initialize rendering and frame timing statistics. + // Initialize rendering and frame timing statistics, and frame pacing + // states. + ps->last_msc_instant = 0; ps->last_msc = 0; ps->last_render = 0; + ps->target_msc = 0; rolling_avg_reset(ps->frame_time); + rolling_max_reset(ps->render_stats); + } else if (ps->frame_pacing) { + log_error("Present extension is not supported, frame pacing disabled."); + ps->frame_pacing = false; } // Must call XSync() here @@ -1383,6 +1463,7 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ if (cne->kind != XCB_PRESENT_COMPLETE_KIND_NOTIFY_MSC) { return; } + if (cne->ust <= ps->last_msc) { return; } @@ -1391,13 +1472,46 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ cne->msc + 1, 0, 0); set_cant_fail_cookie(ps, cookie); + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + uint64_t now_usec = (uint64_t)(now.tv_sec * 1000000 + now.tv_nsec / 1000); + uint64_t drift; + if (cne->ust > now_usec) { + drift = cne->ust - now_usec; + } else { + drift = now_usec - cne->ust; + } + if (ps->last_msc != 0) { int frame_time = (int)(cne->ust - ps->last_msc); rolling_avg_push(ps->frame_time, frame_time); - log_trace("Frame time: %d us, rolling average: %lf us, msc: %" PRIu64, - frame_time, rolling_avg_get_avg(ps->frame_time), cne->ust); + log_trace("Frame time: %d us, rolling average: %lf us, msc: %" PRIu64 + ", offset: %" PRIu64, + frame_time, rolling_avg_get_avg(ps->frame_time), cne->ust, drift); } ps->last_msc = cne->ust; + if (drift > 1000000 && ps->frame_pacing) { + log_error("Temporal anomaly detected, frame pacing disabled. (Are we " + "running inside a time namespace?), %" PRIu64 " %" PRIu64, + now_usec, ps->last_msc); + ps->frame_pacing = false; + // We could have deferred a frame in queue_redraw() because of frame + // pacing. Unconditionally queue a frame for simplicity. + queue_redraw(ps); + } + if (ps->frame_pacing && ps->redraw_needed && !ev_is_active(&ps->draw_timer)) { + log_trace("Frame pacing: queueing redraw"); + // We deferred a frame in queue_redraw() because of frame pacing. Schedule + // it now. + if (!update_render_stats(ps)) { + log_warn("A frame is still being rendered, not queueing redraw."); + } else { + + ev_timer_set(&ps->draw_timer, + next_frame_offset(ps) + ev_now(ps->loop) - ev_time(), 0); + ev_timer_start(ps->loop, &ps->draw_timer); + } + } } static void handle_present_events(session_t *ps) { @@ -1613,6 +1727,12 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { } log_trace("Render end"); + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + uint64_t now_us = + (uint64_t)now.tv_sec * 1000000ULL + (uint64_t)now.tv_nsec / 1000; + ps->last_render = now_us; + ps->first_frame = false; paint++; if (ps->o.benchmark && paint >= ps->o.benchmark) { @@ -1627,18 +1747,21 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { // 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; } -static void draw_callback(EV_P_ ev_idle *w, int revents) { - session_t *ps = session_ptr(w, draw_idle); +static void draw_callback(EV_P_ ev_timer *w, int revents) { + session_t *ps = session_ptr(w, draw_timer); draw_callback_impl(EV_A_ ps, revents); + ev_timer_stop(EV_A_ w); - // Don't do painting non-stop unless we are in benchmark mode, or if - // draw_callback_impl thinks we should continue painting. - if (!ps->o.benchmark && !ps->redraw_needed) { - ev_idle_stop(EV_A_ & ps->draw_idle); + // Immediately start next frame if we are in benchmark mode. + if (ps->o.benchmark) { + ev_timer_set(w, 0, 0); + ev_timer_start(EV_A_ w); } } @@ -2252,7 +2375,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ev_io_init(&ps->xiow, x_event_callback, ConnectionNumber(ps->dpy), EV_READ); ev_io_start(ps->loop, &ps->xiow); ev_init(&ps->unredir_timer, tmout_unredir_callback); - ev_idle_init(&ps->draw_idle, draw_callback); + ev_init(&ps->draw_timer, draw_callback); ev_init(&ps->fade_timer, fade_timer_callback); @@ -2550,7 +2673,7 @@ static void session_destroy(session_t *ps) { ev_timer_stop(ps->loop, &ps->unredir_timer); ev_timer_stop(ps->loop, &ps->fade_timer); ev_timer_stop(ps->loop, &ps->dpms_check_timer); - ev_idle_stop(ps->loop, &ps->draw_idle); + ev_timer_stop(ps->loop, &ps->draw_timer); ev_prepare_stop(ps->loop, &ps->event_check); ev_signal_stop(ps->loop, &ps->usr1_signal); ev_signal_stop(ps->loop, &ps->int_signal); @@ -2562,9 +2685,10 @@ static void session_destroy(session_t *ps) { * @param ps current session */ static void session_run(session_t *ps) { - // In benchmark mode, we want draw_idle handler to always be active + // In benchmark mode, we want draw_timer handler to always be active if (ps->o.benchmark) { - ev_idle_start(ps->loop, &ps->draw_idle); + ev_timer_set(&ps->draw_timer, 0, 0); + ev_timer_start(ps->loop, &ps->draw_timer); } else { // Let's draw our first frame! queue_redraw(ps); From 47ffaf0a38e041c7df9084149ccae1bd812bf74e Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Dec 2022 09:53:31 +0000 Subject: [PATCH 10/37] core: workaround X present event quirk When the screen turns off, X sometimes sends present complete notify for the same frame multiple times, or even events with invalid msc/ust number. This will cause us to ignore it and not send a subsequent NotifyMsc request, causing the complete notify to stop. Now we send NotifyMsc regardless to keep the events going. Also detect when the complete notifies skip frames, divide the interval by frame count to estimate frame time in that case. Upstream bug report: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 Signed-off-by: Yuxuan Shui --- src/common.h | 4 +++- src/picom.c | 38 +++++++++++++++++++++++--------------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/common.h b/src/common.h index e6bbff0624..c3d5f82986 100644 --- a/src/common.h +++ b/src/common.h @@ -242,7 +242,9 @@ typedef struct session { /// Event context for X Present extension. uint32_t present_event_id; xcb_special_event_t *present_event; - /// Last MSC event, in useconds. + /// When last MSC event happened, in useconds. + uint64_t last_msc_instant; + /// The last MSC number uint64_t last_msc; /// When did we render our last frame. uint64_t last_render; diff --git a/src/picom.c b/src/picom.c index b5867413e9..d670fadfe6 100644 --- a/src/picom.c +++ b/src/picom.c @@ -166,7 +166,7 @@ double next_frame_offset(session_t *ps) { return 0; } int frame_time = (int)rolling_avg_get_avg(ps->frame_time); - auto next_msc = ps->last_msc + (uint64_t)frame_time; + auto next_msc = ps->last_msc_instant + (uint64_t)frame_time; auto deadline = next_msc - (uint64_t)render_time; const uint64_t slack = 1000; @@ -177,12 +177,12 @@ double next_frame_offset(session_t *ps) { // We are already late, render immediately. log_trace("Already late, rendering immediately, last_msc: %" PRIu64 ", render_time: %d, frame_time: %d, now_us: %" PRIu64, - ps->last_msc, render_time, frame_time, now_us); + ps->last_msc_instant, render_time, frame_time, now_us); return 0; } log_trace("Delay: %lf s, last_msc: %" PRIu64 ", render_time: %d, frame_time: %d, " "now_us: %" PRIu64 ", next_msc: %" PRIu64, - (double)(deadline - now_us - slack) / 1000000.0, ps->last_msc, + (double)(deadline - now_us - slack) / 1000000.0, ps->last_msc_instant, render_time, frame_time, now_us, next_msc); return (double)(deadline - now_us - slack) / 1000000.0; } @@ -208,7 +208,7 @@ static bool update_render_stats(session_t *ps) { 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. - bool already_rendered = (ps->last_render > ps->last_msc) && ps->frame_pacing; + bool already_rendered = (ps->last_render > ps->last_msc_instant) && ps->frame_pacing; if (already_rendered) { log_debug("Already rendered for the current frame, not queuing " "redraw"); @@ -1464,14 +1464,18 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ return; } - if (cne->ust <= ps->last_msc) { - return; - } - auto cookie = xcb_present_notify_msc(ps->c, session_get_target_window(ps), 0, cne->msc + 1, 0, 0); set_cant_fail_cookie(ps, cookie); + if (cne->msc <= ps->last_msc || cne->ust == 0) { + // X sometimes sends duplicate/bogus MSC events, ignore them + // + // See: + // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 + return; + } + struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); uint64_t now_usec = (uint64_t)(now.tv_sec * 1000000 + now.tv_nsec / 1000); @@ -1482,18 +1486,21 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ drift = now_usec - cne->ust; } - if (ps->last_msc != 0) { - int frame_time = (int)(cne->ust - ps->last_msc); + 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); rolling_avg_push(ps->frame_time, frame_time); - log_trace("Frame time: %d us, rolling average: %lf us, msc: %" PRIu64 - ", offset: %" PRIu64, - frame_time, rolling_avg_get_avg(ps->frame_time), cne->ust, drift); + log_trace("Frame count %lu, frame time: %d us, rolling average: %lf us, " + "msc: %" PRIu64 ", offset: %" PRIu64, + frame_count, frame_time, rolling_avg_get_avg(ps->frame_time), + cne->ust, drift); } - ps->last_msc = cne->ust; + ps->last_msc_instant = cne->ust; + ps->last_msc = cne->msc; if (drift > 1000000 && ps->frame_pacing) { log_error("Temporal anomaly detected, frame pacing disabled. (Are we " "running inside a time namespace?), %" PRIu64 " %" PRIu64, - now_usec, ps->last_msc); + now_usec, ps->last_msc_instant); ps->frame_pacing = false; // We could have deferred a frame in queue_redraw() because of frame // pacing. Unconditionally queue a frame for simplicity. @@ -1516,6 +1523,7 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ static void handle_present_events(session_t *ps) { if (!ps->present_event) { + // Screen not redirected return; } xcb_present_generic_event_t *ev; From a4fae2b60a576d0c50b4134ff0c65d47a5e9fa30 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Dec 2022 17:24:14 +0000 Subject: [PATCH 11/37] core: better frame pacing function Details explained in the comments on schedule_render(). Signed-off-by: Yuxuan Shui --- src/common.h | 3 + src/picom.c | 173 ++++++++++++++++++++++++++++----------------------- 2 files changed, 97 insertions(+), 79 deletions(-) diff --git a/src/common.h b/src/common.h index c3d5f82986..b76863df15 100644 --- a/src/common.h +++ b/src/common.h @@ -246,6 +246,9 @@ typedef struct session { uint64_t last_msc_instant; /// The last MSC number uint64_t last_msc; + /// When the currently rendered frame will be displayed. + /// 0 means there is no pending frame. + uint64_t target_msc; /// When did we render our last frame. uint64_t last_render; /// Whether we can perform frame pacing. diff --git a/src/picom.c b/src/picom.c index d670fadfe6..25d22128bf 100644 --- a/src/picom.c +++ b/src/picom.c @@ -157,86 +157,111 @@ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t } /// How many seconds into the future should we start rendering the next frame. -double next_frame_offset(session_t *ps) { - int render_time = rolling_max_get_max(ps->render_stats); - if (render_time < 0) { - // We don't have render time estimates, maybe there's no frame rendered - // yet, or the backend doesn't support render timing information, - // queue render immediately. - return 0; +/// +/// Renders are scheduled like this: +/// +/// 1. queue_redraw() registers the intention to render. redraw_needed is set to true to +/// indicate what is on screen needs to be updated. +/// 2. then, we need to figure out the best time to start rendering. first, we need to +/// know when the next frame will be displayed on screen. we have this information from +/// the Present extension: we know when was the last frame displayed, and we know the +/// refresh rate. so we can calculate the next frame's display time. if our render time +/// estimation shows we could miss that target, we push the target back one frame. +/// 3. if there is already render completed for that target frame, or there is a render +/// currently underway, we don't do anything, and wait for the next Present Complete +/// Notify event to try to schedule again. +/// 4. otherwise, we schedule a render for that target frame. we use past statistics about +/// how long our renders took to figure out when to start rendering. we start rendering +/// at the latest point of time possible to still hit the target frame. +void schedule_render(session_t *ps) { + double delay_s = 0; + if (!ps->frame_pacing || !ps->redirected) { + // Not doing frame pacing, schedule a render immediately, if not already + // scheduled. + // 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)) { + // We don't know the msc, so we set it to 1, because 0 is a + // special value + ps->target_msc = 1; + goto schedule; + } + return; + } + struct timespec render_time; + bool completed = + ps->backend_data->ops->last_render_time(ps->backend_data, &render_time); + if (!completed || ev_is_active(&ps->draw_timer)) { + // There is already a render underway (either just scheduled, or is + // rendered but awaiting completion), don't schedule another one. + if (ps->target_msc <= ps->last_msc) { + log_debug("Target frame %ld is in the past, but we are still " + "rendering", + ps->target_msc); + // We missed our target, push it back one frame + ps->target_msc = ps->last_msc + 1; + } + return; + } + if (ps->target_msc > ps->last_msc) { + // Render for the target frame is completed, and has yet to be displayed. + // Don't schedule another render. + return; } - int frame_time = (int)rolling_avg_get_avg(ps->frame_time); - auto next_msc = ps->last_msc_instant + (uint64_t)frame_time; - auto deadline = next_msc - (uint64_t)render_time; - const uint64_t slack = 1000; struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); auto now_us = (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_nsec / 1000; - if (now_us + slack >= deadline) { - // We are already late, render immediately. - log_trace("Already late, rendering immediately, last_msc: %" PRIu64 - ", render_time: %d, frame_time: %d, now_us: %" PRIu64, - ps->last_msc_instant, render_time, frame_time, now_us); - return 0; - } - log_trace("Delay: %lf s, last_msc: %" PRIu64 ", render_time: %d, frame_time: %d, " - "now_us: %" PRIu64 ", next_msc: %" PRIu64, - (double)(deadline - now_us - slack) / 1000000.0, ps->last_msc_instant, - render_time, frame_time, now_us, next_msc); - return (double)(deadline - now_us - slack) / 1000000.0; -} -/// Update render stats. Return false if a frame is still being rendered. -static bool update_render_stats(session_t *ps) { - if (!ps->first_frame && ps->redirected && ps->backend_data->ops->last_render_time) { - struct timespec render_time; - if (ps->backend_data->ops->last_render_time(ps->backend_data, &render_time)) { - int render_time_us = - (int)(render_time.tv_sec * 1000000 + render_time.tv_nsec / 1000); - log_trace("Last render call took: %d us, " - "rolling_max: %d us", - render_time_us, rolling_max_get_max(ps->render_stats)); - rolling_max_push(ps->render_stats, render_time_us); - return true; - } - return false; + int render_time_us = + (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); + if (ps->target_msc == ps->last_msc) { + // The frame has just been displayed, record its render time; + log_trace("Last render call took: %d us, rolling_max: %d us", + render_time_us, rolling_max_get_max(ps->render_stats)); + rolling_max_push(ps->render_stats, render_time_us); + ps->target_msc = 0; } - return true; + + // TODO(yshui): why 1500? + const int SLACK = 1500; + int render_time_estimate = rolling_max_get_max(ps->render_stats); + if (render_time_estimate < 0) { + // We don't have render time estimates, maybe there's no frame rendered + // yet, or the backend doesn't support render timing information, + // schedule render immediately. + ps->target_msc = ps->last_msc + 1; + goto schedule; + } + render_time_estimate += SLACK; + + auto frame_time = (uint64_t)rolling_avg_get_avg(ps->frame_time); + auto minimal_target_us = now_us + (uint64_t)render_time_estimate; + auto frame_delay = (uint64_t)ceil( + (double)(minimal_target_us - ps->last_msc_instant) / (double)frame_time); + auto deadline = ps->last_msc_instant + frame_delay * frame_time; + + ps->target_msc = ps->last_msc + frame_delay; + auto delay = deadline - (uint64_t)render_time_estimate - now_us; + delay_s = (double)delay / 1000000.0; + + log_trace("Delay: %lu us, last_msc: %" PRIu64 ", render_time: %d, frame_time: " + "%" PRIu64 ", now_us: %" PRIu64 ", next_msc: %" PRIu64, + delay, ps->last_msc_instant, render_time_estimate, frame_time, now_us, + deadline); + +schedule: + 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) { // Whether we have already rendered for the current frame. // If frame pacing is not enabled, pretend this is false. - bool already_rendered = (ps->last_render > ps->last_msc_instant) && ps->frame_pacing; - if (already_rendered) { - log_debug("Already rendered for the current frame, not queuing " - "redraw"); - } // If --benchmark is used, redraw is always queued - if (!ps->redraw_needed && !ps->o.benchmark && !already_rendered) { - double delay = 0; - if (ps->frame_pacing) { - if (!update_render_stats(ps)) { - // A frame is still being rendered. This means we missed - // previous vblank. We shouldn't queue a new frame until - // the next vblank. - log_debug("A frame is still being rendered, not queueing " - "redraw"); - ps->redraw_needed = true; - return; - } - // Our loop can be blocked by frame present, which cause ev_now to - // drift away from the real time. We need to correct it. - delay = next_frame_offset(ps) + ev_now(ps->loop) - ev_time(); - if (delay < 0) { - delay = 0; - } - } - // Not doing frame pacing, just redraw immediately - ev_timer_set(&ps->draw_timer, delay, 0); - assert(!ev_is_active(&ps->draw_timer)); - ev_timer_start(ps->loop, &ps->draw_timer); + if (!ps->redraw_needed && !ps->o.benchmark) { + schedule_render(ps); } ps->redraw_needed = true; } @@ -1506,18 +1531,8 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ // pacing. Unconditionally queue a frame for simplicity. queue_redraw(ps); } - if (ps->frame_pacing && ps->redraw_needed && !ev_is_active(&ps->draw_timer)) { - log_trace("Frame pacing: queueing redraw"); - // We deferred a frame in queue_redraw() because of frame pacing. Schedule - // it now. - if (!update_render_stats(ps)) { - log_warn("A frame is still being rendered, not queueing redraw."); - } else { - - ev_timer_set(&ps->draw_timer, - next_frame_offset(ps) + ev_now(ps->loop) - ev_time(), 0); - ev_timer_start(ps->loop, &ps->draw_timer); - } + if (ps->frame_pacing && ps->redraw_needed) { + schedule_render(ps); } } From 480fd24da6ebca65e72d32ae6bbdd6e238282434 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Dec 2022 08:52:20 +0000 Subject: [PATCH 12/37] core: stop sending NotifyMsc if frame pacing is disabled Signed-off-by: Yuxuan Shui --- src/picom.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/picom.c b/src/picom.c index 25d22128bf..0727847219 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1489,9 +1489,11 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ return; } - auto cookie = xcb_present_notify_msc(ps->c, session_get_target_window(ps), 0, - cne->msc + 1, 0, 0); - set_cant_fail_cookie(ps, cookie); + if (ps->frame_pacing) { + auto cookie = xcb_present_notify_msc(ps->c, session_get_target_window(ps), + 0, cne->msc + 1, 0, 0); + set_cant_fail_cookie(ps, cookie); + } if (cne->msc <= ps->last_msc || cne->ust == 0) { // X sometimes sends duplicate/bogus MSC events, ignore them From 1899ef0d494a5309cf2dacbd6148186e6fb9c1d1 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Dec 2022 08:53:08 +0000 Subject: [PATCH 13/37] core: only check present timer alignment in the first frame Timer can sometimes drift randomly, like when the system is suspended. we don't want to disable frame pacing for that Signed-off-by: Yuxuan Shui --- src/picom.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/picom.c b/src/picom.c index 0727847219..65883beeec 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1518,22 +1518,21 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ int frame_time = (int)((cne->ust - ps->last_msc_instant) / frame_count); rolling_avg_push(ps->frame_time, frame_time); log_trace("Frame count %lu, frame time: %d us, rolling average: %lf us, " - "msc: %" PRIu64 ", offset: %" PRIu64, + "msc: %" PRIu64 ", offset: %d us", frame_count, frame_time, rolling_avg_get_avg(ps->frame_time), - cne->ust, drift); - } - ps->last_msc_instant = cne->ust; - ps->last_msc = cne->msc; - if (drift > 1000000 && ps->frame_pacing) { + cne->ust, (int)drift); + } else if (drift > 1000000 && ps->frame_pacing) { + // 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?), %" PRIu64 " %" PRIu64, now_usec, ps->last_msc_instant); ps->frame_pacing = false; - // We could have deferred a frame in queue_redraw() because of frame - // pacing. Unconditionally queue a frame for simplicity. - queue_redraw(ps); } - if (ps->frame_pacing && ps->redraw_needed) { + ps->last_msc_instant = cne->ust; + ps->last_msc = cne->msc; + if (ps->redraw_needed) { schedule_render(ps); } } From edeb17ad4637feec72aaba506ad7fc4a5cac811e Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 17 Dec 2022 19:27:40 +0000 Subject: [PATCH 14/37] core: don't fully trust msc from present complete notify Sometimes X sends bogus complete notifies with invalid msc number. Instead use a saved msc number to get the next complete notify. Signed-off-by: Yuxuan Shui --- src/picom.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/picom.c b/src/picom.c index 65883beeec..3e0fd786df 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1489,17 +1489,23 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ return; } + bool event_is_invalid = false; if (ps->frame_pacing) { + auto next_msc = cne->msc + 1; + if (cne->msc <= ps->last_msc || cne->ust == 0) { + // X sometimes sends duplicate/bogus MSC events, don't + // use the msc value. Also ignore these events. + // + // See: + // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 + next_msc = ps->last_msc + 1; + event_is_invalid = true; + } auto cookie = xcb_present_notify_msc(ps->c, session_get_target_window(ps), - 0, cne->msc + 1, 0, 0); + 0, next_msc, 0, 0); set_cant_fail_cookie(ps, cookie); } - - if (cne->msc <= ps->last_msc || cne->ust == 0) { - // X sometimes sends duplicate/bogus MSC events, ignore them - // - // See: - // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 + if (event_is_invalid) { return; } From fcb9dc8cfd41d3f5ae3463ab4625ffd03daa25e4 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 7 Mar 2023 17:03:26 +0000 Subject: [PATCH 15/37] core: make sure unredirection happens when screen is off So when the screen is off, we calls queue_redraw, hoping draw_callback will be called and unredirects the screen. However, queue_redraw doesn't queue another redraw when one is already queued. Redraws can be queued in two ways: one is timer based, which is fine, because it will be triggered no matter what; the other is frame based, which is triggered by Present events. When the screen is off, X server, depends on the driver, could send abnormal Present events, which we ignore. But that also means queued frame based redraw will stop being triggered. Those two factors combined means sometimes unredirection does not happen when the screen is off. Which means we aren't going to free the GL context, which are still receiving Present events, but can't handle them, because we are not rendering anything with GL. In the end all these causes memory usage to balloon, until the screen is turned on and we start rendering again. And all these is not caught by leak checkers because this technically is not a leak, as everything is eventually freed. Signed-off-by: Yuxuan Shui --- src/picom.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/picom.c b/src/picom.c index 3e0fd786df..e97f0e6fae 100644 --- a/src/picom.c +++ b/src/picom.c @@ -133,6 +133,7 @@ void check_dpms_status(EV_P attr_unused, ev_timer *w, int revents attr_unused) { } auto now_screen_is_off = dpms_screen_is_off(r); if (ps->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; queue_redraw(ps); } @@ -145,14 +146,17 @@ void check_dpms_status(EV_P attr_unused, ev_timer *w, int revents attr_unused) { * XXX move to win.c */ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t wid) { - if (!wid || PointerRoot == wid || wid == ps->root || wid == ps->overlay) + if (!wid || PointerRoot == wid || wid == ps->root || wid == ps->overlay) { return NULL; + } auto w = find_managed_win(ps, wid); - if (!w) + if (!w) { w = find_toplevel(ps, wid); - if (!w) + } + if (!w) { w = find_managed_window_or_parent(ps, wid); + } return w; } @@ -244,6 +248,11 @@ void schedule_render(session_t *ps) { ps->target_msc = ps->last_msc + frame_delay; auto delay = deadline - (uint64_t)render_time_estimate - now_us; delay_s = (double)delay / 1000000.0; + if (delay_s > 1) { + log_warn("Delay too long: %f s, render_time: %d us, frame_time: %" PRIu64 + " us, now_us: %" PRIu64 " us, next_msc: %" PRIu64 " us", + delay_s, render_time_estimate, frame_time, now_us, deadline); + } log_trace("Delay: %lu us, last_msc: %" PRIu64 ", render_time: %d, frame_time: " "%" PRIu64 ", now_us: %" PRIu64 ", next_msc: %" PRIu64, @@ -257,6 +266,18 @@ void schedule_render(session_t *ps) { } void queue_redraw(session_t *ps) { + if (ps->screen_is_off) { + // The screen is off, if there is a draw queued for the next frame (i.e. + // ps->redraw_needed == true), it won't be triggered until the screen is + // on again, because the abnormal Present events we will receive from the + // X server when the screen is off. Yet we need the draw_callback to be + // called as soon as possible so the screen can be unredirected. + // So here we unconditionally start the draw timer. + ev_timer_stop(ps->loop, &ps->draw_timer); + ev_timer_set(&ps->draw_timer, 0, 0); + ev_timer_start(ps->loop, &ps->draw_timer); + return; + } // 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 From 336cb0917adbfabe264f431de20f0cffa45fdb18 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 27 Apr 2023 04:06:56 +0100 Subject: [PATCH 16/37] core: check we have both frame time and render time before pacing We only checked render time. If we don't have frame time estimates, we would divide by zero and end up with wild scheduling delays. Signed-off-by: Yuxuan Shui --- src/picom.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/picom.c b/src/picom.c index e97f0e6fae..8f217b1464 100644 --- a/src/picom.c +++ b/src/picom.c @@ -230,16 +230,16 @@ void schedule_render(session_t *ps) { // TODO(yshui): why 1500? const int SLACK = 1500; int render_time_estimate = rolling_max_get_max(ps->render_stats); - if (render_time_estimate < 0) { - // We don't have render time estimates, maybe there's no frame rendered - // yet, or the backend doesn't support render timing information, - // schedule render immediately. + auto frame_time = (uint64_t)rolling_avg_get_avg(ps->frame_time); + if (render_time_estimate < 0 || frame_time == 0) { + // We don't have render time, and/or frame time estimates, maybe there's + // no frame rendered yet, or the backend doesn't support render timing + // information, schedule render immediately. ps->target_msc = ps->last_msc + 1; goto schedule; } render_time_estimate += SLACK; - auto frame_time = (uint64_t)rolling_avg_get_avg(ps->frame_time); auto minimal_target_us = now_us + (uint64_t)render_time_estimate; auto frame_delay = (uint64_t)ceil( (double)(minimal_target_us - ps->last_msc_instant) / (double)frame_time); From 8e1f3c92f54ee10793cf06d260cfd57abf4c28db Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Jun 2023 14:08:49 +0100 Subject: [PATCH 17/37] core: factor out code for estimating the rendering time budget Add a render_statistics type to encapsulate all the statistics done on rendering times. And use that to estimate the time budget for rendering and frame pacing. Tweak the rolling window utilities a bit so we can reuse one rolling window for multiple statistics. Signed-off-by: Yuxuan Shui --- src/backend/backend.c | 14 +++ src/common.h | 12 ++- src/meson.build | 2 +- src/picom.c | 115 ++++++++++++--------- src/statistics.c | 100 ++++++++++++++++++ src/statistics.h | 33 ++++++ src/utils.c | 233 ++++++++++++++++++++++++------------------ src/utils.h | 94 +++++++++++++++-- 8 files changed, 441 insertions(+), 162 deletions(-) create mode 100644 src/statistics.c create mode 100644 src/statistics.h diff --git a/src/backend/backend.c b/src/backend/backend.c index 3543ee127e..bacf29d28b 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -176,6 +176,20 @@ void paint_all_new(session_t *ps, struct managed_win *t) { region_t reg_shadow_clip; pixman_region32_init(®_shadow_clip); + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + auto now_us = (uint64_t)(now.tv_sec * 1000000 + now.tv_nsec / 1000); + if (ps->next_render > 0) { + log_trace("Render schedule deviation: %ld us (%s) %ld %ld", + labs((int64_t)now_us - (int64_t)ps->next_render), + now_us < ps->next_render ? "early" : "late", now_us, + ps->next_render); + ps->last_schedule_delay = 0; + if (now_us > ps->next_render) { + ps->last_schedule_delay = now_us - ps->next_render; + } + } + if (ps->backend_data->ops->prepare) { ps->backend_data->ops->prepare(ps->backend_data, ®_paint); } diff --git a/src/common.h b/src/common.h index b76863df15..65d0e7b65f 100644 --- a/src/common.h +++ b/src/common.h @@ -58,6 +58,7 @@ #include "list.h" #include "region.h" #include "render.h" +#include "statistics.h" #include "types.h" #include "utils.h" #include "win_defs.h" @@ -249,13 +250,16 @@ typedef struct session { /// When the currently rendered frame will be displayed. /// 0 means there is no pending frame. uint64_t target_msc; - /// When did we render our last frame. - uint64_t last_render; + /// The delay between when the last frame was scheduled to be rendered, and when + /// the render actually started. + uint64_t last_schedule_delay; + /// When do we want our next frame to start rendering. + uint64_t next_render; /// Whether we can perform frame pacing. bool frame_pacing; - struct rolling_avg *frame_time; - struct rolling_max *render_stats; + /// Render statistics + struct render_statistics render_stats; // === Operation related === /// Flags related to the root window diff --git a/src/meson.build b/src/meson.build index 0cd87c896d..074683b2a3 100644 --- a/src/meson.build +++ b/src/meson.build @@ -9,7 +9,7 @@ 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') ] + 'options.c', 'event.c', 'cache.c', 'atom.c', 'file_watch.c', 'statistics.c') ] picom_inc = include_directories('.') cflags = [] diff --git a/src/picom.c b/src/picom.c index 8f217b1464..8bb22d7d0e 100644 --- a/src/picom.c +++ b/src/picom.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -43,7 +44,6 @@ #endif #include "backend/backend.h" #include "c2.h" -#include "config.h" #include "diagnostic.h" #include "log.h" #include "region.h" @@ -60,6 +60,7 @@ #include "file_watch.h" #include "list.h" #include "options.h" +#include "statistics.h" #include "uthash_extra.h" /// Get session_t pointer from a pointer to a member of session_t @@ -177,8 +178,17 @@ static inline struct managed_win *find_win_all(session_t *ps, const xcb_window_t /// 4. otherwise, we schedule a render for that target frame. we use past statistics about /// how long our renders took to figure out when to start rendering. we start rendering /// at the latest point of time possible to still hit the target frame. -void schedule_render(session_t *ps) { +/// +/// 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. + +void schedule_render(session_t *ps, bool triggered_by_vblank) { double delay_s = 0; + ps->next_render = 0; if (!ps->frame_pacing || !ps->redirected) { // Not doing frame pacing, schedule a render immediately, if not already // scheduled. @@ -205,59 +215,77 @@ void schedule_render(session_t *ps) { // We missed our target, push it back one frame ps->target_msc = ps->last_msc + 1; } + log_trace("Still rendering for target frame %ld, not scheduling another " + "render", + ps->target_msc); return; } if (ps->target_msc > ps->last_msc) { - // Render for the target frame is completed, and has yet to be displayed. + // Render for the target frame is completed, but is yet to be displayed. // Don't schedule another render. + log_trace("Target frame %ld is in the future, and we have already " + "rendered, last msc: %d", + ps->target_msc, (int)ps->last_msc); return; } struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); auto now_us = (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_nsec / 1000; + if (triggered_by_vblank) { + log_trace("vblank schedule delay: %ld us", now_us - ps->last_msc_instant); + } int render_time_us = (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); if (ps->target_msc == ps->last_msc) { // The frame has just been displayed, record its render time; - log_trace("Last render call took: %d us, rolling_max: %d us", - render_time_us, rolling_max_get_max(ps->render_stats)); - rolling_max_push(ps->render_stats, render_time_us); + 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->target_msc = 0; + ps->last_schedule_delay = 0; } - // TODO(yshui): why 1500? - const int SLACK = 1500; - int render_time_estimate = rolling_max_get_max(ps->render_stats); - auto frame_time = (uint64_t)rolling_avg_get_avg(ps->frame_time); - if (render_time_estimate < 0 || frame_time == 0) { - // We don't have render time, and/or frame time estimates, maybe there's + unsigned int divisor = 0; + 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) { + // 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. ps->target_msc = ps->last_msc + 1; goto schedule; } - render_time_estimate += SLACK; - auto minimal_target_us = now_us + (uint64_t)render_time_estimate; - auto frame_delay = (uint64_t)ceil( - (double)(minimal_target_us - ps->last_msc_instant) / (double)frame_time); - auto deadline = ps->last_msc_instant + frame_delay * frame_time; + const auto deadline = ps->last_msc_instant + (unsigned long)divisor * frame_time; + unsigned int available = 0; + if (deadline > now_us) { + available = (unsigned int)(deadline - now_us); + } - ps->target_msc = ps->last_msc + frame_delay; - auto delay = deadline - (uint64_t)render_time_estimate - now_us; - delay_s = (double)delay / 1000000.0; + ps->target_msc = ps->last_msc + divisor; + if (available > render_budget) { + delay_s = (double)(available - render_budget) / 1000000.0; + ps->next_render = deadline - render_budget; + } else { + delay_s = 0; + ps->next_render = now_us; + } if (delay_s > 1) { - log_warn("Delay too long: %f s, render_time: %d us, frame_time: %" PRIu64 - " us, now_us: %" PRIu64 " us, next_msc: %" PRIu64 " us", - delay_s, render_time_estimate, frame_time, now_us, deadline); + log_warn("Delay too long: %f s, render_budget: %d us, frame_time: " + "%" PRIu32 " us, now_us: %" PRIu64 " us, next_msc: %" PRIu64 " u" + "s", + delay_s, render_budget, frame_time, now_us, deadline); } - log_trace("Delay: %lu us, last_msc: %" PRIu64 ", render_time: %d, frame_time: " - "%" PRIu64 ", now_us: %" PRIu64 ", next_msc: %" PRIu64, - delay, ps->last_msc_instant, render_time_estimate, frame_time, now_us, - deadline); + log_trace("Delay: %.6lf s, last_msc: %" PRIu64 ", render_budget: %d, frame_time: " + "%" PRIu32 ", now_us: %" PRIu64 ", next_msc: %" PRIu64 ", " + "target_msc: %" PRIu64 ", divisor: %d", + delay_s, ps->last_msc_instant, render_budget, frame_time, now_us, + deadline, ps->target_msc, divisor); schedule: assert(!ev_is_active(&ps->draw_timer)); @@ -282,7 +310,7 @@ void queue_redraw(session_t *ps) { // If frame pacing is not enabled, pretend this is false. // If --benchmark is used, redraw is always queued if (!ps->redraw_needed && !ps->o.benchmark) { - schedule_render(ps); + schedule_render(ps, false); } ps->redraw_needed = true; } @@ -758,7 +786,6 @@ static void configure_root(session_t *ps) { } force_repaint(ps); } - return; } static void handle_root_flags(session_t *ps) { @@ -1439,10 +1466,9 @@ static bool redirect_start(session_t *ps) { // states. ps->last_msc_instant = 0; ps->last_msc = 0; - ps->last_render = 0; + ps->last_schedule_delay = 0; ps->target_msc = 0; - rolling_avg_reset(ps->frame_time); - rolling_max_reset(ps->render_stats); + render_statistics_reset(&ps->render_stats); } else if (ps->frame_pacing) { log_error("Present extension is not supported, frame pacing disabled."); ps->frame_pacing = false; @@ -1543,11 +1569,12 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ 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); - rolling_avg_push(ps->frame_time, frame_time); - log_trace("Frame count %lu, frame time: %d us, rolling average: %lf us, " + 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, rolling_avg_get_avg(ps->frame_time), - cne->ust, (int)drift); + frame_count, frame_time, + render_statistics_get_vblank_time(&ps->render_stats), cne->ust, + (int)drift); } else if (drift > 1000000 && ps->frame_pacing) { // 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 @@ -1560,7 +1587,7 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ ps->last_msc_instant = cne->ust; ps->last_msc = cne->msc; if (ps->redraw_needed) { - schedule_render(ps); + schedule_render(ps, true); } } @@ -1651,6 +1678,8 @@ static void tmout_unredir_callback(EV_P attr_unused, ev_timer *w, int revents at } static void fade_timer_callback(EV_P attr_unused, ev_timer *w, int revents attr_unused) { + // TODO(yshui): do we still need the fade timer? we queue redraw automatically in + // draw_callback_impl if animation is running. session_t *ps = session_ptr(w, fade_timer); queue_redraw(ps); } @@ -1778,12 +1807,6 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { } log_trace("Render end"); - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - uint64_t now_us = - (uint64_t)now.tv_sec * 1000000ULL + (uint64_t)now.tv_nsec / 1000; - ps->last_render = now_us; - ps->first_frame = false; paint++; if (ps->o.benchmark && paint >= ps->o.benchmark) { @@ -2004,8 +2027,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, pixman_region32_init(&ps->screen_reg); // TODO(yshui) investigate what's the best window size - ps->render_stats = rolling_max_new(128); - ps->frame_time = rolling_avg_new(16); + render_statistics_init(&ps->render_stats, 128); ps->pending_reply_tail = &ps->pending_reply_head; @@ -2642,8 +2664,7 @@ static void session_destroy(session_t *ps) { free(ps->o.glx_fshader_win_str); x_free_randr_info(ps); - rolling_avg_destroy(ps->frame_time); - rolling_max_destroy(ps->render_stats); + render_statistics_destroy(&ps->render_stats); // Release custom window shaders free(ps->o.window_shader_fg); diff --git a/src/statistics.c b/src/statistics.c new file mode 100644 index 0000000000..5e3d92491d --- /dev/null +++ b/src/statistics.c @@ -0,0 +1,100 @@ +//! Rendering statistics +//! +//! Tracks how long it takes to render a frame, for measuring performance, and for pacing +//! the frames. + +#include "statistics.h" +#include "log.h" +#include "utils.h" + +void render_statistics_init(struct render_statistics *rs, int window_size) { + *rs = (struct render_statistics){0}; + + rolling_window_init(&rs->render_times, window_size); + rolling_quantile_init_with_tolerance(&rs->render_time_quantile, window_size, + /* q */ 0.98, /* tolerance */ 0.01); +} + +void render_statistics_add_vblank_time_sample(struct render_statistics *rs, int time_us) { + auto sample_sd = sqrt(cumulative_mean_and_var_get_var(&rs->vblank_time_us)); + auto current_estimate = render_statistics_get_vblank_time(rs); + if (current_estimate != 0 && fabs((double)time_us - current_estimate) > sample_sd * 3) { + // Deviated from the mean by more than 3 sigma (p < 0.003) + log_debug("vblank time outlier: %d %f %f", time_us, rs->vblank_time_us.mean, + cumulative_mean_and_var_get_var(&rs->vblank_time_us)); + // An outlier sample, this could mean things like refresh rate changes, so + // we reset the statistics. This could also be benign, but we like to be + // cautious. + cumulative_mean_and_var_init(&rs->vblank_time_us); + } + + if (rs->vblank_time_us.mean != 0) { + auto nframes_in_10_seconds = + (unsigned int)(10. * 1000000. / rs->vblank_time_us.mean); + if (rs->vblank_time_us.n > 20 && rs->vblank_time_us.n > nframes_in_10_seconds) { + // We collected 10 seconds worth of samples, we assume the + // estimated refresh rate is stable. We will still reset the + // statistics if we get an outlier sample though, see above. + return; + } + } + cumulative_mean_and_var_update(&rs->vblank_time_us, time_us); +} + +void render_statistics_add_render_time_sample(struct render_statistics *rs, int time_us) { + int oldest; + if (rolling_window_push_back(&rs->render_times, time_us, &oldest)) { + rolling_quantile_pop_front(&rs->render_time_quantile, oldest); + } + + rolling_quantile_push_back(&rs->render_time_quantile, 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) { + 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; +} + +unsigned int render_statistics_get_vblank_time(struct render_statistics *rs) { + if (rs->vblank_time_us.n <= 20 || rs->vblank_time_us.mean < 100) { + // Not enough samples yet, or the vblank time is too short to be + // meaningful. Assume maximum budget. + return 0; + } + return (unsigned int)rs->vblank_time_us.mean; +} + +void render_statistics_reset(struct render_statistics *rs) { + rolling_window_reset(&rs->render_times); + rolling_quantile_reset(&rs->render_time_quantile); + rs->vblank_time_us = (struct cumulative_mean_and_var){0}; +} + +void render_statistics_destroy(struct render_statistics *rs) { + render_statistics_reset(rs); + rolling_window_destroy(&rs->render_times); + rolling_quantile_destroy(&rs->render_time_quantile); +} diff --git a/src/statistics.h b/src/statistics.h new file mode 100644 index 0000000000..67d02119af --- /dev/null +++ b/src/statistics.h @@ -0,0 +1,33 @@ +#pragma once + +#include "utils.h" + +#define NTIERS (3) + +struct render_statistics { + /// Rolling window of rendering times (in us) and the tiers they belong to. + /// We keep track of the tiers because the vblank time estimate can change over + /// time. + struct rolling_window render_times; + /// Estimate the 95-th percentile of rendering times + struct rolling_quantile render_time_quantile; + /// Time between each vblanks + struct cumulative_mean_and_var vblank_time_us; +}; + +void render_statistics_init(struct render_statistics *rs, int window_size); +void render_statistics_reset(struct render_statistics *rs); +void render_statistics_destroy(struct render_statistics *rs); + +void render_statistics_add_vblank_time_sample(struct render_statistics *rs, int time_us); +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_vblank_time(struct render_statistics *rs); diff --git a/src/utils.c b/src/utils.c index a1114a0491..d26afa331e 100644 --- a/src/utils.c +++ b/src/utils.c @@ -48,7 +48,43 @@ int next_power_of_two(int n) { return n; } -/// Track the rolling maximum of a stream of integers. +void rolling_window_destroy(struct rolling_window *rw) { + free(rw->elem); + rw->elem = NULL; +} + +void rolling_window_reset(struct rolling_window *rw) { + rw->nelem = 0; + rw->elem_head = 0; +} + +void rolling_window_init(struct rolling_window *rw, int size) { + rw->elem = ccalloc(size, int); + rw->window_size = size; + rolling_window_reset(rw); +} + +int rolling_window_pop_front(struct rolling_window *rw) { + assert(rw->nelem > 0); + auto ret = rw->elem[rw->elem_head]; + rw->elem_head = (rw->elem_head + 1) % rw->window_size; + rw->nelem--; + return ret; +} + +bool rolling_window_push_back(struct rolling_window *rw, int val, int *front) { + bool full = rw->nelem == rw->window_size; + if (full) { + *front = rolling_window_pop_front(rw); + } + rw->elem[(rw->elem_head + rw->nelem) % rw->window_size] = val; + rw->nelem++; + return full; +} + +/// Track the maximum member of a FIFO queue of integers. Integers are pushed to the back +/// and popped from the front, the maximum of the current members in the queue is +/// tracked. struct rolling_max { /// A priority queue holding the indices of the maximum element candidates. /// The head of the queue is the index of the maximum element. @@ -59,32 +95,26 @@ struct rolling_max { /// it's called the "original" indices. int *p; int p_head, np; - - /// The elemets - int *elem; - int elem_head, nelem; - - int window_size; + /// The maximum number of in flight elements. + int capacity; }; void rolling_max_destroy(struct rolling_max *rm) { - free(rm->elem); free(rm->p); free(rm); } -struct rolling_max *rolling_max_new(int size) { +struct rolling_max *rolling_max_new(int capacity) { auto rm = ccalloc(1, struct rolling_max); if (!rm) { return NULL; } - rm->p = ccalloc(size, int); - rm->elem = ccalloc(size, int); - rm->window_size = size; - if (!rm->p || !rm->elem) { + rm->p = ccalloc(capacity, int); + if (!rm->p) { goto err; } + rm->capacity = capacity; return rm; @@ -96,33 +126,21 @@ struct rolling_max *rolling_max_new(int size) { void rolling_max_reset(struct rolling_max *rm) { rm->p_head = 0; rm->np = 0; - rm->nelem = 0; - rm->elem_head = 0; -} - -void rolling_max_push(struct rolling_max *rm, int val) { -#define IDX(n) ((n) % rm->window_size) - if (rm->nelem == rm->window_size) { - auto old_head = rm->elem_head; - // Discard the oldest element. - // rm->elem.pop_front(); - rm->nelem--; - rm->elem_head = IDX(rm->elem_head + 1); - - // Remove discarded element from the priority queue too. - assert(rm->np); - if (rm->p[rm->p_head] == old_head) { - // rm->p.pop_front() - rm->p_head = IDX(rm->p_head + 1); - rm->np--; - } - } +} - // Add the new element to the queue. - // rm->elem.push_back(val) - rm->elem[IDX(rm->elem_head + rm->nelem)] = val; - rm->nelem++; +#define IDX(n) ((n) % rm->capacity) +/// Remove the oldest element in the window. The caller must maintain the list of elements +/// themselves, i.e. the behavior is undefined if `front` does not 1match the oldest +/// element. +void rolling_max_pop_front(struct rolling_max *rm, int front) { + if (rm->p[rm->p_head] == front) { + // rm->p.pop_front() + rm->p_head = IDX(rm->p_head + 1); + rm->np--; + } +} +void rolling_max_push_back(struct rolling_max *rm, int val) { // Update the prority queue. // Remove all elements smaller than the new element from the queue. Because // the new element will become the maximum element before them, and since they @@ -130,7 +148,7 @@ void rolling_max_push(struct rolling_max *rm, int val) { // element, so they will never become the maximum element. while (rm->np) { int p_tail = IDX(rm->p_head + rm->np - 1); - if (rm->elem[rm->p[p_tail]] > val) { + if (rm->p[p_tail] > val) { break; } // rm->p.pop_back() @@ -138,108 +156,119 @@ void rolling_max_push(struct rolling_max *rm, int val) { } // Add the new element to the end of the queue. // rm->p.push_back(rm->start_index + rm->nelem - 1) - rm->p[IDX(rm->p_head + rm->np)] = IDX(rm->elem_head + rm->nelem - 1); + assert(rm->np < rm->capacity); + rm->p[IDX(rm->p_head + rm->np)] = val; rm->np++; -#undef IDX } +#undef IDX int rolling_max_get_max(struct rolling_max *rm) { if (rm->np == 0) { return INT_MIN; } - return rm->elem[rm->p[rm->p_head]]; + return rm->p[rm->p_head]; } TEST_CASE(rolling_max_test) { #define NELEM 15 + struct rolling_window queue; + rolling_window_init(&queue, 3); auto rm = rolling_max_new(3); const int data[NELEM] = {1, 2, 3, 1, 4, 5, 2, 3, 6, 5, 4, 3, 2, 0, 0}; const int expected_max[NELEM] = {1, 2, 3, 3, 4, 5, 5, 5, 6, 6, 6, 5, 4, 3, 2}; int max[NELEM] = {0}; for (int i = 0; i < NELEM; i++) { - rolling_max_push(rm, data[i]); + int front; + bool full = rolling_window_push_back(&queue, data[i], &front); + if (full) { + rolling_max_pop_front(rm, front); + } + rolling_max_push_back(rm, data[i]); max[i] = rolling_max_get_max(rm); } + rolling_window_destroy(&queue); + rolling_max_destroy(rm); TEST_TRUE(memcmp(max, expected_max, sizeof(max)) == 0); #undef NELEM } -/// A rolling average of a stream of integers. -struct rolling_avg { - /// The sum of the elements in the window. - int64_t sum; - - /// The elements in the window. - int *elem; - int head, nelem; +// Find the k-th smallest element in an array. +int quickselect(int *elems, int nelem, int k) { + int l = 0, r = nelem; // [l, r) is the range of candidates + while (l != r) { + int pivot = elems[l]; + int i = l, j = r; + while (i < j) { + while (i < j && elems[--j] >= pivot) { + } + elems[i] = elems[j]; + while (i < j && elems[++i] <= pivot) { + } + elems[j] = elems[i]; + } + elems[i] = pivot; - int window_size; -}; + if (i == k) { + break; + } -struct rolling_avg *rolling_avg_new(int size) { - auto rm = ccalloc(1, struct rolling_avg); - if (!rm) { - return NULL; + if (i < k) { + l = i + 1; + } else { + r = i; + } } + return elems[k]; +} - rm->elem = ccalloc(size, int); - rm->window_size = size; - if (!rm->elem) { - free(rm); - return NULL; - } +void rolling_quantile_init(struct rolling_quantile *rq, int capacity, int mink, int maxk) { + *rq = (struct rolling_quantile){0}; + rq->tmp_buffer = malloc(sizeof(int) * (size_t)capacity); + rq->capacity = capacity; + rq->min_target_rank = mink; + rq->max_target_rank = maxk; +} - return rm; +void rolling_quantile_init_with_tolerance(struct rolling_quantile *rq, int window_size, + double target, double tolerance) { + rolling_quantile_init(rq, window_size, (int)((target - tolerance) * window_size), + (int)((target + tolerance) * window_size)); } -void rolling_avg_destroy(struct rolling_avg *rm) { - free(rm->elem); - free(rm); +void rolling_quantile_reset(struct rolling_quantile *rq) { + rq->current_rank = 0; + rq->estimate = 0; } -void rolling_avg_reset(struct rolling_avg *ra) { - ra->sum = 0; - ra->nelem = 0; - ra->head = 0; +void rolling_quantile_destroy(struct rolling_quantile *rq) { + free(rq->tmp_buffer); } -void rolling_avg_push(struct rolling_avg *ra, int val) { - if (ra->nelem == ra->window_size) { - // Discard the oldest element. - // rm->elem.pop_front(); - ra->sum -= ra->elem[ra->head % ra->window_size]; - ra->nelem--; - ra->head = (ra->head + 1) % ra->window_size; +int rolling_quantile_estimate(struct rolling_quantile *rq, struct rolling_window *elements) { + if (rq->current_rank < rq->min_target_rank || rq->current_rank > rq->max_target_rank) { + if (elements->nelem != elements->window_size) { + return INT_MIN; + } + // Re-estimate the quantile. + assert(elements->nelem <= rq->capacity); + rolling_window_copy_to_array(elements, rq->tmp_buffer); + const int target_rank = + rq->min_target_rank + (rq->max_target_rank - rq->min_target_rank) / 2; + rq->estimate = quickselect(rq->tmp_buffer, elements->nelem, target_rank); + rq->current_rank = target_rank; } - - // Add the new element to the queue. - // rm->elem.push_back(val) - ra->elem[(ra->head + ra->nelem) % ra->window_size] = val; - ra->sum += val; - ra->nelem++; + return rq->estimate; } -double rolling_avg_get_avg(struct rolling_avg *ra) { - if (ra->nelem == 0) { - return 0; +void rolling_quantile_push_back(struct rolling_quantile *rq, int x) { + if (x <= rq->estimate) { + rq->current_rank++; } - return (double)ra->sum / (double)ra->nelem; } -TEST_CASE(rolling_avg_test) { -#define NELEM 15 - auto rm = rolling_avg_new(3); - const int data[NELEM] = {1, 2, 3, 1, 4, 5, 2, 3, 6, 5, 4, 3, 2, 0, 0}; - const double expected_avg[NELEM] = { - 1, 1.5, 2, 2, 8.0 / 3.0, 10.0 / 3.0, 11.0 / 3.0, 10.0 / 3.0, - 11.0 / 3.0, 14.0 / 3.0, 5, 4, 3, 5.0 / 3.0, 2.0 / 3.0}; - double avg[NELEM] = {0}; - for (int i = 0; i < NELEM; i++) { - rolling_avg_push(rm, data[i]); - avg[i] = rolling_avg_get_avg(rm); - } - for (int i = 0; i < NELEM; i++) { - TEST_EQUAL(avg[i], expected_avg[i]); +void rolling_quantile_pop_front(struct rolling_quantile *rq, int x) { + if (x <= rq->estimate) { + rq->current_rank--; } } diff --git a/src/utils.h b/src/utils.h index b7c452912e..fc6dd306de 100644 --- a/src/utils.h +++ b/src/utils.h @@ -17,6 +17,7 @@ #include #include "compiler.h" +#include "log.h" #include "types.h" #define ARR_SIZE(arr) (sizeof(arr) / sizeof(arr[0])) @@ -289,20 +290,97 @@ static inline void free_charpp(char **str) { /// int next_power_of_two(int n); +struct rolling_window { + int *elem; + int elem_head, nelem; + int window_size; +}; + +void rolling_window_destroy(struct rolling_window *rw); +void rolling_window_reset(struct rolling_window *rw); +void rolling_window_init(struct rolling_window *rw, int size); +int rolling_window_pop_front(struct rolling_window *rw); +bool rolling_window_push_back(struct rolling_window *rw, int val, int *front); + +/// Copy the contents of the rolling window to an array. The array is assumed to +/// have enough space to hold the contents of the rolling window. +static inline void attr_unused rolling_window_copy_to_array(struct rolling_window *rw, + int *arr) { + // The length from head to the end of the array + auto head_len = (size_t)(rw->window_size - rw->elem_head); + if (head_len >= (size_t)rw->nelem) { + memcpy(arr, rw->elem + rw->elem_head, sizeof(int) * (size_t)rw->nelem); + } else { + auto tail_len = (size_t)((size_t)rw->nelem - head_len); + memcpy(arr, rw->elem + rw->elem_head, sizeof(int) * head_len); + memcpy(arr + head_len, rw->elem, sizeof(int) * tail_len); + } +} + struct rolling_max; -struct rolling_max *rolling_max_new(int window_size); +struct rolling_max *rolling_max_new(int capacity); void rolling_max_destroy(struct rolling_max *rm); void rolling_max_reset(struct rolling_max *rm); -void rolling_max_push(struct rolling_max *rm, int val); +void rolling_max_pop_front(struct rolling_max *rm, int front); +void rolling_max_push_back(struct rolling_max *rm, int val); int rolling_max_get_max(struct rolling_max *rm); -struct rolling_avg; -struct rolling_avg *rolling_avg_new(int window_size); -void rolling_avg_destroy(struct rolling_avg *ra); -void rolling_avg_reset(struct rolling_avg *ra); -void rolling_avg_push(struct rolling_avg *ra, int val); -double rolling_avg_get_avg(struct rolling_avg *ra); +/// Estimate the mean and variance of random variable X using Welford's online +/// algorithm. +struct cumulative_mean_and_var { + double mean; + double m2; + unsigned int n; +}; + +static inline attr_unused void +cumulative_mean_and_var_init(struct cumulative_mean_and_var *cmv) { + *cmv = (struct cumulative_mean_and_var){0}; +} + +static inline attr_unused void +cumulative_mean_and_var_update(struct cumulative_mean_and_var *cmv, double x) { + if (cmv->n == UINT_MAX) { + // We have too many elements, let's keep the mean and variance. + return; + } + cmv->n++; + double delta = x - cmv->mean; + cmv->mean += delta / (double)cmv->n; + cmv->m2 += delta * (x - cmv->mean); +} + +static inline attr_unused double +cumulative_mean_and_var_get_var(struct cumulative_mean_and_var *cmv) { + if (cmv->n < 2) { + return 0; + } + return cmv->m2 / (double)(cmv->n - 1); +} + +// Find the k-th smallest element in an array. +int quickselect(int *elems, int nelem, int k); + +/// A naive quantile estimator. +/// +/// Estimates the N-th percentile of a random variable X in a sliding window. +struct rolling_quantile { + int current_rank; + int min_target_rank, max_target_rank; + int estimate; + int capacity; + int *tmp_buffer; +}; + +void rolling_quantile_init(struct rolling_quantile *rq, int capacity, int mink, int maxk); +void rolling_quantile_init_with_tolerance(struct rolling_quantile *rq, int window_size, + double target, double tolerance); +void rolling_quantile_reset(struct rolling_quantile *rq); +void rolling_quantile_destroy(struct rolling_quantile *rq); +int rolling_quantile_estimate(struct rolling_quantile *rq, struct rolling_window *elements); +void rolling_quantile_push_back(struct rolling_quantile *rq, int x); +void rolling_quantile_pop_front(struct rolling_quantile *rq, int x); // Some versions of the Android libc do not have timespec_get(), use // clock_gettime() instead. From b6e7ea5639154078b136b263d7a28a250359b30b Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Jun 2023 14:11:30 +0100 Subject: [PATCH 18/37] core: don't update render statistics if we didn't actually render Sometimes a scheduled render can end up doing nothing, e.g. if the damage region is empty. In that case we don't have valid data to collect and thus shouldn't update the statistics. Signed-off-by: Yuxuan Shui --- src/backend/backend.c | 1 + src/common.h | 3 +++ src/picom.c | 13 ++++++++----- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/backend/backend.c b/src/backend/backend.c index bacf29d28b..36f80cc685 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -189,6 +189,7 @@ void paint_all_new(session_t *ps, struct managed_win *t) { ps->last_schedule_delay = now_us - ps->next_render; } } + ps->did_render = true; if (ps->backend_data->ops->prepare) { ps->backend_data->ops->prepare(ps->backend_data, ®_paint); diff --git a/src/common.h b/src/common.h index 65d0e7b65f..e183a1e6b0 100644 --- a/src/common.h +++ b/src/common.h @@ -255,6 +255,9 @@ typedef struct session { uint64_t last_schedule_delay; /// When do we want our next frame to start rendering. uint64_t next_render; + /// Did we actually render the last frame. Sometimes redraw will be scheduled only + /// to find out nothing has changed. In which case this will be set to false. + bool did_render; /// Whether we can perform frame pacing. bool frame_pacing; diff --git a/src/picom.c b/src/picom.c index 8bb22d7d0e..36c8a34af3 100644 --- a/src/picom.c +++ b/src/picom.c @@ -240,12 +240,15 @@ void schedule_render(session_t *ps, bool triggered_by_vblank) { (int)(render_time.tv_sec * 1000000L + render_time.tv_nsec / 1000L); if (ps->target_msc == ps->last_msc) { // The frame has just been displayed, 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); + if (ps->did_render) { + 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->target_msc = 0; + ps->did_render = false; ps->last_schedule_delay = 0; } From 7d9692360b2eaa2068a465819905f3322f9d57fa Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Jun 2023 14:18:25 +0100 Subject: [PATCH 19/37] core: use SCHED_RR scheduling Make picom realtime to reduce latency, and make rendering times more predictable to help pacing. Signed-off-by: Yuxuan Shui --- src/picom.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/picom.c b/src/picom.c index 36c8a34af3..d6b52b589b 100644 --- a/src/picom.c +++ b/src/picom.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -2553,6 +2554,31 @@ static session_t *session_init(int argc, char **argv, Display *dpy, free(ps); return NULL; } +void set_rr_scheduling(void) { + struct rlimit rlim; + if (getrlimit(RLIMIT_RTPRIO, &rlim) != 0) { + log_warn("Failed to get RLIMIT_RTPRIO, not setting real-time priority"); + return; + } + int old_policy; + int ret; + struct sched_param param; + + ret = pthread_getschedparam(pthread_self(), &old_policy, ¶m); + if (ret != 0) { + log_debug("Failed to get old scheduling priority"); + return; + } + + param.sched_priority = (int)rlim.rlim_cur; + + ret = pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + if (ret != 0) { + log_info("Failed to set scheduling priority to %lu", rlim.rlim_cur); + return; + } + log_info("Set scheduling priority to %lu", rlim.rlim_cur); +} /** * Destroy a session. @@ -2760,6 +2786,7 @@ static void session_destroy(session_t *ps) { * @param ps current session */ static void session_run(session_t *ps) { + set_rr_scheduling(); // In benchmark mode, we want draw_timer handler to always be active if (ps->o.benchmark) { ev_timer_set(&ps->draw_timer, 0, 0); From 6a69cdb002dc6943fd228d73f8b952681b2894b9 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Jun 2023 14:28:43 +0100 Subject: [PATCH 20/37] options: add a no-frame-pacing option Signed-off-by: Yuxuan Shui --- man/picom.1.asciidoc | 3 +++ src/config.c | 1 + src/config.h | 2 ++ src/config_libconfig.c | 3 +++ src/options.c | 2 ++ src/picom.c | 6 +++--- 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/man/picom.1.asciidoc b/man/picom.1.asciidoc index 0a2e3c31a5..1ccf093390 100644 --- a/man/picom.1.asciidoc +++ b/man/picom.1.asciidoc @@ -109,6 +109,9 @@ OPTIONS *--rounded-corners-exclude* 'CONDITION':: Exclude conditions for rounded corners. +*--no-frame-pacing*:: + Disable vsync-aware frame pacing. By default, the compositor tries to make sure it only renders once per vblank interval, and also the render happens as late as possible to minimize the latency from updates to the screen. However this can sometimes cause stuttering, or even lowered frame rate. This option can be used to disable frame pacing. + *--mark-wmwin-focused*:: Try to detect WM windows (a non-override-redirect window with no child that has 'WM_STATE') and mark them as active. diff --git a/src/config.c b/src/config.c index 1013d128fa..3797ffa4a1 100644 --- a/src/config.c +++ b/src/config.c @@ -742,6 +742,7 @@ char *parse_config(options_t *opt, const char *config_file, bool *shadow_enable, .logpath = NULL, .use_damage = true, + .no_frame_pacing = false, .shadow_red = 0.0, .shadow_green = 0.0, diff --git a/src/config.h b/src/config.h index 5ce3ba9635..31e6774c8f 100644 --- a/src/config.h +++ b/src/config.h @@ -140,6 +140,8 @@ typedef struct options { bool vsync_use_glfinish; /// Whether use damage information to help limit the area to paint bool use_damage; + /// Disable frame pacing + bool no_frame_pacing; // === Shadow === /// Red, green and blue tone of the shadow. diff --git a/src/config_libconfig.c b/src/config_libconfig.c index 63f05412bf..40bc47b838 100644 --- a/src/config_libconfig.c +++ b/src/config_libconfig.c @@ -363,6 +363,9 @@ char *parse_config_libconfig(options_t *opt, const char *config_file, bool *shad // --corner-radius-rules parse_cfg_condlst_corner(opt, &cfg, "corner-radius-rules"); + // --no-frame-pacing + lcfg_lookup_bool(&cfg, "no-frame-pacing", &opt->no_frame_pacing); + // -e (frame_opacity) config_lookup_float(&cfg, "frame-opacity", &opt->frame_opacity); // -c (shadow_enable) diff --git a/src/options.c b/src/options.c index 9d500b8e65..b0d956a2bc 100644 --- a/src/options.c +++ b/src/options.c @@ -176,6 +176,7 @@ static const struct picom_option picom_options[] = { "rendered screen. Reduces banding artifacts, but might cause performance " "degradation. Only works with OpenGL."}, // 340 is corner-radius-rules + {"no-frame-pacing" , no_argument , 341, NULL , "Disable frame pacing. This might increase the latency."}, {"legacy-backends" , no_argument , 733, NULL , "Use deprecated version of the backends."}, {"monitor-repaint" , no_argument , 800, NULL , "Highlight the updated area of the screen. For debugging."}, {"diagnostics" , no_argument , 801, NULL , "Print diagnostic information"}, @@ -738,6 +739,7 @@ bool get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, // --dithered-present opt->dithered_present = true; break; + P_CASEBOOL(341, no_frame_pacing); P_CASEBOOL(733, legacy_backends); P_CASEBOOL(800, monitor_repaint); case 801: diff --git a/src/picom.c b/src/picom.c index d6b52b589b..fb7e49e2e2 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1445,9 +1445,9 @@ static bool redirect_start(session_t *ps) { pixman_region32_init(&ps->damage_ring[i]); } - ps->frame_pacing = true; - if (ps->o.legacy_backends || ps->o.benchmark || - !ps->backend_data->ops->last_render_time) { + ps->frame_pacing = !ps->o.no_frame_pacing; + 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 // benchmark mode, or if the backend doesn't report render time log_info("Disabling frame pacing."); From d0c121ec834a4c837ab78f574ece5f547e374054 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Jun 2023 23:12:31 +0100 Subject: [PATCH 21/37] core: print some timing info from draw_callback_impl Signed-off-by: Yuxuan Shui --- src/picom.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/picom.c b/src/picom.c index fb7e49e2e2..8a58fe50f7 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1741,8 +1741,24 @@ static void handle_pending_updates(EV_P_ struct session *ps) { } static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { + struct timespec now; + int64_t draw_callback_enter_us; + clock_gettime(CLOCK_MONOTONIC, &now); + + draw_callback_enter_us = (now.tv_sec * 1000000LL + now.tv_nsec / 1000); + if (ps->next_render != 0) { + log_trace("Schedule delay: %" PRIi64 " us", + draw_callback_enter_us - (int64_t)ps->next_render); + } + handle_pending_updates(EV_A_ ps); + int64_t after_handle_pending_updates_us; + clock_gettime(CLOCK_MONOTONIC, &now); + after_handle_pending_updates_us = (now.tv_sec * 1000000LL + now.tv_nsec / 1000); + log_trace("handle_pending_updates took: %" PRIi64 " us", + after_handle_pending_updates_us - draw_callback_enter_us); + if (ps->first_frame) { // If we are still rendering the first frame, if some of the windows are // unmapped/destroyed during the above handle_pending_updates() call, they @@ -1799,6 +1815,12 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { ev_timer_start(EV_A_ & ps->fade_timer); } + int64_t after_preprocess_us; + clock_gettime(CLOCK_MONOTONIC, &now); + after_preprocess_us = (now.tv_sec * 1000000LL + now.tv_nsec / 1000); + log_trace("paint_preprocess took: %" PRIi64 " us", + after_preprocess_us - after_handle_pending_updates_us); + // If the screen is unredirected, free all_damage to stop painting if (ps->redirected && ps->o.stoppaint_force != ON) { static int paint = 0; From 6af0251f2a856e2f62170cf317363eb95ce8bc33 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 12 Jun 2023 11:49:06 +0100 Subject: [PATCH 22/37] readme: add some badges and a link to our discord Signed-off-by: Yuxuan Shui --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 0fecde197e..5372111362 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,15 @@ picom ===== +[![circleci](https://circleci.com/gh/yshui/picom.svg?style=shield)](https://circleci.com/gh/yshui/picom) +[![codecov](https://codecov.io/gh/yshui/picom/branch/next/graph/badge.svg?token=NRSegi0Gze)](https://codecov.io/gh/yshui/picom) +[![chat on discord](https://img.shields.io/discord/1106224720833159198?logo=discord)](https://discord.gg/uqmNX6dR) + __picom__ is a compositor for X, and a [fork of Compton](History.md). **This is a development branch, bugs to be expected** -You can leave your feedback or thoughts in the [discussion tab](https://github.com/yshui/picom/discussions). +You can leave your feedback or thoughts in the [discussion tab](https://github.com/yshui/picom/discussions), or chat with other users on [discord](https://discord.gg/uqmNX6dR)! ## Change Log From 5b6f6ecbf5877ca18eb7c028247f67650959e5ef Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 12 Jun 2023 12:03:09 +0100 Subject: [PATCH 23/37] backend: egl: print egl error when eglCreateImage failed Signed-off-by: Yuxuan Shui --- src/backend/gl/egl.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/backend/gl/egl.c b/src/backend/gl/egl.c index f1f67d91e6..9f651544a0 100644 --- a/src/backend/gl/egl.c +++ b/src/backend/gl/egl.c @@ -42,6 +42,30 @@ static PFNEGLDESTROYIMAGEKHRPROC eglDestroyImageProc = NULL; static PFNEGLGETPLATFORMDISPLAYPROC eglGetPlatformDisplayProc = NULL; static PFNEGLCREATEPLATFORMWINDOWSURFACEPROC eglCreatePlatformWindowSurfaceProc = NULL; +const char *eglGetErrorString(EGLint error) { +#define CASE_STR(value) \ + case value: return #value; + switch (error) { + CASE_STR(EGL_SUCCESS) + CASE_STR(EGL_NOT_INITIALIZED) + CASE_STR(EGL_BAD_ACCESS) + CASE_STR(EGL_BAD_ALLOC) + CASE_STR(EGL_BAD_ATTRIBUTE) + CASE_STR(EGL_BAD_CONTEXT) + CASE_STR(EGL_BAD_CONFIG) + CASE_STR(EGL_BAD_CURRENT_SURFACE) + CASE_STR(EGL_BAD_DISPLAY) + CASE_STR(EGL_BAD_SURFACE) + CASE_STR(EGL_BAD_MATCH) + CASE_STR(EGL_BAD_PARAMETER) + CASE_STR(EGL_BAD_NATIVE_PIXMAP) + CASE_STR(EGL_BAD_NATIVE_WINDOW) + CASE_STR(EGL_CONTEXT_LOST) + default: return "Unknown"; + } +#undef CASE_STR +} + /** * Free a glx_texture_t. */ @@ -283,7 +307,8 @@ egl_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b eglpixmap->owned = owned; if (eglpixmap->image == EGL_NO_IMAGE) { - log_error("Failed to create eglpixmap for pixmap %#010x", pixmap); + log_error("Failed to create eglpixmap for pixmap %#010x: %s", pixmap, + eglGetErrorString(eglGetError())); goto err; } From 5826adf8533c81086371af1c309600f8660800e5 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Mon, 12 Jun 2023 21:17:28 +0300 Subject: [PATCH 24/37] backend: gl: remove gl_delete_texture function it wasn't used and it's hard to call it a shortcut --- src/backend/gl/gl_common.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index f7e721c289..5e3a505d0b 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -188,10 +188,6 @@ void gl_present(backend_t *base, const region_t *); bool gl_read_pixel(backend_t *base, void *image_data, int x, int y, struct color *output); enum device_status gl_device_status(backend_t *base); -static inline void gl_delete_texture(GLuint texture) { - glDeleteTextures(1, &texture); -} - /** * Get a textual representation of an OpenGL error. */ From 8245de27ed7c329e710c0569021c2829ed9d4fd1 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Mon, 12 Jun 2023 21:24:57 +0300 Subject: [PATCH 25/37] backend: gl: do not leak back and default mask textures they're generated during backend initialization and now deleted during deinitialization --- src/backend/gl/gl_common.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index 42f7136dc6..df502dc0e1 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -976,6 +976,9 @@ void gl_deinit(struct gl_data *gd) { gd->default_shader = NULL; } + glDeleteTextures(1, &gd->default_mask_texture); + glDeleteTextures(1, &gd->back_texture); + gl_check_err(); } From 1e398b9c24fdbf8db76190f51f82b901b15f93de Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Mon, 12 Jun 2023 23:11:20 +0300 Subject: [PATCH 26/37] backend: dummy: fix a typo in the dummy_get_blur_size function "reisze_region" => "resize_region" --- src/backend/dummy/dummy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/dummy/dummy.c b/src/backend/dummy/dummy.c index 7e06facecc..0c39741949 100644 --- a/src/backend/dummy/dummy.c +++ b/src/backend/dummy/dummy.c @@ -162,7 +162,7 @@ void dummy_destroy_blur_context(struct backend_base *base attr_unused, void *ctx } void dummy_get_blur_size(void *ctx attr_unused, int *width, int *height) { - // These numbers are arbitrary, to make sure the reisze_region code path is + // These numbers are arbitrary, to make sure the resize_region code path is // covered. *width = 5; *height = 5; From 223872bc7f65274af24cb441e3ac50dd3ec50bcf Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Mon, 12 Jun 2023 23:41:25 +0300 Subject: [PATCH 27/37] backend: dummy: do not leak owned pixmaps free pixmaps which ownership was transferred to the backend --- src/backend/dummy/dummy.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/backend/dummy/dummy.c b/src/backend/dummy/dummy.c index 0c39741949..5f5e1229a7 100644 --- a/src/backend/dummy/dummy.c +++ b/src/backend/dummy/dummy.c @@ -17,6 +17,7 @@ struct dummy_image { xcb_pixmap_t pixmap; bool transparent; int *refcount; + bool owned; UT_hash_handle hh; }; @@ -42,6 +43,9 @@ void dummy_deinit(struct backend_base *data) { log_warn("Backend image for pixmap %#010x is not freed", img->pixmap); HASH_DEL(dummy->images, img); free(img->refcount); + if (img->owned) { + xcb_free_pixmap(data->c, img->pixmap); + } free(img); } free(dummy); @@ -82,7 +86,7 @@ bool dummy_blur(struct backend_base *backend_data attr_unused, double opacity at } void *dummy_bind_pixmap(struct backend_base *base, xcb_pixmap_t pixmap, - struct xvisual_info fmt, bool owned attr_unused) { + struct xvisual_info fmt, bool owned) { auto dummy = (struct dummy_data *)base; struct dummy_image *img = NULL; HASH_FIND_INT(dummy->images, &pixmap, img); @@ -96,6 +100,7 @@ void *dummy_bind_pixmap(struct backend_base *base, xcb_pixmap_t pixmap, img->transparent = fmt.alpha_size != 0; img->refcount = ccalloc(1, int); *img->refcount = 1; + img->owned = owned; HASH_ADD_INT(dummy->images, pixmap, img); return (void *)img; @@ -112,6 +117,9 @@ void dummy_release_image(backend_t *base, void *image) { if (*img->refcount == 0) { HASH_DEL(dummy->images, img); free(img->refcount); + if (img->owned) { + xcb_free_pixmap(base->c, img->pixmap); + } free(img); } } From 99568446472c8eed9faa7687b2f94aa29302d979 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Jun 2023 00:22:54 +0100 Subject: [PATCH 28/37] backend: log more timing info Signed-off-by: Yuxuan Shui --- src/backend/backend.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/backend/backend.c b/src/backend/backend.c index 36f80cc685..da1fa42996 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) Yuxuan Shui +#include #include #include @@ -82,6 +83,9 @@ void handle_device_reset(session_t *ps) { /// paint all windows void paint_all_new(session_t *ps, struct managed_win *t) { + struct timespec now = get_time_timespec(); + auto paint_all_start_us = + (uint64_t)now.tv_sec * 1000000UL + (uint64_t)now.tv_nsec / 1000; if (ps->backend_data->ops->device_status && ps->backend_data->ops->device_status(ps->backend_data) != DEVICE_STATUS_NORMAL) { return handle_device_reset(ps); @@ -96,6 +100,12 @@ void paint_all_new(session_t *ps, struct managed_win *t) { ps->xsync_exists = false; } } + + now = get_time_timespec(); + auto after_sync_fence_us = + (uint64_t)now.tv_sec * 1000000UL + (uint64_t)now.tv_nsec / 1000; + log_trace("Time spent on sync fence: %" PRIu64 " us", + after_sync_fence_us - paint_all_start_us); // All painting will be limited to the damage, if _some_ of // the paints bleed out of the damage region, it will destroy // part of the image we want to reuse @@ -176,17 +186,17 @@ void paint_all_new(session_t *ps, struct managed_win *t) { region_t reg_shadow_clip; pixman_region32_init(®_shadow_clip); - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - auto now_us = (uint64_t)(now.tv_sec * 1000000 + now.tv_nsec / 1000); + now = get_time_timespec(); + 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) %ld %ld", - labs((int64_t)now_us - (int64_t)ps->next_render), - now_us < ps->next_render ? "early" : "late", now_us, - ps->next_render); + 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); ps->last_schedule_delay = 0; - if (now_us > ps->next_render) { - ps->last_schedule_delay = now_us - ps->next_render; + if (after_damage_us > ps->next_render) { + ps->last_schedule_delay = after_damage_us - ps->next_render; } } ps->did_render = true; From 0deaa9a241fccb30ba363c3b071028b1f846de0a Mon Sep 17 00:00:00 2001 From: Kurenshe Nurdaulet <63652620+EpsilonKu@users.noreply.github.com> Date: Thu, 15 Jun 2023 13:23:49 +0600 Subject: [PATCH 29/37] Update discord link --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5372111362..b7ea2a18e7 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ __picom__ is a compositor for X, and a [fork of Compton](History.md). **This is a development branch, bugs to be expected** -You can leave your feedback or thoughts in the [discussion tab](https://github.com/yshui/picom/discussions), or chat with other users on [discord](https://discord.gg/uqmNX6dR)! +You can leave your feedback or thoughts in the [discussion tab](https://github.com/yshui/picom/discussions), or chat with other users on [discord](https://discord.gg/SY5JJzPgME)! ## Change Log From 4d724047ef0e8ce713e9d4d51773a3af4146b3a3 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Fri, 16 Jun 2023 17:57:59 +0300 Subject: [PATCH 30/37] options: handle max-brightness option better allow it's use with the egl backend and report it's unavailability better --- src/options.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/options.c b/src/options.c index 1fe955adbe..2ee504e91a 100644 --- a/src/options.c +++ b/src/options.c @@ -824,18 +824,16 @@ bool get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, opt->inactive_dim = normalize_d(opt->inactive_dim); opt->frame_opacity = normalize_d(opt->frame_opacity); opt->shadow_opacity = normalize_d(opt->shadow_opacity); - opt->max_brightness = normalize_d(opt->max_brightness); if (opt->max_brightness < 1.0) { - if (opt->use_damage) { - log_warn("--max-brightness requires --no-use-damage. Falling " - "back to 1.0"); + if (opt->backend == BKEND_XRENDER || opt->legacy_backends) { + log_warn("--max-brightness is not supported by the %s backend. " + "Falling back to 1.0.", + opt->backend == BKEND_XRENDER ? "xrender" : "legacy glx"); opt->max_brightness = 1.0; - } - - if (opt->legacy_backends || opt->backend != BKEND_GLX) { - log_warn("--max-brightness requires the new glx " - "backend. Falling back to 1.0"); + } else if (opt->use_damage) { + log_warn("--max-brightness requires --no-use-damage. Falling " + "back to 1.0."); opt->max_brightness = 1.0; } } From 9f9cff3b0669b4dad7050e928b9bf8789c2252a0 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Fri, 16 Jun 2023 18:21:34 +0300 Subject: [PATCH 31/37] options: unify unavailability reporting of some options unify unavailability reporting of the max-brightness, window-shader-fg and window-shader-fg-rule options --- src/options.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/options.c b/src/options.c index 2ee504e91a..2b0bb1771e 100644 --- a/src/options.c +++ b/src/options.c @@ -805,10 +805,10 @@ bool get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, if (opt->window_shader_fg || opt->window_shader_fg_rules) { if (opt->backend == BKEND_XRENDER || opt->legacy_backends) { log_warn(opt->backend == BKEND_XRENDER - ? "Shader interface is not available for the " - "xrender backend." - : "The new shader interface is not available for " - "the legacy glx backend. You may want to use " + ? "Shader interface is not supported by the xrender " + "backend." + : "The new shader interface is not supported by the " + "legacy glx backend. You may want to use " "--glx-fshader-win instead."); opt->window_shader_fg = NULL; c2_list_free(&opt->window_shader_fg_rules, free); From ecbc8b50edd81c8fd1f19570590dd78fd4647d20 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Fri, 16 Jun 2023 19:42:36 +0300 Subject: [PATCH 32/37] backend: xrender: set created picture to repeat when binding a pixmap --- src/backend/xrender/xrender.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/xrender/xrender.c b/src/backend/xrender/xrender.c index 686e6deb8c..0041817aca 100644 --- a/src/backend/xrender/xrender.c +++ b/src/backend/xrender/xrender.c @@ -520,8 +520,9 @@ bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, bool inner->height = img->base.eheight = r->height; inner->pixmap = pixmap; inner->has_alpha = fmt.alpha_size != 0; - inner->pict = - x_create_picture_with_visual_and_pixmap(base->c, fmt.visual, pixmap, 0, NULL); + xcb_render_create_picture_value_list_t pic_attrs = {.repeat = XCB_RENDER_REPEAT_NORMAL}; + inner->pict = x_create_picture_with_visual_and_pixmap( + base->c, fmt.visual, pixmap, XCB_RENDER_CP_REPEAT, &pic_attrs); inner->owned = owned; inner->visual = fmt.visual; inner->refcount = 1; From 07303ce2cb13f207df69b05a4b012a1bbb5263cb Mon Sep 17 00:00:00 2001 From: Nikolay Borodin Date: Fri, 16 Jun 2023 23:50:20 +0200 Subject: [PATCH 33/37] core: added proper event handling for XESetWireToEvent --- src/event.c | 5 +++-- src/meson.build | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/event.c b/src/event.c index afaa02c490..da350b95df 100644 --- a/src/event.c +++ b/src/event.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "atom.h" #include "common.h" @@ -687,9 +688,9 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { // For even more details, see: // https://bugs.freedesktop.org/show_bug.cgi?id=35945 // https://lists.freedesktop.org/archives/xcb/2011-November/007337.html - auto proc = XESetWireToEvent(ps->dpy, ev->response_type, 0); + auto proc = XESetWireToEvent(ps->dpy, XCB_EVENT_RESPONSE_TYPE(ev), 0); if (proc) { - XESetWireToEvent(ps->dpy, ev->response_type, proc); + XESetWireToEvent(ps->dpy, XCB_EVENT_RESPONSE_TYPE(ev), proc); XEvent dummy; // Stop Xlib from complaining about lost sequence numbers. diff --git a/src/meson.build b/src/meson.build index 0cd87c896d..a7f25b0231 100644 --- a/src/meson.build +++ b/src/meson.build @@ -31,6 +31,8 @@ foreach i : required_xcb_packages base_deps += [dependency(i, version: '>=1.12.0', required: true)] endforeach +base_deps += [dependency('xcb-util', version: '>=0.4.0', required: true)] + if not cc.has_header('uthash.h') error('Dependency uthash not found') endif From 56745b64d7a2b4875ce3fa14dc168fd1314ece9f Mon Sep 17 00:00:00 2001 From: Nikolay Borodin Date: Sat, 17 Jun 2023 01:08:47 +0200 Subject: [PATCH 34/37] core: event code refactoring --- src/event.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/event.c b/src/event.c index da350b95df..b140082874 100644 --- a/src/event.c +++ b/src/event.c @@ -107,7 +107,7 @@ static inline xcb_window_t attr_pure ev_window(session_t *ps, xcb_generic_event_ static inline const char *ev_name(session_t *ps, xcb_generic_event_t *ev) { static char buf[128]; - switch (ev->response_type & 0x7f) { + switch (XCB_EVENT_RESPONSE_TYPE(ev)) { CASESTRRET(FocusIn); CASESTRRET(FocusOut); CASESTRRET(CreateNotify); @@ -666,7 +666,7 @@ ev_selection_clear(session_t *ps, xcb_selection_clear_event_t attr_unused *ev) { } void ev_handle(session_t *ps, xcb_generic_event_t *ev) { - if ((ev->response_type & 0x7f) != KeymapNotify) { + if (XCB_EVENT_RESPONSE_TYPE(ev) != KeymapNotify) { discard_pending(ps, ev->full_sequence); } @@ -688,9 +688,10 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { // For even more details, see: // https://bugs.freedesktop.org/show_bug.cgi?id=35945 // https://lists.freedesktop.org/archives/xcb/2011-November/007337.html - auto proc = XESetWireToEvent(ps->dpy, XCB_EVENT_RESPONSE_TYPE(ev), 0); + auto response_type = XCB_EVENT_RESPONSE_TYPE(ev); + auto proc = XESetWireToEvent(ps->dpy, response_type, 0); if (proc) { - XESetWireToEvent(ps->dpy, XCB_EVENT_RESPONSE_TYPE(ev), proc); + XESetWireToEvent(ps->dpy, response_type, proc); XEvent dummy; // Stop Xlib from complaining about lost sequence numbers. @@ -709,6 +710,7 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { // XXX redraw needs to be more fine grained queue_redraw(ps); + // the events sent from SendEvent will be ignored switch (ev->response_type) { case FocusIn: ev_focus_in(ps, (xcb_focus_in_event_t *)ev); break; case FocusOut: ev_focus_out(ps, (xcb_focus_out_event_t *)ev); break; From c065ad1b8d8f7b0dc1db2b9f5a716d96440a62d4 Mon Sep 17 00:00:00 2001 From: Nikolay Borodin Date: Sat, 17 Jun 2023 01:15:38 +0200 Subject: [PATCH 35/37] Refactored meson.build --- src/meson.build | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/meson.build b/src/meson.build index a7f25b0231..98c50664a3 100644 --- a/src/meson.build +++ b/src/meson.build @@ -20,7 +20,7 @@ required_xcb_packages = [ ] required_packages = [ - 'x11', 'x11-xcb', 'xcb-renderutil', 'xcb-image', 'xext', 'pixman-1' + 'x11', 'x11-xcb', 'xcb-renderutil', 'xcb-image', 'xext', 'pixman-1', 'xcb-util' ] foreach i : required_packages @@ -31,8 +31,6 @@ foreach i : required_xcb_packages base_deps += [dependency(i, version: '>=1.12.0', required: true)] endforeach -base_deps += [dependency('xcb-util', version: '>=0.4.0', required: true)] - if not cc.has_header('uthash.h') error('Dependency uthash not found') endif From 73a366ffe04484b68e7596d89dd0e28ceb787286 Mon Sep 17 00:00:00 2001 From: Monsterovich Date: Sat, 17 Jun 2023 01:27:20 +0200 Subject: [PATCH 36/37] Added xcb-util to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b7ea2a18e7..5b358bbe6b 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ Assuming you already have all the usual building tools installed (e.g. gcc, pyth * libXext * xproto * xcb +* xcb-util * xcb-damage * xcb-dpms * xcb-xfixes From c02a1e2a30943d61db0586a26d410995261c68c4 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Sat, 17 Jun 2023 16:41:15 +0300 Subject: [PATCH 37/37] ci: install the xcb-util dependency the xcb-util dependency that was introduced recently isn't installed by the github's ci workflow what results in ci failures --- .github/workflows/codeql-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 2e3dfe7e26..c9ebfa6bc4 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -36,7 +36,7 @@ jobs: languages: ${{ matrix.language }} # Install dependencies - - run: sudo apt update && sudo apt install libxext-dev libxcb1-dev libxcb-dpms0-dev libxcb-damage0-dev libxcb-xfixes0-dev libxcb-shape0-dev libxcb-render-util0-dev libxcb-render0-dev libxcb-randr0-dev libxcb-composite0-dev libxcb-image0-dev libxcb-present-dev libxcb-glx0-dev libpixman-1-dev libdbus-1-dev libconfig-dev libgl1-mesa-dev libpcre2-dev libevdev-dev uthash-dev libev-dev libx11-xcb-dev meson ninja-build + - run: sudo apt update && sudo apt install libxext-dev libxcb1-dev libxcb-dpms0-dev libxcb-damage0-dev libxcb-xfixes0-dev libxcb-shape0-dev libxcb-render-util0-dev libxcb-render0-dev libxcb-randr0-dev libxcb-composite0-dev libxcb-image0-dev libxcb-present-dev libxcb-glx0-dev libxcb-util-dev libpixman-1-dev libdbus-1-dev libconfig-dev libgl1-mesa-dev libpcre2-dev libevdev-dev uthash-dev libev-dev libx11-xcb-dev meson ninja-build if: ${{ matrix.language == 'cpp' }} # Autobuild