[PATCH weston v2 6/9] wayland-backend: split wayland_output_create_for_config

Pekka Paalanen ppaalanen at gmail.com
Wed May 4 12:47:48 UTC 2016


On Thu, 28 Apr 2016 20:33:13 +0200
Benoit Gschwind <gschwind at gnu-log.net> wrote:

> The splitting intend to separate configuration parsing from output
> setup.
> 
> Signed-off-by: Benoit Gschwind <gschwind at gnu-log.net>
> ---
>  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160504/b6670bfe/attachment.sig>


More information about the wayland-devel mailing list