On Fri, 24 Jun 2016 11:39:30 +0200
Quentin Glidic <sardemff7+wayl...@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.ar...@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+...@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);
> >
> >  
> 

Attachment: pgpsgfth98EUO.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to