[PATCH weston v2] clients: Add a weston-autorotator client and rotator protocol
Pekka Paalanen
ppaalanen at gmail.com
Mon Oct 10 14:07:50 UTC 2016
On Mon, 8 Aug 2016 15:41:04 +0100
Emmanuel Gil Peyrot <emmanuel.peyrot at collabora.com> wrote:
> This client uses libiio to retrieve accelerometer values from the iio
> subsystem on Linux (and maybe some other kernels), and automatically
> rotate the output whenever orientation changed.
>
> I tested it with a mma8453 accelerometer, but everything needed should
> be available in the configuration to make it work with any other iio
> device.
>
> v2:
> - Rename rotater to rotator, which is actual English.
> - Add documentation for every new option, and a default value.
> - Add the bare minimum of those options in the sample weston.ini.
> - Fix for an inaccurate sampling frequency due to integer downcasting.
> - Make the server-side a module instead of a simple function that has to be
> called by every shell.
> - Remove the mostly-useless smoothing of the captured input.
> - Make the protocol take an accelerometer name, to let the rotator
> match its name with the outputs using it.
>
> Signed-off-by: Emmanuel Gil Peyrot <emmanuel.peyrot at collabora.com>
> ---
> .gitignore | 1 +
> Makefile.am | 28 +++
> clients/autorotator.c | 435 ++++++++++++++++++++++++++++++++++++++++++++
> compositor/rotator.c | 246 +++++++++++++++++++++++++
> configure.ac | 15 ++
> libweston/compositor.c | 1 +
> libweston/compositor.h | 1 +
> man/weston.ini.man | 41 +++++
> protocol/weston-rotator.xml | 26 +++
> weston.ini.in | 4 +
> 10 files changed, 798 insertions(+)
> create mode 100644 clients/autorotator.c
> create mode 100644 compositor/rotator.c
> create mode 100644 protocol/weston-rotator.xml
Hi,
this rebases to current master with just a couple trivial conflicts.
The architecture looks good now, with the justification from last time.
All the comments below are implementation details, the biggest of them
being the need for a proper interface to change the transform of a
weston_output.
There is nothing even packaged in Gentoo with 'iio' in its name, so
people probably won't build-test this stuff often. Where does one get
libiio?
Maybe mention the URL in at least the commit message or even in the
configure.ac error.
> diff --git a/.gitignore b/.gitignore
> index 41a140b..72cea9f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -51,6 +51,7 @@ protocol/*.[ch]
>
> 00*.patch
>
> +weston-autorotator
> weston-calibrator
> weston-clickdot
> weston-cliptest
> diff --git a/Makefile.am b/Makefile.am
> index 32627f5..0e990ab 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -124,6 +124,8 @@ endif
> nodist_libweston_ at LIBWESTON_MAJOR@_la_SOURCES = \
> protocol/weston-screenshooter-protocol.c \
> protocol/weston-screenshooter-server-protocol.h \
> + protocol/weston-rotator-protocol.c \
> + protocol/weston-rotator-server-protocol.h \
These would belong in nodist_rotator_la_SOURCES.
> protocol/text-cursor-position-protocol.c \
> protocol/text-cursor-position-server-protocol.h \
> protocol/text-input-unstable-v1-protocol.c \
> @@ -610,6 +612,32 @@ nodist_weston_screenshooter_SOURCES = \
> weston_screenshooter_LDADD = $(CLIENT_LIBS) libshared.la
> weston_screenshooter_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
>
> +if BUILD_AUTOROTATOR
> +libexec_PROGRAMS += weston-autorotator
> +
> +weston_autorotator_SOURCES = \
> + clients/autorotator.c
> +nodist_weston_autorotator_SOURCES = \
> + protocol/weston-rotator-protocol.c \
> + protocol/weston-rotator-client-protocol.h
> +weston_autorotator_LDADD = $(CLIENT_LIBS) $(LIBIIO_LIBS) libshared.la
> +weston_autorotator_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS) $(LIBIIO_CFLAGS)
> +
> +module_LTLIBRARIES += rotator.la
> +rotator_la_LDFLAGS = -module -avoid-version
> +rotator_la_LIBADD = $(COMPOSITOR_LIBS) libshared.la
> +rotator_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
> +rotator_la_SOURCES = \
> + compositor/rotator.c
> +
> +BUILT_SOURCES += \
> + protocol/weston-rotator-protocol.c \
> + protocol/weston-rotator-client-protocol.h
> +
> +EXTRA_DIST += \
> + protocol/weston-rotator.xml
> +endif
Not sure BUILT_SOURCES and EXTRA_DIST should be inside 'if'.
Distcheck does fail (when libiio not available).
> +
> weston_terminal_SOURCES = \
> clients/terminal.c \
> shared/helpers.h
> diff --git a/clients/autorotator.c b/clients/autorotator.c
> new file mode 100644
> index 0000000..89697c9
> --- /dev/null
> +++ b/clients/autorotator.c
> @@ -0,0 +1,435 @@
> +/*
> + * Copyright © 2016 Collabora, Ltd.
Is the copyright correct?
> +static bool
> +synchronize(struct wl_display *display, double sampling_frequency)
> +{
> + if (wl_display_roundtrip(display) < 0) {
> + fprintf(stderr, "lost connection, aborting\n");
> + return false;
> + }
> + usleep(1000. * sampling_frequency);
Please, add a big fat comment ranting about the IIO driver quality and
not supporting event-based delivery. This kind of uglyness always
demands justification. :-P
> + return true;
> +}
> +
> +static void
> +handle_global(void *data, struct wl_registry *registry,
> + uint32_t name, const char *interface, uint32_t version)
> +{
> + if (strcmp(interface, "weston_rotator") == 0) {
> + rotator = wl_registry_bind(registry, name,
> + &weston_rotator_interface,
> + 1);
> + }
> +}
> +
> +static void
> +handle_global_remove(void *data, struct wl_registry *registry, uint32_t name)
> +{
> + /* XXX: unimplemented */
> +}
> +
> +static const struct wl_registry_listener registry_listener = {
> + handle_global,
> + handle_global_remove
> +};
> +
> +int main(int argc, char *argv[])
> +{
> + struct wl_display *display;
> + struct wl_registry *registry;
> + struct config *config;
> + struct accelerometer *accel;
> + const char *accelerometer_device;
> + bool ret, found;
> + int x, y;
> + int transform, previous_transform, *allowed_transform;
> +
> + if (getenv("WAYLAND_SOCKET") == NULL) {
> + fprintf(stderr, "%s must be launched by Weston.\n",
> + program_invocation_short_name);
> + return 1;
> + }
> +
> + accelerometer_device = getenv("WESTON_ACCELEROMETER");
> + if (accelerometer_device == NULL) {
> + fprintf(stderr, "Weston must pass an accelerometer device.\n");
> + return 2;
> + }
> +
> + config = read_configuration(accelerometer_device);
> + if (!config) {
> + fprintf(stderr, "failed to read configuration\n");
> + return 3;
> + }
> +
> + display = wl_display_connect(NULL);
> + if (display == NULL) {
> + fprintf(stderr, "failed to create display: %m\n");
> + destroy_configuration(config);
> + return 4;
> + }
> +
> + registry = wl_display_get_registry(display);
> + wl_registry_add_listener(registry, ®istry_listener, NULL);
> + wl_display_dispatch(display);
The dispatch call is racy. See the big comment in
simple-shm.c:create_display() for rationale. I think you only need one
roundtrip here.
> + wl_display_roundtrip(display);
> + if (rotator == NULL) {
> + fprintf(stderr, "display doesn't support rotator\n");
> + free(registry);
> + wl_display_disconnect(display);
> + destroy_configuration(config);
> + return 5;
> + }
> +
> + accel = initialize_iio(config);
> + if (!accel) {
> + fprintf(stderr, "failed to initialize iio subsystem\n");
> + free(registry);
> + wl_display_disconnect(display);
> + destroy_configuration(config);
> + return 6;
> + }
> +
> + previous_transform = WESTON_ROTATOR_TRANSFORM_NORMAL;
> + for (;;) {
> + ret = capture_rotation(accel, &x, &y);
> + if (!ret) {
> + fprintf(stderr, "failed to capture accelerometer "
> + "values\n");
> + break;
> + }
> +
> + transform = get_transform(x, y, config->threshold);
> + if (transform < 0) {
> + if (!synchronize(display, accel->sampling_frequency))
> + return 7;
> + continue;
> + }
> +
> + found = false;
> + wl_array_for_each(allowed_transform,
> + &config->allowed_rotations) {
> + if (transform == *allowed_transform) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + if (!synchronize(display, accel->sampling_frequency))
> + return 7;
> + continue;
> + }
> +
> + if (transform != previous_transform) {
> + weston_rotator_rotate(rotator,
> + accelerometer_device,
> + transform);
> + }
> + previous_transform = transform;
> +
> + if (!synchronize(display, accel->sampling_frequency))
> + return 7;
> + }
> +
> + wl_display_disconnect(display);
> + destroy_accelerometer(accel);
> + destroy_configuration(config);
> +
> + return 0;
> +}
> diff --git a/compositor/rotator.c b/compositor/rotator.c
> new file mode 100644
> index 0000000..c823205
> --- /dev/null
> +++ b/compositor/rotator.c
> @@ -0,0 +1,246 @@
> +/*
> + * Copyright © 2016 Collabora, Ltd.
Is the copyright correct?
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/input-event-codes.h>
> +
> +#include "compositor.h"
> +#include "weston-rotator-server-protocol.h"
> +#include "weston.h"
> +#include "shared/helpers.h"
> +#include "shared/zalloc.h"
> +
> +struct autorotator_process {
> + char *accelerometer;
> + struct wl_client *client;
> + struct weston_process process;
> + struct wl_list link;
> +};
> +
> +struct rotator {
> + struct weston_compositor *ec;
> + struct wl_global *global;
> + struct wl_list process_list;
> + struct wl_listener destroy_listener;
> +};
> +
> +static void
> +rotator_rotate(struct wl_client *client,
> + struct wl_resource *resource,
> + const char *accelerometer_device,
> + int32_t transform)
> +{
> + struct rotator *rotator = wl_resource_get_user_data(resource);
> + int32_t current_width;
> + struct weston_output *output;
> + struct weston_mode *mode;
> +
> + wl_list_for_each(output, &rotator->ec->output_list, link) {
> + if (!output->accelerometer || strcmp(accelerometer_device, output->accelerometer) != 0)
> + continue;
> +
> + output->transform = transform;
Sanitize 'transform'?
> + output->dirty = 1;
> +
> + /* Only swap width and height when the aspect ratio changed. */
> + mode = output->current_mode;
> + if ((transform ^ output->transform) & 1) {
> + current_width = mode->width;
> + mode->width = mode->height;
> + mode->height = current_width;
> + weston_output_mode_set_native(output, mode, output->current_scale);
This is definitely the wrong way to do it.
We never fudge the modes. Instead, output size calculation takes
transform and scale into account.
Current mode is not the native mode, if a client has triggered a
temporary video mode switch. weston_output_mode_set_native() does not
change the current video mode if the current is set by
weston_output_mode_switch_to_temporary().
You are not supposed do a video mode switch at all. Your monitor got
rotated, you want to rotate the content, but the same video mode still
applies.
We have no ready-made interface for changing the output transform
on-the-fly. You need an advance patch adding that interface. I
suppose it could be implemented by following the
weston_output_mode_set_native() mechanics, but doing nothing with video
modes, not touching the scale, and changing only the transform.
When the transform changes, it needs to trigger:
- sending wl_output events to clients to inform about the new transform
- output resize, which leads to
- pointer (input device) clamp check and
- output re-layout
weston_mode_switch_finish() might be of an inspiration there.
Hopefully in the future we'd get the libweston output configuration API
in such a shape that one can use it to also change output configuration
on-the-fly rather than only through disable-configure-enable cycle.
Until then, you need your own function that just changes the transform
immediately.
> + }
> +
> + weston_output_damage(output);
> + }
> +}
> +
> +struct weston_rotator_interface rotator_implementation = {
> + rotator_rotate
> +};
> +
> +static void
> +bind_rotator(struct wl_client *client,
> + void *data, uint32_t version, uint32_t id)
> +{
> + struct rotator *rotator = data;
> + struct autorotator_process *process;
> + struct wl_resource *resource;
> + bool found;
> +
> + resource = wl_resource_create(client,
> + &weston_rotator_interface, 1, id);
> +
> + found = false;
> + wl_list_for_each(process, &rotator->process_list, link)
> + if (client == process->client)
> + found = true;
> + if (!found) {
> + wl_resource_post_error(resource,
> + WL_DISPLAY_ERROR_INVALID_OBJECT,
> + "rotator failed: permission denied");
> + return;
> + }
> +
> + wl_resource_set_implementation(resource, &rotator_implementation,
> + data, NULL);
> +}
> +
> +static void
> +rotator_sigchld(struct weston_process *process, int status)
> +{
> + struct autorotator_process *autorotator_process =
> + container_of(process, struct autorotator_process, process);
> +
> + autorotator_process->client = NULL;
> +}
> +
> +static void
> +rotator_launch_autorotator(struct rotator *rotator)
> +{
> + struct autorotator_process *process;
> + char *rotator_exe;
> + int ret;
> +
> + const char *old_device = getenv("WESTON_ACCELEROMETER");
> +
> + ret = asprintf(&rotator_exe, "%s/%s",
> + weston_config_get_libexec_dir(),
> + "weston-autorotator");
> + if (ret < 0) {
> + weston_log("Could not construct rotator path.\n");
> + return;
> + }
> +
> + wl_list_for_each(process, &rotator->process_list, link) {
> + setenv("WESTON_ACCELEROMETER", process->accelerometer, 1);
> + process->client = weston_client_launch(rotator->ec,
> + &process->process,
> + rotator_exe,
> + rotator_sigchld);
I would recommend weston_client_start(). It not only provides reports
of child process exiting, it also makes it less tempting to create
races between wl_client destruction and the sigchld handler being
called. Thankfully you are not using either really.
Would you want to restart the client if it died?
> + }
> + free(rotator_exe);
> +
> + if (old_device)
> + setenv("WESTON_ACCELEROMETER", old_device, 1);
> + else
> + unsetenv("WESTON_ACCELEROMETER");
> +}
> +
> +static void
> +rotator_destroy(struct wl_listener *listener, void *data)
> +{
> + struct weston_output *output;
> + struct autorotator_process *process, *next;
> + struct rotator *rotator =
> + container_of(listener, struct rotator, destroy_listener);
> +
> + wl_list_for_each(output, &rotator->ec->output_list, link)
> + free(output->accelerometer);
Overkill? You also added a free(output->accelerometer) to
weston_output_destroy().
> +
> + wl_list_for_each_safe(process, next, &rotator->process_list, link) {
What guarantees the clean-up of process->process?
Potential use-after-free?
> + free(process->accelerometer);
> + free(process);
> + }
> +
> + wl_global_destroy(rotator->global);
> + free(rotator);
> +}
> +
> +static bool
> +read_configuration(struct rotator *rotator)
> +{
> + const char *config_file;
> + struct weston_config *config;
> + struct weston_config_section *section = NULL;
> + struct weston_output *output;
> + struct autorotator_process *process;
> + const char *section_name;
> + char *output_name, *accelerometer_name;
> +
> + config_file = weston_config_get_name_from_env();
> + config = weston_config_parse(config_file);
Use wet_get_config() instead of loading from scratch.
> + if (config == NULL)
> + return false;
> +
> + while (weston_config_next_section(config, §ion, §ion_name)) {
> + if (strcmp(section_name, "output") == 0) {
> + weston_config_section_get_string(section, "name", &output_name, NULL);
> + if (output_name == NULL)
> + continue;
> + weston_config_section_get_string(section, "accelerometer", &accelerometer_name, NULL);
> + wl_list_for_each(output, &rotator->ec->output_list, link)
This is not the only output list anymore. The output configuration API
just recently merged changed things: a weston_output can now be either
pending (inactive) or in use. Actually there are three states:
- disconnected, there is no struct weston_output at all
- pending, connected but either not taken into use or disabled
- enabled, being in the output_list
Here you will only tag accelerators to outputs that have been enabled
before the module loads. There is
weston_compositor::output_created_signal where you would get a callback
every time an output gets enabled. Probably you should use these two
ways together.
Rather than storing the accelerometer name in weston_output, how about
storing a list of output names vs. accelerometer names privately? All
that rotator_rotate() needs to do is to find the right outputs.
Why did you choose to have an extra 'accelerometer' key in 'output'
section?
Would it not work just the same if an 'accelerometer' section
referenced any number of outputs by one or more, say, 'control_output'
keys?
Or, you could keep the weston.ini as you designed it, but internally
convert accelerator names into output name lists without storing
anything new in struct weston_output.
> + if (output->name && strcmp(output->name, output_name) == 0) {
> + output->accelerometer = accelerometer_name; //TODO: free at deletion?
Stale TODO.
> + break;
> + }
> + free(output_name);
> + } else if (strcmp(section_name, "accelerometer") == 0) {
> + weston_config_section_get_string(section, "name", &accelerometer_name, NULL);
> + if (accelerometer_name == NULL)
> + continue;
> + process = zalloc(sizeof *process);
> + if (process == NULL)
> + return false;
> + process->accelerometer = accelerometer_name;
> + wl_list_insert(&rotator->process_list, &process->link);
> + }
> + }
> +
> + weston_config_destroy(config);
> +
> + return true;
> +}
> +
> +WL_EXPORT int
> +module_init(struct weston_compositor *ec, int *argc, char *argv[])
> +{
> + struct rotator *rotator;
> +
> + rotator = zalloc(sizeof *rotator);
> + if (rotator == NULL)
> + return -1;
> +
> + rotator->ec = ec;
> + wl_list_init(&rotator->process_list);
> +
> + if (!read_configuration(rotator))
> + return -1;
> +
> + rotator->global = wl_global_create(ec->wl_display,
> + &weston_rotator_interface, 1,
> + rotator, bind_rotator);
> + rotator->destroy_listener.notify = rotator_destroy;
> + wl_signal_add(&ec->destroy_signal, &rotator->destroy_listener);
> +
> + rotator_launch_autorotator(rotator);
> +
> + return 0;
> +}
> diff --git a/configure.ac b/configure.ac
> index 74f931d..186fc25 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -379,6 +379,20 @@ if ! test "x$enable_simple_dmabuf_v4l_client" = "xno"; then
> fi
> AM_CONDITIONAL(BUILD_SIMPLE_DMABUF_V4L_CLIENT, test "x$enable_simple_dmabuf_v4l_client" = "xyes")
>
> +AC_ARG_ENABLE(weston-autorotator,
> + AS_HELP_STRING([--disable-weston-autorotator],
> + [do not build the autorotator client]),,
> + enable_autorotator="auto")
> +if ! test "x$enable_autorotator" = "xno"; then
> + PKG_CHECK_MODULES(LIBIIO, [libiio],
> + have_autorotator=yes, have_autorotator=no)
> + if test "x$have_autorotator" = "xno" -a "x$enable_autorotator" = "xyes"; then
> + AC_MSG_ERROR([autorotator client explicitly enabled, but libiio couldn't be found])
> + fi
> + enable_autorotator="$have_autorotator"
> +fi
> +AM_CONDITIONAL(BUILD_AUTOROTATOR, test "x$enable_autorotator" = "xyes")
> +
> AC_ARG_ENABLE(clients, [ --enable-clients],, enable_clients=yes)
> AM_CONDITIONAL(BUILD_CLIENTS, test x$enable_clients = xyes)
> if test x$enable_clients = xyes; then
> @@ -685,6 +699,7 @@ AC_MSG_RESULT([
> Build EGL Clients ${have_cairo_egl}
> Build Simple Clients ${enable_simple_clients}
> Build Simple EGL Clients ${enable_simple_egl_clients}
> + Build Auto-rotator Client ${enable_autorotator}
>
> Install Demo Clients ${enable_demo_clients_install}
>
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index b17c76d..75c2d2f 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4113,6 +4113,7 @@ weston_output_destroy(struct weston_output *output)
> wl_signal_emit(&output->destroy_signal, output);
>
> free(output->name);
> + free(output->accelerometer);
> pixman_region32_fini(&output->region);
> pixman_region32_fini(&output->previous_damage);
> output->compositor->output_id_pool &= ~(1u << output->id);
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index 0133084..9cf5c19 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -240,6 +240,7 @@ struct weston_output {
> uint16_t *b);
>
> struct weston_timeline_object timeline;
> + char *accelerometer;
> };
>
> enum weston_pointer_motion_mask {
> diff --git a/man/weston.ini.man b/man/weston.ini.man
> index 7aa7810..a527707 100644
> --- a/man/weston.ini.man
> +++ b/man/weston.ini.man
> @@ -412,6 +412,47 @@ multiheaded environment with a single compositor for multiple output and input
> configurations. The default seat is called "default" and will always be
> present. This seat can be constrained like any other.
> .RE
> +.TP 7
> +.BI "accelerometer=" device
> +The name of an accelerometer corresponding to an accelerometer section, which
> +will be used to make this particular output rotate.
> +.RE
> +.SH "ACCELEROMETER SECTION"
> +There can be multiple accelerometer sections, each corresponding to one
> +accelerometer device. Each one should be listed in at least one output section
> +in order to have any effect, to select which outputs it will rotate.
> +.TP 7
> +.BI "name=" device
> +defines the device that will be used for this accelerometer. Must be the same
> +string as the accelerometer field in an output section.
> +.RE
> +.TP 7
> +.BI "channel_x=" attribute
> +sets the iio channel that will be used as the horizontal value. By default,
> +this channel is named accel_x.
> +.RE
> +.TP 7
> +.BI "channel_y=" attribute
> +sets the iio channel that will be used as the vertical value. By default, this
> +channel is named accel_y.
> +.RE
> +.TP 7
> +.BI "sampling_frequency_attr=" attribute
> +sets the iio channel that will be used to set the desired sampling frequency.
> +By default, this channel is named accel_sampling_frequency.
> +.RE
> +.TP 7
> +.BI "sampling_frequency=" number
> +sets the target sampling frequency, for the sampling_frequency_attr. If there
> +is any error while setting it, the current value will be used instead.
> +.RE
> +.TP 7
> +.BI "allowed_rotations=" list
> +restricts the allowed rotation configurations, list can take up to four
> +space-separated values, selected from “normal”, “90”, “180” and “270”. Any
> +value not listed in list will disallow this rotation, and the absence of this
> +option will allow all four of them.
> +.RE
Somewhere you could also mention that one needs to load the module, too.
> .SH "INPUT-METHOD SECTION"
> .TP 7
> .BI "path=" "/usr/libexec/weston-keyboard"
> diff --git a/protocol/weston-rotator.xml b/protocol/weston-rotator.xml
> new file mode 100644
> index 0000000..37f027a
> --- /dev/null
> +++ b/protocol/weston-rotator.xml
> @@ -0,0 +1,26 @@
> +<protocol name="weston_rotator">
> +
License blurb? Though the protocol is kind of trivial.
> + <interface name="weston_rotator" version="1">
> +
> + <enum name="transform">
> + <description summary="copied from wl_output.transform"/>
> + <entry name="normal" value="0"/>
> + <entry name="90" value="1"/>
> + <entry name="180" value="2"/>
> + <entry name="270" value="3"/>
> + <entry name="flipped" value="4"/>
> + <entry name="flipped_90" value="5"/>
> + <entry name="flipped_180" value="6"/>
> + <entry name="flipped_270" value="7"/>
> + </enum>
I think it would be fine to just use the values defined for wl_output
directly rather than duplicate the list. This is very much tied to what
wl_output is tied to, too.
> +
> + <request name="rotate">
> + <arg name="accelerometer" type="string"/>
> + <arg name="transform" type="int" enum="transform"/>
> + </request>
> +
> + <event name="done"/>
I don't recall seeing this event ever being sent or handled.
> +
> + </interface>
> +
> +</protocol>
> diff --git a/weston.ini.in b/weston.ini.in
> index 14a4c0c..5e36e5f 100644
> --- a/weston.ini.in
> +++ b/weston.ini.in
> @@ -46,6 +46,7 @@ path=@libexecdir@/weston-keyboard
> #mode=1680x1050
> #transform=90
> #icc_profile=/usr/share/color/icc/colord/Bluish.icc
> +#accelerometer=iio:device0
>
> #[output]
> #name=VGA1
> @@ -65,6 +66,9 @@ path=@libexecdir@/weston-keyboard
> #min_accel_factor = 0.16
> #max_accel_factor = 1.0
>
> +#[accelerometer]
> +#device=iio:device0
Should this not be 'name'?
> +
> [screen-share]
> command=@bindir@/weston --backend=rdp-backend.so --shell=fullscreen-shell.so --no-clients-resize
>
Everything I didn't comment on seemed fine, possible style issues
notwithtanding.
Thanks,
pq
-------------- 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/20161010/e3a268c2/attachment-0001.sig>
More information about the wayland-devel
mailing list