[PATCH] Bug fix client apps because of output change

[Pekka Paalanen]

On Thu,  6 Mar 2014 16:25:42 +0800
Quanxian Wang <quanxian.wang at intel.com> wrote:

> 1)
> Width and height of Panel and Background depend
> on output's, therefore they should be bound
> with output changes including mode, transform and scale.
> 
> 2)
> Update the min_allocation before resize the panel and
> background window. Add window_set_min_allocation function
> because it is invisible outside window.c.
> 
> Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
> ---
>  clients/desktop-shell.c | 70
> +++++++++++++++++++++++++++++++++++++++++++++++++
> clients/window.c        |  7 +++++ clients/window.h        |  2 ++
>  3 files changed, 79 insertions(+)
> 
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index a0c6b6d..9ccba34 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -96,6 +96,12 @@ struct background {
>  	uint32_t color;
>  };
>  
> +struct mode {
> +	int height;
> +	int width;
> +	uint32_t refresh;
> +};
> +
>  struct output {
>  	struct wl_output *output;
>  	uint32_t server_output_id;
> @@ -103,6 +109,9 @@ struct output {
>  
>  	struct panel *panel;
>  	struct background *background;
> +	struct mode *mode;

Hi,

I think you could simplify this code quite a bit by just having the
struct mode embed in struct output, instead of dynamically allocating
it. You will always have exactly one struct mode for each struct
output, right?

> +	uint32_t transform;
> +	uint32_t scale;
>  };
>  
>  struct panel_launcher {
> @@ -1128,6 +1137,10 @@ output_destroy(struct output *output)
>  {
>  	background_destroy(output->background);
>  	panel_destroy(output->panel);
> +
> +	if (output->mode)
> +		free(output->mode);
> +
>  	wl_output_destroy(output->output);
>  	wl_list_remove(&output->link);
>  
> @@ -1145,6 +1158,44 @@ desktop_destroy_outputs(struct desktop
> *desktop) }
>  
>  static void
> +update_output(struct output *output)
> +{
> +	struct panel *panel = output->panel;
> +	struct background *background = output->background;
> +	int width, height;
> +
> +	if (!output || !output->mode)
> +		return;
> +
> +	width = output->mode->width;
> +	height = output->mode->height;
> +
> +	switch (output->transform) {
> +	case WL_OUTPUT_TRANSFORM_90:
> +	case WL_OUTPUT_TRANSFORM_270:
> +	case WL_OUTPUT_TRANSFORM_FLIPPED_90:
> +	case WL_OUTPUT_TRANSFORM_FLIPPED_270:
> +		/* Swap width and height */
> +		width = output->mode->height;
> +		height = output->mode->width;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (output->scale != 0) {
> +		width /= output->scale;
> +		height /= output->scale;
> +	}
> +
> +	/* Set min_allocation of panel */
> +	window_set_min_allocation(panel->window, width, 32);

Is there a reason why the background window does not need the
min_allocation changed?

> +
> +	window_schedule_resize(background->window, width, height);
> +	window_schedule_resize(panel->window, width, 32);
> +}
> +
> +static void
>  output_handle_geometry(void *data,
>                         struct wl_output *wl_output,
>                         int x, int y,
> @@ -1157,6 +1208,8 @@ output_handle_geometry(void *data,
>  {
>  	struct output *output = data;
>  
> +	output->transform = transform;
> +	update_output(output);
>  	window_set_buffer_transform(output->panel->window,
> transform); window_set_buffer_transform(output->background->window,
> transform); }
> @@ -1169,6 +1222,17 @@ output_handle_mode(void *data,
>  		   int height,
>  		   int refresh)
>  {
> +	struct output *output = data;
> +
> +	if (flags & WL_OUTPUT_MODE_CURRENT) {
> +		if (!output && !output->mode)
> +			return;
> +
> +		output->mode->width = width;
> +		output->mode->height = height;
> +		output->mode->refresh = refresh;
> +		update_output(output);
> +	}
>  }
>  
>  static void
> @@ -1184,6 +1248,8 @@ output_handle_scale(void *data,
>  {
>  	struct output *output = data;
>  
> +	output->scale  = scale;
> +	update_output(output);

I think calling update_output() would be better done from the
wl_output.done event handler, because there you are guaranteed to have
all the relevant events received. Otherwise you race the scheduled
resize against receiving all the related events.

However, the catch is that wl_output.done is in interface version >= 2,
but not in version 1. For version 1 you'd need just call
update_output() directly like you already do.

>  	window_set_buffer_scale(output->panel->window, scale);
>  	window_set_buffer_scale(output->background->window, scale);
>  }
> @@ -1206,6 +1272,10 @@ output_init(struct output *output, struct
> desktop *desktop) output->output, surface);
>  
>  	output->background = background_create(desktop);
> +	output->mode = (struct mode *)xzalloc(sizeof(*output->mode));
> +	if (!output->mode)
> +		fprintf(stderr, "No memory for mode allocation.\n");
> +
>  	surface = window_get_wl_surface(output->background->window);
>  	desktop_shell_set_background(desktop->shell,
>  				     output->output, surface);
> diff --git a/clients/window.c b/clients/window.c
> index c8287e2..6c01222 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -3839,6 +3839,13 @@ undo_resize(struct window *window)
>  }
>  
>  void
> +window_set_min_allocation(struct window *window, int width, int
> height) +{
> +	window->min_allocation.width = width;
> +	window->min_allocation.height = height;
> +}
> +
> +void
>  window_schedule_resize(struct window *window, int width, int height)
>  {
>  	/* We should probably get these numbers from the theme. */
> diff --git a/clients/window.h b/clients/window.h
> index 54b848b..b221b76 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -336,6 +336,8 @@ void
>  window_schedule_redraw(struct window *window);
>  void
>  window_schedule_resize(struct window *window, int width, int height);
> +void
> +window_set_min_allocation(struct window *window, int width, int
> height); 
>  void
>  window_damage(struct window *window, int32_t x, int32_t y,

Otherwise seems fine to me, although I didn't test it.


Thanks,
pq