x11: port the x11 backend to the new init api

Use a "well" defined structure to configure x11-backend and move configuration
file parsing inside the weston compositor code.

Enforce destruction of all backend config objects after initialization.
Since the backend config struct versioning implies that there we expect
potential future descrepancy between main's definition of the config
object and the backend's, don't allow the backend to hang onto the
config object outside the initialization scope.

Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Benoit Gschwind <gschwind@gnu-log.net>
Tested-by: Benoit Gschwind <gschwind@gnu-log.net>

Makefile.am

@@ -75,6 +75,7 @@ weston_SOURCES =					\
 	src/compositor-headless.h			\
 	src/compositor-fbdev.h				\
 	src/compositor-rdp.h				\
+	src/compositor-x11.h			\
 	src/input.c					\
 	src/data-device.c				\
 	src/screenshooter.c				\
@@ -214,6 +215,7 @@ westoninclude_HEADERS =				\
 	src/compositor-headless.h		\
 	src/compositor-fbdev.h			\
 	src/compositor-rdp.h			\
+	src/compositor-x11.h			\
 	src/timeline-object.h			\
 	shared/matrix.h				\
 	shared/config-parser.h			\
@@ -256,6 +258,7 @@ x11_backend_la_CFLAGS =				\
 	$(AM_CFLAGS)
 x11_backend_la_SOURCES = 			\
 	src/compositor-x11.c			\
+	src/compositor-x11.h			\
 	shared/helpers.h
 endif
 

src/compositor-x11.c

@@ -50,21 +50,17 @@
 #include <xkbcommon/xkbcommon.h>
 
 #include "compositor.h"
-#include "gl-renderer.h"
+#include "compositor-x11.h"
-#include "pixman-renderer.h"
 #include "shared/config-parser.h"
 #include "shared/helpers.h"
 #include "shared/image-loader.h"
+#include "gl-renderer.h"
+#include "pixman-renderer.h"
 #include "presentation-time-server-protocol.h"
 #include "linux-dmabuf.h"
 
 #define DEFAULT_AXIS_STEP_DISTANCE 10
 
-static int option_width;
-static int option_height;
-static int option_scale;
-static int option_count;
-
 struct x11_backend {
 	struct weston_backend	 base;
 	struct weston_compositor *compositor;
@@ -1566,25 +1562,16 @@ init_gl_renderer(struct x11_backend *b)
 
 	return ret;
 }
