On Wed, 6 Sep 2017 08:17:19 -0400 nerdopolis <bluescreen_aven...@verizon.net> wrote:
> ---
Hi,
the commit message is missing a rationale and Signed-off-by.
> compositor/main.c | 2 ++
> libweston/compositor-fbdev.c | 5 ++++-
> libweston/compositor-fbdev.h | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/compositor/main.c b/compositor/main.c
> index 61bda282..f88608cd 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -573,6 +573,7 @@ usage(int error_code)
> "Options for fbdev-backend.so:\n\n"
> " --tty=TTY\t\tThe tty to use\n"
> " --device=DEVICE\tThe framebuffer device to use\n"
> + " --seat=SEAT\t\tThe seat that weston should run on, instead
> of the seat defined in XDG_SEAT\n"
> "\n");
> #endif
>
> @@ -1444,6 +1445,7 @@ load_fbdev_backend(struct weston_compositor *c,
> const struct weston_option fbdev_options[] = {
> { WESTON_OPTION_INTEGER, "tty", 0, &config.tty },
> { WESTON_OPTION_STRING, "device", 0, &config.device },
> + { WESTON_OPTION_STRING, "seat", 0, &config.seat_id },
> };
>
> parse_options(fbdev_options, ARRAY_LENGTH(fbdev_options), argc, argv);
> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index dabacbb5..b4f0685c 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -728,6 +728,8 @@ fbdev_backend_create(struct weston_compositor *compositor,
> session_seat=getenv("XDG_SEAT");
> if (session_seat)
> seat_id=session_seat;
> + if (param->seat_id)
> + seat_id = param->seat_id;
This is correct, but there is something else below that is not.
>
> weston_log("initializing fbdev backend\n");
>
> @@ -751,7 +753,7 @@ fbdev_backend_create(struct weston_compositor *compositor,
> wl_signal_add(&compositor->session_signal,
> &backend->session_listener);
> compositor->launcher =
> - weston_launcher_connect(compositor, param->tty, "seat0", false);
> + weston_launcher_connect(compositor, param->tty, seat_id, false);
Ok.
> if (!compositor->launcher) {
> weston_log("fatal: fbdev backend should be run "
> "using weston-launch binary or as root\n");
> @@ -797,6 +799,7 @@ config_init_to_defaults(struct
> weston_fbdev_backend_config *config)
> * udev, rather than passing a device node in as a parameter. */
> config->tty = 0; /* default to current tty */
> config->device = "/dev/fb0"; /* default frame buffer */
> + config->seat_id = "seat0"; /* default seat*/
Setting the default here does not work quite right. If seat_id is not
NULL, then fbdev-backend must use seat_id instead of the default or
XDG_SEAT. Otherwise the compositor cannot choose the seat with a
command line option.
The values set in this function will be used when the compositor uses a
version of weston_fbdev_backend_config struct which does not have these
fields.
If the compositor uses a version of weston_fbdev_backend_config that is
up-to-date, in Weston's case always, then these values get ignored.
I think the right action would be to not assign seat_id here at all,
leaving it NULL. Then the only way for it to be non-NULL is when the
compositor passes in a seat name that must be used instead of XDG_SEAT
or the default.
> }
>
> WL_EXPORT int
> diff --git a/libweston/compositor-fbdev.h b/libweston/compositor-fbdev.h
> index 8b7d900e..c4e3305a 100644
> --- a/libweston/compositor-fbdev.h
> +++ b/libweston/compositor-fbdev.h
> @@ -43,6 +43,7 @@ struct weston_fbdev_backend_config {
>
> int tty;
> char *device;
> + char *seat_id;
This is a ABI breaking change to weston_fbdev_backend_config, so you
should bump WESTON_FBDEV_BACKEND_CONFIG_VERSION as well, or put the new
field as the last one instead.
>
> /** Callback used to configure input devices.
> *
Thanks,
pq
pgprFd273wiKD.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
