Turn around frame update model

Keeping a framebuffer for clients to request from seems to be a better fit for
the VNC standard.

examples/draw.c

@@ -28,27 +28,6 @@ struct draw {
 	struct nvnc_fb* fb;
 };
 
-void on_fb_req(struct nvnc_client *client, bool incremental, uint16_t x, uint16_t y,
-	       uint16_t width, uint16_t height)
-{
-	if (incremental)
-		return;
-
-	struct nvnc* server = nvnc_get_server(client);
-	assert(server);
-
-	struct draw *draw = nvnc_get_userdata(server);
-	assert(draw);
-
-	int fbwidth = nvnc_fb_get_width(draw->fb);
-	int fbheight = nvnc_fb_get_height(draw->fb);
-
-	struct pixman_region16 region;
-	pixman_region_init_rect(&region, 0, 0, fbwidth, fbheight);
-	nvnc_update_fb(server, draw->fb, &region, NULL);
-	pixman_region_fini(&region);
-}
-
 void on_pointer_event(struct nvnc_client *client, uint16_t x, uint16_t y,
 		      enum nvnc_button_mask buttons)
 {
@@ -70,7 +49,7 @@ void on_pointer_event(struct nvnc_client *client, uint16_t x, uint16_t y,
 	struct pixman_region16 region;
 	pixman_region_init_rect(&region, 0, 0, width, height);
 	pixman_region_intersect_rect(&region, &region, x, y, 1, 1);
-	nvnc_update_fb(server, draw->fb, &region, NULL);
+	nvnc_feed_frame(server, draw->fb, &region);
 	pixman_region_fini(&region);
 }
 
@@ -91,7 +70,6 @@ int main(int argc, char *argv[])
 
 	nvnc_set_dimensions(server, width, height, format);
 	nvnc_set_name(server, "Draw");
-	nvnc_set_fb_req_fn(server, on_fb_req);
 	nvnc_set_pointer_fn(server, on_pointer_event);
 	nvnc_set_userdata(server, &draw);
 

examples/png-server.c

@@ -23,25 +23,6 @@
 
 struct nvnc_fb* read_png_file(const char *filename);
 
-void on_fb_req(struct nvnc_client *client, bool incremental,
-	       uint16_t x, uint16_t y, uint16_t width, uint16_t height)
-{
-	if (incremental)
-		return;
-
-	struct nvnc *server = nvnc_get_server(client);
-	assert(server);
-
-	struct nvnc_fb *fb = nvnc_get_userdata(server);
-	assert(fb);
-
-	struct pixman_region16 region;
-	pixman_region_init_rect(&region, 0, 0, nvnc_fb_get_width(fb),
-				nvnc_fb_get_height(fb));
-	nvnc_update_fb(server, fb, &region, NULL);
-	pixman_region_fini(&region);
-}
-
 int main(int argc, char *argv[])
 {
 	const char *file = argv[1];
@@ -65,8 +46,12 @@ int main(int argc, char *argv[])
 
 	nvnc_set_dimensions(server, width, height, fourcc_format);
 	nvnc_set_name(server, file);
-	nvnc_set_fb_req_fn(server, on_fb_req);
+
-	nvnc_set_userdata(server, &fb);
+	struct pixman_region16 region;
+	pixman_region_init_rect(&region, 0, 0, nvnc_fb_get_width(fb),
+				nvnc_fb_get_height(fb));
+	nvnc_feed_frame(server, fb, &region);
+	pixman_region_fini(&region);
 
 	uv_run(uv_default_loop(), UV_RUN_DEFAULT);
 

include/neatvnc.h

@@ -44,7 +44,6 @@ typedef void (*nvnc_fb_req_fn)(struct nvnc_client*, bool is_incremental,
                                uint16_t width, uint16_t height);
 typedef void (*nvnc_client_fn)(struct nvnc_client*);
 typedef void (*nvnc_damage_fn)(struct pixman_region16 *damage, void *userdata);
-typedef void (*nvnc_update_done_fn)(struct nvnc*);
 
 struct nvnc *nvnc_open(const char *addr, uint16_t port);
 void nvnc_close(struct nvnc *self);
@@ -77,13 +76,11 @@ uint16_t nvnc_fb_get_height(const struct nvnc_fb* fb);
 uint32_t nvnc_fb_get_fourcc_format(const struct nvnc_fb* fb);
 
 /*
- * Send an updated framebuffer to all clients with pending update requests.
+ * Feed a new frame to the server. The damaged region is sent to clients
- *
+ * immediately.
- * Only the region specified by the region argument is updated.
  */
-int nvnc_update_fb(struct nvnc *self, const struct nvnc_fb* fb,
+int nvnc_feed_frame(struct nvnc *self, struct nvnc_fb* fb,
-                   const struct pixman_region16* region,
+                    const struct pixman_region16* damage);
-                   nvnc_update_done_fn on_update_done);
 
 /*
  * Find the regions that differ between fb0 and fb1. Regions outside the hinted

src/server.c

@@ -73,7 +73,9 @@ struct nvnc_client {
 	enum rfb_encodings encodings[MAX_ENCODINGS + 1];
 	size_t n_encodings;
 	LIST_ENTRY(nvnc_client) link;
-	struct pixman_region16 requested_region;
+	struct pixman_region16 damage;
+	int n_pending_requests;
+	bool is_updating;
 	nvnc_client_fn cleanup_fn;
 	z_stream z_stream;
 	size_t buffer_index;
@@ -96,11 +98,11 @@ struct nvnc {
 	struct nvnc_client_list clients;
 	struct vnc_display display;
 	void *userdata;
-	int n_pending_updates;
 	nvnc_key_fn key_fn;
 	nvnc_pointer_fn pointer_fn;
 	nvnc_fb_req_fn fb_req_fn;
 	nvnc_client_fn new_client_fn;
+	struct nvnc_fb* frame;
 };
 
 struct fb_update_work {
@@ -110,9 +112,10 @@ struct fb_update_work {
 	struct rfb_pixel_format server_fmt;
 	struct vec frame;
 	const struct nvnc_fb *fb;
-	nvnc_update_done_fn on_done;
 };
 
+int schedule_client_update_fb(struct nvnc_client *client);
+
 static const char* fourcc_to_string(uint32_t fourcc)
 {
 	static char buffer[5];
@@ -148,7 +151,7 @@ static void cleanup_client(uv_handle_t* handle)
 	deflateEnd(&client->z_stream);
 
 	LIST_REMOVE(client, link);
-	pixman_region_fini(&client->requested_region);
+	pixman_region_fini(&client->damage);
 	free(client);
 }
 
@@ -539,6 +542,25 @@ static int on_client_set_encodings(struct nvnc_client *client)
 	return sizeof(*msg) + 4 * n_encodings;
 }
 
+static void process_fb_update_requests(struct nvnc_client *client)
+{
+	if (!client->server->frame)
+		return;
+
+	if (uv_is_closing((uv_handle_t*)&client->stream_handle))
+		return;
+
+	if (!pixman_region_not_empty(&client->damage))
+		return;
+
+	if (client->is_updating)
+		return;
+
+	client->is_updating = true;
+
+	schedule_client_update_fb(client);
+}
+
 static int on_client_fb_update_request(struct nvnc_client *client)
 {
 	struct nvnc *server = client->server;
@@ -553,14 +575,21 @@ static int on_client_fb_update_request(struct nvnc_client *client)
 	int width = ntohs(msg->width);
 	int height = ntohs(msg->height);
 
-	pixman_region_union_rect(&client->requested_region,
+	client->n_pending_requests++;
-				 &client->requested_region,
+
+	/* Note: The region sent from the client is ignored for incremental
+	 * updates. This avoids superfluous complexity.
+	 */
+	if (!incremental)
+		pixman_region_union_rect(&client->damage, &client->damage,
 					 x, y, width, height);
 
 	nvnc_fb_req_fn fn = server->fb_req_fn;
 	if (fn)
 		fn(client, incremental, x, y, width, height);
 
+	process_fb_update_requests(client);
+
 	return sizeof(*msg);
 }
 
@@ -721,7 +750,7 @@ static void on_connection(uv_stream_t *server_stream, int status)
 		return;
 	}
 
