On 18/06/2016 19:15, 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.

Signed-off-by: Armin Krezović <[email protected]>
---

The overall idea is good, let’s see the details.


 clients/keyboard.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/clients/keyboard.c b/clients/keyboard.c
index d719764..4fb4200 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,26 @@ global_handler(struct display *display, uint32_t name,
 }

 static void
+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
 keyboard_create(struct output *output, 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,13 +998,16 @@ keyboard_create(struct output *output, struct 
virtual_keyboard *virtual_keyboard
                               layout->columns * key_width,
                               layout->rows * key_height);

+       if (output)
+               set_toplevel(output, virtual_keyboard);

I would drop it completely from here, see below.


+}

-       ips = 
zwp_input_panel_v1_get_input_panel_surface(virtual_keyboard->input_panel,
-                                                        
window_get_wl_surface(keyboard->window));
+static
+void display_output_handler(struct output *output, void *data) {
+       struct virtual_keyboard *keyboard = data;

-       zwp_input_panel_surface_v1_set_toplevel(ips,
-                                               output_get_wl_output(output),
-                                               
ZWP_INPUT_PANEL_SURFACE_V1_POSITION_CENTER_BOTTOM);
+       if (!keyboard->toplevel)
+               set_toplevel(output, keyboard);
 }

 int
@@ -1006,6 +1026,7 @@ main(int argc, char *argv[])

        display_set_user_data(virtual_keyboard.display, &virtual_keyboard);
        display_set_global_handler(virtual_keyboard.display, global_handler);
+       display_set_output_configure_handler(virtual_keyboard.display, 
display_output_handler);

Is that handler called only on hotplug or is it called at startup too?
IOW, could we rely on it to be called at least one?



        if (virtual_keyboard.input_panel == NULL) {
                fprintf(stderr, "No input panel global\n");



Later in this file, there is:
        output = display_get_output(virtual_keyboard.display);
        keyboard_create(output, &virtual_keyboard);

Could be replaced by:
        keyboard_create(&virtual_keyboard);
        output = display_get_output(virtual_keyboard.display);
        if (output)
                set_toplevel(output, keyboard);

Or keyboard_create only if the handler is assured to be called at least once.

Anyone has comments? :-)

Cheers,

--

Quentin “Sardem FF7” Glidic
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to