[PATCH weston v2 06/12] keyboard: Only set toplevel when there is a valid output

[Quentin Glidic]

On 23/06/2016 11:59, Armin Krezović wrote:
> Currently, the keyboard client is created and the input
> panel surface is set as toplevel on the first output it
> finds. This does not work in a scenario when there are
> no outputs, resulting in weston-keyboard to crash at
> startup due to operating on an invalid output pointer.
>
> This makes input panel toplevel setting depend on a
> valid output, and if there was no output present at
> startup, it will be set toplevel as soon as an output
> gets plugged in.
>
> v2:
>
> - Remove dependency on output pointer at startup
> - Only setup output_configure_handler after the
>   keyboard has been created
> - Let the output_configure_handler handle toplevel
>   setting in all cases
>
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
>  clients/keyboard.c | 42 ++++++++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/clients/keyboard.c b/clients/keyboard.c
> index d719764..83082ad 100644
> --- a/clients/keyboard.c
> +++ b/clients/keyboard.c
> @@ -24,6 +24,7 @@
>
>  #include "config.h"
>
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -56,6 +57,7 @@ struct virtual_keyboard {
>  	char *surrounding_text;
>  	uint32_t surrounding_cursor;
>  	struct keyboard *keyboard;
> +	bool toplevel;
>  };
>
>  enum key_type {
> @@ -953,11 +955,34 @@ global_handler(struct display *display, uint32_t name,
>  }
>
>  static void
> -keyboard_create(struct output *output, struct virtual_keyboard *virtual_keyboard)
> +set_toplevel(struct output *output, struct virtual_keyboard *virtual_keyboard)
> +{
> +	struct zwp_input_panel_surface_v1 *ips;
> +	struct keyboard *keyboard = virtual_keyboard->keyboard;
> +
> +	ips = zwp_input_panel_v1_get_input_panel_surface(virtual_keyboard->input_panel,
> +							 window_get_wl_surface(keyboard->window));
> +
> +	zwp_input_panel_surface_v1_set_toplevel(ips,
> +						output_get_wl_output(output),
> +						ZWP_INPUT_PANEL_SURFACE_V1_POSITION_CENTER_BOTTOM);
> +
> +	virtual_keyboard->toplevel = true;
> +}
> +
> +static
> +void display_output_handler(struct output *output, void *data) {

Nit: void on the previous line.


> +	struct virtual_keyboard *keyboard = data;
> +
> +	if (!keyboard->toplevel)
> +		set_toplevel(output, keyboard);
> +}
> +
> +static void
> +keyboard_create(struct virtual_keyboard *virtual_keyboard)
>  {
>  	struct keyboard *keyboard;
>  	const struct layout *layout;
> -	struct zwp_input_panel_surface_v1 *ips;
>
>  	layout = get_current_layout(virtual_keyboard);
>
> @@ -981,20 +1006,14 @@ keyboard_create(struct output *output, struct virtual_keyboard *virtual_keyboard
>  			       layout->columns * key_width,
>  			       layout->rows * key_height);
>
> -
> -	ips = zwp_input_panel_v1_get_input_panel_surface(virtual_keyboard->input_panel,
> -							 window_get_wl_surface(keyboard->window));
> -
> -	zwp_input_panel_surface_v1_set_toplevel(ips,
> -						output_get_wl_output(output),
> -						ZWP_INPUT_PANEL_SURFACE_V1_POSITION_CENTER_BOTTOM);
> +	display_set_output_configure_handler(virtual_keyboard->display,
> +					     display_output_handler);

Maybe we do not want to hide it in there, but instead in main() (but 
after keyboard_create())?

Anyway:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>

Cheers,


>  }
>
>  int
>  main(int argc, char *argv[])
>  {
>  	struct virtual_keyboard virtual_keyboard;
> -	struct output *output;
>
>  	memset(&virtual_keyboard, 0, sizeof virtual_keyboard);
>
> @@ -1012,8 +1031,7 @@ main(int argc, char *argv[])
>  		return -1;
>  	}
>
> -	output = display_get_output(virtual_keyboard.display);
> -	keyboard_create(output, &virtual_keyboard);
> +	keyboard_create(&virtual_keyboard);
>
>  	display_run(virtual_keyboard.display);
>
>

-- 

Quentin “Sardem FF7” Glidic