Merge pull request #915 from emersion/redesign-compositor-resources

compositor: redesign how resources are managed

5 files changed, 90 insertions, 76 deletions

diff --git a/include/wlr/types/wlr_region.h b/include/wlr/types/wlr_region.h
index a79ab61d..be2f8b84 100644
--- a/include/wlr/types/wlr_region.h
+++ b/include/wlr/types/wlr_region.h
@@ -6,9 +6,11 @@
 struct wl_resource;
 
 /*
- * Implements the given resource as region.
+ * Creates a new region resource with the provided new ID. If `resource_list` is
+ * non-NULL, adds the region's resource to the list.
  */
-struct wl_resource *wlr_region_create(struct wl_client *client, uint32_t id);
+struct wl_resource *wlr_region_create(struct wl_client *client,
+	uint32_t version, uint32_t id, struct wl_list *resource_list);
 
 pixman_region32_t *wlr_region_from_resource(struct wl_resource *resource);
 
diff --git a/include/wlr/types/wlr_surface.h b/include/wlr/types/wlr_surface.h
index 5ff9996d..c6eb3c7d 100644
--- a/include/wlr/types/wlr_surface.h
+++ b/include/wlr/types/wlr_surface.h
@@ -80,10 +80,6 @@ struct wlr_surface {
 		struct wl_signal destroy;
 	} events;
 
-	// destroy listener used by compositor
-	struct wl_listener compositor_listener;
-	void *compositor_data;
-
 	// surface commit callback for the role that runs before all others
 	void (*role_committed)(struct wlr_surface *surface, void *role_data);
 	void *role_data;
@@ -102,8 +98,14 @@ typedef void (*wlr_surface_iterator_func_t)(struct wlr_surface *surface,
 	int sx, int sy, void *data);
 
 struct wlr_renderer;
-struct wlr_surface *wlr_surface_create(struct wl_resource *res,
-		struct wlr_renderer *renderer);
+
+/**
+ * Create a new surface resource with the provided new ID. If `resource_list`
+ * is non-NULL, adds the surface's resource to the list.
+ */
+struct wlr_surface *wlr_surface_create(struct wl_client *client,
+		uint32_t version, uint32_t id, struct wlr_renderer *renderer,
+		struct wl_list *resource_list);
 
 /**
  * Set the lifetime role for this surface. Returns 0 on success or -1 if the
@@ -121,10 +123,12 @@ int wlr_surface_set_role(struct wlr_surface *surface, const char *role,
 bool wlr_surface_has_buffer(struct wlr_surface *surface);
 
 /**
- * Create the subsurface implementation for this surface.
+ * Create a new subsurface resource with the provided new ID. If `resource_list`
+ * is non-NULL, adds the subsurface's resource to the list.
  */
