Improve frame callback and commit handling

The few places that required a surface commit relied on render_frame and
render_frame_background to form it form them after they had set up frame
callback events. This would fail if render_frame ran out of buffers and
exited early, as the caller would still wait indefinitely on the frame
callback.

swaylock would quite consistently run out of buffers when rendering
immediately after a configure event, such as when the keypress causing
render also caused outputs to be enabled from idle.

Restructure the render and commit handling slightly so that the whole
frame callback and commit setup is handled by the render code, which now
has a single render entrypoint. This both avoids stalls from lacking
commits, but also fixes the case the configure path to respect frame
callbacks so we do not run out of buffers in the first place.

include/swaylock.h

@@ -113,12 +113,13 @@ struct swaylock_surface {
 	struct ext_session_lock_surface_v1 *ext_session_lock_surface_v1;
 	struct pool_buffer indicator_buffers[2];
 	bool created;
-	bool frame_pending, dirty;
+	bool dirty;
 	uint32_t width, height;
 	int32_t scale;
 	enum wl_output_subpixel subpixel;
 	char *output_name;
 	struct wl_list link;
+	struct wl_callback *frame;
 	// Dimensions of last wl_buffer committed to background surface
 	int last_buffer_width, last_buffer_height;
 };
@@ -133,9 +134,8 @@ struct swaylock_image {
 
 void swaylock_handle_key(struct swaylock_state *state,
 		xkb_keysym_t keysym, uint32_t codepoint);
-void render_frame_background(struct swaylock_surface *surface);
-void render_frame(struct swaylock_surface *surface);
-void damage_surface(struct swaylock_surface *surface);
+
+void render(struct swaylock_surface *surface);
 void damage_state(struct swaylock_state *state);
 void clear_password_buffer(struct swaylock_password *pw);
 void schedule_auth_idle(struct swaylock_state *state);

main.c

@@ -163,59 +163,19 @@ static void ext_session_lock_surface_v1_handle_configure(void *data,
 	surface->width = width;
 	surface->height = height;
 	ext_session_lock_surface_v1_ack_configure(lock_surface, serial);
-	render_frame_background(surface);
-	render_frame(surface);
+	surface->dirty = true;
+	render(surface);
 }
 
 static const struct ext_session_lock_surface_v1_listener ext_session_lock_surface_v1_listener = {
 	.configure = ext_session_lock_surface_v1_handle_configure,
 };
 
-static const struct wl_callback_listener surface_frame_listener;
-
-static void surface_frame_handle_done(void *data, struct wl_callback *callback,
-		uint32_t time) {
-	struct swaylock_surface *surface = data;
-
-	wl_callback_destroy(callback);
-	surface->frame_pending = false;
-
-	if (surface->dirty) {
-		// Schedule a frame in case the surface is damaged again
-		struct wl_callback *callback = wl_surface_frame(surface->surface);
-		wl_callback_add_listener(callback, &surface_frame_listener, surface);
-		surface->frame_pending = true;
-
-		render_frame(surface);
-		surface->dirty = false;
-	}
-}
-
-static const struct wl_callback_listener surface_frame_listener = {
-	.done = surface_frame_handle_done,
-};
-
-void damage_surface(struct swaylock_surface *surface) {
-	if (surface->width == 0 || surface->height == 0) {
-		// Not yet configured
-		return;
-	}
-
-	surface->dirty = true;
-	if (surface->frame_pending) {
-		return;
-	}
-
-	struct wl_callback *callback = wl_surface_frame(surface->surface);
-	wl_callback_add_listener(callback, &surface_frame_listener, surface);
-	surface->frame_pending = true;
-	wl_surface_commit(surface->surface);
-}
-
 void damage_state(struct swaylock_state *state) {
 	struct swaylock_surface *surface;
 	wl_list_for_each(surface, &state->surfaces, link) {
-		damage_surface(surface);
+		surface->dirty = true;
+		render(surface);
 	}
 }
 
@@ -226,7 +186,8 @@ static void handle_wl_output_geometry(void *data, struct wl_output *wl_output,
 	struct swaylock_surface *surface = data;
 	surface->subpixel = subpixel;
 	if (surface->state->run_display) {
-		damage_surface(surface);
+		surface->dirty = true;
+		render(surface);
 	}
 }
 
@@ -247,7 +208,8 @@ static void handle_wl_output_scale(void *data, struct wl_output *output,
 	struct swaylock_surface *surface = data;
 	surface->scale = factor;
 	if (surface->state->run_display) {
-		damage_surface(surface);
+		surface->dirty = true;
+		render(surface);
 	}
 }
 

render.c

@@ -33,7 +33,23 @@ static void set_color_for_state(cairo_t *cairo, struct swaylock_state *state,
 	}
 }
 