+
 static struct x11_backend *
 x11_backend_create(struct weston_compositor *compositor,
-		   int fullscreen,
+		   struct weston_x11_backend_config *config)
-		   int no_input,
-		   int use_pixman,
-		   int *argc, char *argv[],
-		   struct weston_config *config)
 {
 	struct x11_backend *b;
 	struct x11_output *output;
 	struct wl_event_loop *loop;
-	struct weston_config_section *section;
+	int x = 0;
-	int i, x = 0, output_count = 0;
+	unsigned i;
-	int width, height, scale, count;
-	const char *section_name;
-	char *name, *t, *mode;
-	uint32_t transform;
-
-	weston_log("initializing x11 backend\n");
 
 	b = zalloc(sizeof *b);
 	if (b == NULL)
@@ -1610,13 +1597,13 @@ x11_backend_create(struct weston_compositor *compositor,
 	x11_backend_get_resources(b);
 	x11_backend_get_wm_info(b);
 
-	if (!b->has_net_wm_state_fullscreen && fullscreen) {
+	if (!b->has_net_wm_state_fullscreen && config->fullscreen) {
 		weston_log("Can not fullscreen without window manager support"
 			   "(need _NET_WM_STATE_FULLSCREEN)\n");
-		fullscreen = 0;
+		config->fullscreen = 0;
 	}
 
-	b->use_pixman = use_pixman;
+	b->use_pixman = config->use_pixman;
 	if (b->use_pixman) {
 		if (pixman_renderer_init(compositor) < 0) {
 			weston_log("Failed to initialize pixman renderer for X11 backend\n");
@@ -1626,83 +1613,51 @@ x11_backend_create(struct weston_compositor *compositor,
 	else if (init_gl_renderer(b) < 0) {
 		goto err_xdisplay;
 	}
-	weston_log("Using %s renderer\n", use_pixman ? "pixman" : "gl");
+	weston_log("Using %s renderer\n", config->use_pixman ? "pixman" : "gl");
 
 	b->base.destroy = x11_destroy;
 	b->base.restore = x11_restore;
 
-	if (x11_input_create(b, no_input) < 0) {
+	if (x11_input_create(b, config->no_input) < 0) {
 		weston_log("Failed to create X11 input\n");
 		goto err_renderer;
 	}
 
-	width = option_width ? option_width : 1024;
+	for (i = 0; i < config->num_outputs; ++i) {
-	height = option_height ? option_height : 640;
+		struct weston_x11_backend_output_config *output_iterator =
-	scale = option_scale ? option_scale : 1;
+			&config->outputs[i];
-	count = option_count ? option_count : 1;
 
-	section = NULL;
+		if (output_iterator->name == NULL) {
-	while (weston_config_next_section(config,
-					  &section, &section_name)) {
-		if (strcmp(section_name, "output") != 0)
-			continue;
-		weston_config_section_get_string(section, "name", &name, NULL);
-		if (name == NULL || name[0] != 'X') {
-			free(name);
 			continue;
 		}
 
-		weston_config_section_get_string(section,
+		if (output_iterator->width < 1) {
-						 "mode", &mode, "1024x600");
+			weston_log("Invalid width \"%d\" for output %s\n",
-		if (sscanf(mode, "%dx%d", &width, &height) != 2) {
+				   output_iterator->width, output_iterator->name);
-			weston_log("Invalid mode \"%s\" for output %s\n",
-				   mode, name);
-			width = 1024;
-			height = 600;
-		}
-		free(mode);
-
-		if (option_width)
-			width = option_width;
-		if (option_height)
-			height = option_height;
-
-		weston_config_section_get_int(section, "scale", &scale, 1);
-		if (option_scale)
-			scale = option_scale;
-
-		weston_config_section_get_string(section,
-						 "transform", &t, "normal");
-		if (weston_parse_transform(t, &transform) < 0)
-			weston_log("Invalid transform \"%s\" for output %s\n",
-				   t, name);
-		free(t);
-
-		output = x11_backend_create_output(b, x, 0,
-						   width, height,
-						   fullscreen, no_input,
-						   name, transform, scale);
-		free(name);
-		if (output == NULL) {
-			weston_log("Failed to create configured x11 output\n");
 			goto err_x11_input;
 		}
 
-		x = pixman_region32_extents(&output->base.region)->x2;
+		if (output_iterator->height < 1) {
-
+			weston_log("Invalid height \"%d\" for output %s\n",
-		output_count++;
+				   output_iterator->height, output_iterator->name);
-		if (option_count && output_count >= option_count)
+			goto err_x11_input;
-			break;
+		}
-	}
 
-	for (i = output_count; i < count; i++) {
+		output = x11_backend_create_output(b,
-		output = x11_backend_create_output(b, x, 0, width, height,
+						   x,
-						   fullscreen, no_input, NULL,
+						   0,
-						   WL_OUTPUT_TRANSFORM_NORMAL, scale);
+						   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 x11 output #%d\n", i);
+			weston_log("Failed to create configured x11 output\n");
 			goto err_x11_input;
 		}
+
 		x = pixman_region32_extents(&output->base.region)->x2;
 	}
 
@@ -1735,34 +1690,33 @@ err_free:
 	return NULL;
 }
 
+static void
+config_init_to_defaults(struct weston_x11_backend_config *config)
+{
+}
+
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor, int *argc, char *argv[],
+backend_init(struct weston_compositor *compositor,
-	     struct weston_config *config,
+	     int *argc, char *argv[],
+	     struct weston_config *wc,
 	     struct weston_backend_config *config_base)
 {
 	struct x11_backend *b;
-	int fullscreen = 0;
+	struct weston_x11_backend_config config = {{ 0, }};
-	int no_input = 0;
-	int use_pixman = 0;
-
-	const struct weston_option x11_options[] = {
-		{ WESTON_OPTION_INTEGER, "width", 0, &option_width },
-		{ WESTON_OPTION_INTEGER, "height", 0, &option_height },
-		{ WESTON_OPTION_INTEGER, "scale", 0, &option_scale },
-		{ WESTON_OPTION_BOOLEAN, "fullscreen", 'f', &fullscreen },
-		{ WESTON_OPTION_INTEGER, "output-count", 0, &option_count },
-		{ WESTON_OPTION_BOOLEAN, "no-input", 0, &no_input },
-		{ WESTON_OPTION_BOOLEAN, "use-pixman", 0, &use_pixman },
-	};
 
-	parse_options(x11_options, ARRAY_LENGTH(x11_options), argc, argv);
+	if (config_base == NULL ||
+	    config_base->struct_version != WESTON_X11_BACKEND_CONFIG_VERSION ||
+	    config_base->struct_size > sizeof(struct weston_x11_backend_config)) {
+		weston_log("X11 backend config structure is invalid\n");
+		return -1;
+	}
+
+	config_init_to_defaults(&config);
+	memcpy(&config, config_base, config_base->struct_size);
 
-	b = x11_backend_create(compositor,
+	b = x11_backend_create(compositor, &config);
-			       fullscreen,
-			       no_input,
-			       use_pixman,
-			       argc, argv, config);
 	if (b == NULL)
 		return -1;
+
 	return 0;
 }

src/compositor-x11.h

@@ -0,0 +1,62 @@
+/*
+ * Copyright © 2016 Benoit Gschwind
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef WESTON_COMPOSITOR_X11_H
+#define WESTON_COMPOSITOR_X11_H
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
+#include "compositor.h"
+
+#define WESTON_X11_BACKEND_CONFIG_VERSION 1
+
+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;
+
+	bool fullscreen;
+	bool no_input;
+
+	/** 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
+}
+#endif
+
+#endif /* WESTON_COMPOSITOR_X11_H_ */

src/main.c

@@ -50,6 +50,7 @@
 #include "compositor-headless.h"
 #include "compositor-rdp.h"
 #include "compositor-fbdev.h"
+#include "compositor-x11.h"
 
 static struct wl_list child_process_list;
 static struct weston_compositor *segv_compositor;
@@ -805,6 +806,146 @@ load_fbdev_backend(struct weston_compositor *c, char const * backend,
 	ret = load_backend_new(c, backend, &config.base);
 
 	free(config.device);
+
+	return ret;
+}
+
+static int
+weston_x11_backend_config_append_output_config(struct weston_x11_backend_config *config,
+					       struct weston_x11_backend_output_config *output_config) {
+	struct weston_x11_backend_output_config *new_outputs;
+
+	new_outputs = realloc(config->outputs, (config->num_outputs+1) *
+			      sizeof(struct weston_x11_backend_output_config));
+	if (new_outputs == NULL)
+		return -1;
+
+	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;
+}
+
+static int
+load_x11_backend(struct weston_compositor *c, char const * backend,
+		 int *argc, char **argv, struct weston_config *wc)
+{
+	struct weston_x11_backend_output_config default_output;
+	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;
+
+	const struct weston_option options[] = {
+	       { WESTON_OPTION_INTEGER, "width", 0, &option_width },
+	       { WESTON_OPTION_INTEGER, "height", 0, &option_height },
+	       { WESTON_OPTION_INTEGER, "scale", 0, &option_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 },
+	       { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
+	};
+
+	parse_options(options, ARRAY_LENGTH(options), argc, argv);
+
+	section = NULL;
+	while (weston_config_next_section(wc, &section, &section_name)) {
+		struct weston_x11_backend_output_config current_output = { 0, };
+		char *t;
+		char *mode;
+
+		if (strcmp(section_name, "output") != 0) {
+			continue;
+		}
+
+		weston_config_section_get_string(section, "name", &current_output.name, NULL);
+		if (current_output.name == NULL || current_output.name[0] != 'X') {
+			free(current_output.name);
+			continue;
+		}
+
+		weston_config_section_get_string(section, "mode", &mode, "1024x600");
+		if (sscanf(mode, "%dx%d", &current_output.width,
+			   &current_output.height) != 2) {
+			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;
+		}
+
+		output_count++;
+		if (option_count && output_count >= option_count)
+			break;
+	}
+
+	default_output.name = 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) {
+			ret = -1;
+			goto out;
+		}
+
+		if (weston_x11_backend_config_append_output_config(&config, &default_output) < 0) {
+			ret = -1;
+			free(default_output.name);
+			goto out;
+		}
+		free(default_output.name);
+	}
+
+	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 = load_backend_new(c, backend, &config.base);
+
+out:
+	for (j = 0; j < config.num_outputs; ++j)
+		free(config.outputs[j].name);
+	free(config.outputs);
+
 	return ret;
 }
 
@@ -818,13 +959,13 @@ load_backend(struct weston_compositor *compositor, const char *backend,
 		return load_rdp_backend(compositor, backend, argc, argv, config);
 	else if (strstr(backend, "fbdev-backend.so"))
 		return load_fbdev_backend(compositor, backend, argc, argv, config);
+	else if (strstr(backend, "x11-backend.so"))
+		return load_x11_backend(compositor, backend, argc, argv, config);
 #if 0
 	else if (strstr(backend, "drm-backend.so"))
 		return load_drm_backend(compositor, backend, argc, argv, config);
 	else if (strstr(backend, "wayland-backend.so"))
 		return load_wayland_backend(compositor, backend, argc, argv, config);
-	else if (strstr(backend, "x11-backend.so"))
-		return load_x11_backend(compositor, backend, argc, argv, config);
 	else if (strstr(backend, "rpi-backend.so"))
 		return load_rpi_backend(compositor, backend, argc, argv, config);
 #endif