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

[Pekka Paalanen]

On Fri, 24 Jun 2016 11:39:30 +0200
Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:

> 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(-)

Hi Armin,

looks good to me. See below.

> >
> > 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.

I fixed the nit when merging.


> > +	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())?

Perhaps, but this is good enough for me.

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

And so patches 1 - 6 pushed:
   5c2f20e..ea0316a  master -> master

The rest I am still looking at.


Thanks,
pq

> 
> 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);
> >
> >  
> 

-------------- 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/20160627/c201648d/attachment.sig>