From 64f4eaa237a8488ca8373cac1413247ea6c44db3 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 5 Jul 2023 05:54:44 +0100 Subject: [PATCH] core: don't unredir when display is turned off We unredirect because we receive bad vblank events, and also vblank events at a different interval compared to when the screen is on. But it is enough to just not record the vblank interval statistics when the screen is off. Although, unredirecting when display is off can also fix the problem where use-damage causes the screen to flicker when the display is turned off then back on. So we need something else for that. Signed-off-by: Yuxuan Shui --- src/common.h | 2 -- src/picom.c | 63 +++++----------------------------------------------- src/x.c | 20 +++++++++++++++++ src/x.h | 3 +++ 4 files changed, 28 insertions(+), 60 deletions(-) diff --git a/src/common.h b/src/common.h index 6480f0e93c..2d9096ad93 100644 --- a/src/common.h +++ b/src/common.h @@ -150,8 +150,6 @@ typedef struct session { // === Event handlers === /// ev_io for X connection ev_io xiow; - /// Timer for checking DPMS power level - ev_timer dpms_check_timer; /// Timeout for delayed unredirection. ev_timer unredir_timer; /// Timer for fading diff --git a/src/picom.c b/src/picom.c index 193a53d94b..44c35f0dca 100644 --- a/src/picom.c +++ b/src/picom.c @@ -122,27 +122,6 @@ static inline int64_t get_time_ms(void) { return (int64_t)tp.tv_sec * 1000 + (int64_t)tp.tv_nsec / 1000000; } -static inline bool dpms_screen_is_off(xcb_dpms_info_reply_t *info) { - // state is a bool indicating whether dpms is enabled - return info->state && (info->power_level != XCB_DPMS_DPMS_MODE_ON); -} - -void check_dpms_status(EV_P attr_unused, ev_timer *w, int revents attr_unused) { - auto ps = session_ptr(w, dpms_check_timer); - auto r = xcb_dpms_info_reply(ps->c.c, xcb_dpms_info(ps->c.c), NULL); - if (!r) { - log_fatal("Failed to query DPMS status."); - abort(); - } - 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); - } - free(r); -} - /** * Find matched window. * @@ -329,18 +308,6 @@ void handle_end_of_vblank(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 @@ -1041,19 +1008,6 @@ paint_preprocess(session_t *ps, bool *fade_running, bool *animation) { // If there's no window to paint, and the screen isn't redirected, // don't redirect it. unredir_possible = true; - } else if (ps->screen_is_off) { - // Screen is off, unredirect - // We do this unconditionally disregarding "unredir_if_possible" - // because it's important for correctness, because we need to - // workaround problems X server has around screen off. - // - // Known problems: - // 1. Sometimes OpenGL front buffer can lose content, and if we - // are doing partial updates (i.e. use-damage = true), the - // result will be wrong. - // 2. For frame pacing, X server sends bogus - // PresentCompleteNotify events when screen is off. - unredir_possible = true; } if (unredir_possible) { if (ps->redirected) { @@ -1555,6 +1509,8 @@ handle_present_complete_notify(session_t *ps, xcb_present_complete_notify_event_ return; } + x_check_dpms_status(ps); + if (ps->last_msc_instant != 0) { auto frame_count = cne->msc - ps->last_msc; int frame_time = (int)((cne->ust - ps->last_msc_instant) / frame_count); @@ -2173,17 +2129,9 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ext_info = xcb_get_extension_data(ps->c.c, &xcb_dpms_id); ps->dpms_exists = ext_info && ext_info->present; - if (ps->dpms_exists) { - auto r = xcb_dpms_info_reply(ps->c.c, xcb_dpms_info(ps->c.c), NULL); - if (!r) { - log_fatal("Failed to query DPMS info"); - goto err; - } - ps->screen_is_off = dpms_screen_is_off(r); - // Check screen status every half second - ev_timer_init(&ps->dpms_check_timer, check_dpms_status, 0, 0.5); - ev_timer_start(ps->loop, &ps->dpms_check_timer); - free(r); + if (!ps->dpms_exists) { + log_fatal("No DPMS extension"); + exit(1); } // Parse configuration file @@ -2792,7 +2740,6 @@ static void session_destroy(session_t *ps) { // Stop libev event handlers 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_timer_stop(ps->loop, &ps->draw_timer); ev_prepare_stop(ps->loop, &ps->event_check); ev_signal_stop(ps->loop, &ps->usr1_signal); diff --git a/src/x.c b/src/x.c index 54cf702eca..89786e18e3 100644 --- a/src/x.c +++ b/src/x.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -789,6 +790,25 @@ void x_request_vblank_event(session_t *ps, uint64_t msc) { ps->vblank_event_requested = true; } +static inline bool dpms_screen_is_off(xcb_dpms_info_reply_t *info) { + // state is a bool indicating whether dpms is enabled + return info->state && (info->power_level != XCB_DPMS_DPMS_MODE_ON); +} + +void x_check_dpms_status(session_t *ps) { + auto r = xcb_dpms_info_reply(ps->c.c, xcb_dpms_info(ps->c.c), NULL); + if (!r) { + log_fatal("Failed to query DPMS status."); + abort(); + } + 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; + } + free(r); +} + /** * Convert a struct conv to a X picture convolution filter, normalizing the kernel * in the process. Allow the caller to specify the element at the center of the kernel, diff --git a/src/x.h b/src/x.h index 39bf5ab0c1..60bcfef492 100644 --- a/src/x.h +++ b/src/x.h @@ -422,3 +422,6 @@ uint32_t attr_deprecated xcb_generate_id(xcb_connection_t *c); /// Ask X server to send us a notification for the next end of vblank. void x_request_vblank_event(session_t *ps, uint64_t msc); + +/// Update ps->screen_is_off to reflect the current DPMS state. +void x_check_dpms_status(session_t *ps);