[PATCH] compositor-x11: Allow output configuration from config file.
Kristian Høgsberg
hoegsberg at gmail.com
Fri Aug 3 10:10:44 PDT 2012
On Thu, Aug 02, 2012 at 08:42:17PM -0600, Scott Moreau wrote:
> This patch provides a way to define outputs for the x11 backend. It
> parses [output] sections and checks for 'name', 'width' and 'height'
> keys. The 'name' key must start with an 'X' to distinguish between
> drm output names. Command line options --width and --height supersede
> what is in the config file. When --output-count is passed, the number
> of outputs are limited or additional outputs added with default
> values.
Nice, that all sounds good.
> ---
> src/compositor-x11.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++----
> weston.ini | 5 ++
> 2 files changed, 132 insertions(+), 9 deletions(-)
>
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 4507463..992b68f 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -50,6 +50,20 @@
> #include "compositor.h"
> #include "../shared/config-parser.h"
>
> +static char *output_name;
> +static char *output_width;
> +static char *output_height;
> +static int width_arg;
> +static int height_arg;
> +static int count_arg;
We're using option_* for globals that we parse from the command line options.
> +static struct wl_list configured_output_list;
> +
> +struct x11_configured_output {
> + char *name;
> + int width, height;
> + struct wl_list link;
> +};
> +
> struct x11_compositor {
> struct weston_compositor base;
>
> @@ -1033,9 +1047,20 @@ x11_restore(struct weston_compositor *ec)
> }
>
> static void
> +x11_free_configured_output(struct x11_configured_output *output)
> +{
> + free(output->name);
> + free(output);
> +}
> +
> +static void
> x11_destroy(struct weston_compositor *ec)
> {
> struct x11_compositor *compositor = (struct x11_compositor *)ec;
> + struct x11_configured_output *o, *n;
> +
> + wl_list_for_each_safe(o, n, &configured_output_list, link)
> + x11_free_configured_output(o);
>
> wl_event_source_remove(compositor->xcb_source);
> x11_input_destroy(compositor);
> @@ -1055,8 +1080,9 @@ x11_compositor_create(struct wl_display *display,
> int argc, char *argv[], const char *config_file)
> {
> struct x11_compositor *c;
> + struct x11_configured_output *o;
> xcb_screen_iterator_t s;
> - int i, x;
> + int i, x = 0, output_count = 0;
>
> weston_log("initializing x11 backend\n");
>
> @@ -1099,11 +1125,35 @@ x11_compositor_create(struct wl_display *display,
> if (x11_input_create(c, no_input) < 0)
> goto err_egl;
>
> - for (i = 0, x = 0; i < count; i++) {
> - if (x11_compositor_create_output(c, x, 0, width, height,
> - fullscreen, no_input) < 0)
> - goto err_x11_input;
> - x += width;
> + if (wl_list_empty(&configured_output_list))
I don't think you need this special case, just let
wl_list_for_each_reverse() run through the empty list and fall through
to the output_count case below.
> + for (i = 0; i < count; i++) {
> + if (x11_compositor_create_output(c, x, 0, width, height,
> + fullscreen, no_input) < 0)
> + goto err_x11_input;
> + x += width;
> + }
> + else {
> + wl_list_for_each_reverse(o, &configured_output_list, link) {
> + if (x11_compositor_create_output(c, x, 0,
> + width_arg ? width_arg :
> + o->width,
> + height_arg ? height_arg :
> + o->height,
> + fullscreen, no_input) < 0)
> + goto err_x11_input;
> + x += width_arg ? width_arg : o->width;
> + output_count++;
> + if (count_arg && output_count >= count_arg)
> + break;
> + }
> +
> + if (count > output_count)
You also don't need this condition, if we've created enoguh outputs,
the loop below wont run.
> + for (i = output_count; i < count; i++) {
> + if (x11_compositor_create_output(c, x, 0, width, height,
> + fullscreen, no_input) < 0)
I think no_input and fullscreen is something we should just store in
x11_compositor instead of passing it around everywhere. But that's a
different patch.
> + goto err_x11_input;
> + x += width;
> + }
> }
>
> c->xcb_source =
> @@ -1126,6 +1176,51 @@ err_free:
> return NULL;
> }
>
> +static void
> +output_section_done(void *data)
> +{
> + struct x11_configured_output *output;
> + int error = 0;
> +
> + output = malloc(sizeof *output);
> +
> + if (!output || !output_name || !output_width || !output_height) {
> + free(output_name);
> + output_name = NULL;
> +
> + goto err_free;
> + }
> +
> + output->name = output_name;
> +
> + if (sscanf(output_width, "%d", &output->width) != 1) {
> + weston_log("Invalid width \"%s\" for output %s\n",
> + output_width, output_name);
> + error = 1;
> + }
> +
> + if (sscanf(output_height, "%d", &output->height) != 1) {
> + weston_log("Invalid height \"%s\" for output %s\n",
> + output_height, output_name);
> + error = 1;
> + }
I think we should just do
mode=600x400
like we do for the drm backend (but not support the mode line syntax,
of course).
> + if (error) {
> + x11_free_configured_output(output);
> + goto err_free;
> + } else if (output_name[0] != 'X') {
> + weston_log("Invalid output name \"%s\"\n", output_name);
> + x11_free_configured_output(output);
Won't this generate a warning if I have an entry for my LVDS or such
in the config file? Maybe we should check for output_name[0] != 'X'
early (in the top if-statement) and just silenty ignore it.
> + } else
> + wl_list_insert(&configured_output_list, &output->link);
Use
wl_list_insert(configured_output_list.prev, &output->link);
to insert at the end of the list and avoid wl_list_for_each_reverse above.
> +
> +err_free:
> + free(output_width);
> + output_width = NULL;
> + free(output_height);
> + output_height = NULL;
> +}
> +
> WL_EXPORT struct weston_compositor *
> backend_init(struct wl_display *display, int argc, char *argv[],
> const char *config_file)
> @@ -1134,15 +1229,38 @@ backend_init(struct wl_display *display, int argc, char *argv[],
> int no_input = 0;
>
> const struct weston_option x11_options[] = {
> - { WESTON_OPTION_INTEGER, "width", 0, &width },
> - { WESTON_OPTION_INTEGER, "height", 0, &height },
> + { WESTON_OPTION_INTEGER, "width", 0, &width_arg },
> + { WESTON_OPTION_INTEGER, "height", 0, &height_arg },
> { WESTON_OPTION_BOOLEAN, "fullscreen", 0, &fullscreen },
> - { WESTON_OPTION_INTEGER, "output-count", 0, &count },
> + { WESTON_OPTION_INTEGER, "output-count", 0, &count_arg },
> { WESTON_OPTION_BOOLEAN, "no-input", 0, &no_input },
> };
>
> parse_options(x11_options, ARRAY_LENGTH(x11_options), argc, argv);
>
> + if (width_arg)
> + width = width_arg;
> + if (height_arg)
> + height = height_arg;
> + if (count_arg)
> + count = count_arg;
Just move the default values up to the global variables (eg, static
int option_width = 1024;) and the just pass option_width to
x11_compositor_create() below.
> + wl_list_init(&configured_output_list);
> +
> + const struct config_key x11_config_keys[] = {
> + { "name", CONFIG_KEY_STRING, &output_name },
> + { "width", CONFIG_KEY_STRING, &output_width },
> + { "height", CONFIG_KEY_STRING, &output_height },
> + };
> +
> + const struct config_section config_section[] = {
> + { "output", x11_config_keys,
> + ARRAY_LENGTH(x11_config_keys), output_section_done },
> + };
> +
> + parse_config_file(config_file, config_section,
> + ARRAY_LENGTH(config_section), NULL);
> +
> return x11_compositor_create(display,
> width, height, count, fullscreen,
> no_input,
> diff --git a/weston.ini b/weston.ini
> index c2d6369..1de8e29 100644
> --- a/weston.ini
> +++ b/weston.ini
> @@ -41,3 +41,8 @@ duration=600
> #[output]
> #name=VGA1
> #mode=173.00 1920 2048 2248 2576 1080 1083 1088 1120 -hsync +vsync
> +
> +#[output]
> +#name=X1
> +#width=1024
> +#height=768
> --
> 1.7.11.2
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list