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 <yshuiv7@gmail.com>

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

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
@@ -1039,19 +1006,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) {
@@ -1553,6 +1507,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);
@@ -2171,17 +2127,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
@@ -2790,7 +2738,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);

src/x.c

@@ -9,6 +9,7 @@
 #include <pixman.h>
 #include <xcb/composite.h>
 #include <xcb/damage.h>
+#include <xcb/dpms.h>
 #include <xcb/glx.h>
 #include <xcb/present.h>
 #include <xcb/randr.h>
@@ -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,

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);