-	pixman_region_init(&client->requested_region);
+	pixman_region_init(&client->damage);
 
 	uv_tcp_init(uv_default_loop(), &client->stream_handle);
 
@@ -798,6 +827,9 @@ void nvnc_close(struct nvnc *self)
 {
 	struct nvnc_client *client;
 
+	if (self->frame)
+		nvnc_fb_unref(self->frame);
+
 	LIST_FOREACH(client, &self->clients, link)
 		client_unref(client);
 
@@ -862,19 +894,17 @@ void on_client_update_fb_done(uv_work_t *work, int status)
 		vnc__write((uv_stream_t*)&client->stream_handle,
 			   frame->data, frame->len, free_write_buffer);
 
-	if (--server->n_pending_updates == 0)
+	client->is_updating = false;
-		if (update->on_done)
+	client->n_pending_requests--;
-			update->on_done(server);
+	process_fb_update_requests(client);
-
-	assert(server->n_pending_updates >= 0);
 	client_unref(client);
 }
 
-int schedule_client_update_fb(struct nvnc_client *client,
+int schedule_client_update_fb(struct nvnc_client *client)
-			      const struct nvnc_fb *fb,
-			      struct pixman_region16 *region,
-			      nvnc_update_done_fn on_update_done)
 {
+	struct nvnc_fb* fb = client->server->frame;
+	assert(fb);
+
 	struct fb_update_work *work = calloc(1, sizeof(*work));
 	if (!work)
 		return -1;
@@ -884,11 +914,10 @@ int schedule_client_update_fb(struct nvnc_client *client,
 
 	work->client = client;
 	work->fb = fb;
-	work->on_done = on_update_done;
 
-	/* The client's region is exchanged for an empty one */
+	/* The client's damage is exchanged for an empty one */
-	work->region = client->requested_region;
+	work->region = client->damage;
-	pixman_region_init(&client->requested_region);
+	pixman_region_init(&client->damage);
 
 	int rc = vec_init(&work->frame, fb->width * fb->height * 3 / 2);
 	if (rc < 0)
@@ -901,8 +930,6 @@ int schedule_client_update_fb(struct nvnc_client *client,
 	if (rc < 0)
 		goto queue_failure;
 
-	client->server->n_pending_updates++;
-
 	return 0;
 
 queue_failure:
@@ -915,47 +942,30 @@ pixfmt_failure:
 }
 
 EXPORT
-int nvnc_update_fb(struct nvnc *self, const struct nvnc_fb *fb,
+int nvnc_feed_frame(struct nvnc *self, struct nvnc_fb *fb,
-		   const struct pixman_region16 *input_region,
+		    const struct pixman_region16 *damage)
-		   nvnc_update_done_fn on_update_done)
 {
-	int rc = -1;
-
-	/* TODO: Support some more tiling modes */
-	if (fb->fourcc_modifier != DRM_FORMAT_MOD_LINEAR)
-		return -1;
-
-	/* Scheduling new updates would cause racing */
-	if (self->n_pending_updates > 0)
-		return -1;
-
-	struct pixman_region16 region;
-	pixman_region_init(&region);
-
-	pixman_region_intersect_rect(&region,
-				     (struct pixman_region16*)input_region,
-				     0, 0, fb->width, fb->height);
-
 	struct nvnc_client *client;
 
+	if (self->frame)
+		nvnc_fb_unref(self->frame);
+
+	self->frame = fb;
+	nvnc_fb_ref(self->frame);
+
 	LIST_FOREACH(client, &self->clients, link) {
 		if (uv_is_closing((uv_handle_t*)&client->stream_handle))
 			continue;
 
-		struct pixman_region16* cregion = &client->requested_region;
+		pixman_region_union(&client->damage, &client->damage,
+				    (struct pixman_region16*)damage);
+		pixman_region_intersect_rect(&client->damage, &client->damage,
+					     0, 0, fb->width, fb->height);
 
-		pixman_region_intersect(cregion, cregion, &region);
+		process_fb_update_requests(client);
-
-		if (!pixman_region_not_empty(cregion))
-			continue;
-
-		schedule_client_update_fb(client, fb, &region, on_update_done);
 	}
 
-	rc = self->n_pending_updates > 0 ? 0 : -1;
+	return 0;
-failure:
-	pixman_region_fini(&region);
-	return rc;
 }
 
 EXPORT