-void render_frame_background(struct swaylock_surface *surface) {
+static void surface_frame_handle_done(void *data, struct wl_callback *callback,
+		uint32_t time) {
+	struct swaylock_surface *surface = data;
+
+	wl_callback_destroy(callback);
+	surface->frame = NULL;
+
+	render(surface);
+}
+
+static const struct wl_callback_listener surface_frame_listener = {
+	.done = surface_frame_handle_done,
+};
+
+static bool render_frame(struct swaylock_surface *surface);
+
+void render(struct swaylock_surface *surface) {
 	struct swaylock_state *state = surface->state;
 
 	int buffer_width = surface->width * surface->scale;
@@ -42,11 +58,17 @@ void render_frame_background(struct swaylock_surface *surface) {
 		return; // not yet configured
 	}
 
-	wl_surface_set_buffer_scale(surface->surface, surface->scale);
+	if (!surface->dirty || surface->frame) {
+		// Nothing to do or frame already pending
+		return;
+	}
+
+	bool need_destroy = false;
+	struct pool_buffer buffer;
 
 	if (buffer_width != surface->last_buffer_width ||
 			buffer_height != surface->last_buffer_height) {
-		struct pool_buffer buffer;
+		need_destroy = true;
 		if (!create_buffer(state->shm, &buffer, buffer_width, buffer_height,
 				WL_SHM_FORMAT_ARGB8888)) {
 			swaylock_log(LOG_ERROR,
@@ -69,15 +91,23 @@ void render_frame_background(struct swaylock_surface *surface) {
 		cairo_restore(cairo);
 		cairo_identity_matrix(cairo);
 
+		wl_surface_set_buffer_scale(surface->surface, surface->scale);
 		wl_surface_attach(surface->surface, buffer.buffer, 0, 0);
 		wl_surface_damage_buffer(surface->surface, 0, 0, INT32_MAX, INT32_MAX);
-		wl_surface_commit(surface->surface);
-		destroy_buffer(&buffer);
+		need_destroy = true;
 
 		surface->last_buffer_width = buffer_width;
 		surface->last_buffer_height = buffer_height;
-	} else {
-		wl_surface_commit(surface->surface);
+	}
+
+	render_frame(surface);
+	surface->dirty = false;
+	surface->frame = wl_surface_frame(surface->surface);
+	wl_callback_add_listener(surface->frame, &surface_frame_listener, surface);
+	wl_surface_commit(surface->surface);
+
+	if (need_destroy) {
+		destroy_buffer(&buffer);
 	}
 }
 
@@ -99,7 +129,7 @@ static void configure_font_drawing(cairo_t *cairo, struct swaylock_state *state,
 	cairo_font_options_destroy(fo);
 }
 
-void render_frame(struct swaylock_surface *surface) {
+static bool render_frame(struct swaylock_surface *surface) {
 	struct swaylock_state *state = surface->state;
 
 	// First, compute the text that will be drawn, if any, since this
@@ -210,7 +240,8 @@ void render_frame(struct swaylock_surface *surface) {
 	struct pool_buffer *buffer = get_next_buffer(state->shm,
 			surface->indicator_buffers, buffer_width, buffer_height);
 	if (buffer == NULL) {
-		return;
+		swaylock_log(LOG_ERROR, "No buffer");
+		return false;
 	}
 
 	// Render the buffer
@@ -352,5 +383,5 @@ void render_frame(struct swaylock_surface *surface) {
 	wl_surface_damage_buffer(surface->child, 0, 0, INT32_MAX, INT32_MAX);
 	wl_surface_commit(surface->child);
 
-	wl_surface_commit(surface->surface);
+	return true;
 }