On Thu, 28 Apr 2016 20:33:13 +0200
Benoit Gschwind <[email protected]> wrote:

> The splitting intend to separate configuration parsing from output
> setup.
> 
> Signed-off-by: Benoit Gschwind <[email protected]>
> ---
>  src/compositor-wayland.c | 96 
> ++++++++++++++++++++++++++++++------------------
>  1 file changed, 60 insertions(+), 36 deletions(-)
> 
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index f218c9e..e40403f 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -1107,61 +1107,68 @@ err_name:
>       return NULL;
>  }

Btw. wayland_backend_create() is no longer using argc, argv, nor config
arguments. They should be removed from its signature. I don't see any
latter patches doing that either.

>  
> -static struct wayland_output *
> -wayland_output_create_for_config(struct wayland_backend *b,
> -                              struct weston_config_section *config_section,
> -                              int option_width, int option_height,
> -                              int option_scale, int32_t x, int32_t y)
> +static void
> +wayland_output_init_from_config(struct weston_wayland_backend_output_config 
> *output,
> +                             struct weston_config_section *config_section,
> +                             int option_width, int option_height,
> +                             int option_scale)

This should be called wayland_output_config_init(). I was first baffled
how come it does not return a struct wayland_output nor even take one
as an argument.

Also the argument 'output' is not an output but a config, the name is
misleading.

>  {
> -     struct wayland_output *output;
> -     char *mode, *t, *name, *str;
> -     int width, height, scale;
> -     uint32_t transform;
> +     char *mode, *t, *str;
>       unsigned int slen;
>  
> -     weston_config_section_get_string(config_section, "name", &name, NULL);
> -     if (name) {
> -             slen = strlen(name);
> +     weston_config_section_get_string(config_section, "name", &output->name,
> +                                      NULL);
> +     if (output->name) {
> +             slen = strlen(output->name);
>               slen += strlen(WINDOW_TITLE " - ");
>               str = malloc(slen + 1);
>               if (str)
> -                     snprintf(str, slen + 1, WINDOW_TITLE " - %s", name);
> -             free(name);
> -             name = str;
> +                     snprintf(str, slen + 1, WINDOW_TITLE " - %s",
> +                              output->name);
> +             free(output->name);
> +             output->name = str;

That block would be easy to simplify by using asprintf(), if anyone
cared.

>       }
> -     if (!name)
> -             name = strdup(WINDOW_TITLE);
> +     if (!output->name)
> +             output->name = strdup(WINDOW_TITLE);
>  
>       weston_config_section_get_string(config_section,
>                                        "mode", &mode, "1024x600");
> -     if (sscanf(mode, "%dx%d", &width, &height) != 2) {
> +     if (sscanf(mode, "%dx%d", &output->width, &output->height) != 2) {
>               weston_log("Invalid mode \"%s\" for output %s\n",
> -                        mode, name);
> -             width = 1024;
> -             height = 640;
> +                        mode, output->name);
> +             output->width = 1024;
> +             output->height = 640;
>       }
>       free(mode);
>  
>       if (option_width)
> -             width = option_width;
> +             output->width = option_width;
>       if (option_height)
> -             height = option_height;
> +             output->height = option_height;
>  
> -     weston_config_section_get_int(config_section, "scale", &scale, 1);
> +     weston_config_section_get_int(config_section, "scale", &output->scale, 
> 1);
>  
>       if (option_scale)
> -             scale = option_scale;
> +             output->scale = option_scale;
>  
>       weston_config_section_get_string(config_section,
>                                        "transform", &t, "normal");
> -     if (weston_parse_transform(t, &transform) < 0)
> +     if (weston_parse_transform(t, &output->transform) < 0)
>               weston_log("Invalid transform \"%s\" for output %s\n",
> -                        t, name);
> +                        t, output->name);
>       free(t);
>  
> -     output = wayland_output_create(b, x, y, width, height, name, 0,
> -                                    transform, scale);
> -     free(name);
> +}

There is a hilarious confusion with "output name" in the wayland
backend it seems. In weston.ini, the wayland outputs are named "WLx"
where 'x' is whatever. However, the string stored in struct
wayland_output::name is not the name but the window title.

And then wayland_output_create_for_parent_output() does not create a
name at all.

I suppose the output names aren't used much.

> +
> +static struct wayland_output *
> +wayland_output_create_for_config(struct wayland_backend *b,
> +                              struct weston_wayland_backend_output_config 
> *oc,
> +                              int fullscreen, int32_t x, int32_t y)
> +{
> +     struct wayland_output *output;
> +
> +     output = wayland_output_create(b, x, y, oc->width, oc->height, oc->name,
> +                                    fullscreen, oc->transform, oc->scale);
>  
>       return output;
>  }
> @@ -2344,6 +2351,7 @@ backend_init(struct weston_compositor *compositor, int 
> *argc, char *argv[],
>       struct wayland_parent_output *poutput;
>       struct weston_config_section *section;
>       struct weston_wayland_backend_config new_config;
> +     struct weston_wayland_backend_output_config output_config;
>       int x, count, width, height, scale;
>       const char *section_name;
>       char *name;
> @@ -2395,8 +2403,14 @@ backend_init(struct weston_compositor *compositor, int 
> *argc, char *argv[],
>       }
>  
>       if (new_config.fullscreen) {
> -             output = wayland_output_create(b, 0, 0, width, height,
> -                                            NULL, 1, 0, 1);
> +             output_config.width = width;
> +             output_config.height = height;
> +             output_config.name = NULL;
> +             output_config.transform = WL_OUTPUT_TRANSFORM_NORMAL;
> +             output_config.scale = scale;

output_config.scale should be hardcoded to 1, right?

Otherwise it defaults to 0 through 'scale' if --scale is not given on
the command line, and that would break.

I see it reverts back to the hardcoded 1 in a following patch.

> +
> +             output = wayland_output_create_for_config(b, &output_config,
> +                                                       1, 0, 0);
>               if (!output)
>                       goto err_outputs;
>  
> @@ -2419,8 +2433,12 @@ backend_init(struct weston_compositor *compositor, int 
> *argc, char *argv[],
>               }
>               free(name);
>  
> -             output = wayland_output_create_for_config(b, section, width,
> -                                                       height, scale, x, 0);
> +             wayland_output_init_from_config(&output_config, section, width,
> +                                             height, scale);
> +             output = wayland_output_create_for_config(b, &output_config, 0,
> +                                                       x, 0);
> +             free(output_config.name);
> +
>               if (!output)
>                       goto err_outputs;
>               if (wayland_output_set_windowed(output))
> @@ -2437,8 +2455,14 @@ backend_init(struct weston_compositor *compositor, int 
> *argc, char *argv[],
>       if (!scale)
>               scale = 1;
>       while (count > 0) {
> -             output = wayland_output_create(b, x, 0, width, height,
> -                                            NULL, 0, 0, scale);
> +             output_config.width = width;
> +             output_config.height = height;
> +             output_config.name = NULL;
> +             output_config.transform = WL_OUTPUT_TRANSFORM_NORMAL;
> +             output_config.scale = scale;
> +
> +             output = wayland_output_create_for_config(b, &output_config,
> +                                                       0, x, 0);
>               if (!output)
>                       goto err_outputs;
>               if (wayland_output_set_windowed(output))

Thanks,
pq

Attachment: pgpSP4hTsEU61.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to