On Tue,  9 Jun 2015 20:24:55 +0000
Murray Calavera <[email protected]> wrote:

> Whether a input method is used should be the responsibility
> of the shell because some shells may not want to implement
> an input method at all.
> 
> Signed-off-by: Murray Calavera <[email protected]>

Hi,

very good.

> ---
>  desktop-shell/shell.c | 3 +++
>  ivi-shell/ivi-shell.c | 3 +++
>  src/compositor.c      | 2 --
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index fe620cb..a697d04 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -6719,6 +6719,9 @@ module_init(struct weston_compositor *ec,
>                            shell, bind_workspace_manager) == NULL)
>               return -1;
>  
> +     if (text_backend_init(ec) < 0)
> +             return -1;

Any reason to put this this late in module_init()?

It is related to the input_panel_setup() call, so it would be nice to
group them together.

> +
>       shell->child.deathstamp = weston_compositor_get_time();
>  
>       shell->panel_position = DESKTOP_SHELL_PANEL_POSITION_TOP;
> diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
> index 4a688cc..b70f1f4 100644
> --- a/ivi-shell/ivi-shell.c
> +++ b/ivi-shell/ivi-shell.c
> @@ -428,6 +428,9 @@ module_init(struct weston_compositor *compositor,
>                            shell, bind_ivi_application) == NULL)
>               goto out_settings;
>  
> +     if (text_backend_init(compositor) < 0)
> +             goto out_settings;

The same here.

> +
>       ivi_layout_init_with_compositor(compositor);
>  
>       /* Call module_init of ivi-modules which are defined in weston.ini */
> diff --git a/src/compositor.c b/src/compositor.c
> index 8f02b4d..38c0775 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -4578,8 +4578,6 @@ weston_compositor_init(struct weston_compositor *ec,
>       weston_config_section_get_int(s, "repeat-delay",
>                                     &ec->kb_repeat_delay, 400);
>  
> -     text_backend_init(ec);
> -
>       wl_data_device_manager_init(ec->wl_display);
>  
>       wl_display_init_shm(display);

Looking good overall.

If you put the text_backend_init() calls right after the
input_panel_setup() calls, then this patch is:
Reviewed-by: Pekka Paalanen <[email protected]>

(You can do that exact change and then re-send the patch with my R-b
tag already added.)


Thanks,
pq
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to