-struct wlr_subsurface *wlr_surface_make_subsurface(struct wlr_surface *surface,
-		struct wlr_surface *parent, uint32_t id);
+struct wlr_subsurface *wlr_subsurface_create(struct wlr_surface *surface,
+		struct wlr_surface *parent, uint32_t version, uint32_t id,
+		struct wl_list *resource_list);
 
 /**
  * Get the root of the subsurface tree for this surface.
diff --git a/types/wlr_compositor.c b/types/wlr_compositor.c
index 103b1b61..f10d891c 100644
--- a/types/wlr_compositor.c
+++ b/types/wlr_compositor.c
@@ -75,14 +75,8 @@ static void subcompositor_handle_get_subsurface(struct wl_client *client,
 		return;
 	}
 
-	struct wlr_subsurface *subsurface =
-		wlr_surface_make_subsurface(surface, parent, id);
-	if (subsurface == NULL) {
-		return;
-	}
-
-	wl_list_insert(&subcompositor->subsurface_resources,
-		wl_resource_get_link(subsurface->resource));
+	wlr_subsurface_create(surface, parent, wl_resource_get_version(resource),
+		id, &subcompositor->subsurface_resources);
 }
 
 static const struct wl_subcompositor_interface subcompositor_impl = {
@@ -142,36 +136,17 @@ static struct wlr_compositor *compositor_from_resource(
 	return wl_resource_get_user_data(resource);
 }
 
-static void compositor_handle_surface_destroy(struct wl_listener *listener,
-		void *data) {
-	wl_list_remove(wl_resource_get_link(data));
-}
-
 static void wl_compositor_create_surface(struct wl_client *client,
 		struct wl_resource *resource, uint32_t id) {
 	struct wlr_compositor *compositor = compositor_from_resource(resource);
 
-	struct wl_resource *surface_resource = wl_resource_create(client,
-		&wl_surface_interface, wl_resource_get_version(resource), id);
-	if (surface_resource == NULL) {
-		wl_resource_post_no_memory(resource);
-		return;
-	}
-
-	struct wlr_surface *surface = wlr_surface_create(surface_resource,
-		compositor->renderer);
+	struct wlr_surface *surface = wlr_surface_create(client,
+		wl_resource_get_version(resource), id, compositor->renderer,
+		&compositor->surface_resources);
 	if (surface == NULL) {
-		wl_resource_destroy(surface_resource);
-		wl_resource_post_no_memory(resource);
 		return;
 	}
-	surface->compositor_data = compositor;
-	surface->compositor_listener.notify = compositor_handle_surface_destroy;
-	wl_resource_add_destroy_listener(surface_resource,
-		&surface->compositor_listener);
 
-	wl_list_insert(&compositor->surface_resources,
-		wl_resource_get_link(surface_resource));
 	wlr_signal_emit_safe(&compositor->events.new_surface, surface);
 }
 
@@ -179,14 +154,7 @@ static void wl_compositor_create_region(struct wl_client *client,
 		struct wl_resource *resource, uint32_t id) {
 	struct wlr_compositor *compositor = compositor_from_resource(resource);
 
-	struct wl_resource *region_resource = wlr_region_create(client, id);
-	if (region_resource == NULL) {
-		wl_resource_post_no_memory(resource);
-		return;
-	}
-
-	wl_list_insert(&compositor->region_resources,
-		wl_resource_get_link(region_resource));
+	wlr_region_create(client, 1, id, &compositor->region_resources);
 }
 
 static const struct wl_compositor_interface compositor_impl = {
diff --git a/types/wlr_region.c b/types/wlr_region.c
index 860864c1..6996ab14 100644
--- a/types/wlr_region.c
+++ b/types/wlr_region.c
@@ -34,29 +34,40 @@ static const struct wl_region_interface region_impl = {
 
 static void region_handle_resource_destroy(struct wl_resource *resource) {
 	pixman_region32_t *reg = wlr_region_from_resource(resource);
-	pixman_region32_fini(reg);
-	free(reg);
 
-	// Set by wlr_compositor
 	wl_list_remove(wl_resource_get_link(resource));
+
+	pixman_region32_fini(reg);
+	free(reg);
 }
 
-struct wl_resource *wlr_region_create(struct wl_client *client, uint32_t id) {
+struct wl_resource *wlr_region_create(struct wl_client *client,
+		uint32_t version, uint32_t id, struct wl_list *resource_list) {
 	pixman_region32_t *region = calloc(1, sizeof(pixman_region32_t));
 	if (region == NULL) {
+		wl_client_post_no_memory(client);
 		return NULL;
 	}
 
 	pixman_region32_init(region);
 
 	struct wl_resource *region_resource = wl_resource_create(client,
-		&wl_region_interface, 1, id);
+		&wl_region_interface, version, id);
 	if (region_resource == NULL) {
 		free(region);
+		wl_client_post_no_memory(client);
 		return NULL;
 	}
 	wl_resource_set_implementation(region_resource, &region_impl, region,
 		region_handle_resource_destroy);
+
+	struct wl_list *resource_link = wl_resource_get_link(region_resource);
+	if (resource_list != NULL) {
+		wl_list_insert(resource_list, resource_link);
+	} else {
+		wl_list_init(resource_link);
+	}
+
 	return region_resource;
 }
 
diff --git a/types/wlr_surface.c b/types/wlr_surface.c
index e228574a..0cf8fec2 100644
--- a/types/wlr_surface.c
+++ b/types/wlr_surface.c
@@ -615,6 +615,8 @@ static void subsurface_destroy(struct wlr_subsurface *subsurface) {
 		wl_list_remove(&subsurface->parent_destroy.link);
 	}
 
+	wl_list_remove(wl_resource_get_link(subsurface->resource));
+
 	wl_resource_set_user_data(subsurface->resource, NULL);
 	if (subsurface->surface) {
 		subsurface->surface->role_data = NULL;
@@ -627,6 +629,9 @@ static void surface_handle_resource_destroy(struct wl_resource *resource) {
 
 	wlr_signal_emit_safe(&surface->events.destroy, surface);
 
+	wl_list_remove(wl_resource_get_link(surface->resource));
+
+	wl_list_remove(&surface->renderer_destroy.link);
 	wlr_texture_destroy(surface->texture);
 	surface_state_destroy(surface->pending);
 	surface_state_destroy(surface->current);
@@ -641,16 +646,27 @@ static void surface_handle_renderer_destroy(struct wl_listener *listener,
 	wl_resource_destroy(surface->resource);
 }
 
-struct wlr_surface *wlr_surface_create(struct wl_resource *res,
-		struct wlr_renderer *renderer) {
+struct wlr_surface *wlr_surface_create(struct wl_client *client,
+		uint32_t version, uint32_t id, struct wlr_renderer *renderer,
+		struct wl_list *resource_list) {
 	struct wlr_surface *surface = calloc(1, sizeof(struct wlr_surface));
 	if (!surface) {
-		wl_resource_post_no_memory(res);
+		wl_client_post_no_memory(client);
+		return NULL;
+	}
+	surface->resource = wl_resource_create(client, &wl_surface_interface,
+		version, id);
+	if (surface->resource == NULL) {
+		free(surface);
+		wl_client_post_no_memory(client);
 		return NULL;
 	}
-	wlr_log(L_DEBUG, "New wlr_surface %p (res %p)", surface, res);
+	wl_resource_set_implementation(surface->resource, &surface_interface,
+		surface, surface_handle_resource_destroy);
+
+	wlr_log(L_DEBUG, "New wlr_surface %p (res %p)", surface, surface->resource);
+
 	surface->renderer = renderer;
-	surface->resource = res;
 
 	surface->current = surface_state_create();
 	surface->pending = surface_state_create();
@@ -660,12 +676,17 @@ struct wlr_surface *wlr_surface_create(struct wl_resource *res,
 	wl_signal_init(&surface->events.new_subsurface);
 	wl_list_init(&surface->subsurfaces);
 	wl_list_init(&surface->subsurface_pending_list);
-	wl_resource_set_implementation(res, &surface_interface,
-		surface, surface_handle_resource_destroy);
 
 	wl_signal_add(&renderer->events.destroy, &surface->renderer_destroy);
 	surface->renderer_destroy.notify = surface_handle_renderer_destroy;
 
+	struct wl_list *resource_link = wl_resource_get_link(surface->resource);
+	if (resource_list != NULL) {
+		wl_list_insert(resource_list, resource_link);
+	} else {
+		wl_list_init(resource_link);
+	}
+
 	return surface;
 }
 
@@ -838,8 +859,9 @@ static void subsurface_handle_surface_destroy(struct wl_listener *listener,
 	subsurface_destroy(subsurface);
 }
 
-struct wlr_subsurface *wlr_surface_make_subsurface(struct wlr_surface *surface,
-		struct wlr_surface *parent, uint32_t id) {
+struct wlr_subsurface *wlr_subsurface_create(struct wlr_surface *surface,
+		struct wlr_surface *parent, uint32_t version, uint32_t id,
+		struct wl_list *resource_list) {
 	struct wl_client *client = wl_resource_get_client(surface->resource);
 
 	struct wlr_subsurface *subsurface =
@@ -856,7 +878,20 @@ struct wlr_subsurface *wlr_surface_make_subsurface(struct wlr_surface *surface,
 	}
 	subsurface->synchronized = true;
 	subsurface->surface = surface;
+	subsurface->resource =
+		wl_resource_create(client, &wl_subsurface_interface, version, id);
+	if (subsurface->resource == NULL) {
+		surface_state_destroy(subsurface->cached);
+		free(subsurface);
+		wl_client_post_no_memory(client);
+		return NULL;
+	}
+	wl_resource_set_implementation(subsurface->resource,
+		&subsurface_implementation, subsurface,
+		subsurface_resource_destroy);
+
 	wl_signal_init(&subsurface->events.destroy);
+
 	wl_signal_add(&surface->events.destroy, &subsurface->surface_destroy);
 	subsurface->surface_destroy.notify = subsurface_handle_surface_destroy;
 
@@ -868,21 +903,15 @@ struct wlr_subsurface *wlr_surface_make_subsurface(struct wlr_surface *surface,
 	wl_list_insert(&parent->subsurface_pending_list,
 		&subsurface->parent_pending_link);
 
-	subsurface->resource =
-		wl_resource_create(client, &wl_subsurface_interface, 1, id);
-	if (subsurface->resource == NULL) {
-		surface_state_destroy(subsurface->cached);
-		free(subsurface);
-		wl_client_post_no_memory(client);
-		return NULL;
-	}
-
-	wl_resource_set_implementation(subsurface->resource,
-		&subsurface_implementation, subsurface,
-		subsurface_resource_destroy);
-
 	surface->role_data = subsurface;
 
+	struct wl_list *resource_link = wl_resource_get_link(subsurface->resource);
+	if (resource_list != NULL) {
+		wl_list_insert(resource_list, resource_link);
+	} else {
+		wl_list_init(resource_link);
+	}
+
 	wlr_signal_emit_safe(&parent->events.new_subsurface, subsurface);
 
 	return subsurface;