weston: Port X11 backend to new output handling API

This is a complete port of the X11 backend that
uses recently added output handling API for output
configuration.

- Output can be configured at runtime by passing the
  necessary configuration parameters, which can be
  filled in manually, obtained from the configuration
  file or obtained from the command line using
  previously added functionality. It is required that
  the scale and transform values are set using the
  previously added functionality.

- Output can be created at runtime using the output
  API. The output creation only creates a pending
  output, which needs to be configured the same way as
  mentioned above.

Same as before, a single output is created at runtime
using the default configuration or a configuration
parsed from the command line. The output-count
functionality is also preserved, which means more than
one output can be created initially, and more outputs can
be added at runtime using the output API.

v2:

 - Fix wet_configure_windowed_output_from_config() usage.
 - Call x11_output_disable() explicitly from
   x11_output_destroy().

v3:

 - Remove unneeded free().
 - Disallow calling x11_output_configure more than once.
 - Remove unneeded checks for output->name == NULL as that
   has been disallowed.
 - Use weston_compositor_add_pending_output().
 - Bump weston_x11_backend_config version to 2.

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>

compositor/main.c

@@ -1426,48 +1426,43 @@ out:
 	return ret;
 }
 
-static int
+static void
-weston_x11_backend_config_append_output_config(struct weston_x11_backend_config *config,
+x11_backend_output_configure(struct wl_listener *listener, void *data)
-					       struct weston_x11_backend_output_config *output_config) {
+{
-	struct weston_x11_backend_output_config *new_outputs;
+	struct weston_output *output = data;
-
+	struct wet_output_config defaults = {
-	new_outputs = realloc(config->outputs, (config->num_outputs+1) *
+		.width = 1024,
-			      sizeof(struct weston_x11_backend_output_config));
+		.height = 600,
-	if (new_outputs == NULL)
+		.scale = 1,
-		return -1;
+		.transform = WL_OUTPUT_TRANSFORM_NORMAL
-
+	};
-	config->outputs = new_outputs;
-	config->outputs[config->num_outputs].width = output_config->width;
-	config->outputs[config->num_outputs].height = output_config->height;
-	config->outputs[config->num_outputs].transform = output_config->transform;
-	config->outputs[config->num_outputs].scale = output_config->scale;
-	config->outputs[config->num_outputs].name = strdup(output_config->name);
-	config->num_outputs++;
 
-	return 0;
+	if (wet_configure_windowed_output_from_config(output, &defaults) < 0)
+		weston_log("Cannot configure output \"%s\".\n", output->name);
 }
 
 static int
 load_x11_backend(struct weston_compositor *c,
 		 int *argc, char **argv, struct weston_config *wc)
 {
-	struct weston_x11_backend_output_config default_output;
+	char *default_output;
+	const struct weston_windowed_output_api *api;
 	struct weston_x11_backend_config config = {{ 0, }};
 	struct weston_config_section *section;
 	int ret = 0;
-	int option_width = 0;
-	int option_height = 0;
-	int option_scale = 0;
 	int option_count = 1;
 	int output_count = 0;
 	char const *section_name;
 	int i;
-	uint32_t j;
+
+	struct wet_output_config *parsed_options = wet_init_parsed_options(c);
+	if (!parsed_options)
+		return -1;
 
 	const struct weston_option options[] = {
-	       { WESTON_OPTION_INTEGER, "width", 0, &option_width },
+	       { WESTON_OPTION_INTEGER, "width", 0, &parsed_options->width },
-	       { WESTON_OPTION_INTEGER, "height", 0, &option_height },
+	       { WESTON_OPTION_INTEGER, "height", 0, &parsed_options->height },
-	       { WESTON_OPTION_INTEGER, "scale", 0, &option_scale },
+	       { WESTON_OPTION_INTEGER, "scale", 0, &parsed_options->scale },
 	       { WESTON_OPTION_BOOLEAN, "fullscreen", 'f', &config.fullscreen },
 	       { WESTON_OPTION_INTEGER, "output-count", 0, &option_count },
 	       { WESTON_OPTION_BOOLEAN, "no-input", 0, &config.no_input },
@@ -1476,94 +1471,66 @@ load_x11_backend(struct weston_compositor *c,
 
 	parse_options(options, ARRAY_LENGTH(options), argc, argv);
 
+	config.base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
+	config.base.struct_size = sizeof(struct weston_x11_backend_config);
+
+	/* load the actual backend and configure it */
+	ret = weston_compositor_load_backend(c, WESTON_BACKEND_X11,
+					     &config.base);
+
+	if (ret < 0)
+		return ret;
+
+	wet_set_pending_output_handler(c, x11_backend_output_configure);
+
+	api = weston_windowed_output_get_api(c);
+
+	if (!api) {
+		weston_log("Cannot use weston_windowed_output_api.\n");
+		return -1;
+	}
+
 	section = NULL;
 	while (weston_config_next_section(wc, &section, &section_name)) {
-		struct weston_x11_backend_output_config current_output = { 0, };
+		char *output_name;
-		char *t;
+
-		char *mode;
+		if (output_count >= option_count)
+			break;
 
 		if (strcmp(section_name, "output") != 0) {
 			continue;
 		}
 
-		weston_config_section_get_string(section, "name", &current_output.name, NULL);
+		weston_config_section_get_string(section, "name", &output_name, NULL);
-		if (current_output.name == NULL || current_output.name[0] != 'X') {
+		if (output_name == NULL || output_name[0] != 'X') {
-			free(current_output.name);
+			free(output_name);
 			continue;
 		}
 
-		weston_config_section_get_string(section, "mode", &mode, "1024x600");
+		if (api->output_create(c, output_name) < 0) {
-		if (sscanf(mode, "%dx%d", &current_output.width,
+			free(output_name);
-			   &current_output.height) != 2) {
+			return -1;
-			weston_log("Invalid mode \"%s\" for output %s\n",
-				   mode, current_output.name);
-			current_output.width = 1024;
-			current_output.height = 600;
-		}
-		free(mode);
-		if (current_output.width < 1)
-			current_output.width = 1024;
-		if (current_output.height < 1)
-			current_output.height = 600;
-		if (option_width)
-			current_output.width = option_width;
-		if (option_height)
-			current_output.height = option_height;
-
-		weston_config_section_get_int(section, "scale", &current_output.scale, 1);
-		if (option_scale)
-			current_output.scale = option_scale;
-
-		weston_config_section_get_string(section,
-						 "transform", &t, "normal");
-		if (weston_parse_transform(t, &current_output.transform) < 0)
-			weston_log("Invalid transform \"%s\" for output %s\n",
-				   t, current_output.name);
-		free(t);
-
-		if (weston_x11_backend_config_append_output_config(&config, &current_output) < 0) {
-			ret = -1;
-			goto out;
 		}
+		free(output_name);
 
 		output_count++;
-		if (output_count >= option_count)
-			break;
 	}
 
-	default_output.name = NULL;
+	default_output = NULL;
-	default_output.width = option_width ? option_width : 1024;
-	default_output.height = option_height ? option_height : 600;
-	default_output.scale = option_scale ? option_scale : 1;
-	default_output.transform = WL_OUTPUT_TRANSFORM_NORMAL;
 
 	for (i = output_count; i < option_count; i++) {
-		if (asprintf(&default_output.name, "screen%d", i) < 0) {
+		if (asprintf(&default_output, "screen%d", i) < 0) {
-			ret = -1;
+			return -1;
-			goto out;
 		}
 
-		if (weston_x11_backend_config_append_output_config(&config, &default_output) < 0) {
+		if (api->output_create(c, default_output) < 0) {
-			ret = -1;
+			free(default_output);
-			free(default_output.name);
+			return -1;
-			goto out;
 		}
-		free(default_output.name);
+		free(default_output);
 	}
 
-	config.base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
+	return 0;
-	config.base.struct_size = sizeof(struct weston_x11_backend_config);
-
-	/* load the actual backend and configure it */
-	ret = weston_compositor_load_backend(c, WESTON_BACKEND_X11,
-					     &config.base);
-
-out:
-	for (j = 0; j < config.num_outputs; ++j)
-		free(config.outputs[j].name);
-	free(config.outputs);
-
-	return ret;
 }
 
 static void

libweston/compositor-x11.c

@@ -59,6 +59,7 @@
 #include "pixman-renderer.h"
 #include "presentation-time-server-protocol.h"
 #include "linux-dmabuf.h"
+#include "windowed-output-api.h"
 
 #define DEFAULT_AXIS_STEP_DISTANCE 10
 
@@ -75,6 +76,8 @@ struct x11_backend {
 	struct xkb_keymap	*xkb_keymap;
 	unsigned int		 has_xkb;
 	uint8_t			 xkb_event_base;
+	int			 fullscreen;
+	int			 no_input;
 	int			 use_pixman;
 
 	int			 has_net_wm_state_fullscreen;
@@ -515,30 +518,6 @@ x11_output_deinit_shm(struct x11_backend *b, struct x11_output *output)
 	shmdt(output->buf);
 }
 
-static void
-x11_output_destroy(struct weston_output *output_base)
-{
-	struct x11_output *output = to_x11_output(output_base);
-	struct x11_backend *backend =
-		to_x11_backend(output->base.compositor);
-
-	wl_event_source_remove(output->finish_frame_timer);
-
-	if (backend->use_pixman) {
-		pixman_renderer_output_destroy(output_base);
-		x11_output_deinit_shm(backend, output);
-	} else
-		gl_renderer->output_destroy(output_base);
-
-	xcb_destroy_window(backend->conn, output->window);
-
-	xcb_flush(backend->conn);
-
-	weston_output_destroy(&output->base);
-
-	free(output);
-}
-
 static void
 x11_output_set_wm_protocols(struct x11_backend *b,
 			    struct x11_output *output)
@@ -789,20 +768,54 @@ x11_output_init_shm(struct x11_backend *b, struct x11_output *output,
 	return 0;
 }
 
-static struct x11_output *
+static int
-x11_backend_create_output(struct x11_backend *b, int x, int y,
+x11_output_disable(struct weston_output *base)
-			     int width, int height, int fullscreen,
+{
-			     int no_input, char *configured_name,
+	struct x11_output *output = to_x11_output(base);
-			     uint32_t transform, int32_t scale)
+	struct x11_backend *backend = to_x11_backend(base->compositor);
+
+	if (!output->base.enabled)
+		return 0;
+
+	wl_event_source_remove(output->finish_frame_timer);
+
+	if (backend->use_pixman) {
+		pixman_renderer_output_destroy(&output->base);
+		x11_output_deinit_shm(backend, output);
+	} else {
+		gl_renderer->output_destroy(&output->base);
+	}
+
+	xcb_destroy_window(backend->conn, output->window);
+	xcb_flush(backend->conn);
+
+	return 0;
+}
+
+static void
+x11_output_destroy(struct weston_output *base)
+{
+	struct x11_output *output = to_x11_output(base);
+
+	x11_output_disable(&output->base);
+	weston_output_destroy(&output->base);
+
+	free(output);
+}
+
+static int
+x11_output_enable(struct weston_output *base)
 {
+	struct x11_output *output = to_x11_output(base);
+	struct x11_backend *b = to_x11_backend(base->compositor);
+
 	static const char name[] = "Weston Compositor";
 	static const char class[] = "weston-1\0Weston Compositor";
 	char *title = NULL;
-	struct x11_output *output;
 	xcb_screen_t *screen;
 	struct wm_normal_hints normal_hints;
 	struct wl_event_loop *loop;
-	int output_width, output_height, width_mm, height_mm;
+
 	int ret;
 	uint32_t mask = XCB_CW_EVENT_MASK | XCB_CW_CURSOR;
 	xcb_atom_t atom_list[1];
@@ -812,10 +825,7 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 		0
 	};
 
-	output_width = width * scale;
+	if (!b->no_input)
-	output_height = height * scale;
-
-	if (!no_input)
 		values[0] |=
 			XCB_EVENT_MASK_KEY_PRESS |
 			XCB_EVENT_MASK_KEY_RELEASE |
@@ -827,22 +837,6 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 			XCB_EVENT_MASK_KEYMAP_STATE |
 			XCB_EVENT_MASK_FOCUS_CHANGE;
 
-	output = zalloc(sizeof *output);
-	if (output == NULL) {
-		perror("zalloc");
-		return NULL;
-	}
-
-	output->mode.flags =
-		WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
-
-	output->mode.width = output_width;
-	output->mode.height = output_height;
-	output->mode.refresh = 60000;
-	output->scale = scale;
-	wl_list_init(&output->base.mode_list);
-	wl_list_insert(&output->base.mode_list, &output->mode.link);
-
 	values[1] = b->null_cursor;
 	output->window = xcb_generate_id(b->conn);
 	screen = x11_compositor_get_default_screen(b);
@@ -851,13 +845,14 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 			  output->window,
 			  screen->root,
 			  0, 0,
-			  output_width, output_height,
+			  output->base.current_mode->width,
+			  output->base.current_mode->height,
 			  0,
 			  XCB_WINDOW_CLASS_INPUT_OUTPUT,
 			  screen->root_visual,
 			  mask, values);
 
-	if (fullscreen) {
+	if (b->fullscreen) {
 		atom_list[0] = b->atom.net_wm_state_fullscreen;
 		xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE,
 				    output->window,
@@ -869,10 +864,10 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 		memset(&normal_hints, 0, sizeof normal_hints);
 		normal_hints.flags =
 			WM_NORMAL_HINTS_MAX_SIZE | WM_NORMAL_HINTS_MIN_SIZE;
-		normal_hints.min_width = output_width;
+		normal_hints.min_width = output->base.current_mode->width;
-		normal_hints.min_height = output_height;
+		normal_hints.min_height = output->base.current_mode->height;
-		normal_hints.max_width = output_width;
+		normal_hints.max_width = output->base.current_mode->width;
-		normal_hints.max_height = output_height;
+		normal_hints.max_height = output->base.current_mode->height;
 		xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
 				    b->atom.wm_normal_hints,
 				    b->atom.wm_size_hints, 32,
@@ -881,8 +876,8 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 	}
 
 	/* Set window name.  Don't bother with non-EWMH WMs. */
-	if (configured_name) {
+	if (output->base.name) {
-		if (asprintf(&title, "%s - %s", name, configured_name) < 0)
+		if (asprintf(&title, "%s - %s", name, output->base.name) < 0)
 			title = NULL;
 	} else {
 		title = strdup(name);
@@ -894,9 +889,7 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 				    strlen(title), title);
 		free(title);
 	} else {
-		xcb_destroy_window(b->conn, output->window);
+		goto err;
-		free(output);
-		return NULL;
 	}
 
 	xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
@@ -909,44 +902,20 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 
 	xcb_map_window(b->conn, output->window);
 
-	if (fullscreen)
+	if (b->fullscreen)
 		x11_output_wait_for_map(b, output);
 
-	output->base.start_repaint_loop = x11_output_start_repaint_loop;
-	if (b->use_pixman)
-		output->base.repaint = x11_output_repaint_shm;
-	else
-		output->base.repaint = x11_output_repaint_gl;
-	output->base.destroy = x11_output_destroy;
-	output->base.assign_planes = NULL;
-	output->base.set_backlight = NULL;
-	output->base.set_dpms = NULL;
-	output->base.switch_mode = NULL;
-	output->base.current_mode = &output->mode;
-	output->base.make = "weston-X11";
-	output->base.model = "none";
-
-	if (configured_name)
-		output->base.name = strdup(configured_name);
-
-	width_mm = width * b->screen->width_in_millimeters /
-		b->screen->width_in_pixels;
-	height_mm = height * b->screen->height_in_millimeters /
-		b->screen->height_in_pixels;
-	weston_output_init(&output->base, b->compositor,
-			   x, y, width_mm, height_mm, transform, scale);
-
 	if (b->use_pixman) {
 		if (x11_output_init_shm(b, output,
-					output->mode.width,
+					output->base.current_mode->width,
-					output->mode.height) < 0) {
+					output->base.current_mode->height) < 0) {
 			weston_log("Failed to initialize SHM for the X11 output\n");
-			return NULL;
+			goto err;
 		}
 		if (pixman_renderer_output_create(&output->base) < 0) {
 			weston_log("Failed to create pixman renderer for output\n");
 			x11_output_deinit_shm(b, output);
-			return NULL;
+			goto err;
 		}
 	} else {
 		/* eglCreatePlatformWindowSurfaceEXT takes a Window*
@@ -960,19 +929,112 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
 						 NULL,
 						 0);
 		if (ret < 0)
-			return NULL;
+			goto err;
 	}
 
 	loop = wl_display_get_event_loop(b->compositor->wl_display);
 	output->finish_frame_timer =
 		wl_event_loop_add_timer(loop, finish_frame_handler, output);
 
-	weston_compositor_add_output(b->compositor, &output->base);
-
 	weston_log("x11 output %dx%d, window id %d\n",
-		   width, height, output->window);
+		   output->base.current_mode->width,
+		   output->base.current_mode->height,
+		   output->window);
+
+	return 0;
 
-	return output;
+err:
+	xcb_destroy_window(b->conn, output->window);
+	xcb_flush(b->conn);
+
+	return -1;
+}
+
+static int
+x11_output_set_size(struct weston_output *base, int width, int height)
+{
+	struct x11_output *output = to_x11_output(base);
+	struct x11_backend *b = to_x11_backend(base->compositor);
+	int output_width, output_height;
+
+	/* We can only be called once. */
+	assert(!output->base.current_mode);
+
+	/* Make sure we have scale set. */
+	assert(output->base.scale);
+
+	if (width < 1) {
+		weston_log("Invalid width \"%d\" for output %s\n",
+			   width, output->base.name);
+		return -1;
+	}
+
+	if (height < 1) {
+		weston_log("Invalid height \"%d\" for output %s\n",
+			   height, output->base.name);
+		return -1;
+	}
+
+	output_width = width * output->base.scale;
+	output_height = height * output->base.scale;
+
+	output->mode.flags =
+		WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
+
+	output->mode.width = output_width;
+	output->mode.height = output_height;
+	output->mode.refresh = 60000;
+	output->scale = output->base.scale;
+	wl_list_init(&output->base.mode_list);
+	wl_list_insert(&output->base.mode_list, &output->mode.link);
+
+	output->base.current_mode = &output->mode;
+	output->base.make = "weston-X11";
+	output->base.model = "none";
+
+	output->base.mm_width = width * b->screen->width_in_millimeters /
+		b->screen->width_in_pixels;
+	output->base.mm_height = height * b->screen->height_in_millimeters /
+		b->screen->height_in_pixels;
+
+	if (b->use_pixman)
+		output->base.repaint = x11_output_repaint_shm;
+	else
+		output->base.repaint = x11_output_repaint_gl;
+
+	output->base.start_repaint_loop = x11_output_start_repaint_loop;
+	output->base.assign_planes = NULL;
+	output->base.set_backlight = NULL;
+	output->base.set_dpms = NULL;
+	output->base.switch_mode = NULL;
+
+	return 0;
+}
+
+static int
+x11_output_create(struct weston_compositor *compositor,
+		  const char *name)
+{
+	struct x11_output *output;
+
+	/* name can't be NULL. */
+	assert(name);
+
+	output = zalloc(sizeof *output);
+	if (output == NULL) {
+		perror("zalloc");
+		return -1;
+	}
+
+	output->base.name = strdup(name);
+	output->base.destroy = x11_output_destroy;
+	output->base.disable = x11_output_disable;
+	output->base.enable = x11_output_enable;
+
+	weston_output_init_pending(&output->base, compositor);
+	weston_compositor_add_pending_output(&output->base, compositor);
+
+	return 0;
 }
 
 static struct x11_output *
@@ -1588,21 +1650,27 @@ init_gl_renderer(struct x11_backend *b)
 	return ret;
 }
 
+static const struct weston_windowed_output_api api = {
+	x11_output_set_size,
+	x11_output_create,
+};
+
 static struct x11_backend *
 x11_backend_create(struct weston_compositor *compositor,
 		   struct weston_x11_backend_config *config)
 {
 	struct x11_backend *b;
-	struct x11_output *output;
 	struct wl_event_loop *loop;
-	int x = 0;
+	int ret;
-	unsigned i;
 
 	b = zalloc(sizeof *b);
 	if (b == NULL)
 		return NULL;
 
 	b->compositor = compositor;
+	b->fullscreen = config->fullscreen;
+	b->no_input = config->no_input;
+
 	if (weston_compositor_set_presentation_clock_software(compositor) < 0)
 		goto err_free;
 
@@ -1648,44 +1716,6 @@ x11_backend_create(struct weston_compositor *compositor,
 		goto err_renderer;
 	}
 
-	for (i = 0; i < config->num_outputs; ++i) {
-		struct weston_x11_backend_output_config *output_iterator =
-			&config->outputs[i];
-
-		if (output_iterator->name == NULL) {
-			continue;
-		}
-
-		if (output_iterator->width < 1) {
-			weston_log("Invalid width \"%d\" for output %s\n",
-				   output_iterator->width, output_iterator->name);
-			goto err_x11_input;
-		}
-
-		if (output_iterator->height < 1) {
-			weston_log("Invalid height \"%d\" for output %s\n",
-				   output_iterator->height, output_iterator->name);
-			goto err_x11_input;
-		}
-
-		output = x11_backend_create_output(b,
-						   x,
-						   0,
-						   output_iterator->width,
-						   output_iterator->height,
-						   config->fullscreen,
-						   config->no_input,
-						   output_iterator->name,
-						   output_iterator->transform,
-						   output_iterator->scale);
-		if (output == NULL) {
-			weston_log("Failed to create configured x11 output\n");
-			goto err_x11_input;
-		}
-
-		x = pixman_region32_extents(&output->base.region)->x2;
-	}
-
 	loop = wl_display_get_event_loop(compositor->wl_display);
 	b->xcb_source =
 		wl_event_loop_add_fd(loop,
@@ -1702,6 +1732,14 @@ x11_backend_create(struct weston_compositor *compositor,
 
 	compositor->backend = &b->base;
 
+	ret = weston_plugin_api_register(compositor, WESTON_WINDOWED_OUTPUT_API_NAME,
+					 &api, sizeof(api));
+
+	if (ret < 0) {
+		weston_log("Failed to register output API.\n");
+		goto err_x11_input;
+	}
+
 	return b;
 
 err_x11_input:

libweston/compositor-x11.h

@@ -34,15 +34,7 @@ extern "C" {
 
 #include "compositor.h"
 
-#define WESTON_X11_BACKEND_CONFIG_VERSION 1
+#define WESTON_X11_BACKEND_CONFIG_VERSION 2
-
-struct weston_x11_backend_output_config {
-	int width;
-	int height;
-	char *name;
-	uint32_t transform;
-	int32_t scale;
-};
 
 struct weston_x11_backend_config {
 	struct weston_backend_config base;
@@ -52,9 +44,6 @@ struct weston_x11_backend_config {
 
 	/** Whether to use the pixman renderer instead of the OpenGL ES renderer. */
 	bool use_pixman;
-
-	uint32_t num_outputs;
-	struct weston_x11_backend_output_config *outputs;
 };
 
 #ifdef  __cplusplus