Discussion:
[PATCH weston 00/25] A new touchscreen calibrator
Pekka Paalanen
2018-03-23 12:00:40 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Hi all,

the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.

The benefits of the new tool, which the old tool lacks, are:

- You can unambiguously pick a physical touch device to calibrate.

- You can be sure your touch events come only from that particular
device, and that you cannot miss touch events even if the current
calibration is horribly wrong.

- You can be sure the calibration window (pattern) is shown on the right
output with the right coordinates.

- You can unambiguously calibrate even multiple touchscreens that are
all cloned (showing the same image).

- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.

- You can load a new calibration into the compositor without playing
tricks with udev or restarting the compositor.

There is more discussion about the topic at:
https://phabricator.freedesktop.org/T7868

This patch series depends on the clone mode series:
https://patchwork.freedesktop.org/series/32898/

There is a full branch available at:
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1

This series contains many patches that could be landed separately, but I
have not pulled them out at this point.

A notable single patch is patch 3 which deprecates the udev property
WL_CALIBRATION. It is completely already superseded by
LIBINPUT_CALIBRATION_MATRIX property. I would hope to remove
WL_CALIBRATION support after few releases and the old calibrator tool
with it.

Patch 21 adds the touchscreen calibration protocol, patch 24 adds the
new calibrator tool, and patch 25 gives suggestions on how to let the
server permanently store a new calibration into udev rules.

As LIBINPUT_CALIBRATION_MATRIX is automatically handled by libinput,
being a description of the physical input device (like e.g. mouse dpi),
it perfectly possible to run Weston and the calibrator once to calibrate
your touchscreens, and then run whatever compositor you want.

One major thing this patch series does not address is how a Wayland
client gets authorized to use the touchscreen calibration interface. The
interface offers two privileged actions: the ability to grab all touch
input (but with provision for the server to cancel at will - not
implemented in weston so far), and the ability to upload a new
calibration for a touch device. Obviously these should not be free for
all. The method implemented here is a simple global configuration
setting to advertise or not the touchscreen calibration interface. If
advertised, it is free for all. Therefore it is not advertised by
default.


Thanks,
pq


Louis-Francis Ratté-Boulianne (7):
input: introduce weston_touch_device
libweston: fix weston_touch_start_grab() arg name
input: move touchpoint counting up
input: introduce touch event mode for calibrator
libweston: implement touch calibration protocol
weston: add touchscreen_calibrator option
clients: add a new touchscreen calibrator

Pekka Paalanen (18):
libinput: remove evdev_device::devnode
libinput: note if calibrating without an output
libinput: deprecate WL_CALIBRATION
libinput: log input device to output associations
libinput: make setting the same output a no-op
libinput: allow evdev_device_set_output(dev, NULL)
libinput: use head names for output matching
libweston: require connected heads for input devices
libinput: do not switch output associations on disable
man: document WESTON_LIBINPUT_LOG_PRIORITY env
tests: add test_seat_release() for symmetry
libinput: move calibration printing into do_set_calibration()
libweston: notify_touch API to use weston_touch_device
libweston: unexport weston_{pointer,keyboard,touch}_{create,destroy}()
libweston: introduce notify_touch_cal() and doc
input: do not forward unmatched touch-ups
protocol: add weston_touch_calibration
doc: add example calibration-helper script

.gitignore | 1 +
Makefile.am | 21 +-
clients/touch-calibrator.c | 774 ++++++++++++++++++++++++++++++++++
clients/window.c | 4 +-
clients/window.h | 4 +
compositor/main.c | 68 +++
doc/calibration-helper.bash | 44 ++
libweston/compositor-wayland.c | 31 +-
libweston/compositor.c | 32 ++
libweston/compositor.h | 174 +++++++-
libweston/input.c | 330 +++++++++++++--
libweston/libinput-device.c | 238 ++++++++---
libweston/libinput-device.h | 4 +-
libweston/libinput-seat.c | 88 +++-
libweston/libinput-seat.h | 1 +
libweston/touch-calibration.c | 667 +++++++++++++++++++++++++++++
man/weston-drm.man | 4 +
man/weston.ini.man | 36 ++
protocol/weston-touch-calibration.xml | 320 ++++++++++++++
tests/weston-test.c | 64 ++-
20 files changed, 2771 insertions(+), 134 deletions(-)
create mode 100644 clients/touch-calibrator.c
create mode 100755 doc/calibration-helper.bash
create mode 100644 libweston/touch-calibration.c
create mode 100644 protocol/weston-touch-calibration.xml
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:42 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Print a note that calibration got skipped if the input device supports a
calibration matrix but there is no associated output to compute it from.
Helps with debugging touchscreen calibration issues.

The code is reorganized and commented a bit, but this does not change
the behaviour.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 9b2577b6..48bfd6f1 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -463,21 +463,29 @@ evdev_device_set_calibration(struct evdev_device *device)
float calibration[6];
enum libinput_config_status status;

- if (!device->output)
+ if (!libinput_device_config_calibration_has_matrix(device->device))
+ return;
+
+ /* If LIBINPUT_CALIBRATION_MATRIX was set to non-identity, we will not
+ * override it with WL_CALIBRATION. It also means we don't need an
+ * output to load a calibration. */
+ if (libinput_device_config_calibration_get_default_matrix(
+ device->device,
+ calibration) != 0)
+ return;
+
+ if (!device->output) {
+ weston_log("input device %s has no enabled output associated "
+ "(%s named), skipping calibration for now.\n",
+ sysname, device->output_name ?: "none");
return;
+ }

width = device->output->width;
height = device->output->height;
if (width == 0 || height == 0)
return;

- /* If libinput has a pre-set calibration matrix, don't override it */
- if (!libinput_device_config_calibration_has_matrix(device->device) ||
- libinput_device_config_calibration_get_default_matrix(
- device->device,
- calibration) != 0)
- return;
-
udev = udev_new();
if (!udev)
return;
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:41 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Struct 'evdev_device' has field 'devnode' which is initialized to NULL,
never assigned, and finally free()'d. Therefore it is useless.

Remove the dead field.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 1 -
libweston/libinput-device.h | 1 -
2 files changed, 2 deletions(-)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 1b9fc9c8..9b2577b6 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -596,7 +596,6 @@ evdev_device_destroy(struct evdev_device *device)
wl_list_remove(&device->output_destroy_listener.link);
wl_list_remove(&device->link);
libinput_device_unref(device->device);
- free(device->devnode);
free(device->output_name);
free(device);
}
diff --git a/libweston/libinput-device.h b/libweston/libinput-device.h
index 5041a4aa..8e9f5608 100644
--- a/libweston/libinput-device.h
+++ b/libweston/libinput-device.h
@@ -47,7 +47,6 @@ struct evdev_device {
struct wl_list link;
struct weston_output *output;
struct wl_listener output_destroy_listener;
- char *devnode;
char *output_name;
int fd;
};
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:43 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

The udev property WL_CALIBRATION is an old way of giving Weston a
touchscreen calibration matrix. It is Weston-specific.

The recommended way of setting up a calibration is to use the udev
property LIBINPUT_CALIBRATION_MATRIX, which libinput will load
automatically and therefore applies to all libinput using display
servers and applications.

The syntax of WL_CALIBRATION and LIBINPUT_CALIBRATION_MATRIX is
different as well: WL_CALIBRATION uses pixels as the translation part
units, which makes the values depend on the output resolution.
LIBINPUT_CALIBRATION_MATRIX on the other hand uses normalized units.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 48bfd6f1..64d99f75 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -500,6 +500,13 @@ evdev_device_set_calibration(struct evdev_device *device)
udev_device_get_property_value(udev_device,
"WL_CALIBRATION");

+ if (calibration_values) {
+ weston_log("Warning: input device %s has WL_CALIBRATION property set. "
+ "Support for it will be removed in the future. "
+ "Please use LIBINPUT_CALIBRATION_MATRIX instead.\n",
+ sysname);
+ }
+
if (!calibration_values || sscanf(calibration_values,
"%f %f %f %f %f %f",
&calibration[0],
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:46 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Removing the output association from an evdev_device is more than just
setting the pointer to NULL, one also needs to remove the destroy
listener and flag the destroy listener as unused (notify == NULL).

evdev_device_set_output() can already remove associations, so let it
also handle an assignment to NULL output.

Fix notify_output_destroy() to handle removing an association correctly.
Previously, the listener was left "used", which would mean the next call
to evdev_device_set_output() would have wl_list_remove()'d, accessing
freed memory. This could be triggered by having a touchscreen with a
specified output association, and unplugging then re-plugging the
corresponding output.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 62f4cee8..dbbaae32 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -442,7 +442,7 @@ notify_output_destroy(struct wl_listener *listener, void *data)
struct weston_output, link);
evdev_device_set_output(device, output);
} else {
- device->output = NULL;
+ evdev_device_set_output(device, NULL);
}
}

@@ -557,6 +557,14 @@ evdev_device_set_output(struct evdev_device *device,
device->output_destroy_listener.notify = NULL;
}

+ if (!output) {
+ weston_log("output for input device %s removed\n",
+ libinput_device_get_sysname(device->device));
+
+ device->output = NULL;
+ return;
+ }
+
weston_log("associating input device %s with output %s "
"(%s by udev)\n",
libinput_device_get_sysname(device->device),
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:47 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Associating input devices with weston_outputs by the output name fails
when one output has several heads. We need to match against head names
instead of output names to be able to find all names.

This fixes touchscreen output association in shared-CRTC clone mode when
outputs or input devices appear or disappear.

Even though notify_output_create() is called only when new outputs
appear, the implementation is prepared to also remove output
associations. This will be handy in the future when this function will
handle also head detaching from an output that remains enabled.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-seat.c | 51 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/libweston/libinput-seat.c b/libweston/libinput-seat.c
index 953f6205..3cece3a1 100644
--- a/libweston/libinput-seat.c
+++ b/libweston/libinput-seat.c
@@ -58,6 +58,27 @@ get_udev_seat(struct udev_input *input, struct libinput_device *device)
return udev_seat_get_named(input, seat_name);
}

+static struct weston_output *
+output_find_by_head_name(struct weston_compositor *compositor,
+ const char *head_name)
+{
+ struct weston_output *output;
+ struct weston_head *head;
+
+ if (!head_name)
+ return NULL;
+
+ /* only enabled outputs */
+ wl_list_for_each(output, &compositor->output_list, link) {
+ wl_list_for_each(head, &output->head_list, output_link) {
+ if (strcmp(head_name, head->name) == 0)
+ return output;
+ }
+ }
+
+ return NULL;
+}
+
static void
device_added(struct udev_input *input, struct libinput_device *libinput_device)
{
@@ -95,11 +116,10 @@ device_added(struct udev_input *input, struct libinput_device *libinput_device)
output_name = libinput_device_get_output_name(libinput_device);
if (output_name) {
device->output_name = strdup(output_name);
- wl_list_for_each(output, &c->output_list, link)
- if (output->name &&
- strcmp(output->name, device->output_name) == 0)
- evdev_device_set_output(device, output);
- } else if (device->output == NULL && !wl_list_empty(&c->output_list)) {
+ output = output_find_by_head_name(c, output_name);
+ evdev_device_set_output(device, output);
+ } else if (!wl_list_empty(&c->output_list)) {
+ /* default assignment to an arbitrary output */
output = container_of(c->output_list.next,
struct weston_output, link);
evdev_device_set_output(device, output);
@@ -363,15 +383,26 @@ notify_output_create(struct wl_listener *listener, void *data)
output_create_listener);
struct evdev_device *device;
struct weston_output *output = data;
+ struct weston_output *found;

wl_list_for_each(device, &seat->devices_list, link) {
- if (device->output_name &&
- strcmp(output->name, device->output_name) == 0) {
- evdev_device_set_output(device, output);
+ /* If we find any input device without an associated output
+ * or an output name to associate with, just tie it with the
+ * output we got here - the default assingment.
+ */
+ if (!device->output_name) {
+ if (!device->output)
+ evdev_device_set_output(device, output);
+
+ continue;
}

- if (device->output_name == NULL && device->output == NULL)
- evdev_device_set_output(device, output);
+ /* Update all devices' output associations, may they gain or
+ * lose it.
+ */
+ found = output_find_by_head_name(output->compositor,
+ device->output_name);
+ evdev_device_set_output(device, found);
}
}
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:50 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Commit c81c4241d9c9fc5f60c08177dd8a33ae4e4ddca1 added this environment
variable. Document it.

It applies also to the fbdev-backend but that has no man page.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
man/weston-drm.man | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/man/weston-drm.man b/man/weston-drm.man
index 257237df..97f55e50 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -125,6 +125,10 @@ instead of using the current tty.
.SH ENVIRONMENT
.
.TP
+.B WESTON_LIBINPUT_LOG_PRIORITY
+Libinput verbosity level as appearing in Weston's log. Valid values are
+.BR debug ", " info ", and " error ". Default is " info .
+.TP
.B WESTON_TTY_FD
The file descriptor (integer) of the opened tty where
.B weston
--
2.16.1
Peter Hutterer
2018-04-05 05:59:18 UTC
Permalink
Post by Pekka Paalanen
Commit c81c4241d9c9fc5f60c08177dd8a33ae4e4ddca1 added this environment
variable. Document it.
It applies also to the fbdev-backend but that has no man page.
---
man/weston-drm.man | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/man/weston-drm.man b/man/weston-drm.man
index 257237df..97f55e50 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -125,6 +125,10 @@ instead of using the current tty.
.SH ENVIRONMENT
.
.TP
+.B WESTON_LIBINPUT_LOG_PRIORITY
+Libinput verbosity level as appearing in Weston's log. Valid values are
+.BR debug ", " info ", and " error ". Default is " info .
not that I care that much, but Libinput looks weird - should be 'libinput',
i.e. lowercase.

Cheers,
Peter
Post by Pekka Paalanen
+.TP
.B WESTON_TTY_FD
The file descriptor (integer) of the opened tty where
.B weston
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-04-05 13:53:53 UTC
Permalink
On Thu, 5 Apr 2018 15:59:18 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Commit c81c4241d9c9fc5f60c08177dd8a33ae4e4ddca1 added this environment
variable. Document it.
It applies also to the fbdev-backend but that has no man page.
---
man/weston-drm.man | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/man/weston-drm.man b/man/weston-drm.man
index 257237df..97f55e50 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -125,6 +125,10 @@ instead of using the current tty.
.SH ENVIRONMENT
.
.TP
+.B WESTON_LIBINPUT_LOG_PRIORITY
+Libinput verbosity level as appearing in Weston's log. Valid values are
+.BR debug ", " info ", and " error ". Default is " info .
not that I care that much, but Libinput looks weird - should be 'libinput',
i.e. lowercase.
It's the first word in a sentence, so my brain says it needs to be
capitalized. I also try very very hard to never start a sentence with a
function name or such because my OCD would explode on the conflict. :-)


Thanks,
pq
Post by Peter Hutterer
Post by Pekka Paalanen
+.TP
.B WESTON_TTY_FD
The file descriptor (integer) of the opened tty where
.B weston
--
2.16.1
Peter Hutterer
2018-04-05 22:37:41 UTC
Permalink
Post by Pekka Paalanen
On Thu, 5 Apr 2018 15:59:18 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Commit c81c4241d9c9fc5f60c08177dd8a33ae4e4ddca1 added this environment
variable. Document it.
It applies also to the fbdev-backend but that has no man page.
---
man/weston-drm.man | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/man/weston-drm.man b/man/weston-drm.man
index 257237df..97f55e50 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -125,6 +125,10 @@ instead of using the current tty.
.SH ENVIRONMENT
.
.TP
+.B WESTON_LIBINPUT_LOG_PRIORITY
+Libinput verbosity level as appearing in Weston's log. Valid values are
+.BR debug ", " info ", and " error ". Default is " info .
not that I care that much, but Libinput looks weird - should be 'libinput',
i.e. lowercase.
It's the first word in a sentence, so my brain says it needs to be
capitalized. I also try very very hard to never start a sentence with a
function name or such because my OCD would explode on the conflict. :-)
sort-of. That's not the case for proper nouns (i.e. names) of which libinput
is arguably one. The same would be true for e.g. systemd and almost all unix
commands. e.g. you wouldn't start a sentence like "Ls shows you the list of
files"

Reading this again, it's grammatically a tad weird anyway, reword to "The
minimum libinput verbosity level to be printed to Weston's log", it removes
some ambiguity and is OCD-compatible :)

Cheers,
Peter
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+.TP
.B WESTON_TTY_FD
The file descriptor (integer) of the opened tty where
.B weston
--
2.16.1
Pekka Paalanen
2018-04-06 07:43:55 UTC
Permalink
On Fri, 6 Apr 2018 08:37:41 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
On Thu, 5 Apr 2018 15:59:18 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Commit c81c4241d9c9fc5f60c08177dd8a33ae4e4ddca1 added this environment
variable. Document it.
It applies also to the fbdev-backend but that has no man page.
---
man/weston-drm.man | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/man/weston-drm.man b/man/weston-drm.man
index 257237df..97f55e50 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -125,6 +125,10 @@ instead of using the current tty.
.SH ENVIRONMENT
.
.TP
+.B WESTON_LIBINPUT_LOG_PRIORITY
+Libinput verbosity level as appearing in Weston's log. Valid values are
+.BR debug ", " info ", and " error ". Default is " info .
not that I care that much, but Libinput looks weird - should be 'libinput',
i.e. lowercase.
It's the first word in a sentence, so my brain says it needs to be
capitalized. I also try very very hard to never start a sentence with a
function name or such because my OCD would explode on the conflict. :-)
sort-of. That's not the case for proper nouns (i.e. names) of which libinput
is arguably one. The same would be true for e.g. systemd and almost all unix
commands. e.g. you wouldn't start a sentence like "Ls shows you the list of
files"
Reading this again, it's grammatically a tad weird anyway, reword to "The
minimum libinput verbosity level to be printed to Weston's log", it removes
some ambiguity and is OCD-compatible :)
Yeah, much better!


Thanks,
pq
Pekka Paalanen
2018-03-23 12:00:49 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

If an input device is associated to an output that then gets disabled,
there is no case where associating to a different output would be
correct.

The output association is used for absolute positioned input devices,
and an input device like a touchscreen cannot ever be automatically
valid for more than one possible output - the touchscreen display
device.

Therefore do not automatically reassing implicitly associated input
devices to another output. This removes some log spam on shutdown.

In fact, if there can be more than one output at any time, absolute
input devices must be explicitly configured to associate with the
correct output, or the results are essentially undefined in any case.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index dbbaae32..e1738613 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -434,16 +434,8 @@ notify_output_destroy(struct wl_listener *listener, void *data)
struct evdev_device *device =
container_of(listener,
struct evdev_device, output_destroy_listener);
- struct weston_compositor *c = device->seat->compositor;
- struct weston_output *output;
-
- if (!device->output_name && !wl_list_empty(&c->output_list)) {
- output = container_of(c->output_list.next,
- struct weston_output, link);
- evdev_device_set_output(device, output);
- } else {
- evdev_device_set_output(device, NULL);
- }
+
+ evdev_device_set_output(device, NULL);
}

/**
--
2.16.1
Peter Hutterer
2018-04-05 06:10:53 UTC
Permalink
Post by Pekka Paalanen
If an input device is associated to an output that then gets disabled,
there is no case where associating to a different output would be
correct.
just a general note here, especially in light of Jason's comments:
this statement above isn't true for external tablet devices like
the Wacom Intuos series (the ones *not* integrated into a touchscreen).
There are workflows where the device is used in absolute mode and mapped to
a specific monitor even though it's external like a touchpad. In fact, IMO
it's the more sane mode for these devices as the only other alternative is
to have the tablet span all outputs, making everything a bit weird.

With those devices, unplugging an output should thus map the tablet to the
remaining one.

It's not yet something weston needs to worry about given the other things
that you need in place for this to work (specifically: tablet area ratio to
monitor ratio). But keep this filed away in the back of your head anyway.

Cheers,
Peter
Post by Pekka Paalanen
The output association is used for absolute positioned input devices,
and an input device like a touchscreen cannot ever be automatically
valid for more than one possible output - the touchscreen display
device.
Therefore do not automatically reassing implicitly associated input
devices to another output. This removes some log spam on shutdown.
In fact, if there can be more than one output at any time, absolute
input devices must be explicitly configured to associate with the
correct output, or the results are essentially undefined in any case.
---
libweston/libinput-device.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index dbbaae32..e1738613 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -434,16 +434,8 @@ notify_output_destroy(struct wl_listener *listener, void *data)
struct evdev_device *device =
container_of(listener,
struct evdev_device, output_destroy_listener);
- struct weston_compositor *c = device->seat->compositor;
- struct weston_output *output;
-
- if (!device->output_name && !wl_list_empty(&c->output_list)) {
- output = container_of(c->output_list.next,
- struct weston_output, link);
- evdev_device_set_output(device, output);
- } else {
- evdev_device_set_output(device, NULL);
- }
+
+ evdev_device_set_output(device, NULL);
}
/**
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-04-05 14:15:44 UTC
Permalink
On Thu, 5 Apr 2018 16:10:53 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
If an input device is associated to an output that then gets disabled,
there is no case where associating to a different output would be
correct.
this statement above isn't true for external tablet devices like
the Wacom Intuos series (the ones *not* integrated into a touchscreen).
There are workflows where the device is used in absolute mode and mapped to
a specific monitor even though it's external like a touchpad. In fact, IMO
it's the more sane mode for these devices as the only other alternative is
to have the tablet span all outputs, making everything a bit weird.
With those devices, unplugging an output should thus map the tablet to the
remaining one.
It's not yet something weston needs to worry about given the other things
that you need in place for this to work (specifically: tablet area ratio to
monitor ratio). But keep this filed away in the back of your head anyway.
Hi,

ok, good to know. So while this patch is correct for actual
touchscreens, it's not correct for other kinds of devices. We'll need
to do it by device type. Given only touchscreens are supported for now,
this patch has the correct behaviour.

The previous patch "libweston: require connected heads for input
devices" should have the same effect and the same issue. By your
rationale, unplugging a force-enabled output should lead to the tablet
device jumping onto another output rather than being disabled.


Thanks,
pq
Post by Peter Hutterer
Cheers,
Peter
Post by Pekka Paalanen
The output association is used for absolute positioned input devices,
and an input device like a touchscreen cannot ever be automatically
valid for more than one possible output - the touchscreen display
device.
Therefore do not automatically reassing implicitly associated input
devices to another output. This removes some log spam on shutdown.
In fact, if there can be more than one output at any time, absolute
input devices must be explicitly configured to associate with the
correct output, or the results are essentially undefined in any case.
---
libweston/libinput-device.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index dbbaae32..e1738613 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -434,16 +434,8 @@ notify_output_destroy(struct wl_listener *listener, void *data)
struct evdev_device *device =
container_of(listener,
struct evdev_device, output_destroy_listener);
- struct weston_compositor *c = device->seat->compositor;
- struct weston_output *output;
-
- if (!device->output_name && !wl_list_empty(&c->output_list)) {
- output = container_of(c->output_list.next,
- struct weston_output, link);
- evdev_device_set_output(device, output);
- } else {
- evdev_device_set_output(device, NULL);
- }
+
+ evdev_device_set_output(device, NULL);
}
/**
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:48 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

The use case driving this change is a clone mode setup, where the user
is hotplugging or unplugging a cloned touchscreen. Even if the output
and head are force-enabled, the touch device should still follow the
connector connection status. If there is no video signal for the
touchscreen (disconnected connector), then the touch input should be
ignored as well.

When the output is force-enabled, we need to trigger
output_heads_changed from connector status changes. If the head or
output are not force-enabled, the compositor will likely attach and
detach the head as appropriate. In clone mode, the attach or detach
needs to trigger output_heads_changed directly. In other cases, it may
be handled through the output getting enabled or disabled which are
different signals.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor.c | 30 ++++++++++++++++++++++++++++++
libweston/compositor.h | 1 +
libweston/libinput-seat.c | 39 ++++++++++++++++++++++++++++++++++-----
libweston/libinput-seat.h | 1 +
4 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 362074b6..8452c5d2 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -4454,11 +4454,31 @@ weston_head_init(struct weston_head *head, const char *name)
head->name = strdup(name);
}

+/** Send output heads changed signal
+ *
+ * \param output The output that changed.
+ *
+ * Notify that the enabled output gained and/or lost heads, or that the
+ * associated heads may have changed their connection status. This does not
+ * include cases where the output becomes enabled or disabled. The registered
+ * callbacks are called after the change has successfully happened.
+ *
+ * If connection status change causes the compositor to attach or detach a head
+ * to an enabled output, the registered callbacks may be called multiple times.
+ */
+static void
+weston_output_emit_heads_changed(struct weston_output *output)
+{
+ wl_signal_emit(&output->compositor->output_heads_changed_signal,
+ output);
+}
+
/** Idle task for calling heads_changed callback */
static void
weston_compositor_call_heads_changed(void *data)
{
struct weston_compositor *compositor = data;
+ struct weston_head *head;

compositor->heads_changed_source = NULL;

@@ -4466,6 +4486,11 @@ weston_compositor_call_heads_changed(void *data)
return;

compositor->heads_changed(compositor);
+
+ wl_list_for_each(head, &compositor->head_list, compositor_link) {
+ if (head->output && head->output->enabled)
+ weston_output_emit_heads_changed(head->output);
+ }
}

/** Schedule a call on idle to heads_changed callback
@@ -4672,6 +4697,8 @@ weston_output_attach_head(struct weston_output *output,
weston_log("Output '%s' updated to have head(s) %s\n",
output->name, head_names);
free(head_names);
+
+ weston_output_emit_heads_changed(output);
}

return 0;
@@ -4717,6 +4744,8 @@ weston_head_detach(struct weston_head *head)
weston_log("Output '%s' updated to have head(s) %s\n",
output->name, head_names);
free(head_names);
+
+ weston_output_emit_heads_changed(output);
}
}
}
@@ -6251,6 +6280,7 @@ weston_compositor_create(struct wl_display *display, void *user_data)
wl_signal_init(&ec->output_destroyed_signal);
wl_signal_init(&ec->output_moved_signal);
wl_signal_init(&ec->output_resized_signal);
+ wl_signal_init(&ec->output_heads_changed_signal);
wl_signal_init(&ec->session_signal);
ec->session_active = 1;

diff --git a/libweston/compositor.h b/libweston/compositor.h
index 4e69f932..675ee68f 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -964,6 +964,7 @@ struct weston_compositor {
struct wl_signal output_destroyed_signal;
struct wl_signal output_moved_signal;
struct wl_signal output_resized_signal; /* callback argument: resized output */
+ struct wl_signal output_heads_changed_signal; /* arg: weston_output */

struct wl_signal session_signal;
int session_active;
diff --git a/libweston/libinput-seat.c b/libweston/libinput-seat.c
index 3cece3a1..ac1e8e99 100644
--- a/libweston/libinput-seat.c
+++ b/libweston/libinput-seat.c
@@ -68,9 +68,16 @@ output_find_by_head_name(struct weston_compositor *compositor,
if (!head_name)
return NULL;

- /* only enabled outputs */
+ /* Only enabled outputs with connected heads.
+ * This means force-enabled outputs but with disconnected heads
+ * will be ignored; if the touchscreen doesn't have a video signal,
+ * touching it is meaningless.
+ */
wl_list_for_each(output, &compositor->output_list, link) {
wl_list_for_each(head, &output->head_list, output_link) {
+ if (!weston_head_is_connected(head))
+ continue;
+
if (strcmp(head_name, head->name) == 0)
return output;
}
@@ -377,12 +384,9 @@ udev_seat_led_update(struct weston_seat *seat_base, enum weston_led leds)
}

static void
-notify_output_create(struct wl_listener *listener, void *data)
+udev_seat_output_changed(struct udev_seat *seat, struct weston_output *output)
{
- struct udev_seat *seat = container_of(listener, struct udev_seat,
- output_create_listener);
struct evdev_device *device;
- struct weston_output *output = data;
struct weston_output *found;

wl_list_for_each(device, &seat->devices_list, link) {
@@ -406,6 +410,26 @@ notify_output_create(struct wl_listener *listener, void *data)
}
}

+static void
+notify_output_create(struct wl_listener *listener, void *data)
+{
+ struct udev_seat *seat = container_of(listener, struct udev_seat,
+ output_create_listener);
+ struct weston_output *output = data;
+
+ udev_seat_output_changed(seat, output);
+}
+
+static void
+notify_output_heads_changed(struct wl_listener *listener, void *data)
+{
+ struct udev_seat *seat = container_of(listener, struct udev_seat,
+ output_heads_listener);
+ struct weston_output *output = data;
+
+ udev_seat_output_changed(seat, output);
+}
+
static struct udev_seat *
udev_seat_create(struct udev_input *input, const char *seat_name)
{
@@ -423,6 +447,10 @@ udev_seat_create(struct udev_input *input, const char *seat_name)
wl_signal_add(&c->output_created_signal,
&seat->output_create_listener);

+ seat->output_heads_listener.notify = notify_output_heads_changed;
+ wl_signal_add(&c->output_heads_changed_signal,
+ &seat->output_heads_listener);
+
wl_list_init(&seat->devices_list);

return seat;
@@ -440,6 +468,7 @@ udev_seat_destroy(struct udev_seat *seat)
udev_seat_remove_devices(seat);
weston_seat_release(&seat->base);
wl_list_remove(&seat->output_create_listener.link);
+ wl_list_remove(&seat->output_heads_listener.link);
free(seat);
}

diff --git a/libweston/libinput-seat.h b/libweston/libinput-seat.h
index 65c9b64b..8c6a5bf7 100644
--- a/libweston/libinput-seat.h
+++ b/libweston/libinput-seat.h
@@ -39,6 +39,7 @@ struct udev_seat {
struct weston_seat base;
struct wl_list devices_list;
struct wl_listener output_create_listener;
+ struct wl_listener output_heads_listener;
};

typedef void (*udev_configure_device_t)(struct weston_compositor *compositor,
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:51 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Add test_seat_release() as the counterpart of test_seat_init() instead
of open-coding it. This helps adding more code to test_seat_release()
later.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
tests/weston-test.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/weston-test.c b/tests/weston-test.c
index 73409cac..ae08b02f 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -80,6 +80,9 @@ test_client_sigchld(struct weston_process *process, int status)
static int
test_seat_init(struct weston_test *test)
{
+ assert(!test->is_seat_initialized &&
+ "Trying to add already added test seat");
+
/* create our own seat */
weston_seat_init(&test->seat, test->compositor, "test-seat");
test->is_seat_initialized = true;
@@ -93,6 +96,16 @@ test_seat_init(struct weston_test *test)
return 0;
}

+static void
+test_seat_release(struct weston_test *test)
+{
+ assert(test->is_seat_initialized &&
+ "Trying to release already released test seat");
+ test->is_seat_initialized = false;
+ weston_seat_release(&test->seat);
+ memset(&test->seat, 0, sizeof test->seat);
+}
+
static struct weston_seat *
get_seat(struct weston_test *test)
{
@@ -270,10 +283,7 @@ device_release(struct wl_client *client,
} else if (strcmp(device, "touch") == 0) {
weston_seat_release_touch(seat);
} else if (strcmp(device, "seat") == 0) {
- assert(test->is_seat_initialized &&
- "Trying to release already released test seat");
- weston_seat_release(seat);
- test->is_seat_initialized = false;
+ test_seat_release(test);
} else {
assert(0 && "Unsupported device");
}
@@ -293,8 +303,6 @@ device_add(struct wl_client *client,
} else if (strcmp(device, "touch") == 0) {
weston_seat_init_touch(seat);
} else if (strcmp(device, "seat") == 0) {
- assert(!test->is_seat_initialized &&
- "Trying to add already added test seat");
test_seat_init(test);
} else {
assert(0 && "Unsupported device");
--
2.16.1
Peter Hutterer
2018-04-05 06:26:58 UTC
Permalink
Post by Pekka Paalanen
Add test_seat_release() as the counterpart of test_seat_init() instead
of open-coding it. This helps adding more code to test_seat_release()
later.
Patches 01 to 11 are Reviewed-by: Peter Hutterer <***@who-t.net>
still working on the rest

Cheers,
Peter
Post by Pekka Paalanen
---
tests/weston-test.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/tests/weston-test.c b/tests/weston-test.c
index 73409cac..ae08b02f 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -80,6 +80,9 @@ test_client_sigchld(struct weston_process *process, int status)
static int
test_seat_init(struct weston_test *test)
{
+ assert(!test->is_seat_initialized &&
+ "Trying to add already added test seat");
+
/* create our own seat */
weston_seat_init(&test->seat, test->compositor, "test-seat");
test->is_seat_initialized = true;
@@ -93,6 +96,16 @@ test_seat_init(struct weston_test *test)
return 0;
}
+static void
+test_seat_release(struct weston_test *test)
+{
+ assert(test->is_seat_initialized &&
+ "Trying to release already released test seat");
+ test->is_seat_initialized = false;
+ weston_seat_release(&test->seat);
+ memset(&test->seat, 0, sizeof test->seat);
+}
+
static struct weston_seat *
get_seat(struct weston_test *test)
{
@@ -270,10 +283,7 @@ device_release(struct wl_client *client,
} else if (strcmp(device, "touch") == 0) {
weston_seat_release_touch(seat);
} else if (strcmp(device, "seat") == 0) {
- assert(test->is_seat_initialized &&
- "Trying to release already released test seat");
- weston_seat_release(seat);
- test->is_seat_initialized = false;
+ test_seat_release(test);
} else {
assert(0 && "Unsupported device");
}
@@ -293,8 +303,6 @@ device_add(struct wl_client *client,
} else if (strcmp(device, "touch") == 0) {
weston_seat_init_touch(seat);
} else if (strcmp(device, "seat") == 0) {
- assert(!test->is_seat_initialized &&
- "Trying to add already added test seat");
test_seat_init(test);
} else {
assert(0 && "Unsupported device");
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-04-20 12:40:00 UTC
Permalink
On Thu, 5 Apr 2018 16:26:58 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Add test_seat_release() as the counterpart of test_seat_init() instead
of open-coding it. This helps adding more code to test_seat_release()
later.
still working on the rest
Hi Peter,

I fixed the WESTON_LIBINPUT_LOG_PRIORITY as you suggested, and pushed
patches 1-11:
6fae2be9..f39249ad master -> master


Thanks,
pq
Pekka Paalanen
2018-03-23 12:00:52 UTC
Permalink
From: Louis-Francis Ratté-Boulianne <***@collabora.com>

Introduce weston_touch_device for libweston core to track individual
touchscreen input devices. A weston_seat/weston_touch may be an
aggregation of several physical touchscreen input devices. Separating
the physical devices will be required for implementing touchscreen
calibration. One can only calibrate one device at a time, and we want to
make sure to handle the right one.

Both backends that support touch devices are updated to create
weston_touch_devices. Wayland-backend provides touch devices that cannot
be calibrated, because we have no access to raw touch coordinates from
the device - calibration is the responsibility of the parent display
server. Libinput backend provides touch devices that can be calibrated,
hence implementing the set and get calibration hooks.

Backends need to maintain an output pointer in any case, so we have a
get_output() hook instead of having to maintain an identical field in
weston_touch_device. The same justification applies to
get_calibration_head_name.

Also update the test plugin to manage weston_touch_device objects.

Co-developed by Louis-Francis and Pekka.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor-wayland.c | 21 ++++++++
libweston/compositor.h | 56 ++++++++++++++++++++
libweston/input.c | 65 +++++++++++++++++++++++
libweston/libinput-device.c | 116 ++++++++++++++++++++++++++++++++++++++++-
libweston/libinput-device.h | 3 ++
tests/weston-test.c | 38 ++++++++++++++
6 files changed, 298 insertions(+), 1 deletion(-)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index f186681c..9d5b3551 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -198,6 +198,8 @@ struct wayland_input {
} cursor;
} parent;

+ struct weston_touch_device *touch_device;
+
enum weston_key_state_update keyboard_state_update;
uint32_t key_serial;
uint32_t enter_serial;
@@ -2246,6 +2248,22 @@ static const struct wl_touch_listener touch_listener = {
};


+static struct weston_touch_device *
+create_touch_device(struct wayland_input *input)
+{
+ struct weston_touch_device *touch_device;
+ char str[128];
+
+ /* manufacture a unique'ish name */
+ snprintf(str, sizeof str, "wayland-touch[%u]",
+ wl_proxy_get_id((struct wl_proxy *)input->parent.seat));
+
+ touch_device = weston_touch_create_touch_device(input->base.touch_state,
+ str, NULL, NULL);
+
+ return touch_device;
+}
+
static void
input_handle_capabilities(void *data, struct wl_seat *seat,
enum wl_seat_capability caps)
@@ -2287,7 +2305,10 @@ input_handle_capabilities(void *data, struct wl_seat *seat,
wl_touch_add_listener(input->parent.touch,
&touch_listener, input);
weston_seat_init_touch(&input->base);
+ input->touch_device = create_touch_device(input);
} else if (!(caps & WL_SEAT_CAPABILITY_TOUCH) && input->parent.touch) {
+ weston_touch_device_destroy(input->touch_device);
+ input->touch_device = NULL;
if (input->seat_version >= WL_TOUCH_RELEASE_SINCE_VERSION)
wl_touch_release(input->parent.touch);
else
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 675ee68f..b08031ac 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -453,10 +453,54 @@ struct weston_pointer {
struct wl_list timestamps_list;
};

+/** libinput style calibration matrix
+ *
+ * See https://wayland.freedesktop.org/libinput/doc/latest/absolute_axes.html
+ * and libinput_device_config_calibration_set_matrix().
+ */
+struct weston_touch_device_matrix {
+ float m[6];
+};
+
+struct weston_touch_device;
+
+/** Operations for a calibratable touchscreen */
+struct weston_touch_device_ops {
+ /** Get the associated output if existing. */
+ struct weston_output *(*get_output)(struct weston_touch_device *device);
+
+ /** Get the name of the associated head if existing. */
+ const char *
+ (*get_calibration_head_name)(struct weston_touch_device *device);
+
+ /** Retrieve the current calibration matrix. */
+ void (*get_calibration)(struct weston_touch_device *device,
+ struct weston_touch_device_matrix *matrix);
+
+ /** Set a new calibration matrix. */
+ void (*set_calibration)(struct weston_touch_device *device,
+ const struct weston_touch_device_matrix *matrix);
+};
+
+/** Represents a physical touchscreen input device */
+struct weston_touch_device {
+ char *devpath; /**< unique name */

+ struct weston_touch *aggregate; /**< weston_touch this is part of */
+ struct wl_list link; /**< in weston_touch::device_list */
+ struct wl_signal destroy_signal; /**< destroy notifier */
+
+ void *backend_data; /**< backend-specific private */
+
+ const struct weston_touch_device_ops *ops;
+};
+
+/** Represents a set of touchscreen devices aggregated under a seat */
struct weston_touch {
struct weston_seat *seat;

+ struct wl_list device_list; /* struct weston_touch_device::link */
+
struct wl_list resource_list;
struct wl_list focus_resource_list;
struct weston_view *focus;
@@ -590,6 +634,18 @@ weston_touch_send_motion(struct weston_touch *touch,
void
weston_touch_send_frame(struct weston_touch *touch);

+struct weston_touch_device *
+weston_touch_create_touch_device(struct weston_touch *touch,
+ const char *devpath,
+ void *backend_data,
+ const struct weston_touch_device_ops *ops);
+
+void
+weston_touch_device_destroy(struct weston_touch_device *device);
+
+bool
+weston_touch_device_can_calibrate(struct weston_touch_device *device);
+
void
wl_data_device_set_keyboard_focus(struct weston_seat *seat);

diff --git a/libweston/input.c b/libweston/input.c
index 3e91c266..feea9e72 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -123,6 +123,68 @@ remove_input_resource_from_timestamps(struct wl_resource *input_resource,
}
}

+/** Register a touchscreen input device
+ *
+ * \param touch The parent weston_touch that identifies the seat.
+ * \param devpath Unique device name.
+ * \param backend_data Backend private data if necessary.
+ * \param ops Calibration operations, or NULL for not able to run calibration.
+ * \return New touch device, or NULL on failure.
+ */
+WL_EXPORT struct weston_touch_device *
+weston_touch_create_touch_device(struct weston_touch *touch,
+ const char *devpath,
+ void *backend_data,
+ const struct weston_touch_device_ops *ops)
+{
+ struct weston_touch_device *device;
+
+ assert(devpath);
+ if (ops) {
+ assert(ops->get_output);
+ assert(ops->get_calibration_head_name);
+ assert(ops->get_calibration);
+ assert(ops->set_calibration);
+ }
+
+ device = zalloc(sizeof *device);
+ if (!device)
+ return NULL;
+
+ wl_signal_init(&device->destroy_signal);
+
+ device->devpath = strdup(devpath);
+ if (!device->devpath) {
+ free(device);
+ return NULL;
+ }
+
+ device->backend_data = backend_data;
+ device->ops = ops;
+
+ device->aggregate = touch;
+ wl_list_insert(touch->device_list.prev, &device->link);
+
+ return device;
+}
+
+/** Destroy the touch device. */
+WL_EXPORT void
+weston_touch_device_destroy(struct weston_touch_device *device)
+{
+ wl_list_remove(&device->link);
+ wl_signal_emit(&device->destroy_signal, device);
+ free(device->devpath);
+ free(device);
+}
+
+/** Is it possible to run calibration on this touch device? */
+WL_EXPORT bool
+weston_touch_device_can_calibrate(struct weston_touch_device *device)
+{
+ return !!device->ops;
+}
+
static struct weston_pointer_client *
weston_pointer_client_create(struct wl_client *client)
{
@@ -1277,6 +1339,7 @@ weston_touch_create(void)
if (touch == NULL)
return NULL;

+ wl_list_init(&touch->device_list);
wl_list_init(&touch->resource_list);
wl_list_init(&touch->focus_resource_list);
wl_list_init(&touch->focus_view_listener.link);
@@ -1297,6 +1360,8 @@ weston_touch_destroy(struct weston_touch *touch)
{
struct wl_resource *resource;

+ assert(wl_list_empty(&touch->device_list));
+
wl_resource_for_each(resource, &touch->resource_list) {
wl_resource_set_user_data(resource, NULL);
}
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index e1738613..cb0bafe8 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -296,6 +296,111 @@ handle_pointer_axis(struct libinput_device *libinput_device,
return true;
}

+static struct weston_output *
+touch_get_output(struct weston_touch_device *device)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+
+ return evdev_device->output;
+}
+
+static const char *
+touch_get_calibration_head_name(struct weston_touch_device *device)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+ struct weston_output *output = evdev_device->output;
+ struct weston_head *head;
+
+ if (!output)
+ return NULL;
+
+ assert(output->enabled);
+ if (evdev_device->output_name)
+ return evdev_device->output_name;
+
+ /* No specific head was configured, so the association was made by
+ * the default rule. Just grab whatever head's name.
+ */
+ wl_list_for_each(head, &output->head_list, output_link)
+ return head->name;
+
+ assert(0);
+ return NULL;
+}
+
+static void
+touch_get_calibration(struct weston_touch_device *device,
+ struct weston_touch_device_matrix *calb)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+
+ libinput_device_config_calibration_get_matrix(evdev_device->device,
+ calb->m);
+}
+
+static void
+do_set_calibration(struct evdev_device *evdev_device,
+ const struct weston_touch_device_matrix *calb)
+{
+ enum libinput_config_status status;
+
+ status = libinput_device_config_calibration_set_matrix(evdev_device->device,
+ calb->m);
+ if (status != LIBINPUT_CONFIG_STATUS_SUCCESS)
+ weston_log("Failed to apply calibration.\n");
+}
+
+static void
+touch_set_calibration(struct weston_touch_device *device,
+ const struct weston_touch_device_matrix *calb)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+
+ /* Stop output hotplug from reloading the WL_CALIBRATION values.
+ * libinput will maintain the latest calibration for us.
+ */
+ evdev_device->override_wl_calibration = true;
+
+ do_set_calibration(evdev_device, calb);
+}
+
+static const struct weston_touch_device_ops touch_calibration_ops = {
+ .get_output = touch_get_output,
+ .get_calibration_head_name = touch_get_calibration_head_name,
+ .get_calibration = touch_get_calibration,
+ .set_calibration = touch_set_calibration
+};
+
+static struct weston_touch_device *
+create_touch_device(struct evdev_device *device)
+{
+ const struct weston_touch_device_ops *ops = NULL;
+ struct weston_touch_device *touch_device;
+ struct udev_device *udev_device;
+
+ if (libinput_device_config_calibration_has_matrix(device->device))
+ ops = &touch_calibration_ops;
+
+ udev_device = libinput_device_get_udev_device(device->device);
+ if (!udev_device)
+ return NULL;
+
+ touch_device = weston_touch_create_touch_device(device->seat->touch_state,
+ udev_device_get_devpath(udev_device),
+ device, ops);
+
+ udev_device_unref(udev_device);
+
+ if (!touch_device)
+ return NULL;
+
+ weston_log("Touchscreen - %s - %s\n",
+ libinput_device_get_name(device->device),
+ touch_device->devpath);
+
+ return touch_device;
+}
+
static void
handle_touch_with_coords(struct libinput_device *libinput_device,
struct libinput_event_touch *touch_event,
@@ -466,6 +571,12 @@ evdev_device_set_calibration(struct evdev_device *device)
calibration) != 0)
return;

+ /* touch_set_calibration() has updated the values, do not load old
+ * values from WL_CALIBRATION.
+ */
+ if (device->override_wl_calibration)
+ return;
+
if (!device->output) {
weston_log("input device %s has no enabled output associated "
"(%s named), skipping calibration for now.\n",
@@ -598,6 +709,7 @@ evdev_device_create(struct libinput_device *libinput_device,
LIBINPUT_DEVICE_CAP_TOUCH)) {
weston_seat_init_touch(seat);
device->seat_caps |= EVDEV_SEAT_TOUCH;
+ device->touch_device = create_touch_device(device);
}

libinput_device_set_user_data(libinput_device, device);
@@ -613,8 +725,10 @@ evdev_device_destroy(struct evdev_device *device)
weston_seat_release_pointer(device->seat);
if (device->seat_caps & EVDEV_SEAT_KEYBOARD)
weston_seat_release_keyboard(device->seat);
- if (device->seat_caps & EVDEV_SEAT_TOUCH)
+ if (device->seat_caps & EVDEV_SEAT_TOUCH) {
+ weston_touch_device_destroy(device->touch_device);
weston_seat_release_touch(device->seat);
+ }

if (device->output)
wl_list_remove(&device->output_destroy_listener.link);
diff --git a/libweston/libinput-device.h b/libweston/libinput-device.h
index 8e9f5608..6147a513 100644
--- a/libweston/libinput-device.h
+++ b/libweston/libinput-device.h
@@ -31,6 +31,7 @@
#include <linux/input.h>
#include <wayland-util.h>
#include <libinput.h>
+#include <stdbool.h>

#include "compositor.h"

@@ -44,11 +45,13 @@ struct evdev_device {
struct weston_seat *seat;
enum evdev_device_seat_capability seat_caps;
struct libinput_device *device;
+ struct weston_touch_device *touch_device;
struct wl_list link;
struct weston_output *output;
struct wl_listener output_destroy_listener;
char *output_name;
int fd;
+ bool override_wl_calibration;
};

void
diff --git a/tests/weston-test.c b/tests/weston-test.c
index ae08b02f..9662b67c 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -45,11 +45,15 @@
#include "shared/helpers.h"
#include "shared/timespec-util.h"

+#define MAX_TOUCH_DEVICES 32
+
struct weston_test {
struct weston_compositor *compositor;
struct weston_layer layer;
struct weston_process process;
struct weston_seat seat;
+ struct weston_touch_device *touch_device[MAX_TOUCH_DEVICES];
+ int nr_touch_devices;
bool is_seat_initialized;
};

@@ -77,6 +81,34 @@ test_client_sigchld(struct weston_process *process, int status)
wl_display_terminate(test->compositor->wl_display);
}

+static void
+touch_device_add(struct weston_test *test)
+{
+ char buf[128];
+ int i = test->nr_touch_devices;
+
+ assert(i < MAX_TOUCH_DEVICES);
+ assert(!test->touch_device[i]);
+
+ snprintf(buf, sizeof buf, "test-touch-device-%d", i);
+
+ test->touch_device[i] = weston_touch_create_touch_device(
+ test->seat.touch_state, buf, NULL, NULL);
+ test->nr_touch_devices++;
+}
+
+static void
+touch_device_remove(struct weston_test *test)
+{
+ int i = test->nr_touch_devices - 1;
+
+ assert(i >= 0);
+ assert(test->touch_device[i]);
+ weston_touch_device_destroy(test->touch_device[i]);
+ test->touch_device[i] = NULL;
+ --test->nr_touch_devices;
+}
+
static int
test_seat_init(struct weston_test *test)
{
@@ -92,6 +124,7 @@ test_seat_init(struct weston_test *test)
if (weston_seat_init_keyboard(&test->seat, NULL) < 0)
return -1;
weston_seat_init_touch(&test->seat);
+ touch_device_add(test);

return 0;
}
@@ -99,6 +132,9 @@ test_seat_init(struct weston_test *test)
static void
test_seat_release(struct weston_test *test)
{
+ while (test->nr_touch_devices > 0)
+ touch_device_remove(test);
+
assert(test->is_seat_initialized &&
"Trying to release already released test seat");
test->is_seat_initialized = false;
@@ -281,6 +317,7 @@ device_release(struct wl_client *client,
} else if (strcmp(device, "keyboard") == 0) {
weston_seat_release_keyboard(seat);
} else if (strcmp(device, "touch") == 0) {
+ touch_device_remove(test);
weston_seat_release_touch(seat);
} else if (strcmp(device, "seat") == 0) {
test_seat_release(test);
@@ -302,6 +339,7 @@ device_add(struct wl_client *client,
weston_seat_init_keyboard(seat, NULL);
} else if (strcmp(device, "touch") == 0) {
weston_seat_init_touch(seat);
+ touch_device_add(test);
} else if (strcmp(device, "seat") == 0) {
test_seat_init(test);
} else {
--
2.16.1
Peter Hutterer
2018-04-09 01:55:51 UTC
Permalink
Post by Pekka Paalanen
Introduce weston_touch_device for libweston core to track individual
touchscreen input devices. A weston_seat/weston_touch may be an
aggregation of several physical touchscreen input devices. Separating
the physical devices will be required for implementing touchscreen
calibration. One can only calibrate one device at a time, and we want to
make sure to handle the right one.
Both backends that support touch devices are updated to create
weston_touch_devices. Wayland-backend provides touch devices that cannot
be calibrated, because we have no access to raw touch coordinates from
the device - calibration is the responsibility of the parent display
server. Libinput backend provides touch devices that can be calibrated,
hence implementing the set and get calibration hooks.
Backends need to maintain an output pointer in any case, so we have a
get_output() hook instead of having to maintain an identical field in
weston_touch_device. The same justification applies to
get_calibration_head_name.
Also update the test plugin to manage weston_touch_device objects.
Co-developed by Louis-Francis and Pekka.
just a little nit: you're going to make the world an every so slightly
angrier place by choosing "calb" instead of "calib" or "cal" as shortcut for
"calibration" :)

I predict that almost everyone working on that code will mistype that the
first few times around.

Cheers,
Peter
Post by Pekka Paalanen
---
libweston/compositor-wayland.c | 21 ++++++++
libweston/compositor.h | 56 ++++++++++++++++++++
libweston/input.c | 65 +++++++++++++++++++++++
libweston/libinput-device.c | 116 ++++++++++++++++++++++++++++++++++++++++-
libweston/libinput-device.h | 3 ++
tests/weston-test.c | 38 ++++++++++++++
6 files changed, 298 insertions(+), 1 deletion(-)
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index f186681c..9d5b3551 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -198,6 +198,8 @@ struct wayland_input {
} cursor;
} parent;
+ struct weston_touch_device *touch_device;
+
enum weston_key_state_update keyboard_state_update;
uint32_t key_serial;
uint32_t enter_serial;
@@ -2246,6 +2248,22 @@ static const struct wl_touch_listener touch_listener = {
};
+static struct weston_touch_device *
+create_touch_device(struct wayland_input *input)
+{
+ struct weston_touch_device *touch_device;
+ char str[128];
+
+ /* manufacture a unique'ish name */
+ snprintf(str, sizeof str, "wayland-touch[%u]",
+ wl_proxy_get_id((struct wl_proxy *)input->parent.seat));
+
+ touch_device = weston_touch_create_touch_device(input->base.touch_state,
+ str, NULL, NULL);
+
+ return touch_device;
+}
+
static void
input_handle_capabilities(void *data, struct wl_seat *seat,
enum wl_seat_capability caps)
@@ -2287,7 +2305,10 @@ input_handle_capabilities(void *data, struct wl_seat *seat,
wl_touch_add_listener(input->parent.touch,
&touch_listener, input);
weston_seat_init_touch(&input->base);
+ input->touch_device = create_touch_device(input);
} else if (!(caps & WL_SEAT_CAPABILITY_TOUCH) && input->parent.touch) {
+ weston_touch_device_destroy(input->touch_device);
+ input->touch_device = NULL;
if (input->seat_version >= WL_TOUCH_RELEASE_SINCE_VERSION)
wl_touch_release(input->parent.touch);
else
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 675ee68f..b08031ac 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -453,10 +453,54 @@ struct weston_pointer {
struct wl_list timestamps_list;
};
+/** libinput style calibration matrix
+ *
+ * See https://wayland.freedesktop.org/libinput/doc/latest/absolute_axes.html
+ * and libinput_device_config_calibration_set_matrix().
+ */
+struct weston_touch_device_matrix {
+ float m[6];
+};
+
+struct weston_touch_device;
+
+/** Operations for a calibratable touchscreen */
+struct weston_touch_device_ops {
+ /** Get the associated output if existing. */
+ struct weston_output *(*get_output)(struct weston_touch_device *device);
+
+ /** Get the name of the associated head if existing. */
+ const char *
+ (*get_calibration_head_name)(struct weston_touch_device *device);
+
+ /** Retrieve the current calibration matrix. */
+ void (*get_calibration)(struct weston_touch_device *device,
+ struct weston_touch_device_matrix *matrix);
+
+ /** Set a new calibration matrix. */
+ void (*set_calibration)(struct weston_touch_device *device,
+ const struct weston_touch_device_matrix *matrix);
+};
+
+/** Represents a physical touchscreen input device */
+struct weston_touch_device {
+ char *devpath; /**< unique name */
+ struct weston_touch *aggregate; /**< weston_touch this is part of */
+ struct wl_list link; /**< in weston_touch::device_list */
+ struct wl_signal destroy_signal; /**< destroy notifier */
+
+ void *backend_data; /**< backend-specific private */
+
+ const struct weston_touch_device_ops *ops;
+};
+
+/** Represents a set of touchscreen devices aggregated under a seat */
struct weston_touch {
struct weston_seat *seat;
+ struct wl_list device_list; /* struct weston_touch_device::link */
+
struct wl_list resource_list;
struct wl_list focus_resource_list;
struct weston_view *focus;
@@ -590,6 +634,18 @@ weston_touch_send_motion(struct weston_touch *touch,
void
weston_touch_send_frame(struct weston_touch *touch);
+struct weston_touch_device *
+weston_touch_create_touch_device(struct weston_touch *touch,
+ const char *devpath,
+ void *backend_data,
+ const struct weston_touch_device_ops *ops);
+
+void
+weston_touch_device_destroy(struct weston_touch_device *device);
+
+bool
+weston_touch_device_can_calibrate(struct weston_touch_device *device);
+
void
wl_data_device_set_keyboard_focus(struct weston_seat *seat);
diff --git a/libweston/input.c b/libweston/input.c
index 3e91c266..feea9e72 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -123,6 +123,68 @@ remove_input_resource_from_timestamps(struct wl_resource *input_resource,
}
}
+/** Register a touchscreen input device
+ *
+ * \param touch The parent weston_touch that identifies the seat.
+ * \param devpath Unique device name.
+ * \param backend_data Backend private data if necessary.
+ * \param ops Calibration operations, or NULL for not able to run calibration.
+ * \return New touch device, or NULL on failure.
+ */
+WL_EXPORT struct weston_touch_device *
+weston_touch_create_touch_device(struct weston_touch *touch,
+ const char *devpath,
+ void *backend_data,
+ const struct weston_touch_device_ops *ops)
+{
+ struct weston_touch_device *device;
+
+ assert(devpath);
+ if (ops) {
+ assert(ops->get_output);
+ assert(ops->get_calibration_head_name);
+ assert(ops->get_calibration);
+ assert(ops->set_calibration);
+ }
+
+ device = zalloc(sizeof *device);
+ if (!device)
+ return NULL;
+
+ wl_signal_init(&device->destroy_signal);
+
+ device->devpath = strdup(devpath);
+ if (!device->devpath) {
+ free(device);
+ return NULL;
+ }
+
+ device->backend_data = backend_data;
+ device->ops = ops;
+
+ device->aggregate = touch;
+ wl_list_insert(touch->device_list.prev, &device->link);
+
+ return device;
+}
+
+/** Destroy the touch device. */
+WL_EXPORT void
+weston_touch_device_destroy(struct weston_touch_device *device)
+{
+ wl_list_remove(&device->link);
+ wl_signal_emit(&device->destroy_signal, device);
+ free(device->devpath);
+ free(device);
+}
+
+/** Is it possible to run calibration on this touch device? */
+WL_EXPORT bool
+weston_touch_device_can_calibrate(struct weston_touch_device *device)
+{
+ return !!device->ops;
+}
+
static struct weston_pointer_client *
weston_pointer_client_create(struct wl_client *client)
{
@@ -1277,6 +1339,7 @@ weston_touch_create(void)
if (touch == NULL)
return NULL;
+ wl_list_init(&touch->device_list);
wl_list_init(&touch->resource_list);
wl_list_init(&touch->focus_resource_list);
wl_list_init(&touch->focus_view_listener.link);
@@ -1297,6 +1360,8 @@ weston_touch_destroy(struct weston_touch *touch)
{
struct wl_resource *resource;
+ assert(wl_list_empty(&touch->device_list));
+
wl_resource_for_each(resource, &touch->resource_list) {
wl_resource_set_user_data(resource, NULL);
}
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index e1738613..cb0bafe8 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -296,6 +296,111 @@ handle_pointer_axis(struct libinput_device *libinput_device,
return true;
}
+static struct weston_output *
+touch_get_output(struct weston_touch_device *device)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+
+ return evdev_device->output;
+}
+
+static const char *
+touch_get_calibration_head_name(struct weston_touch_device *device)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+ struct weston_output *output = evdev_device->output;
+ struct weston_head *head;
+
+ if (!output)
+ return NULL;
+
+ assert(output->enabled);
+ if (evdev_device->output_name)
+ return evdev_device->output_name;
+
+ /* No specific head was configured, so the association was made by
+ * the default rule. Just grab whatever head's name.
+ */
+ wl_list_for_each(head, &output->head_list, output_link)
+ return head->name;
+
+ assert(0);
+ return NULL;
+}
+
+static void
+touch_get_calibration(struct weston_touch_device *device,
+ struct weston_touch_device_matrix *calb)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+
+ libinput_device_config_calibration_get_matrix(evdev_device->device,
+ calb->m);
+}
+
+static void
+do_set_calibration(struct evdev_device *evdev_device,
+ const struct weston_touch_device_matrix *calb)
+{
+ enum libinput_config_status status;
+
+ status = libinput_device_config_calibration_set_matrix(evdev_device->device,
+ calb->m);
+ if (status != LIBINPUT_CONFIG_STATUS_SUCCESS)
+ weston_log("Failed to apply calibration.\n");
+}
+
+static void
+touch_set_calibration(struct weston_touch_device *device,
+ const struct weston_touch_device_matrix *calb)
+{
+ struct evdev_device *evdev_device = device->backend_data;
+
+ /* Stop output hotplug from reloading the WL_CALIBRATION values.
+ * libinput will maintain the latest calibration for us.
+ */
+ evdev_device->override_wl_calibration = true;
+
+ do_set_calibration(evdev_device, calb);
+}
+
+static const struct weston_touch_device_ops touch_calibration_ops = {
+ .get_output = touch_get_output,
+ .get_calibration_head_name = touch_get_calibration_head_name,
+ .get_calibration = touch_get_calibration,
+ .set_calibration = touch_set_calibration
+};
+
+static struct weston_touch_device *
+create_touch_device(struct evdev_device *device)
+{
+ const struct weston_touch_device_ops *ops = NULL;
+ struct weston_touch_device *touch_device;
+ struct udev_device *udev_device;
+
+ if (libinput_device_config_calibration_has_matrix(device->device))
+ ops = &touch_calibration_ops;
+
+ udev_device = libinput_device_get_udev_device(device->device);
+ if (!udev_device)
+ return NULL;
+
+ touch_device = weston_touch_create_touch_device(device->seat->touch_state,
+ udev_device_get_devpath(udev_device),
+ device, ops);
+
+ udev_device_unref(udev_device);
+
+ if (!touch_device)
+ return NULL;
+
+ weston_log("Touchscreen - %s - %s\n",
+ libinput_device_get_name(device->device),
+ touch_device->devpath);
+
+ return touch_device;
+}
+
static void
handle_touch_with_coords(struct libinput_device *libinput_device,
struct libinput_event_touch *touch_event,
@@ -466,6 +571,12 @@ evdev_device_set_calibration(struct evdev_device *device)
calibration) != 0)
return;
+ /* touch_set_calibration() has updated the values, do not load old
+ * values from WL_CALIBRATION.
+ */
+ if (device->override_wl_calibration)
+ return;
+
if (!device->output) {
weston_log("input device %s has no enabled output associated "
"(%s named), skipping calibration for now.\n",
@@ -598,6 +709,7 @@ evdev_device_create(struct libinput_device *libinput_device,
LIBINPUT_DEVICE_CAP_TOUCH)) {
weston_seat_init_touch(seat);
device->seat_caps |= EVDEV_SEAT_TOUCH;
+ device->touch_device = create_touch_device(device);
}
libinput_device_set_user_data(libinput_device, device);
@@ -613,8 +725,10 @@ evdev_device_destroy(struct evdev_device *device)
weston_seat_release_pointer(device->seat);
if (device->seat_caps & EVDEV_SEAT_KEYBOARD)
weston_seat_release_keyboard(device->seat);
- if (device->seat_caps & EVDEV_SEAT_TOUCH)
+ if (device->seat_caps & EVDEV_SEAT_TOUCH) {
+ weston_touch_device_destroy(device->touch_device);
weston_seat_release_touch(device->seat);
+ }
if (device->output)
wl_list_remove(&device->output_destroy_listener.link);
diff --git a/libweston/libinput-device.h b/libweston/libinput-device.h
index 8e9f5608..6147a513 100644
--- a/libweston/libinput-device.h
+++ b/libweston/libinput-device.h
@@ -31,6 +31,7 @@
#include <linux/input.h>
#include <wayland-util.h>
#include <libinput.h>
+#include <stdbool.h>
#include "compositor.h"
@@ -44,11 +45,13 @@ struct evdev_device {
struct weston_seat *seat;
enum evdev_device_seat_capability seat_caps;
struct libinput_device *device;
+ struct weston_touch_device *touch_device;
struct wl_list link;
struct weston_output *output;
struct wl_listener output_destroy_listener;
char *output_name;
int fd;
+ bool override_wl_calibration;
};
void
diff --git a/tests/weston-test.c b/tests/weston-test.c
index ae08b02f..9662b67c 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -45,11 +45,15 @@
#include "shared/helpers.h"
#include "shared/timespec-util.h"
+#define MAX_TOUCH_DEVICES 32
+
struct weston_test {
struct weston_compositor *compositor;
struct weston_layer layer;
struct weston_process process;
struct weston_seat seat;
+ struct weston_touch_device *touch_device[MAX_TOUCH_DEVICES];
+ int nr_touch_devices;
bool is_seat_initialized;
};
@@ -77,6 +81,34 @@ test_client_sigchld(struct weston_process *process, int status)
wl_display_terminate(test->compositor->wl_display);
}
+static void
+touch_device_add(struct weston_test *test)
+{
+ char buf[128];
+ int i = test->nr_touch_devices;
+
+ assert(i < MAX_TOUCH_DEVICES);
+ assert(!test->touch_device[i]);
+
+ snprintf(buf, sizeof buf, "test-touch-device-%d", i);
+
+ test->touch_device[i] = weston_touch_create_touch_device(
+ test->seat.touch_state, buf, NULL, NULL);
+ test->nr_touch_devices++;
+}
+
+static void
+touch_device_remove(struct weston_test *test)
+{
+ int i = test->nr_touch_devices - 1;
+
+ assert(i >= 0);
+ assert(test->touch_device[i]);
+ weston_touch_device_destroy(test->touch_device[i]);
+ test->touch_device[i] = NULL;
+ --test->nr_touch_devices;
+}
+
static int
test_seat_init(struct weston_test *test)
{
@@ -92,6 +124,7 @@ test_seat_init(struct weston_test *test)
if (weston_seat_init_keyboard(&test->seat, NULL) < 0)
return -1;
weston_seat_init_touch(&test->seat);
+ touch_device_add(test);
return 0;
}
@@ -99,6 +132,9 @@ test_seat_init(struct weston_test *test)
static void
test_seat_release(struct weston_test *test)
{
+ while (test->nr_touch_devices > 0)
+ touch_device_remove(test);
+
assert(test->is_seat_initialized &&
"Trying to release already released test seat");
test->is_seat_initialized = false;
@@ -281,6 +317,7 @@ device_release(struct wl_client *client,
} else if (strcmp(device, "keyboard") == 0) {
weston_seat_release_keyboard(seat);
} else if (strcmp(device, "touch") == 0) {
+ touch_device_remove(test);
weston_seat_release_touch(seat);
} else if (strcmp(device, "seat") == 0) {
test_seat_release(test);
@@ -302,6 +339,7 @@ device_add(struct wl_client *client,
weston_seat_init_keyboard(seat, NULL);
} else if (strcmp(device, "touch") == 0) {
weston_seat_init_touch(seat);
+ touch_device_add(test);
} else if (strcmp(device, "seat") == 0) {
test_seat_init(test);
} else {
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-04-10 09:31:26 UTC
Permalink
On Mon, 9 Apr 2018 11:55:51 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Introduce weston_touch_device for libweston core to track individual
touchscreen input devices. A weston_seat/weston_touch may be an
aggregation of several physical touchscreen input devices. Separating
the physical devices will be required for implementing touchscreen
calibration. One can only calibrate one device at a time, and we want to
make sure to handle the right one.
Both backends that support touch devices are updated to create
weston_touch_devices. Wayland-backend provides touch devices that cannot
be calibrated, because we have no access to raw touch coordinates from
the device - calibration is the responsibility of the parent display
server. Libinput backend provides touch devices that can be calibrated,
hence implementing the set and get calibration hooks.
Backends need to maintain an output pointer in any case, so we have a
get_output() hook instead of having to maintain an identical field in
weston_touch_device. The same justification applies to
get_calibration_head_name.
Also update the test plugin to manage weston_touch_device objects.
Co-developed by Louis-Francis and Pekka.
just a little nit: you're going to make the world an every so slightly
angrier place by choosing "calb" instead of "calib" or "cal" as shortcut for
"calibration" :)
I predict that almost everyone working on that code will mistype that the
first few times around.
Right, I can try to think of another name for the calibration matrix
local variable.


Thanks,
pq
Pekka Paalanen
2018-03-23 12:00:53 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Move calibration printing here and call do_set_calibration() from
evdev_device_set_calibration() so that all matrix setting paths print
the same way.

Print the matrix values in a matrix style to help readability, and
mention the input device.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 51 +++++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index cb0bafe8..742cb5c9 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -344,10 +344,17 @@ do_set_calibration(struct evdev_device *evdev_device,
{
enum libinput_config_status status;

+ weston_log("input device %s: applying calibration:\n",
+ libinput_device_get_sysname(evdev_device->device));
+ weston_log_continue(STAMP_SPACE " %f %f %f\n",
+ calb->m[0], calb->m[1], calb->m[2]);
+ weston_log_continue(STAMP_SPACE " %f %f %f\n",
+ calb->m[3], calb->m[4], calb->m[5]);
+
status = libinput_device_config_calibration_set_matrix(evdev_device->device,
calb->m);
if (status != LIBINPUT_CONFIG_STATUS_SUCCESS)
- weston_log("Failed to apply calibration.\n");
+ weston_log("Error: Failed to apply calibration.\n");
}

static void
@@ -557,8 +564,7 @@ evdev_device_set_calibration(struct evdev_device *device)
const char *sysname = libinput_device_get_sysname(device->device);
const char *calibration_values;
uint32_t width, height;
- float calibration[6];
- enum libinput_config_status status;
+ struct weston_touch_device_matrix calibration;

if (!libinput_device_config_calibration_has_matrix(device->device))
return;
@@ -568,7 +574,7 @@ evdev_device_set_calibration(struct evdev_device *device)
* output to load a calibration. */
if (libinput_device_config_calibration_get_default_matrix(
device->device,
- calibration) != 0)
+ calibration.m) != 0)
return;

/* touch_set_calibration() has updated the values, do not load old
@@ -612,35 +618,26 @@ evdev_device_set_calibration(struct evdev_device *device)

if (!calibration_values || sscanf(calibration_values,
"%f %f %f %f %f %f",
- &calibration[0],
- &calibration[1],
- &calibration[2],
- &calibration[3],
- &calibration[4],
- &calibration[5]) != 6)
+ &calibration.m[0],
+ &calibration.m[1],
+ &calibration.m[2],
+ &calibration.m[3],
+ &calibration.m[4],
+ &calibration.m[5]) != 6)
goto out;

- weston_log("Applying calibration: %f %f %f %f %f %f "
- "(normalized %f %f)\n",
- calibration[0],
- calibration[1],
- calibration[2],
- calibration[3],
- calibration[4],
- calibration[5],
- calibration[2] / width,
- calibration[5] / height);
-
/* normalize to a format libinput can use. There is a chance of
this being wrong if the width/height don't match the device
width/height but I'm not sure how to fix that */
- calibration[2] /= width;
- calibration[5] /= height;
+ calibration.m[2] /= width;
+ calibration.m[5] /= height;

- status = libinput_device_config_calibration_set_matrix(device->device,
- calibration);
- if (status != LIBINPUT_CONFIG_STATUS_SUCCESS)
- weston_log("Failed to apply calibration.\n");
+ do_set_calibration(device, &calibration);
+
+ weston_log_continue(STAMP_SPACE " raw translation %f %f for output %s\n",
+ calibration.m[2] * width,
+ calibration.m[5] * height,
+ device->output->name);

out:
if (udev_device)
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:44 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Helps admins ensure the configuration is correct.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 64d99f75..d391d63e 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -554,6 +554,12 @@ evdev_device_set_output(struct evdev_device *device,
device->output_destroy_listener.notify = NULL;
}

+ weston_log("associating input device %s with output %s "
+ "(%s by udev)\n",
+ libinput_device_get_sysname(device->device),
+ output->name,
+ device->output_name ?: "none");
+
device->output = output;
device->output_destroy_listener.notify = notify_output_destroy;
wl_signal_add(&output->destroy_signal,
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:45 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

In the future evdev_device_set_output() will start getting called more
often, redundantly. Short-circuit the setting if the chosen output is
already set for an input device. This reduces churn in the logs.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/libinput-device.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index d391d63e..62f4cee8 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -549,6 +549,9 @@ void
evdev_device_set_output(struct evdev_device *device,
struct weston_output *output)
{
+ if (device->output == output)
+ return;
+
if (device->output_destroy_listener.notify) {
wl_list_remove(&device->output_destroy_listener.link);
device->output_destroy_listener.notify = NULL;
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:54 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Relay touch input events into libweston core through the
weston_touch_device, so that the core can tell which individual physical
device they come from.

This is necessary for supporting touchscreen calibration, where one
needs to process a single physical device at a time instead of the
aggregate of all touch devices on the weston_seat.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor-wayland.c | 10 +++++-----
libweston/compositor.h | 6 +++---
libweston/input.c | 18 ++++++++----------
libweston/libinput-device.c | 7 +++----
tests/weston-test.c | 6 ++++--
5 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 9d5b3551..27fe5f59 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -2144,7 +2144,7 @@ input_handle_touch_down(void *data, struct wl_touch *wl_touch,

weston_output_transform_coordinate(&output->base, x, y, &x, &y);

- notify_touch(&input->base, &ts, id, x, y, WL_TOUCH_DOWN);
+ notify_touch(input->touch_device, &ts, id, x, y, WL_TOUCH_DOWN);
input->touch_active = true;
}

@@ -2185,7 +2185,7 @@ input_handle_touch_up(void *data, struct wl_touch *wl_touch,
}

if (active)
- notify_touch(&input->base, &ts, id, 0, 0, WL_TOUCH_UP);
+ notify_touch(input->touch_device, &ts, id, 0, 0, WL_TOUCH_UP);
}

static void
@@ -2214,7 +2214,7 @@ input_handle_touch_motion(void *data, struct wl_touch *wl_touch,

weston_output_transform_coordinate(&output->base, x, y, &x, &y);

- notify_touch(&input->base, &ts, id, x, y, WL_TOUCH_MOTION);
+ notify_touch(input->touch_device, &ts, id, x, y, WL_TOUCH_MOTION);
}

static void
@@ -2225,7 +2225,7 @@ input_handle_touch_frame(void *data, struct wl_touch *wl_touch)
if (!input->touch_focus || !input->touch_active)
return;

- notify_touch_frame(&input->base);
+ notify_touch_frame(input->touch_device);
}

static void
@@ -2236,7 +2236,7 @@ input_handle_touch_cancel(void *data, struct wl_touch *wl_touch)
if (!input->touch_focus || !input->touch_active)
return;

- notify_touch_cancel(&input->base);
+ notify_touch_cancel(input->touch_device);
}

static const struct wl_touch_listener touch_listener = {
diff --git a/libweston/compositor.h b/libweston/compositor.h
index b08031ac..27daf76a 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1545,13 +1545,13 @@ void
notify_keyboard_focus_out(struct weston_seat *seat);

void
-notify_touch(struct weston_seat *seat, const struct timespec *time,
+notify_touch(struct weston_touch_device *device, const struct timespec *time,
int touch_id, double x, double y, int touch_type);
void
-notify_touch_frame(struct weston_seat *seat);
+notify_touch_frame(struct weston_touch_device *device);

void
-notify_touch_cancel(struct weston_seat *seat);
+notify_touch_cancel(struct weston_touch_device *device);

void
weston_layer_entry_insert(struct weston_layer_entry *list,
diff --git a/libweston/input.c b/libweston/input.c
index feea9e72..db710da3 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2345,12 +2345,12 @@ weston_touch_set_focus(struct weston_touch *touch, struct weston_view *view)
*
*/
WL_EXPORT void
-notify_touch(struct weston_seat *seat, const struct timespec *time,
+notify_touch(struct weston_touch_device *device, const struct timespec *time,
int touch_id, double double_x, double double_y, int touch_type)
{
- struct weston_compositor *ec = seat->compositor;
- struct weston_touch *touch = weston_seat_get_touch(seat);
- struct weston_touch_grab *grab = touch->grab;
+ struct weston_touch *touch = device->aggregate;
+ struct weston_touch_grab *grab = device->aggregate->grab;
+ struct weston_compositor *ec = device->aggregate->seat->compositor;
struct weston_view *ev;
wl_fixed_t sx, sy;
wl_fixed_t x = wl_fixed_from_double(double_x);
@@ -2424,19 +2424,17 @@ notify_touch(struct weston_seat *seat, const struct timespec *time,
}

WL_EXPORT void
-notify_touch_frame(struct weston_seat *seat)
+notify_touch_frame(struct weston_touch_device *device)
{
- struct weston_touch *touch = weston_seat_get_touch(seat);
- struct weston_touch_grab *grab = touch->grab;
+ struct weston_touch_grab *grab = device->aggregate->grab;

grab->interface->frame(grab);
}

WL_EXPORT void
-notify_touch_cancel(struct weston_seat *seat)
+notify_touch_cancel(struct weston_touch_device *device)
{
- struct weston_touch *touch = weston_seat_get_touch(seat);
- struct weston_touch_grab *grab = touch->grab;
+ struct weston_touch_grab *grab = device->aggregate->grab;

grab->interface->cancel(grab);
}
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 742cb5c9..9f5793aa 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -436,7 +436,7 @@ handle_touch_with_coords(struct libinput_device *libinput_device,
weston_output_transform_coordinate(device->output,
x, y, &x, &y);

- notify_touch(device->seat, &time, slot, x, y, touch_type);
+ notify_touch(device->touch_device, &time, slot, x, y, touch_type);
}

static void
@@ -465,7 +465,7 @@ handle_touch_up(struct libinput_device *libinput_device,
timespec_from_usec(&time,
libinput_event_touch_get_time_usec(touch_event));

- notify_touch(device->seat, &time, slot, 0, 0, WL_TOUCH_UP);
+ notify_touch(device->touch_device, &time, slot, 0, 0, WL_TOUCH_UP);
}

static void
@@ -474,9 +474,8 @@ handle_touch_frame(struct libinput_device *libinput_device,
{
struct evdev_device *device =
libinput_device_get_user_data(libinput_device);
- struct weston_seat *seat = device->seat;

- notify_touch_frame(seat);
+ notify_touch_frame(device->touch_device);
}

int
diff --git a/tests/weston-test.c b/tests/weston-test.c
index 9662b67c..412eb243 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -595,12 +595,14 @@ send_touch(struct wl_client *client, struct wl_resource *resource,
int32_t touch_id, wl_fixed_t x, wl_fixed_t y, uint32_t touch_type)
{
struct weston_test *test = wl_resource_get_user_data(resource);
- struct weston_seat *seat = get_seat(test);
+ struct weston_touch_device *device = test->touch_device[0];
struct timespec time;

+ assert(device);
+
timespec_from_proto(&time, tv_sec_hi, tv_sec_lo, tv_nsec);

- notify_touch(seat, &time, touch_id, wl_fixed_to_double(x),
+ notify_touch(device, &time, touch_id, wl_fixed_to_double(x),
wl_fixed_to_double(y), touch_type);
}
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:56 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

notify_touch_cal() is an extended form of notify_touch(), adding
normalized touch coordinates which are necessary for calibrating a
touchscreen.

It would be possible to invert the transformation and convert from
global coordinates to normalized device coordinates in input.c without
adding this API, but this way it is more robust against code changes.

Recovering normalized device coordinates is necessary because libinput
calibration matrix must be given in normalized units, and it would be
difficult to compute otherwise. Libinput API does not offer normalized
coordinates directly either, but those can be fetched by pretending the
output resolution is 1x1.

Anticipating touch calibration mode, the old notify_touch() is renamed
into a private process_touch_normal(), and the new notify_touch_cal()
delegates to it.

Co-developed by Louis-Francis and Pekka.

Cc: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor.h | 21 +++++++++++++++-
libweston/input.c | 60 ++++++++++++++++++++++++++++++++++++---------
libweston/libinput-device.c | 11 ++++++++-
3 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/libweston/compositor.h b/libweston/compositor.h
index b992b7eb..a1264e5a 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1532,9 +1532,28 @@ notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
void
notify_keyboard_focus_out(struct weston_seat *seat);

+/* an arbitrary unlikely value */
+#define WESTON_INVALID_TOUCH_COORDINATE ((double)777e+7)
+
void
+notify_touch_cal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double x, double y,
+ double norm_x, double norm_y, int touch_type);
+
+/** Feed in touch down, motion, and up events, non-calibratable device.
+ *
+ * @sa notify_touch_cal
+ */
+static inline void
notify_touch(struct weston_touch_device *device, const struct timespec *time,
- int touch_id, double x, double y, int touch_type);
+ int touch_id, double x, double y, int touch_type)
+{
+ notify_touch_cal(device, time, touch_id, x, y,
+ WESTON_INVALID_TOUCH_COORDINATE,
+ WESTON_INVALID_TOUCH_COORDINATE, touch_type);
+}
+
void
notify_touch_frame(struct weston_touch_device *device);

diff --git a/libweston/input.c b/libweston/input.c
index bd7a9167..1658422c 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2336,17 +2336,10 @@ weston_touch_set_focus(struct weston_touch *touch, struct weston_view *view)
touch->focus = view;
}

-/**
- * notify_touch - emulates button touches and notifies surfaces accordingly.
- *
- * It assumes always the correct cycle sequence until it gets here: touch_down
- * → touch_update → ... → touch_update → touch_end. The driver is responsible
- * for sending along such order.
- *
- */
-WL_EXPORT void
-notify_touch(struct weston_touch_device *device, const struct timespec *time,
- int touch_id, double double_x, double double_y, int touch_type)
+static void
+process_touch_normal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double double_x, double double_y, int touch_type)
{
struct weston_touch *touch = device->aggregate;
struct weston_touch_grab *grab = device->aggregate->grab;
@@ -2423,6 +2416,51 @@ notify_touch(struct weston_touch_device *device, const struct timespec *time,
}
}

+/** Feed in touch down, motion, and up events, calibratable device.
+ *
+ * It assumes always the correct cycle sequence until it gets here: touch_down
+ * → touch_update → ... → touch_update → touch_end. The driver is responsible
+ * for sending along such order.
+ *
+ * \param device The physical device that generated the event.
+ * \param time The event timestamp.
+ * \param touch_id ID for the touch point of this event (multi-touch).
+ * \param double_x X coordinate in compositor global space.
+ * \param double_y Y coordinate in compositor global space.
+ * \param norm_x Normalized device X coordinate in calibration space.
+ * \param norm_y Normalized device Y coordinate in calibration space.
+ * \param touch_type Either WL_TOUCH_DOWN, WL_TOUCH_UP, or WL_TOUCH_MOTION.
+ *
+ * Coordinates double_x and double_y are used for normal operation.
+ *
+ * Coordinates norm_x and norm_y are only used for touch device calibration.
+ * If and only if the weston_touch_device does not support calibrating,
+ * norm_x and norm_y must be WESTON_INVALID_TOUCH_COORDINATE.
+ *
+ * The calibration space is the normalized coordinate space
+ * [0.0, 1.0]×[0.0, 1.0] of the weston_touch_device. This is assumed to
+ * map to the similar normalized coordinate space of the associated
+ * weston_output.
+ */
+WL_EXPORT void
+notify_touch_cal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double x, double y, double norm_x, double norm_y,
+ int touch_type)
+{
+ if (touch_type != WL_TOUCH_UP) {
+ if (weston_touch_device_can_calibrate(device)) {
+ assert(norm_x != WESTON_INVALID_TOUCH_COORDINATE);
+ assert(norm_y != WESTON_INVALID_TOUCH_COORDINATE);
+ } else {
+ assert(norm_x == WESTON_INVALID_TOUCH_COORDINATE);
+ assert(norm_y == WESTON_INVALID_TOUCH_COORDINATE);
+ }
+ }
+
+ process_touch_normal(device, time, touch_id, x, y, touch_type);
+}
+
WL_EXPORT void
notify_touch_frame(struct weston_touch_device *device)
{
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 9f5793aa..6fd49572 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -417,6 +417,8 @@ handle_touch_with_coords(struct libinput_device *libinput_device,
libinput_device_get_user_data(libinput_device);
double x;
double y;
+ double norm_x;
+ double norm_y;
uint32_t width, height;
struct timespec time;
int32_t slot;
@@ -436,7 +438,14 @@ handle_touch_with_coords(struct libinput_device *libinput_device,
weston_output_transform_coordinate(device->output,
x, y, &x, &y);

- notify_touch(device->touch_device, &time, slot, x, y, touch_type);
+ if (weston_touch_device_can_calibrate(device->touch_device)) {
+ norm_x = libinput_event_touch_get_x_transformed(touch_event, 1);
+ norm_y = libinput_event_touch_get_y_transformed(touch_event, 1);
+ notify_touch_cal(device->touch_device, &time, slot, x, y,
+ norm_x, norm_y, touch_type);
+ } else {
+ notify_touch(device->touch_device, &time, slot, x, y, touch_type);
+ }
}

static void
--
2.16.1
Peter Hutterer
2018-04-09 02:12:49 UTC
Permalink
Post by Pekka Paalanen
notify_touch_cal() is an extended form of notify_touch(), adding
normalized touch coordinates which are necessary for calibrating a
touchscreen.
It would be possible to invert the transformation and convert from
global coordinates to normalized device coordinates in input.c without
adding this API, but this way it is more robust against code changes.
Recovering normalized device coordinates is necessary because libinput
calibration matrix must be given in normalized units, and it would be
difficult to compute otherwise. Libinput API does not offer normalized
coordinates directly either, but those can be fetched by pretending the
output resolution is 1x1.
Anticipating touch calibration mode, the old notify_touch() is renamed
into a private process_touch_normal(), and the new notify_touch_cal()
delegates to it.
Co-developed by Louis-Francis and Pekka.
---
libweston/compositor.h | 21 +++++++++++++++-
libweston/input.c | 60 ++++++++++++++++++++++++++++++++++++---------
libweston/libinput-device.c | 11 ++++++++-
3 files changed, 79 insertions(+), 13 deletions(-)
diff --git a/libweston/compositor.h b/libweston/compositor.h
index b992b7eb..a1264e5a 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1532,9 +1532,28 @@ notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
void
notify_keyboard_focus_out(struct weston_seat *seat);
+/* an arbitrary unlikely value */
+#define WESTON_INVALID_TOUCH_COORDINATE ((double)777e+7)
+
void
+notify_touch_cal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double x, double y,
+ double norm_x, double norm_y, int touch_type);
+
+/** Feed in touch down, motion, and up events, non-calibratable device.
+ *
+ */
+static inline void
notify_touch(struct weston_touch_device *device, const struct timespec *time,
- int touch_id, double x, double y, int touch_type);
+ int touch_id, double x, double y, int touch_type)
+{
+ notify_touch_cal(device, time, touch_id, x, y,
+ WESTON_INVALID_TOUCH_COORDINATE,
+ WESTON_INVALID_TOUCH_COORDINATE, touch_type);
+}
+
void
notify_touch_frame(struct weston_touch_device *device);
diff --git a/libweston/input.c b/libweston/input.c
index bd7a9167..1658422c 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2336,17 +2336,10 @@ weston_touch_set_focus(struct weston_touch *touch, struct weston_view *view)
touch->focus = view;
}
-/**
- * notify_touch - emulates button touches and notifies surfaces accordingly.
- *
- * It assumes always the correct cycle sequence until it gets here: touch_down
- * → touch_update → ... → touch_update → touch_end. The driver is responsible
- * for sending along such order.
- *
- */
-WL_EXPORT void
-notify_touch(struct weston_touch_device *device, const struct timespec *time,
- int touch_id, double double_x, double double_y, int touch_type)
+static void
+process_touch_normal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double double_x, double double_y, int touch_type)
IMO just process_touch() is better, otherwise it's more confusing ("wait?
why is this normal? and what's not normal? or is this an abbreviation of
normalized?")
Post by Pekka Paalanen
{
struct weston_touch *touch = device->aggregate;
struct weston_touch_grab *grab = device->aggregate->grab;
@@ -2423,6 +2416,51 @@ notify_touch(struct weston_touch_device *device, const struct timespec *time,
}
}
+/** Feed in touch down, motion, and up events, calibratable device.
+ *
+ * It assumes always the correct cycle sequence until it gets here: touch_down
+ * → touch_update → ... → touch_update → touch_end. The driver is responsible
+ * for sending along such order.
+ *
+ * \param device The physical device that generated the event.
+ * \param time The event timestamp.
+ * \param touch_id ID for the touch point of this event (multi-touch).
+ * \param double_x X coordinate in compositor global space.
+ * \param double_y Y coordinate in compositor global space.
+ * \param norm_x Normalized device X coordinate in calibration space.
+ * \param norm_y Normalized device Y coordinate in calibration space.
+ * \param touch_type Either WL_TOUCH_DOWN, WL_TOUCH_UP, or WL_TOUCH_MOTION.
+ *
+ * Coordinates double_x and double_y are used for normal operation.
+ *
+ * Coordinates norm_x and norm_y are only used for touch device calibration.
+ * If and only if the weston_touch_device does not support calibrating,
+ * norm_x and norm_y must be WESTON_INVALID_TOUCH_COORDINATE.
+ *
+ * The calibration space is the normalized coordinate space
+ * [0.0, 1.0]×[0.0, 1.0] of the weston_touch_device. This is assumed to
+ * map to the similar normalized coordinate space of the associated
+ * weston_output.
+ */
+WL_EXPORT void
+notify_touch_cal(struct weston_touch_device *device,
this should be notify_touch_normalized() because that's what the extra
values are. That calibration affects those is just an implementation detail.

Also, at this point I can only say creating structs for each coordinate type
in libinput has helped greatly in understanding what exactly you're dealing
with at any point in time. see libinput-private.h:

/* A coordinate pair in device coordinates */
struct device_coords {
int x, y;
};

/* A dpi-normalized coordinate pair */
struct normalized_coords {
double x, y;
};

/* A pair of coordinates normalized to a [0,1] or [-1, 1] range */
struct normalized_range_coords {
double x, y;
};


etc. These are passed through the various functions, so the compiler will
tell you when you're passing a device coordinate into something that should
take a [0, 1] normalized range. I cannot recommend this enough when you're
dealing with more than one coordinate system.
Post by Pekka Paalanen
+ const struct timespec *time, int touch_id,
+ double x, double y, double norm_x, double norm_y,
+ int touch_type)
+{
+ if (touch_type != WL_TOUCH_UP) {
+ if (weston_touch_device_can_calibrate(device)) {
+ assert(norm_x != WESTON_INVALID_TOUCH_COORDINATE);
+ assert(norm_y != WESTON_INVALID_TOUCH_COORDINATE);
+ } else {
+ assert(norm_x == WESTON_INVALID_TOUCH_COORDINATE);
+ assert(norm_y == WESTON_INVALID_TOUCH_COORDINATE);
+ }
+ }
+
+ process_touch_normal(device, time, touch_id, x, y, touch_type);
+}
+
WL_EXPORT void
notify_touch_frame(struct weston_touch_device *device)
{
diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
index 9f5793aa..6fd49572 100644
--- a/libweston/libinput-device.c
+++ b/libweston/libinput-device.c
@@ -417,6 +417,8 @@ handle_touch_with_coords(struct libinput_device *libinput_device,
libinput_device_get_user_data(libinput_device);
double x;
double y;
+ double norm_x;
+ double norm_y;
uint32_t width, height;
struct timespec time;
int32_t slot;
@@ -436,7 +438,14 @@ handle_touch_with_coords(struct libinput_device *libinput_device,
weston_output_transform_coordinate(device->output,
x, y, &x, &y);
- notify_touch(device->touch_device, &time, slot, x, y, touch_type);
+ if (weston_touch_device_can_calibrate(device->touch_device)) {
+ norm_x = libinput_event_touch_get_x_transformed(touch_event, 1);
+ norm_y = libinput_event_touch_get_y_transformed(touch_event, 1);
one space to many

Cheers,
Peter
Post by Pekka Paalanen
+ notify_touch_cal(device->touch_device, &time, slot, x, y,
+ norm_x, norm_y, touch_type);
+ } else {
+ notify_touch(device->touch_device, &time, slot, x, y, touch_type);
+ }
}
static void
--
2.16.1
Pekka Paalanen
2018-04-10 09:37:15 UTC
Permalink
On Mon, 9 Apr 2018 12:12:49 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
notify_touch_cal() is an extended form of notify_touch(), adding
normalized touch coordinates which are necessary for calibrating a
touchscreen.
It would be possible to invert the transformation and convert from
global coordinates to normalized device coordinates in input.c without
adding this API, but this way it is more robust against code changes.
Recovering normalized device coordinates is necessary because libinput
calibration matrix must be given in normalized units, and it would be
difficult to compute otherwise. Libinput API does not offer normalized
coordinates directly either, but those can be fetched by pretending the
output resolution is 1x1.
Anticipating touch calibration mode, the old notify_touch() is renamed
into a private process_touch_normal(), and the new notify_touch_cal()
delegates to it.
Co-developed by Louis-Francis and Pekka.
---
libweston/compositor.h | 21 +++++++++++++++-
libweston/input.c | 60 ++++++++++++++++++++++++++++++++++++---------
libweston/libinput-device.c | 11 ++++++++-
3 files changed, 79 insertions(+), 13 deletions(-)
-/**
- * notify_touch - emulates button touches and notifies surfaces accordingly.
- *
- * It assumes always the correct cycle sequence until it gets here: touch_down
- * → touch_update → ... → touch_update → touch_end. The driver is responsible
- * for sending along such order.
- *
- */
-WL_EXPORT void
-notify_touch(struct weston_touch_device *device, const struct timespec *time,
- int touch_id, double double_x, double double_y, int touch_type)
+static void
+process_touch_normal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double double_x, double double_y, int touch_type)
IMO just process_touch() is better, otherwise it's more confusing ("wait?
why is this normal? and what's not normal? or is this an abbreviation of
normalized?")
Ok.
Post by Peter Hutterer
Post by Pekka Paalanen
{
struct weston_touch *touch = device->aggregate;
struct weston_touch_grab *grab = device->aggregate->grab;
@@ -2423,6 +2416,51 @@ notify_touch(struct weston_touch_device *device, const struct timespec *time,
}
}
+/** Feed in touch down, motion, and up events, calibratable device.
+ *
+ * It assumes always the correct cycle sequence until it gets here: touch_down
+ * → touch_update → ... → touch_update → touch_end. The driver is responsible
+ * for sending along such order.
+ *
+ * \param device The physical device that generated the event.
+ * \param time The event timestamp.
+ * \param touch_id ID for the touch point of this event (multi-touch).
+ * \param double_x X coordinate in compositor global space.
+ * \param double_y Y coordinate in compositor global space.
+ * \param norm_x Normalized device X coordinate in calibration space.
+ * \param norm_y Normalized device Y coordinate in calibration space.
+ * \param touch_type Either WL_TOUCH_DOWN, WL_TOUCH_UP, or WL_TOUCH_MOTION.
+ *
+ * Coordinates double_x and double_y are used for normal operation.
+ *
+ * Coordinates norm_x and norm_y are only used for touch device calibration.
+ * If and only if the weston_touch_device does not support calibrating,
+ * norm_x and norm_y must be WESTON_INVALID_TOUCH_COORDINATE.
+ *
+ * The calibration space is the normalized coordinate space
+ * [0.0, 1.0]×[0.0, 1.0] of the weston_touch_device. This is assumed to
+ * map to the similar normalized coordinate space of the associated
+ * weston_output.
+ */
+WL_EXPORT void
+notify_touch_cal(struct weston_touch_device *device,
this should be notify_touch_normalized() because that's what the extra
values are. That calibration affects those is just an implementation detail.
Also, at this point I can only say creating structs for each coordinate type
in libinput has helped greatly in understanding what exactly you're dealing
/* A coordinate pair in device coordinates */
struct device_coords {
int x, y;
};
/* A dpi-normalized coordinate pair */
struct normalized_coords {
double x, y;
};
/* A pair of coordinates normalized to a [0,1] or [-1, 1] range */
struct normalized_range_coords {
double x, y;
};
etc. These are passed through the various functions, so the compiler will
tell you when you're passing a device coordinate into something that should
take a [0, 1] normalized range. I cannot recommend this enough when you're
dealing with more than one coordinate system.
That's a good idea indeed. I think I'll re-spin with that.
Post by Peter Hutterer
Post by Pekka Paalanen
+ const struct timespec *time, int touch_id,
+ double x, double y, double norm_x, double norm_y,
+ int touch_type)
+{
- notify_touch(device->touch_device, &time, slot, x, y, touch_type);
+ if (weston_touch_device_can_calibrate(device->touch_device)) {
+ norm_x = libinput_event_touch_get_x_transformed(touch_event, 1);
+ norm_y = libinput_event_touch_get_y_transformed(touch_event, 1);
one space to many
Yup.


Thanks,
pq
Pekka Paalanen
2018-04-23 10:51:32 UTC
Permalink
On Tue, 10 Apr 2018 12:37:15 +0300
Post by Pekka Paalanen
On Mon, 9 Apr 2018 12:12:49 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
notify_touch_cal() is an extended form of notify_touch(), adding
normalized touch coordinates which are necessary for calibrating a
touchscreen.
It would be possible to invert the transformation and convert from
global coordinates to normalized device coordinates in input.c without
adding this API, but this way it is more robust against code changes.
Recovering normalized device coordinates is necessary because libinput
calibration matrix must be given in normalized units, and it would be
difficult to compute otherwise. Libinput API does not offer normalized
coordinates directly either, but those can be fetched by pretending the
output resolution is 1x1.
Anticipating touch calibration mode, the old notify_touch() is renamed
into a private process_touch_normal(), and the new notify_touch_cal()
delegates to it.
Co-developed by Louis-Francis and Pekka.
---
libweston/compositor.h | 21 +++++++++++++++-
libweston/input.c | 60 ++++++++++++++++++++++++++++++++++++---------
libweston/libinput-device.c | 11 ++++++++-
3 files changed, 79 insertions(+), 13 deletions(-)
Also, at this point I can only say creating structs for each coordinate type
in libinput has helped greatly in understanding what exactly you're dealing
/* A coordinate pair in device coordinates */
struct device_coords {
int x, y;
};
/* A dpi-normalized coordinate pair */
struct normalized_coords {
double x, y;
};
/* A pair of coordinates normalized to a [0,1] or [-1, 1] range */
struct normalized_range_coords {
double x, y;
};
etc. These are passed through the various functions, so the compiler will
tell you when you're passing a device coordinate into something that should
take a [0, 1] normalized range. I cannot recommend this enough when you're
dealing with more than one coordinate system.
That's a good idea indeed. I think I'll re-spin with that.
On second thought, even adding just

struct weston_point_global_double {
double x;
double y;
};

would be touching quite many places, and it would prompt me to add

struct weston_point_global_fixed {
wl_fixed_t x;
wl_fixed_t y;
};

which would touch even more that I'd probably be touching almost all
APIs there are. Those would need to be complemented with struct
weston_point_surface_{double,fixed} as well.

So if you don't mind, I would leave that yak for another time.

But, maybe I can start with struct weston_2d_normalized_range_coords
since that's not used elsewhere at all. If I can figure out a bit
shorter name... weston_point2d_output_normalized...


Thanks,
pq
Pekka Paalanen
2018-04-23 10:34:19 UTC
Permalink
On Mon, 9 Apr 2018 12:12:49 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
notify_touch_cal() is an extended form of notify_touch(), adding
normalized touch coordinates which are necessary for calibrating a
touchscreen.
It would be possible to invert the transformation and convert from
global coordinates to normalized device coordinates in input.c without
adding this API, but this way it is more robust against code changes.
Recovering normalized device coordinates is necessary because libinput
calibration matrix must be given in normalized units, and it would be
difficult to compute otherwise. Libinput API does not offer normalized
coordinates directly either, but those can be fetched by pretending the
output resolution is 1x1.
Anticipating touch calibration mode, the old notify_touch() is renamed
into a private process_touch_normal(), and the new notify_touch_cal()
delegates to it.
Co-developed by Louis-Francis and Pekka.
---
libweston/compositor.h | 21 +++++++++++++++-
libweston/input.c | 60 ++++++++++++++++++++++++++++++++++++---------
libweston/libinput-device.c | 11 ++++++++-
3 files changed, 79 insertions(+), 13 deletions(-)
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2336,17 +2336,10 @@ weston_touch_set_focus(struct weston_touch *touch, struct weston_view *view)
touch->focus = view;
}
-/**
- * notify_touch - emulates button touches and notifies surfaces accordingly.
- *
- * It assumes always the correct cycle sequence until it gets here: touch_down
- * → touch_update → ... → touch_update → touch_end. The driver is responsible
- * for sending along such order.
- *
- */
-WL_EXPORT void
-notify_touch(struct weston_touch_device *device, const struct timespec *time,
- int touch_id, double double_x, double double_y, int touch_type)
+static void
+process_touch_normal(struct weston_touch_device *device,
+ const struct timespec *time, int touch_id,
+ double double_x, double double_y, int touch_type)
IMO just process_touch() is better, otherwise it's more confusing ("wait?
why is this normal? and what's not normal? or is this an abbreviation of
normalized?")
Actually, this is really the normal path. The not normal path is the
one for calibrator. You see the non-normal path added in "input:
introduce touch event mode for calibrator".

I didn't want to call it process_touch() originally, because it doesn't
process all touches, just the usual case. I even call it
WESTON_TOUCH_MODE_NORMAL.

Would you have other suggestions?


Thanks,
pq
Pekka Paalanen
2018-03-23 12:00:57 UTC
Permalink
From: Louis-Francis Ratté-Boulianne <***@collabora.com>

compositor.c has 'touch', so use 'touch' here as well. It is not a
device to begin with.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libweston/compositor.h b/libweston/compositor.h
index a1264e5a..129e15b8 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -602,7 +602,7 @@ void
weston_touch_set_focus(struct weston_touch *touch,
struct weston_view *view);
void
-weston_touch_start_grab(struct weston_touch *device,
+weston_touch_start_grab(struct weston_touch *touch,
struct weston_touch_grab *grab);
void
weston_touch_end_grab(struct weston_touch *touch);
--
2.16.1
Pekka Paalanen
2018-04-30 12:17:10 UTC
Permalink
On Fri, 23 Mar 2018 14:00:57 +0200
Post by Pekka Paalanen
compositor.c has 'touch', so use 'touch' here as well. It is not a
device to begin with.
Hi,

with Peter's R-b, I have pushed the two patches:

[PATCH weston 15/25] libweston: unexport weston_{pointer,keyboard,touch}_{create,destroy}()
[PATCH weston 17/25] libweston: fix weston_touch_start_grab() arg name

925788fc..8549e165 master -> master


Thanks,
pq

Pekka Paalanen
2018-03-23 12:00:55 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

We have weston_seat_{init,release}_{pointer,keyboard,touch}() as the
backend-facing API. There is no need to expose the create/destroy
functions which have been for internal use only for quite a long time.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor.h | 12 ------------
libweston/input.c | 12 ++++++------
2 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/libweston/compositor.h b/libweston/compositor.h
index 27daf76a..b992b7eb 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -526,10 +526,6 @@ weston_pointer_motion_to_abs(struct weston_pointer *pointer,
struct weston_pointer_motion_event *event,
wl_fixed_t *x, wl_fixed_t *y);

-struct weston_pointer *
-weston_pointer_create(struct weston_seat *seat);
-void
-weston_pointer_destroy(struct weston_pointer *pointer);
void
weston_pointer_send_motion(struct weston_pointer *pointer,
const struct timespec *time,
@@ -574,10 +570,6 @@ weston_pointer_set_default_grab(struct weston_pointer *pointer,
void
weston_pointer_constraint_destroy(struct weston_pointer_constraint *constraint);

-struct weston_keyboard *
-weston_keyboard_create(void);
-void
-weston_keyboard_destroy(struct weston_keyboard *keyboard);
void
weston_keyboard_set_focus(struct weston_keyboard *keyboard,
struct weston_surface *surface);
@@ -606,10 +598,6 @@ weston_keyboard_send_modifiers(struct weston_keyboard *keyboard,
uint32_t mods_latched,
uint32_t mods_locked, uint32_t group);

-struct weston_touch *
-weston_touch_create(void);
-void
-weston_touch_destroy(struct weston_touch *touch);
void
weston_touch_set_focus(struct weston_touch *touch,
struct weston_view *view);
diff --git a/libweston/input.c b/libweston/input.c
index db710da3..bd7a9167 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -1198,7 +1198,7 @@ weston_pointer_reset_state(struct weston_pointer *pointer)
static void
weston_pointer_handle_output_destroy(struct wl_listener *listener, void *data);

-WL_EXPORT struct weston_pointer *
+static struct weston_pointer *
weston_pointer_create(struct weston_seat *seat)
{
struct weston_pointer *pointer;
@@ -1237,7 +1237,7 @@ weston_pointer_create(struct weston_seat *seat)
return pointer;
}

-WL_EXPORT void
+static void
weston_pointer_destroy(struct weston_pointer *pointer)
{
struct weston_pointer_client *pointer_client, *tmp;
@@ -1271,7 +1271,7 @@ weston_pointer_set_default_grab(struct weston_pointer *pointer,
&default_pointer_grab_interface;
}

-WL_EXPORT struct weston_keyboard *
+static struct weston_keyboard *
weston_keyboard_create(void)
{
struct weston_keyboard *keyboard;
@@ -1297,7 +1297,7 @@ weston_keyboard_create(void)
static void
weston_xkb_info_destroy(struct weston_xkb_info *xkb_info);

-WL_EXPORT void
+static void
weston_keyboard_destroy(struct weston_keyboard *keyboard)
{
struct wl_resource *resource;
@@ -1330,7 +1330,7 @@ weston_touch_reset_state(struct weston_touch *touch)
touch->num_tp = 0;
}

-WL_EXPORT struct weston_touch *
+static struct weston_touch *
weston_touch_create(void)
{
struct weston_touch *touch;
@@ -1355,7 +1355,7 @@ weston_touch_create(void)
return touch;
}

-WL_EXPORT void
+static void
weston_touch_destroy(struct weston_touch *touch)
{
struct wl_resource *resource;
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:58 UTC
Permalink
From: Louis-Francis Ratté-Boulianne <***@collabora.com>

The touchpoint counting is needed regardless of what we do with the
touch events, so move it out of process_touch_normal() into the caller
notify_touch_cal().

This is pure refactoring.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/input.c | 42 +++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/libweston/input.c b/libweston/input.c
index 1658422c..17a0c051 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2357,10 +2357,6 @@ process_touch_normal(struct weston_touch_device *device,

switch (touch_type) {
case WL_TOUCH_DOWN:
- weston_compositor_idle_inhibit(ec);
-
- touch->num_tp++;
-
/* the first finger down picks the view, and all further go
* to that view for the remainder of the touch session i.e.
* until all touch points are up again. */
@@ -2398,17 +2394,6 @@ process_touch_normal(struct weston_touch_device *device,
grab->interface->motion(grab, time, touch_id, x, y);
break;
case WL_TOUCH_UP:
- if (touch->num_tp == 0) {
- /* This can happen if we start out with one or
- * more fingers on the touch screen, in which
- * case we didn't get the corresponding down
- * event. */
- weston_log("unmatched touch up event\n");
- break;
- }
- weston_compositor_idle_release(ec);
- touch->num_tp--;
-
grab->interface->up(grab, time, touch_id);
if (touch->num_tp == 0)
weston_touch_set_focus(touch, NULL);
@@ -2448,6 +2433,9 @@ notify_touch_cal(struct weston_touch_device *device,
double x, double y, double norm_x, double norm_y,
int touch_type)
{
+ struct weston_seat *seat = device->aggregate->seat;
+ struct weston_touch *touch = device->aggregate;
+
if (touch_type != WL_TOUCH_UP) {
if (weston_touch_device_can_calibrate(device)) {
assert(norm_x != WESTON_INVALID_TOUCH_COORDINATE);
@@ -2458,6 +2446,30 @@ notify_touch_cal(struct weston_touch_device *device,
}
}

+ /* Update touchpoints count regardless of the current mode. */
+ switch (touch_type) {
+ case WL_TOUCH_DOWN:
+ weston_compositor_idle_inhibit(seat->compositor);
+
+ touch->num_tp++;
+ break;
+ case WL_TOUCH_UP:
+ if (touch->num_tp == 0) {
+ /* This can happen if we start out with one or
+ * more fingers on the touch screen, in which
+ * case we didn't get the corresponding down
+ * event. */
+ weston_log("unmatched touch up event\n");
+ break;
+ }
+ weston_compositor_idle_release(seat->compositor);
+
+ touch->num_tp--;
+ break;
+ default:
+ break;
+ }
+
process_touch_normal(device, time, touch_id, x, y, touch_type);
}
--
2.16.1
Pekka Paalanen
2018-03-23 12:00:59 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

Commit a30e29af2e4d0ad6fc476ae7cc13c4cad5119217 introduced the code to
deal with a touchscreen with touches already down when Weston starts
using it. It fixed the touchpoint counting problem.

However, Weston still should not forward or process the unmatched
touch-ups either. Code inspection says it would confuse the
idle-inhibit counting, and it could probably confuse clients as well.
Hence, just drop unmatched touch-ups.

Enhance the warning message to allow identifying where the event came
from.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/input.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libweston/input.c b/libweston/input.c
index 17a0c051..7e4677e6 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2459,8 +2459,9 @@ notify_touch_cal(struct weston_touch_device *device,
* more fingers on the touch screen, in which
* case we didn't get the corresponding down
* event. */
- weston_log("unmatched touch up event\n");
- break;
+ weston_log("Unmatched touch up event on seat %s, device %s\n",
+ seat->seat_name, device->devpath);
+ return;
}
weston_compositor_idle_release(seat->compositor);
--
2.16.1
Pekka Paalanen
2018-03-23 12:01:02 UTC
Permalink
From: Louis-Francis Ratté-Boulianne <***@collabora.com>

This implements a new global interface weston_touch_calibration, which
allows one client at a time to perform touchscreen calibration. This
also implements the calibrator window management.

A client asks to calibrate a specific physical touch device (not a
wl_seat which may have several physical touch devices aggregated).
Libweston grabs all touch devices and prevents normal touch event
handling during the calibation sequence.

API is added to enable this new global interface, but it not yet called
by anything. Since the implementation allows clients to grab touch devices
arbitrarily, it is not enabled by default. The compositor should take
measures to prevent unexpected access to the interface.

A client may upload a new calibration to the compositor. There is a
vfunc to allow the compositor to reject/accept it and save it to
persistent storage. The persistent storage could be a udev rule
setting LIBINPUT_CALIBRATION_MATRIX, so that all display server would
load the new calibration automatically.

Co-developed by Louis-Francis and Pekka.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
Makefile.am | 1 +
libweston/compositor.h | 41 ++-
libweston/input.c | 4 +
libweston/touch-calibration.c | 667 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 711 insertions(+), 2 deletions(-)
create mode 100644 libweston/touch-calibration.c

diff --git a/Makefile.am b/Makefile.am
index 5e830777..ca878824 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -81,6 +81,7 @@ ***@LIBWESTON_MAJOR@_la_SOURCES = \
libweston/input.c \
libweston/data-device.c \
libweston/screenshooter.c \
+ libweston/touch-calibration.c \
libweston/clipboard.c \
libweston/zoom.c \
libweston/bindings.c \
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 48a9af55..71f001bd 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -521,6 +521,7 @@ struct weston_touch_device {
void *backend_data; /**< backend-specific private */

const struct weston_touch_device_ops *ops;
+ struct weston_touch_device_matrix saved_calibration;
};

/** Represents a set of touchscreen devices aggregated under a seat */
@@ -1006,6 +1007,21 @@ struct weston_backend {
const char *name);
};

+/** Callback for saving calibration
+ *
+ * \param compositor The compositor.
+ * \param device The physical touch device to save for.
+ * \param calibration The new calibration from a client.
+ * \return -1 on failure, 0 on success.
+ *
+ * Failure will prevent taking the new calibration into use.
+ */
+typedef int (*weston_touch_calibration_save_func)(
+ struct weston_compositor *compositor,
+ struct weston_touch_device *device,
+ const struct weston_touch_device_matrix *calibration);
+struct weston_touch_calibrator;
+
struct weston_desktop_xwayland;
struct weston_desktop_xwayland_interface;

@@ -1109,7 +1125,12 @@ struct weston_compositor {
weston_heads_changed_func_t heads_changed;
struct wl_event_source *heads_changed_source;

+ /* Touchscreen calibrator support: */
enum weston_touch_mode touch_mode;
+ struct wl_global *touch_calibration;
+ weston_touch_calibration_save_func touch_calibration_save;
+ struct weston_layer calibrator_layer;
+ struct weston_touch_calibrator *touch_calibrator;
};

struct weston_buffer {
@@ -1596,8 +1617,20 @@ weston_compositor_set_touch_mode_normal(struct weston_compositor *compositor);
void
weston_compositor_set_touch_mode_calib(struct weston_compositor *compositor);

-static inline void
-touch_calibrator_mode_changed(struct weston_compositor *compositor) { /* stub */ }
+void
+touch_calibrator_mode_changed(struct weston_compositor *compositor);
+
+void
+notify_touch_calibrator(struct weston_touch_device *device,
+ const struct timespec *time, int32_t slot,
+ double x, double y,
+ int touch_type);
+
+void
+notify_touch_calibrator_frame(struct weston_touch_device *device);
+
+void
+notify_touch_calibrator_cancel(struct weston_touch_device *device);

void
weston_layer_entry_insert(struct weston_layer_entry *list,
@@ -2244,6 +2277,10 @@ weston_head_from_resource(struct wl_resource *resource);
struct weston_head *
weston_output_get_first_head(struct weston_output *output);

+int
+weston_compositor_enable_touch_calibrator(struct weston_compositor *compositor,
+ weston_touch_calibration_save_func save);
+
#ifdef __cplusplus
}
#endif
diff --git a/libweston/input.c b/libweston/input.c
index 4a82203c..c6ae8fda 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2584,6 +2584,8 @@ notify_touch_cal(struct weston_touch_device *device,
break;
case WESTON_TOUCH_MODE_CALIB:
case WESTON_TOUCH_MODE_PREP_NORMAL:
+ notify_touch_calibrator(device, time, touch_id,
+ norm_x, norm_y, touch_type);
break;
}
}
@@ -2601,6 +2603,7 @@ notify_touch_frame(struct weston_touch_device *device)
break;
case WESTON_TOUCH_MODE_CALIB:
case WESTON_TOUCH_MODE_PREP_NORMAL:
+ notify_touch_calibrator_frame(device);
break;
}

@@ -2620,6 +2623,7 @@ notify_touch_cancel(struct weston_touch_device *device)
break;
case WESTON_TOUCH_MODE_CALIB:
case WESTON_TOUCH_MODE_PREP_NORMAL:
+ notify_touch_calibrator_cancel(device);
break;
}

diff --git a/libweston/touch-calibration.c b/libweston/touch-calibration.c
new file mode 100644
index 00000000..4f5ddf19
--- /dev/null
+++ b/libweston/touch-calibration.c
@@ -0,0 +1,667 @@
+/*
+ * Copyright (c) 2017 Collabora, Ltd.
+ * Copyright (c) 2017 General Electric Company
+ *
+ * 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 <string.h>
+#include <wayland-server.h>
+
+#include "shared/helpers.h"
+#include "shared/string-helpers.h"
+#include "shared/zalloc.h"
+#include "shared/timespec-util.h"
+#include "compositor.h"
+
+#include "weston-touch-calibration-server-protocol.h"
+
+struct weston_touch_calibrator {
+ struct wl_resource *resource;
+
+ struct weston_compositor *compositor;
+
+ struct weston_surface *surface;
+ struct wl_listener surface_destroy_listener;
+ struct wl_listener surface_commit_listener;
+
+ struct weston_touch_device *device;
+ struct wl_listener device_destroy_listener;
+
+ struct weston_output *output;
+ struct wl_listener output_destroy_listener;
+
+ struct weston_view *view;
+
+ bool cancelled;
+};
+
+static struct weston_touch_calibrator *
+calibrator_from_device(struct weston_touch_device *device)
+{
+ return device->aggregate->seat->compositor->touch_calibrator;
+}
+
+static uint32_t
+calcoord_from_double(double c)
+{
+ if (c < 0.0) {
+ weston_log("warning %s: %f is negative\n", __func__, c);
+ c = 0.0;
+ }
+
+ if (c > 1.0) {
+ weston_log("warning %s: %f is larger than 1.0\n", __func__, c);
+ c = 1.0;
+ }
+
+ return round(c * 0xffffffff);
+}
+
+WL_EXPORT void
+notify_touch_calibrator(struct weston_touch_device *device,
+ const struct timespec *time, int32_t slot,
+ double norm_x, double norm_y,
+ int touch_type)
+{
+ struct weston_touch_calibrator *calibrator;
+ struct wl_resource *res;
+ uint32_t msecs;
+ uint32_t x = 0;
+ uint32_t y = 0;
+
+ calibrator = calibrator_from_device(device);
+ if (!calibrator)
+ return;
+
+ res = calibrator->resource;
+
+ /* Ignore any touch events coming from another device */
+ if (device != calibrator->device) {
+ if (touch_type == WL_TOUCH_DOWN)
+ weston_touch_calibrator_send_wrong_touch(res);
+ return;
+ }
+
+ msecs = timespec_to_msec(time);
+ if (touch_type != WL_TOUCH_UP) {
+ x = calcoord_from_double(norm_x);
+ y = calcoord_from_double(norm_y);
+ }
+
+ switch (touch_type) {
+ case WL_TOUCH_UP:
+ weston_touch_calibrator_send_up(res, msecs, slot);
+ break;
+ case WL_TOUCH_DOWN:
+ weston_touch_calibrator_send_down(res, msecs, slot, x, y);
+ break;
+ case WL_TOUCH_MOTION:
+ weston_touch_calibrator_send_motion(res, msecs, slot, x, y);
+ break;
+ default:
+ return;
+ }
+}
+
+WL_EXPORT void
+notify_touch_calibrator_frame(struct weston_touch_device *device)
+{
+ struct weston_touch_calibrator *calibrator;
+
+ calibrator = calibrator_from_device(device);
+ if (!calibrator)
+ return;
+
+ weston_touch_calibrator_send_frame(calibrator->resource);
+}
+
+WL_EXPORT void
+notify_touch_calibrator_cancel(struct weston_touch_device *device)
+{
+ struct weston_touch_calibrator *calibrator;
+
+ calibrator = calibrator_from_device(device);
+ if (!calibrator)
+ return;
+
+ weston_touch_calibrator_send_cancel(calibrator->resource);
+}
+
+static void
+map_calibrator(struct weston_touch_calibrator *calibrator)
+{
+ struct weston_compositor *c = calibrator->compositor;
+ struct weston_touch_device *device = calibrator->device;
+ static const struct weston_touch_device_matrix identity = {
+ .m = { 1, 0, 0, 0, 1, 0}
+ };
+
+ assert(!calibrator->view);
+ assert(calibrator->output);
+ assert(calibrator->surface);
+ assert(calibrator->surface->resource);
+
+ calibrator->view = weston_view_create(calibrator->surface);
+ if (!calibrator->view) {
+ wl_resource_post_no_memory(calibrator->surface->resource);
+ return;
+ }
+
+ weston_layer_entry_insert(&c->calibrator_layer.view_list,
+ &calibrator->view->layer_link);
+
+ weston_view_set_position(calibrator->view,
+ calibrator->output->x,
+ calibrator->output->y);
+ calibrator->view->output = calibrator->surface->output;
+ calibrator->view->is_mapped = true;
+
+ calibrator->surface->output = calibrator->output;
+ calibrator->surface->is_mapped = true;
+
+ weston_output_schedule_repaint(calibrator->output);
+
+ device->ops->get_calibration(device, &device->saved_calibration);
+ device->ops->set_calibration(device, &identity);
+}
+
+static void
+unmap_calibrator(struct weston_touch_calibrator *calibrator)
+{
+ struct weston_touch_device *device = calibrator->device;
+
+ wl_list_remove(&calibrator->surface_commit_listener.link);
+ wl_list_init(&calibrator->surface_commit_listener.link);
+
+ if (!calibrator->view)
+ return;
+
+ weston_view_destroy(calibrator->view);
+ calibrator->view = NULL;
+ weston_surface_unmap(calibrator->surface);
+
+ /* Reload saved calibration */
+ if (device)
+ device->ops->set_calibration(device,
+ &device->saved_calibration);
+}
+
+void
+touch_calibrator_mode_changed(struct weston_compositor *compositor)
+{
+ struct weston_touch_calibrator *calibrator;
+
+ calibrator = compositor->touch_calibrator;
+ if (!calibrator)
+ return;
+
+ if (calibrator->cancelled)
+ return;
+
+ if (compositor->touch_mode == WESTON_TOUCH_MODE_CALIB)
+ map_calibrator(calibrator);
+}
+
+static void
+touch_calibrator_surface_committed(struct wl_listener *listener, void *data)
+{
+ struct weston_touch_calibrator *calibrator =
+ container_of(listener, struct weston_touch_calibrator,
+ surface_commit_listener);
+ struct weston_surface *surface = calibrator->surface;
+
+ wl_list_remove(&calibrator->surface_commit_listener.link);
+ wl_list_init(&calibrator->surface_commit_listener.link);
+
+ if (surface->width != calibrator->output->width ||
+ surface->height != calibrator->output->height) {
+ wl_resource_post_error(calibrator->resource,
+ WESTON_TOUCH_CALIBRATOR_ERROR_BAD_SIZE,
+ "calibrator surface size does not match");
+ return;
+ }
+
+ weston_compositor_set_touch_mode_calib(calibrator->compositor);
+ /* results in call to touch_calibrator_mode_changed() */
+}
+
+static void
+touch_calibrator_convert(struct wl_client *client,
+ struct wl_resource *resource,
+ int32_t x,
+ int32_t y,
+ uint32_t coordinate_id)
+{
+ struct weston_touch_calibrator *calibrator;
+ struct wl_resource *coordinate_resource;
+ struct weston_output *output;
+ uint32_t version;
+ struct weston_vector p = { { 0.0, 0.0, 0.0, 1.0 } };
+ double norm_x, norm_y;
+
+ version = wl_resource_get_version(resource);
+ calibrator = wl_resource_get_user_data(resource);
+ output = calibrator->output;
+
+ if (!calibrator->surface || !calibrator->surface->is_mapped) {
+ wl_resource_post_error(resource,
+ WESTON_TOUCH_CALIBRATOR_ERROR_NOT_MAPPED,
+ "calibrator surface is not mapped");
+ return;
+ }
+ assert(calibrator->view);
+ assert(output);
+
+ /* Convert from surface-local coordinates into global, from global
+ * into output-raw, do perspective division and normalize.
+ */
+ weston_view_to_global_float(calibrator->view, x, y, &p.f[0], &p.f[1]);
+ weston_matrix_transform(&output->matrix, &p);
+ norm_x = p.f[0] / (p.f[3] * output->current_mode->width);
+ norm_y = p.f[1] / (p.f[3] * output->current_mode->height);
+
+ coordinate_resource =
+ wl_resource_create(client, &weston_touch_coordinate_interface,
+ version, coordinate_id);
+ if (!coordinate_resource) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ weston_touch_coordinate_send_result(coordinate_resource,
+ calcoord_from_double(norm_x),
+ calcoord_from_double(norm_y));
+ wl_resource_destroy(coordinate_resource);
+}
+
+static void
+destroy_touch_calibrator(struct wl_resource *resource)
+{
+ struct weston_touch_calibrator *calibrator;
+
+ calibrator = wl_resource_get_user_data(resource);
+
+ calibrator->compositor->touch_calibrator = NULL;
+
+ weston_compositor_set_touch_mode_normal(calibrator->compositor);
+
+ if (calibrator->surface) {
+ unmap_calibrator(calibrator);
+ weston_surface_set_role(calibrator->surface, NULL,
+ calibrator->surface->resource, 0);
+ wl_list_remove(&calibrator->surface_destroy_listener.link);
+ wl_list_remove(&calibrator->surface_commit_listener.link);
+ }
+
+ if (calibrator->device)
+ wl_list_remove(&calibrator->device_destroy_listener.link);
+
+ if (calibrator->output)
+ wl_list_remove(&calibrator->output_destroy_listener.link);
+
+ free(calibrator);
+}
+
+static void
+touch_calibrator_destroy(struct wl_client *client,
+ struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+}
+
+static const struct weston_touch_calibrator_interface
+touch_calibrator_implementation = {
+ touch_calibrator_destroy,
+ touch_calibrator_convert
+};
+
+static void
+touch_calibrator_cancel_calibration(struct weston_touch_calibrator *calibrator)
+{
+ weston_touch_calibrator_send_cancel_calibration(calibrator->resource);
+ calibrator->cancelled = true;
+
+ if (calibrator->surface)
+ unmap_calibrator(calibrator);
+}
+
+static void
+touch_calibrator_output_destroyed(struct wl_listener *listener, void *data)
+{
+ struct weston_touch_calibrator *calibrator =
+ container_of(listener, struct weston_touch_calibrator,
+ output_destroy_listener);
+
+ assert(calibrator->output == data);
+ calibrator->output = NULL;
+
+ touch_calibrator_cancel_calibration(calibrator);
+}
+
+static void
+touch_calibrator_device_destroyed(struct wl_listener *listener, void *data)
+{
+ struct weston_touch_calibrator *calibrator =
+ container_of(listener, struct weston_touch_calibrator,
+ device_destroy_listener);
+
+ assert(calibrator->device == data);
+ calibrator->device = NULL;
+
+ touch_calibrator_cancel_calibration(calibrator);
+}
+
+static void
+touch_calibrator_surface_destroyed(struct wl_listener *listener, void *data)
+{
+ struct weston_touch_calibrator *calibrator =
+ container_of(listener, struct weston_touch_calibrator,
+ surface_destroy_listener);
+
+ assert(calibrator->surface->resource == data);
+
+ unmap_calibrator(calibrator);
+ calibrator->surface = NULL;
+}
+
+static void
+touch_calibration_destroy(struct wl_client *client,
+ struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+}
+
+static struct weston_touch_device *
+weston_compositor_find_touch_device_by_devpath(
+ struct weston_compositor *compositor,
+ const char *devpath)
+{
+ struct weston_seat *seat;
+ struct weston_touch *touch;
+ struct weston_touch_device *device;
+
+ if (!devpath)
+ return NULL;
+
+ wl_list_for_each(seat, &compositor->seat_list, link) {
+ touch = weston_seat_get_touch(seat);
+ if (!touch)
+ continue;
+
+ wl_list_for_each(device, &touch->device_list, link) {
+ if (strcmp(device->devpath, devpath) == 0)
+ return device;
+ }
+ }
+
+ return NULL;
+}
+
+static void
+touch_calibration_create_calibrator(
+ struct wl_client *client,
+ struct wl_resource *touch_calibration_resource,
+ struct wl_resource *surface_resource,
+ const char *devpath,
+ uint32_t calibrator_id)
+{
+ struct weston_compositor *compositor;
+ struct weston_touch_calibrator *calibrator;
+ struct weston_touch_device *device;
+ struct weston_output *output = NULL;
+ struct weston_surface *surface;
+ uint32_t version;
+ int ret;
+
+ version = wl_resource_get_version(touch_calibration_resource);
+ compositor = wl_resource_get_user_data(touch_calibration_resource);
+
+ if (compositor->touch_calibrator != NULL) {
+ wl_resource_post_error(touch_calibration_resource,
+ WESTON_TOUCH_CALIBRATION_ERROR_ALREADY_EXISTS,
+ "a calibrator has already been created");
+ return;
+ }
+
+ calibrator = zalloc(sizeof *calibrator);
+ if (!calibrator) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ calibrator->compositor = compositor;
+ calibrator->resource = wl_resource_create(client,
+ &weston_touch_calibrator_interface,
+ version, calibrator_id);
+ if (!calibrator->resource) {
+ wl_client_post_no_memory(client);
+ goto err_dealloc;
+ }
+
+ surface = wl_resource_get_user_data(surface_resource);
+ assert(surface);
+ ret = weston_surface_set_role(surface, "weston_touch_calibrator",
+ touch_calibration_resource,
+ WESTON_TOUCH_CALIBRATION_ERROR_INVALID_SURFACE);
+ if (ret < 0)
+ goto err_destroy_resource;
+
+ calibrator->surface_destroy_listener.notify =
+ touch_calibrator_surface_destroyed;
+ wl_resource_add_destroy_listener(surface->resource,
+ &calibrator->surface_destroy_listener);
+ calibrator->surface = surface;
+
+ calibrator->surface_commit_listener.notify =
+ touch_calibrator_surface_committed;
+ wl_signal_add(&surface->commit_signal,
+ &calibrator->surface_commit_listener);
+
+ device = weston_compositor_find_touch_device_by_devpath(compositor,
+ devpath);
+ if (device) {
+ output = device->ops->get_output(device);
+ if (weston_touch_device_can_calibrate(device) && output)
+ calibrator->device = device;
+ }
+
+ if (!calibrator->device) {
+ wl_resource_post_error(touch_calibration_resource,
+ WESTON_TOUCH_CALIBRATION_ERROR_INVALID_DEVICE,
+ "the given touch device '%s' is not valid",
+ devpath ?: "");
+ goto err_unlink_surface;
+ }
+
+ calibrator->device_destroy_listener.notify =
+ touch_calibrator_device_destroyed;
+ wl_signal_add(&calibrator->device->destroy_signal,
+ &calibrator->device_destroy_listener);
+
+ wl_resource_set_implementation(calibrator->resource,
+ &touch_calibrator_implementation,
+ calibrator, destroy_touch_calibrator);
+
+ assert(output);
+ calibrator->output_destroy_listener.notify =
+ touch_calibrator_output_destroyed;
+ wl_signal_add(&output->destroy_signal,
+ &calibrator->output_destroy_listener);
+ calibrator->output = output;
+
+ weston_touch_calibrator_send_configure(calibrator->resource,
+ output->width,
+ output->height);
+
+ compositor->touch_calibrator = calibrator;
+
+ return;
+
+err_unlink_surface:
+ wl_list_remove(&calibrator->surface_commit_listener.link);
+ wl_list_remove(&calibrator->surface_destroy_listener.link);
+
+err_destroy_resource:
+ wl_resource_destroy(calibrator->resource);
+
+err_dealloc:
+ free(calibrator);
+}
+
+static void
+touch_calibration_save(struct wl_client *client,
+ struct wl_resource *touch_calibration_resource,
+ const char *device_name,
+ struct wl_array *matrix_data)
+{
+ struct weston_touch_device *device;
+ struct weston_compositor *compositor;
+ struct weston_touch_device_matrix calibration;
+ struct weston_touch_calibrator *calibrator;
+ int i = 0;
+ float *c;
+
+ compositor = wl_resource_get_user_data(touch_calibration_resource);
+
+ device = weston_compositor_find_touch_device_by_devpath(compositor,
+ device_name);
+ if (!device || !weston_touch_device_can_calibrate(device)) {
+ wl_resource_post_error(touch_calibration_resource,
+ WESTON_TOUCH_CALIBRATION_ERROR_INVALID_DEVICE,
+ "the given device is not valid");
+ return;
+ }
+
+ wl_array_for_each(c, matrix_data) {
+ calibration.m[i++] = *c;
+ }
+
+ /* If calibration can't be saved, don't set it as current */
+ if (compositor->touch_calibration_save &&
+ compositor->touch_calibration_save(compositor, device,
+ &calibration) < 0)
+ return;
+
+ /* If calibrator is still mapped, the compositor will use
+ * saved_calibration when going back to normal touch handling.
+ * Continuing calibrating after save request is undefined. */
+ calibrator = compositor->touch_calibrator;
+ if (calibrator &&
+ calibrator->surface &&
+ weston_surface_is_mapped(calibrator->surface))
+ device->saved_calibration = calibration;
+ else
+ device->ops->set_calibration(device, &calibration);
+}
+
+static const struct weston_touch_calibration_interface
+touch_calibration_implementation = {
+ touch_calibration_destroy,
+ touch_calibration_create_calibrator,
+ touch_calibration_save
+};
+
+static void
+bind_touch_calibration(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct weston_compositor *compositor = data;
+ struct wl_resource *resource;
+ struct weston_touch_device *device;
+ struct weston_seat *seat;
+ struct weston_touch *touch;
+ const char *name;
+
+ resource = wl_resource_create(client,
+ &weston_touch_calibration_interface,
+ version, id);
+ if (resource == NULL) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ wl_resource_set_implementation(resource,
+ &touch_calibration_implementation,
+ compositor, NULL);
+
+ wl_list_for_each(seat, &compositor->seat_list, link) {
+ touch = weston_seat_get_touch(seat);
+ if (!touch)
+ continue;
+
+ wl_list_for_each(device, &touch->device_list, link) {
+ if (!weston_touch_device_can_calibrate(device))
+ continue;
+
+ name = device->ops->get_calibration_head_name(device);
+ if (!name)
+ continue;
+
+ weston_touch_calibration_send_touch_device(resource,
+ device->devpath, name);
+ }
+ }
+}
+
+/** Advertise touch_calibration support
+ *
+ * \param compositor The compositor to init for.
+ * \param save The callback function for saving a new calibration, or NULL.
+ * \return Zero on success, -1 on failure or if already enabled.
+ *
+ * Calling this initializes the weston_touch_calibration protocol support,
+ * so that the interface will be advertised to clients. It is recommended
+ * to use some mechanism, e.g. wl_display_set_global_filter(), to restrict
+ * access to the interface.
+ *
+ * There is no way to disable this once enabled.
+ *
+ * If the save callback is NULL, a new calibration provided by a client will
+ * always be accepted. If the save callback is not NULL, it must return
+ * success for the new calibration to be accepted.
+ */
+WL_EXPORT int
+weston_compositor_enable_touch_calibrator(struct weston_compositor *compositor,
+ weston_touch_calibration_save_func save)
+{
+ if (compositor->touch_calibration)
+ return -1;
+
+ compositor->touch_calibration = wl_global_create(compositor->wl_display,
+ &weston_touch_calibration_interface, 1,
+ compositor, bind_touch_calibration);
+ if (!compositor->touch_calibration)
+ return -1;
+
+ compositor->touch_calibration_save = save;
+ weston_layer_init(&compositor->calibrator_layer, compositor);
+
+ /* needs to be stacked above everything except lock screen and cursor,
+ * otherwise the position value is arbitrary */
+ weston_layer_set_position(&compositor->calibrator_layer,
+ WESTON_LAYER_POSITION_TOP_UI + 120);
+
+ return 0;
+}
--
2.16.1
Pekka Paalanen
2018-03-23 12:01:00 UTC
Permalink
From: Louis-Francis Ratté-Boulianne <***@collabora.com>

In addition to the normal touch event processing mode, introduce a new
mode for calibrating a touchscreen input device.

In the calibration mode, normal touch event processing is skipped, and
the raw events are forwarded to the calibrator instead. The calibrator
is not yet implemented, so the calls will be added in a following patch.

To switch between modes, two functions are added, one for entering each
mode. The mode switch happens only when no touches are down on any touch
device, to avoid confusing touch grabs and clients. To realise this, the
state machine has four states: prepare and actual state for both normal
and calibrator modes.

At this point nothing will attempt to change the touch event mode.

The new calibrator mode is necessary, because when calibrating a
touchscreen, the touch events must be routed to the calibration client
directly. The touch coordinates are expected to be wrong, so they cannot
go through the normal focus surface picking. The calibrator code also
cannot use the normal touch grab interface, because it needs to be able
to distinguish between different physical touch input devices, even if
they are part of the same weston_seat. This requirement makes
calibration special enough to warrant the new mode, a sort of "super
grab".

Co-developed by Louis-Francis and Pekka.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
libweston/compositor.c | 2 +
libweston/compositor.h | 39 +++++++++++++
libweston/input.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 182 insertions(+), 5 deletions(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 8452c5d2..d7922799 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -6289,6 +6289,8 @@ weston_compositor_create(struct wl_display *display, void *user_data)

ec->activate_serial = 1;

+ ec->touch_mode = WESTON_TOUCH_MODE_NORMAL;
+
if (!wl_global_create(ec->wl_display, &wl_compositor_interface, 4,
ec, compositor_bind))
goto fail;
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 129e15b8..48a9af55 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -482,6 +482,34 @@ struct weston_touch_device_ops {
const struct weston_touch_device_matrix *matrix);
};

+enum weston_touch_mode {
+ /** Normal touch event handling */
+ WESTON_TOUCH_MODE_NORMAL,
+
+ /** Prepare moving to WESTON_TOUCH_MODE_CALIB.
+ *
+ * Move to WESTON_TOUCH_MODE_CALIB as soon as no touches are down on
+ * any seat. Until then, all touch events are routed normally.
+ */
+ WESTON_TOUCH_MODE_PREP_CALIB,
+
+ /** Calibration mode
+ *
+ * Only a single weston_touch_device forwards events to the calibrator
+ * all other touch device cause a calibrator "wrong device" event to
+ * be sent.
+ */
+ WESTON_TOUCH_MODE_CALIB,
+
+ /** Prepare moving to WESTON_TOUCH_MODE_NORMAL.
+ *
+ * Move to WESTON_TOUCH_MODE_NORMAL as soon as no touches are down on
+ * any seat. Until then, touch events are routed as in
+ * WESTON_TOUCH_MODE_CALIB except "wrong device" events are not sent.
+ */
+ WESTON_TOUCH_MODE_PREP_NORMAL
+};
+
/** Represents a physical touchscreen input device */
struct weston_touch_device {
char *devpath; /**< unique name */
@@ -1080,6 +1108,8 @@ struct weston_compositor {

weston_heads_changed_func_t heads_changed;
struct wl_event_source *heads_changed_source;
+
+ enum weston_touch_mode touch_mode;
};

struct weston_buffer {
@@ -1560,6 +1590,15 @@ notify_touch_frame(struct weston_touch_device *device);
void
notify_touch_cancel(struct weston_touch_device *device);

+void
+weston_compositor_set_touch_mode_normal(struct weston_compositor *compositor);
+
+void
+weston_compositor_set_touch_mode_calib(struct weston_compositor *compositor);
+
+static inline void
+touch_calibrator_mode_changed(struct weston_compositor *compositor) { /* stub */ }
+
void
weston_layer_entry_insert(struct weston_layer_entry *list,
struct weston_layer_entry *entry);
diff --git a/libweston/input.c b/libweston/input.c
index 7e4677e6..4a82203c 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -185,6 +185,12 @@ weston_touch_device_can_calibrate(struct weston_touch_device *device)
return !!device->ops;
}

+static enum weston_touch_mode
+weston_touch_device_get_mode(struct weston_touch_device *device)
+{
+ return device->aggregate->seat->compositor->touch_mode;
+}
+
static struct weston_pointer_client *
weston_pointer_client_create(struct wl_client *client)
{
@@ -2401,6 +2407,105 @@ process_touch_normal(struct weston_touch_device *device,
}
}

+static enum weston_touch_mode
+get_next_touch_mode(enum weston_touch_mode from)
+{
+ switch (from) {
+ case WESTON_TOUCH_MODE_PREP_NORMAL:
+ return WESTON_TOUCH_MODE_NORMAL;
+
+ case WESTON_TOUCH_MODE_PREP_CALIB:
+ return WESTON_TOUCH_MODE_CALIB;
+
+ case WESTON_TOUCH_MODE_NORMAL:
+ case WESTON_TOUCH_MODE_CALIB:
+ return from;
+ }
+
+ return WESTON_TOUCH_MODE_NORMAL;
+}
+
+/** Global touch mode update
+ *
+ * If no seat has a touch down and the compositor is in a PREP touch mode,
+ * set the compositor to the goal touch mode.
+ *
+ * Calls calibrator if touch mode changed.
+ */
+static void
+weston_compositor_update_touch_mode(struct weston_compositor *compositor)
+{
+ struct weston_seat *seat;
+ struct weston_touch *touch;
+ enum weston_touch_mode goal;
+
+ wl_list_for_each(seat, &compositor->seat_list, link) {
+ touch = weston_seat_get_touch(seat);
+ if (!touch)
+ continue;
+
+ if (touch->num_tp > 0)
+ return;
+ }
+
+ goal = get_next_touch_mode(compositor->touch_mode);
+ if (compositor->touch_mode != goal) {
+ compositor->touch_mode = goal;
+ touch_calibrator_mode_changed(compositor);
+ }
+}
+
+/** Start transition to normal touch event handling
+ *
+ * The touch event mode changes when all touches on all touch devices have
+ * been lifted. If no touches are currently down, the transition is immediate.
+ *
+ * \sa weston_touch_mode
+ */
+void
+weston_compositor_set_touch_mode_normal(struct weston_compositor *compositor)
+{
+ switch (compositor->touch_mode) {
+ case WESTON_TOUCH_MODE_PREP_NORMAL:
+ case WESTON_TOUCH_MODE_NORMAL:
+ return;
+ case WESTON_TOUCH_MODE_PREP_CALIB:
+ compositor->touch_mode = WESTON_TOUCH_MODE_NORMAL;
+ touch_calibrator_mode_changed(compositor);
+ return;
+ case WESTON_TOUCH_MODE_CALIB:
+ compositor->touch_mode = WESTON_TOUCH_MODE_PREP_NORMAL;
+ }
+
+ weston_compositor_update_touch_mode(compositor);
+}
+
+/** Start transition to calibrator touch event handling
+ *
+ * The touch event mode changes when all touches on all touch devices have
+ * been lifted. If no touches are currently down, the transition is immediate.
+ *
+ * \sa weston_touch_mode
+ */
+void
+weston_compositor_set_touch_mode_calib(struct weston_compositor *compositor)
+{
+ switch (compositor->touch_mode) {
+ case WESTON_TOUCH_MODE_PREP_CALIB:
+ case WESTON_TOUCH_MODE_CALIB:
+ assert(0);
+ return;
+ case WESTON_TOUCH_MODE_PREP_NORMAL:
+ compositor->touch_mode = WESTON_TOUCH_MODE_CALIB;
+ touch_calibrator_mode_changed(compositor);
+ return;
+ case WESTON_TOUCH_MODE_NORMAL:
+ compositor->touch_mode = WESTON_TOUCH_MODE_PREP_CALIB;
+ }
+
+ weston_compositor_update_touch_mode(compositor);
+}
+
/** Feed in touch down, motion, and up events, calibratable device.
*
* It assumes always the correct cycle sequence until it gets here: touch_down
@@ -2471,23 +2576,54 @@ notify_touch_cal(struct weston_touch_device *device,
break;
}

- process_touch_normal(device, time, touch_id, x, y, touch_type);
+ /* Properly forward the touch event */
+ switch (weston_touch_device_get_mode(device)) {
+ case WESTON_TOUCH_MODE_NORMAL:
+ case WESTON_TOUCH_MODE_PREP_CALIB:
+ process_touch_normal(device, time, touch_id, x, y, touch_type);
+ break;
+ case WESTON_TOUCH_MODE_CALIB:
+ case WESTON_TOUCH_MODE_PREP_NORMAL:
+ break;
+ }
}

WL_EXPORT void
notify_touch_frame(struct weston_touch_device *device)
{
- struct weston_touch_grab *grab = device->aggregate->grab;
+ struct weston_touch_grab *grab;

- grab->interface->frame(grab);
+ switch (weston_touch_device_get_mode(device)) {
+ case WESTON_TOUCH_MODE_NORMAL:
+ case WESTON_TOUCH_MODE_PREP_CALIB:
+ grab = device->aggregate->grab;
+ grab->interface->frame(grab);
+ break;
+ case WESTON_TOUCH_MODE_CALIB:
+ case WESTON_TOUCH_MODE_PREP_NORMAL:
+ break;
+ }
+
+ weston_compositor_update_touch_mode(device->aggregate->seat->compositor);
}

WL_EXPORT void
notify_touch_cancel(struct weston_touch_device *device)
{
- struct weston_touch_grab *grab = device->aggregate->grab;
+ struct weston_touch_grab *grab;
+
+ switch (weston_touch_device_get_mode(device)) {
+ case WESTON_TOUCH_MODE_NORMAL:
+ case WESTON_TOUCH_MODE_PREP_CALIB:
+ grab = device->aggregate->grab;
+ grab->interface->cancel(grab);
+ break;
+ case WESTON_TOUCH_MODE_CALIB:
+ case WESTON_TOUCH_MODE_PREP_NORMAL:
+ break;
+ }

- grab->interface->cancel(grab);
+ weston_compositor_update_touch_mode(device->aggregate->seat->compositor);
}

static int
--
2.16.1
Peter Hutterer
2018-04-09 04:24:59 UTC
Permalink
Post by Pekka Paalanen
In addition to the normal touch event processing mode, introduce a new
mode for calibrating a touchscreen input device.
In the calibration mode, normal touch event processing is skipped, and
the raw events are forwarded to the calibrator instead. The calibrator
is not yet implemented, so the calls will be added in a following patch.
To switch between modes, two functions are added, one for entering each
mode. The mode switch happens only when no touches are down on any touch
device, to avoid confusing touch grabs and clients. To realise this, the
state machine has four states: prepare and actual state for both normal
and calibrator modes.
At this point nothing will attempt to change the touch event mode.
The new calibrator mode is necessary, because when calibrating a
touchscreen, the touch events must be routed to the calibration client
directly. The touch coordinates are expected to be wrong, so they cannot
go through the normal focus surface picking. The calibrator code also
cannot use the normal touch grab interface, because it needs to be able
to distinguish between different physical touch input devices, even if
they are part of the same weston_seat. This requirement makes
calibration special enough to warrant the new mode, a sort of "super
grab".
Co-developed by Louis-Francis and Pekka.
series 13-20 Reviewed-by: Peter Hutterer <***@who-t.net>, pls see
my comments in the emails though.

Cheers,
Peter
Post by Pekka Paalanen
---
libweston/compositor.c | 2 +
libweston/compositor.h | 39 +++++++++++++
libweston/input.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 182 insertions(+), 5 deletions(-)
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 8452c5d2..d7922799 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -6289,6 +6289,8 @@ weston_compositor_create(struct wl_display *display, void *user_data)
ec->activate_serial = 1;
+ ec->touch_mode = WESTON_TOUCH_MODE_NORMAL;
+
if (!wl_global_create(ec->wl_display, &wl_compositor_interface, 4,
ec, compositor_bind))
goto fail;
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 129e15b8..48a9af55 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -482,6 +482,34 @@ struct weston_touch_device_ops {
const struct weston_touch_device_matrix *matrix);
};
+enum weston_touch_mode {
+ /** Normal touch event handling */
+ WESTON_TOUCH_MODE_NORMAL,
+
+ /** Prepare moving to WESTON_TOUCH_MODE_CALIB.
+ *
+ * Move to WESTON_TOUCH_MODE_CALIB as soon as no touches are down on
+ * any seat. Until then, all touch events are routed normally.
+ */
+ WESTON_TOUCH_MODE_PREP_CALIB,
+
+ /** Calibration mode
+ *
+ * Only a single weston_touch_device forwards events to the calibrator
+ * all other touch device cause a calibrator "wrong device" event to
+ * be sent.
+ */
+ WESTON_TOUCH_MODE_CALIB,
+
+ /** Prepare moving to WESTON_TOUCH_MODE_NORMAL.
+ *
+ * Move to WESTON_TOUCH_MODE_NORMAL as soon as no touches are down on
+ * any seat. Until then, touch events are routed as in
+ * WESTON_TOUCH_MODE_CALIB except "wrong device" events are not sent.
+ */
+ WESTON_TOUCH_MODE_PREP_NORMAL
+};
+
/** Represents a physical touchscreen input device */
struct weston_touch_device {
char *devpath; /**< unique name */
@@ -1080,6 +1108,8 @@ struct weston_compositor {
weston_heads_changed_func_t heads_changed;
struct wl_event_source *heads_changed_source;
+
+ enum weston_touch_mode touch_mode;
};
struct weston_buffer {
@@ -1560,6 +1590,15 @@ notify_touch_frame(struct weston_touch_device *device);
void
notify_touch_cancel(struct weston_touch_device *device);
+void
+weston_compositor_set_touch_mode_normal(struct weston_compositor *compositor);
+
+void
+weston_compositor_set_touch_mode_calib(struct weston_compositor *compositor);
+
+static inline void
+touch_calibrator_mode_changed(struct weston_compositor *compositor) { /* stub */ }
+
void
weston_layer_entry_insert(struct weston_layer_entry *list,
struct weston_layer_entry *entry);
diff --git a/libweston/input.c b/libweston/input.c
index 7e4677e6..4a82203c 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -185,6 +185,12 @@ weston_touch_device_can_calibrate(struct weston_touch_device *device)
return !!device->ops;
}
+static enum weston_touch_mode
+weston_touch_device_get_mode(struct weston_touch_device *device)
+{
+ return device->aggregate->seat->compositor->touch_mode;
+}
+
static struct weston_pointer_client *
weston_pointer_client_create(struct wl_client *client)
{
@@ -2401,6 +2407,105 @@ process_touch_normal(struct weston_touch_device *device,
}
}
+static enum weston_touch_mode
+get_next_touch_mode(enum weston_touch_mode from)
+{
+ switch (from) {
+ return WESTON_TOUCH_MODE_NORMAL;
+
+ return WESTON_TOUCH_MODE_CALIB;
+
+ return from;
+ }
+
+ return WESTON_TOUCH_MODE_NORMAL;
+}
+
+/** Global touch mode update
+ *
+ * If no seat has a touch down and the compositor is in a PREP touch mode,
+ * set the compositor to the goal touch mode.
+ *
+ * Calls calibrator if touch mode changed.
+ */
+static void
+weston_compositor_update_touch_mode(struct weston_compositor *compositor)
+{
+ struct weston_seat *seat;
+ struct weston_touch *touch;
+ enum weston_touch_mode goal;
+
+ wl_list_for_each(seat, &compositor->seat_list, link) {
+ touch = weston_seat_get_touch(seat);
+ if (!touch)
+ continue;
+
+ if (touch->num_tp > 0)
+ return;
+ }
+
+ goal = get_next_touch_mode(compositor->touch_mode);
+ if (compositor->touch_mode != goal) {
+ compositor->touch_mode = goal;
+ touch_calibrator_mode_changed(compositor);
+ }
+}
+
+/** Start transition to normal touch event handling
+ *
+ * The touch event mode changes when all touches on all touch devices have
+ * been lifted. If no touches are currently down, the transition is immediate.
+ *
+ * \sa weston_touch_mode
+ */
+void
+weston_compositor_set_touch_mode_normal(struct weston_compositor *compositor)
+{
+ switch (compositor->touch_mode) {
+ return;
+ compositor->touch_mode = WESTON_TOUCH_MODE_NORMAL;
+ touch_calibrator_mode_changed(compositor);
+ return;
+ compositor->touch_mode = WESTON_TOUCH_MODE_PREP_NORMAL;
+ }
+
+ weston_compositor_update_touch_mode(compositor);
+}
+
+/** Start transition to calibrator touch event handling
+ *
+ * The touch event mode changes when all touches on all touch devices have
+ * been lifted. If no touches are currently down, the transition is immediate.
+ *
+ * \sa weston_touch_mode
+ */
+void
+weston_compositor_set_touch_mode_calib(struct weston_compositor *compositor)
+{
+ switch (compositor->touch_mode) {
+ assert(0);
+ return;
+ compositor->touch_mode = WESTON_TOUCH_MODE_CALIB;
+ touch_calibrator_mode_changed(compositor);
+ return;
+ compositor->touch_mode = WESTON_TOUCH_MODE_PREP_CALIB;
+ }
+
+ weston_compositor_update_touch_mode(compositor);
+}
+
/** Feed in touch down, motion, and up events, calibratable device.
*
* It assumes always the correct cycle sequence until it gets here: touch_down
@@ -2471,23 +2576,54 @@ notify_touch_cal(struct weston_touch_device *device,
break;
}
- process_touch_normal(device, time, touch_id, x, y, touch_type);
+ /* Properly forward the touch event */
+ switch (weston_touch_device_get_mode(device)) {
+ process_touch_normal(device, time, touch_id, x, y, touch_type);
+ break;
+ break;
+ }
}
WL_EXPORT void
notify_touch_frame(struct weston_touch_device *device)
{
- struct weston_touch_grab *grab = device->aggregate->grab;
+ struct weston_touch_grab *grab;
- grab->interface->frame(grab);
+ switch (weston_touch_device_get_mode(device)) {
+ grab = device->aggregate->grab;
+ grab->interface->frame(grab);
+ break;
+ break;
+ }
+
+ weston_compositor_update_touch_mode(device->aggregate->seat->compositor);
}
WL_EXPORT void
notify_touch_cancel(struct weston_touch_device *device)
{
- struct weston_touch_grab *grab = device->aggregate->grab;
+ struct weston_touch_grab *grab;
+
+ switch (weston_touch_device_get_mode(device)) {
+ grab = device->aggregate->grab;
+ grab->interface->cancel(grab);
+ break;
+ break;
+ }
- grab->interface->cancel(grab);
+ weston_compositor_update_touch_mode(device->aggregate->seat->compositor);
}
static int
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-03-23 12:01:01 UTC
Permalink
From: Pekka Paalanen <***@collabora.co.uk>

This is a Wayland protocol extension to allow the calibration of
touchscreens in Weston.

See: https://phabricator.freedesktop.org/T7868

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
Makefile.am | 5 +-
protocol/weston-touch-calibration.xml | 320 ++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 protocol/weston-touch-calibration.xml

diff --git a/Makefile.am b/Makefile.am
index e028a2a1..5e830777 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -168,7 +168,9 @@ ***@LIBWESTON_MAJOR@_la_SOURCES = \
protocol/pointer-constraints-unstable-v1-protocol.c \
protocol/pointer-constraints-unstable-v1-server-protocol.h \
protocol/input-timestamps-unstable-v1-protocol.c \
- protocol/input-timestamps-unstable-v1-server-protocol.h
+ protocol/input-timestamps-unstable-v1-server-protocol.h \
+ protocol/weston-touch-calibration-protocol.c \
+ protocol/weston-touch-calibration-server-protocol.h

BUILT_SOURCES += $(***@LIBWESTON_MAJOR@_la_SOURCES)

@@ -1534,6 +1536,7 @@ EXTRA_DIST += \
protocol/weston-screenshooter.xml \
protocol/text-cursor-position.xml \
protocol/weston-test.xml \
+ protocol/weston-touch-calibration.xml \
protocol/ivi-application.xml \
protocol/ivi-hmi-controller.xml

diff --git a/protocol/weston-touch-calibration.xml b/protocol/weston-touch-calibration.xml
new file mode 100644
index 00000000..b1e19f9b
--- /dev/null
+++ b/protocol/weston-touch-calibration.xml
@@ -0,0 +1,320 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="weston_touch_calibration">
+
+ <copyright>
+ Copyright © 2017 Collabora, Ltd.
+ Copyright © 2017 General Electric Company
+
+ 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.
+ </copyright>
+
+ <interface name="weston_touch_calibration" version="1">
+ <description summary="weston touchscreen calibration interface">
+ This is the global interface for calibrating a touchscreen input
+ coordinate transformation. It is recommended to make this interface
+ privileged.
+
+ This interface can be used by a client to show a calibration pattern and
+ receive uncalibrated touch coordinates, facilitating the computation of
+ a calibration transformation that will align actual touch positions
+ on screen with their expected coordinates.
+
+ Immediately after being bound by a client, the server sends the
+ touch_device events.
+
+ The client chooses a touch device from the touch_device events,
+ creates a wl_surface and then a weston_touch_calibrator for the
+ wl_surface and the chosen touch device. The client waits for the server
+ to send a configure event before it starts drawing the first calibration
+ pattern. After receiving the configure event, the client will iterate
+ drawing a pattern, getting touch input via weston_touch_calibrator,
+ and converting pixel coordinates to expected touch coordinates with
+ weston_touch_calibrator.convert until it has enough correspondences to
+ compute the calibration transformation or the server cancels the
+ calibration.
+
+ Once the client has successfully computed a new calibration, it can use
+ weston_touch_calibration.save request to load the new calibration into
+ the server. The server may take this new calibration into use and may
+ write it into persistent storage.
+ </description>
+
+ <enum name="error">
+ <description summary="global interface errors"/>
+ <entry name="invalid_surface" value="0"
+ summary="the given wl_surface already has a role"/>
+ <entry name="invalid_device" value="1"
+ summary="the given device is not valid"/>
+ <entry name="already_exists" value="2"
+ summary="a calibrator has already been created"/>
+ </enum>
+
+ <request name="destroy" type="destructor">
+ <description summary="unbind">
+ Destroy the binding to the global interface, does not affect any
+ objects already created through this interface.
+ </description>
+ </request>
+
+ <request name="create_calibrator">
+ <description summary="give the calibrator role to a surface">
+ This gives the calibrator role to the surface and ties it with the
+ given touch input device.
+
+ The surface must not already have a role, otherwise invalid_surface
+ error is raised.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ There must not exist a weston_touch_calibrator protocol object in the
+ server already, otherwise already_exists error is raised. This
+ limitation is server-wide and not specific to any particular client.
+ </description>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface to give the role to"/>
+ <arg name="device" type="string" summary="the touch device to calibrate"/>
+ <arg name="cal" type="new_id" interface="weston_touch_calibrator"
+ summary="a new calibrator object"/>
+ </request>
+
+ <request name="save">
+ <description summary="save calibration for a touch device">
+ This request asks the server to save the calibration data for the
+ given touch input device. The server may ignore this request.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ The array must contain exactly six 'float' (the 32-bit floating
+ point format used by the C language on the system) numbers. The numbers
+ form a libinput style 2-by-3 calibration matrix in row-major order.
+ </description>
+ <arg name="device" type="string" summary="the target touch device"/>
+ <arg name="matrix" type="array" summary="the new calibration matrix"/>
+ </request>
+
+ <event name="touch_device">
+ <description summary="advertise a touchscreen input device">
+ When a client binds to weston_touch_calibration, one touch_device
+ event is sent for each touchscreen that is available to be calibrated.
+ These events are not sent later even if new touch devices appear.
+
+ An event carries the touch device identification and the associated
+ output or head (display connector) name.
+
+ On platforms using udev, the device identification is the udev DEVPATH.
+ </description>
+ <arg name="device" type="string"
+ summary="the touch device identification"/>
+ <arg name="head" type="string"
+ summary="name of the head or display connector"/>
+ </event>
+ </interface>
+
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
+ converted into a real value by dividing by 2^32-1. A calibration matrix
+ must be computed from the [0.0, 1.0] real values, but the matrix elements
+ do not need to fall into that range.
+ </description>
+
+ <enum name="error">
+ <description summary="calibrator object errors"/>
+ <entry name="bad_size" value="0"
+ summary="surface size does not match"/>
+ <entry name="not_mapped" value="0"
+ summary="requested operation is not possible without mapping the surface"/>
+ </enum>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy the calibrator">
+ This unmaps the surface if it was mapped. The input device grab
+ is dropped, if it was present. The surface loses its role as a
+ calibrator.
+ </description>
+ </request>
+
+ <request name="convert">
+ <description summary="convert from surface to raw coordinates">
+ This request asks the server to convert the surface-local coordinates
+ into the expected touch input coordinates appropriate for the
+ associated touch device.
+
+ The coordinates given as arguments to this request are relative to
+ the associated wl_surface.
+
+ If a client asks for conversion before it has committed valid
+ content to the wl_surface, the not_mapped error is raised.
+
+ If the server has cancelled the calibration, the conversion result
+ shall be zeroes.
+
+ The intention is that a client uses this request to convert marker
+ positions that the user is supposed to touch during calibration.
+ </description>
+ <arg name="x" type="int" summary="surface-local X coordinate"/>
+ <arg name="y" type="int" summary="surface-local Y coordinate"/>
+ <arg name="reply" type="new_id" interface="weston_touch_coordinate"
+ summary="object delivering the result"/>
+ </request>
+
+ <event name="configure">
+ <description summary="surface size">
+ This event tells the client what size to make the surface. The client
+ must obey the size exactly on the next commit with a wl_buffer.
+
+ This event shall be sent once as a response to creating a
+ weston_touch_calibrator object.
+ </description>
+ <arg name="width" type="int" summary="surface width"/>
+ <arg name="height" type="int" summary="surface height"/>
+ </event>
+
+ <event name="cancel_calibration">
+ <description summary="cancel the calibration procedure">
+ This is sent when the server wants to cancel the calibration and
+ drop the touch device grab. The server unmaps the surface, if
+ it was mapped.
+
+ The weston_touch_calibrator object will not send any more events. The
+ client should destroy it.
+ </description>
+ </event>
+
+ <event name="wrong_touch">
+ <description summary="user touched a wrong touchscreen">
+ Calibration can only be done on one touch input device at a time. If
+ the user touches another touch device during calibration, the server
+ sends this event to notify the calibration client. The client should
+ show feedback to the user that he touched a wrong screen. This is
+ useful particularly when it is impossible for a user to tell which
+ screen he should touch (when the screens are cloned).
+ </description>
+ </event>
+
+ <!-- touch events copied from wl_touch interface -->
+ <event name="down">
+ <description summary="touch down event and beginning of a touch sequence">
+ A new touch point has appeared on the surface. This touch point is
+ assigned a unique ID. Future events from this touch point reference
+ this ID. The ID ceases to be valid after a touch up event and may be
+ reused in the future.
+
+ For the coordinate units, see weston_touch_calibrator.
+ </description>
+ <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
+ <arg name="id" type="int" summary="the unique ID of this touch point"/>
+ <arg name="x" type="uint" summary="x coordinate in calibration units"/>
+ <arg name="y" type="uint" summary="y coordinate in calibration units"/>
+ </event>
+
+ <event name="up">
+ <description summary="end of a touch event sequence">
+ The touch point has disappeared. No further events will be sent for
+ this touch point and the touch point's ID is released and may be
+ reused in a future touch down event.
+ </description>
+ <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
+ <arg name="id" type="int" summary="the unique ID of this touch point"/>
+ </event>
+
+ <event name="motion">
+ <description summary="update of touch point coordinates">
+ A touch point has changed coordinates.
+
+ For the coordinate units, see weston_touch_calibrator.
+ </description>
+ <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
+ <arg name="id" type="int" summary="the unique ID of this touch point"/>
+ <arg name="x" type="uint" summary="x coordinate in calibration units"/>
+ <arg name="y" type="uint" summary="y coordinate in calibration units"/>
+ </event>
+
+ <event name="frame">
+ <description summary="end of touch frame event">
+ Indicates the end of a set of events that logically belong together.
+ A client is expected to accumulate the data in all events within the
+ frame before proceeding.
+
+ A wl_touch.frame terminates at least one event but otherwise no
+ guarantee is provided about the set of events within a frame. A client
+ must assume that any state not updated in a frame is unchanged from the
+ previously known state.
+ </description>
+ </event>
+
+ <event name="cancel">
+ <description summary="touch session cancelled">
+ Sent if the compositor decides the touch stream is a global
+ gesture. No further events are sent to the clients from that
+ particular gesture. Touch cancellation applies to all touch points
+ currently active on this client's surface. The client is
+ responsible for finalizing the touch points, future touch points on
+ this surface may reuse the touch point ID.
+ </description>
+ </event>
+ <!-- END of touch events copied from wl_touch interface -->
+
+ </interface>
+
+ <interface name="weston_touch_coordinate" version="1">
+ <description summary="coordinate conversion reply"/>
+
+ <!-- no destructor request defined, no requests possible -->
+
+ <event name="result">
+ <description summary="coordinates in raw touch space">
+ This event returns the conversion result from surface coordinates to
+ raw touch device coordinates.
+
+ For details, see weston_touch_calibrator.convert. For the coordinate
+ units, see weston_touch_calibrator.
+
+ This event destroys the weston_touch_coordinate object.
+ </description>
+ <arg name="x" type="uint" summary="x coordinate in calibration units"/>
+ <arg name="y" type="uint" summary="y coordinate in calibration units"/>
+ </event>
+ </interface>
+</protocol>
--
2.16.1
Peter Hutterer
2018-04-09 02:54:43 UTC
Permalink
Post by Pekka Paalanen
This is a Wayland protocol extension to allow the calibration of
touchscreens in Weston.
See: https://phabricator.freedesktop.org/T7868
---
Makefile.am | 5 +-
protocol/weston-touch-calibration.xml | 320 ++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 protocol/weston-touch-calibration.xml
diff --git a/Makefile.am b/Makefile.am
index e028a2a1..5e830777 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -168,7 +168,9 @@ ***@LIBWESTON_MAJOR@_la_SOURCES = \
protocol/pointer-constraints-unstable-v1-protocol.c \
protocol/pointer-constraints-unstable-v1-server-protocol.h \
protocol/input-timestamps-unstable-v1-protocol.c \
- protocol/input-timestamps-unstable-v1-server-protocol.h
+ protocol/input-timestamps-unstable-v1-server-protocol.h \
+ protocol/weston-touch-calibration-protocol.c \
+ protocol/weston-touch-calibration-server-protocol.h
@@ -1534,6 +1536,7 @@ EXTRA_DIST += \
protocol/weston-screenshooter.xml \
protocol/text-cursor-position.xml \
protocol/weston-test.xml \
+ protocol/weston-touch-calibration.xml \
protocol/ivi-application.xml \
protocol/ivi-hmi-controller.xml
diff --git a/protocol/weston-touch-calibration.xml b/protocol/weston-touch-calibration.xml
new file mode 100644
index 00000000..b1e19f9b
--- /dev/null
+++ b/protocol/weston-touch-calibration.xml
@@ -0,0 +1,320 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="weston_touch_calibration">
+
+ <copyright>
+ Copyright © 2017 Collabora, Ltd.
+ Copyright © 2017 General Electric Company
+
+ 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
+
+ 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.
+ </copyright>
+
+ <interface name="weston_touch_calibration" version="1">
+ <description summary="weston touchscreen calibration interface">
+ This is the global interface for calibrating a touchscreen input
+ coordinate transformation. It is recommended to make this interface
+ privileged.
+
+ This interface can be used by a client to show a calibration pattern and
+ receive uncalibrated touch coordinates, facilitating the computation of
+ a calibration transformation that will align actual touch positions
+ on screen with their expected coordinates.
+
+ Immediately after being bound by a client, the server sends the
+ touch_device events.
s/server/compositor/, in a few more places
Post by Pekka Paalanen
+
+ The client chooses a touch device from the touch_device events,
+ creates a wl_surface and then a weston_touch_calibrator for the
+ wl_surface and the chosen touch device. The client waits for the server
+ to send a configure event before it starts drawing the first calibration
+ pattern. After receiving the configure event, the client will iterate
+ drawing a pattern, getting touch input via weston_touch_calibrator,
+ and converting pixel coordinates to expected touch coordinates with
+ weston_touch_calibrator.convert until it has enough correspondences to
+ compute the calibration transformation or the server cancels the
+ calibration.
+
+ Once the client has successfully computed a new calibration, it can use
+ weston_touch_calibration.save request to load the new calibration into
+ the server. The server may take this new calibration into use and may
+ write it into persistent storage.
+ </description>
+
+ <enum name="error">
+ <description summary="global interface errors"/>
+ <entry name="invalid_surface" value="0"
+ summary="the given wl_surface already has a role"/>
+ <entry name="invalid_device" value="1"
+ summary="the given device is not valid"/>
+ <entry name="already_exists" value="2"
+ summary="a calibrator has already been created"/>
+ </enum>
+
+ <request name="destroy" type="destructor">
+ <description summary="unbind">
+ Destroy the binding to the global interface, does not affect any
+ objects already created through this interface.
+ </description>
+ </request>
+
+ <request name="create_calibrator">
+ <description summary="give the calibrator role to a surface">
+ This gives the calibrator role to the surface and ties it with the
+ given touch input device.
+
+ The surface must not already have a role, otherwise invalid_surface
+ error is raised.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ There must not exist a weston_touch_calibrator protocol object in the
+ server already, otherwise already_exists error is raised. This
+ limitation is server-wide and not specific to any particular client.
Personal preference: I find it easier to understand when a sentence is "if
blah then error E". As opposed to "if not blah, otherwise E". It also
narrows down the error cases a bit better too because you're only describing
the error case rather than the not-error case.
Post by Pekka Paalanen
+ </description>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface to give the role to"/>
+ <arg name="device" type="string" summary="the touch device to calibrate"/>
+ <arg name="cal" type="new_id" interface="weston_touch_calibrator"
+ summary="a new calibrator object"/>
+ </request>
+
+ <request name="save">
+ <description summary="save calibration for a touch device">
+ This request asks the server to save the calibration data for the
+ given touch input device. The server may ignore this request.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ The array must contain exactly six 'float' (the 32-bit floating
+ point format used by the C language on the system) numbers. The numbers
+ form a libinput style 2-by-3 calibration matrix in row-major order.
'libinput-style', i.e. hyphenated. but tbh, no need to mention libinput
here, just spell out the matrix:
"""
For a 3D matrix in the form
( a b c )
( d e f )
( 0 0 1 )
the array must contain the values [a, b, c, d, e, f] with a at index 0.
"""

Much easier than figuring out what "row-major order" means.
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string" summary="the target touch device"/>
+ <arg name="matrix" type="array" summary="the new calibration matrix"/>
+ </request>
+
+ <event name="touch_device">
+ <description summary="advertise a touchscreen input device">
+ When a client binds to weston_touch_calibration, one touch_device
+ event is sent for each touchscreen that is available to be calibrated.
+ These events are not sent later even if new touch devices appear.
Unclear what "later" means, use "Touch devices added after the initial burst
of events will not generate a touch_device event".

Though arguably - why not? Because it makes things easier to implement?
Post by Pekka Paalanen
+
+ An event carries the touch device identification and the associated
+ output or head (display connector) name.
+
+ On platforms using udev, the device identification is the udev DEVPATH.
do we want to really set this in stone in the protocol? Also, the DEVPATH
is without /sys, right? Why not make this an open format and let the client
figure it out. It's not hard to strcmp for /sys or /dev/input/event in the
client and iirc we have precedence for that in the tablet protocol. or in
libinput for tablets, can't remember now :)
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string"
+ summary="the touch device identification"/>
+ <arg name="head" type="string"
+ summary="name of the head or display connector"/>
+ </event>
+ </interface>
+
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
Post by Pekka Paalanen
+ converted into a real value by dividing by 2^32-1. A calibration matrix
+ must be computed from the [0.0, 1.0] real values, but the matrix elements
+ do not need to fall into that range.
+ </description>
+
+ <enum name="error">
+ <description summary="calibrator object errors"/>
+ <entry name="bad_size" value="0"
+ summary="surface size does not match"/>
+ <entry name="not_mapped" value="0"
+ summary="requested operation is not possible without mapping the surface"/>
+ </enum>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy the calibrator">
+ This unmaps the surface if it was mapped. The input device grab
+ is dropped, if it was present. The surface loses its role as a
+ calibrator.
+ </description>
+ </request>
+
+ <request name="convert">
+ <description summary="convert from surface to raw coordinates">
+ This request asks the server to convert the surface-local coordinates
+ into the expected touch input coordinates appropriate for the
+ associated touch device.
+
+ The coordinates given as arguments to this request are relative to
+ the associated wl_surface.
+
+ If a client asks for conversion before it has committed valid
+ content to the wl_surface, the not_mapped error is raised.
+
+ If the server has cancelled the calibration, the conversion result
+ shall be zeroes.
+
+ The intention is that a client uses this request to convert marker
+ positions that the user is supposed to touch during calibration.
+ </description>
+ <arg name="x" type="int" summary="surface-local X coordinate"/>
+ <arg name="y" type="int" summary="surface-local Y coordinate"/>
+ <arg name="reply" type="new_id" interface="weston_touch_coordinate"
+ summary="object delivering the result"/>
+ </request>
in theory, you could use this to figure out where on the screen you are
which is something we've avoided elsewhere. Since it's very confined I'm not
sure you could do anything with that information but is this something we
need to worry about?
Post by Pekka Paalanen
+
+ <event name="configure">
+ <description summary="surface size">
+ This event tells the client what size to make the surface. The client
+ must obey the size exactly on the next commit with a wl_buffer.
+
+ This event shall be sent once as a response to creating a
+ weston_touch_calibrator object.
+ </description>
+ <arg name="width" type="int" summary="surface width"/>
+ <arg name="height" type="int" summary="surface height"/>
+ </event>
+
+ <event name="cancel_calibration">
+ <description summary="cancel the calibration procedure">
+ This is sent when the server wants to cancel the calibration and
+ drop the touch device grab. The server unmaps the surface, if
+ it was mapped.
+
+ The weston_touch_calibrator object will not send any more events. The
+ client should destroy it.
+ </description>
+ </event>
+
+ <event name="wrong_touch">
"invalid_device_touch" maybe? 'wrong' feels.... wrong :)
also, while explained in the description, wrong_touch could also refer to a
touch that's outside the boundary, at the wrong time, etc. All these things
one could assume if one doesn't read the docs.

Cheers,
Peter
Post by Pekka Paalanen
+ <description summary="user touched a wrong touchscreen">
+ Calibration can only be done on one touch input device at a time. If
+ the user touches another touch device during calibration, the server
+ sends this event to notify the calibration client. The client should
+ show feedback to the user that he touched a wrong screen. This is
+ useful particularly when it is impossible for a user to tell which
+ screen he should touch (when the screens are cloned).
+ </description>
+ </event>
+
+ <!-- touch events copied from wl_touch interface -->
+ <event name="down">
+ <description summary="touch down event and beginning of a touch sequence">
+ A new touch point has appeared on the surface. This touch point is
+ assigned a unique ID. Future events from this touch point reference
+ this ID. The ID ceases to be valid after a touch up event and may be
+ reused in the future.
+
+ For the coordinate units, see weston_touch_calibrator.
+ </description>
+ <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
+ <arg name="id" type="int" summary="the unique ID of this touch point"/>
+ <arg name="x" type="uint" summary="x coordinate in calibration units"/>
+ <arg name="y" type="uint" summary="y coordinate in calibration units"/>
+ </event>
+
+ <event name="up">
+ <description summary="end of a touch event sequence">
+ The touch point has disappeared. No further events will be sent for
+ this touch point and the touch point's ID is released and may be
+ reused in a future touch down event.
+ </description>
+ <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
+ <arg name="id" type="int" summary="the unique ID of this touch point"/>
+ </event>
+
+ <event name="motion">
+ <description summary="update of touch point coordinates">
+ A touch point has changed coordinates.
+
+ For the coordinate units, see weston_touch_calibrator.
+ </description>
+ <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
+ <arg name="id" type="int" summary="the unique ID of this touch point"/>
+ <arg name="x" type="uint" summary="x coordinate in calibration units"/>
+ <arg name="y" type="uint" summary="y coordinate in calibration units"/>
+ </event>
+
+ <event name="frame">
+ <description summary="end of touch frame event">
+ Indicates the end of a set of events that logically belong together.
+ A client is expected to accumulate the data in all events within the
+ frame before proceeding.
+
+ A wl_touch.frame terminates at least one event but otherwise no
+ guarantee is provided about the set of events within a frame. A client
+ must assume that any state not updated in a frame is unchanged from the
+ previously known state.
+ </description>
+ </event>
+
+ <event name="cancel">
+ <description summary="touch session cancelled">
+ Sent if the compositor decides the touch stream is a global
+ gesture. No further events are sent to the clients from that
+ particular gesture. Touch cancellation applies to all touch points
+ currently active on this client's surface. The client is
+ responsible for finalizing the touch points, future touch points on
+ this surface may reuse the touch point ID.
+ </description>
+ </event>
+ <!-- END of touch events copied from wl_touch interface -->
+
+ </interface>
+
+ <interface name="weston_touch_coordinate" version="1">
+ <description summary="coordinate conversion reply"/>
+
+ <!-- no destructor request defined, no requests possible -->
+
+ <event name="result">
+ <description summary="coordinates in raw touch space">
+ This event returns the conversion result from surface coordinates to
+ raw touch device coordinates.
+
+ For details, see weston_touch_calibrator.convert. For the coordinate
+ units, see weston_touch_calibrator.
+
+ This event destroys the weston_touch_coordinate object.
+ </description>
+ <arg name="x" type="uint" summary="x coordinate in calibration units"/>
+ <arg name="y" type="uint" summary="y coordinate in calibration units"/>
+ </event>
+ </interface>
+</protocol>
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-04-10 11:01:10 UTC
Permalink
On Mon, 9 Apr 2018 12:54:43 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
This is a Wayland protocol extension to allow the calibration of
touchscreens in Weston.
See: https://phabricator.freedesktop.org/T7868
---
Makefile.am | 5 +-
protocol/weston-touch-calibration.xml | 320 ++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 protocol/weston-touch-calibration.xml
+ <interface name="weston_touch_calibration" version="1">
+ <description summary="weston touchscreen calibration interface">
+ This is the global interface for calibrating a touchscreen input
+ coordinate transformation. It is recommended to make this interface
+ privileged.
+
+ This interface can be used by a client to show a calibration pattern and
+ receive uncalibrated touch coordinates, facilitating the computation of
+ a calibration transformation that will align actual touch positions
+ on screen with their expected coordinates.
+
+ Immediately after being bound by a client, the server sends the
+ touch_device events.
s/server/compositor/, in a few more places
I'm a bit mixed there. I tried to be consistent with "server", but one
"compositor" still remained. There are a couple dozen "server".

Is your point that all Wayland spec language should be using
"compositor" to talk about the server-side?
Post by Peter Hutterer
Post by Pekka Paalanen
+ <request name="create_calibrator">
+ <description summary="give the calibrator role to a surface">
+ This gives the calibrator role to the surface and ties it with the
+ given touch input device.
+
+ The surface must not already have a role, otherwise invalid_surface
+ error is raised.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ There must not exist a weston_touch_calibrator protocol object in the
+ server already, otherwise already_exists error is raised. This
+ limitation is server-wide and not specific to any particular client.
Personal preference: I find it easier to understand when a sentence is "if
blah then error E". As opposed to "if not blah, otherwise E". It also
narrows down the error cases a bit better too because you're only describing
the error case rather than the not-error case.
Yup.
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface to give the role to"/>
+ <arg name="device" type="string" summary="the touch device to calibrate"/>
+ <arg name="cal" type="new_id" interface="weston_touch_calibrator"
+ summary="a new calibrator object"/>
+ </request>
+
+ <request name="save">
+ <description summary="save calibration for a touch device">
+ This request asks the server to save the calibration data for the
+ given touch input device. The server may ignore this request.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ The array must contain exactly six 'float' (the 32-bit floating
+ point format used by the C language on the system) numbers. The numbers
+ form a libinput style 2-by-3 calibration matrix in row-major order.
'libinput-style', i.e. hyphenated. but tbh, no need to mention libinput
"""
For a 3D matrix in the form
( a b c )
( d e f )
( 0 0 1 )
the array must contain the values [a, b, c, d, e, f] with a at index 0.
"""
Not only that, but it also needs to define the coordinate spaces where
it applies, and probably also the ordering with other transformations
as well. Hence I just punted to "how libinput uses it", because
everything here is specifically designed for libinput.

I'm also not sure I can actually lay out a matrix so that it will still
look ok in generated output and doxygen docs generated from that.
Post by Peter Hutterer
Much easier than figuring out what "row-major order" means.
It's a standard term.
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string" summary="the target touch device"/>
+ <arg name="matrix" type="array" summary="the new calibration matrix"/>
+ </request>
+
+ <event name="touch_device">
+ <description summary="advertise a touchscreen input device">
+ When a client binds to weston_touch_calibration, one touch_device
+ event is sent for each touchscreen that is available to be calibrated.
+ These events are not sent later even if new touch devices appear.
Unclear what "later" means, use "Touch devices added after the initial burst
of events will not generate a touch_device event".
More like:

Touch devices added in the server will not generate touch_device events
for existing weston_touch_calibration objects.

Or simply:

This is the only time when touch_device events are sent.
Post by Peter Hutterer
Though arguably - why not? Because it makes things easier to implement?
Yes. There is no event to remove a touch device, and there is no need
for a client to maintain an accurate list of touch devices.

Since there is no event to remove a touch device, there also is no
protocol race between server removing and the client using one.

There is still the race of server internally removing a device and a
client starting to use it after touch_device event, but that could
be fixed by having the server remember what devices it advertised and
if the client uses a deleted device, just ignore the saving or
immediately cancel the calibration. This is not implemented in this
patch series, though.
Post by Peter Hutterer
Post by Pekka Paalanen
+
+ An event carries the touch device identification and the associated
+ output or head (display connector) name.
+
+ On platforms using udev, the device identification is the udev DEVPATH.
do we want to really set this in stone in the protocol? Also, the DEVPATH
is without /sys, right? Why not make this an open format and let the client
figure it out. It's not hard to strcmp for /sys or /dev/input/event in the
client and iirc we have precedence for that in the tablet protocol. or in
libinput for tablets, can't remember now :)
Yes, I wanted to have a clear, unambgious definition. What udev calls
the DEVPATH is the path without /sys. We can revise this decision
if there ever arises a need for anything else, but it's still a Weston
private protocol for now.

When would one use /dev paths?

Btw. are device node and DEVPATH equally racy wrt. device getting
replaced with a different one?

We could have rules like if it starts with /sys, remove /sys and you
have the DEVPATH; if it starts with /dev, it's the device node, and so
on, but I don't see the point. So rather than pretending that I cater
for other environments, I just rule them out for now.
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string"
+ summary="the touch device identification"/>
+ <arg name="head" type="string"
+ summary="name of the head or display connector"/>
+ </event>
+ </interface>
+
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
I'm not sure. The underlying assumption is that there is a finite and
closed range of values a sensor can output, and that is mapped to [0,
1]. I don't know what nominal width and height are or can the values
extend to negative. It might be easier to define the calibration units
without defining those first.

It doesn't really matter, the important point is that when the
resulting matrix is loaded into libinput and then libinput is used
according to its API, the result is as expected. I'd be tempted to just
punt all this to libinput.

Libinput doc for libinput_device_config_calibration_set_matrix() says:

"The translation component (c, f) is expected to be normalized
to the device coordinate range."

I suppose rephrasing with "the device coordinate range" would be ok?
But is "device coordinates" well defined here?

How about adding:

The calibration units cover the device coordinate range exactly.
Post by Peter Hutterer
Post by Pekka Paalanen
+ converted into a real value by dividing by 2^32-1. A calibration matrix
+ must be computed from the [0.0, 1.0] real values, but the matrix elements
+ do not need to fall into that range.
+ </description>
+
+ <enum name="error">
+ <description summary="calibrator object errors"/>
+ <entry name="bad_size" value="0"
+ summary="surface size does not match"/>
+ <entry name="not_mapped" value="0"
+ summary="requested operation is not possible without mapping the surface"/>
+ </enum>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy the calibrator">
+ This unmaps the surface if it was mapped. The input device grab
+ is dropped, if it was present. The surface loses its role as a
+ calibrator.
+ </description>
+ </request>
+
+ <request name="convert">
+ <description summary="convert from surface to raw coordinates">
+ This request asks the server to convert the surface-local coordinates
+ into the expected touch input coordinates appropriate for the
+ associated touch device.
+
+ The coordinates given as arguments to this request are relative to
+ the associated wl_surface.
+
+ If a client asks for conversion before it has committed valid
+ content to the wl_surface, the not_mapped error is raised.
+
+ If the server has cancelled the calibration, the conversion result
+ shall be zeroes.
+
+ The intention is that a client uses this request to convert marker
+ positions that the user is supposed to touch during calibration.
+ </description>
+ <arg name="x" type="int" summary="surface-local X coordinate"/>
+ <arg name="y" type="int" summary="surface-local Y coordinate"/>
+ <arg name="reply" type="new_id" interface="weston_touch_coordinate"
+ summary="object delivering the result"/>
+ </request>
in theory, you could use this to figure out where on the screen you are
which is something we've avoided elsewhere. Since it's very confined I'm not
sure you could do anything with that information but is this something we
need to worry about?
I wouldn't worry about it. The global interface is intended to be
privileged anyway, as it practically allows a client to grab a physical
touch device and overwrite calibration matrices. One can retrieve [0,
1] domain coordinates of the output where the calibration window
happens to be on, so you can find out where the calibration window is,
but then again, the calibration window is quite likely fullscreen on
the output you chose in the touch device list. You cannot transfer the
coordinate mapping to any other window either.

So if you get far enough to actually use this request, you already have
a window covering a specific output and likely being top-most. Even
still, if the compositor does something like 1.5 output scaling factor,
you still probably cannot transform your coordinates into physical
output coordinates. Global compositor coordinate system is not exposed
at all as these are per-output coordinates.
Post by Peter Hutterer
Post by Pekka Paalanen
+
+ <event name="configure">
+ <description summary="surface size">
+ This event tells the client what size to make the surface. The client
+ must obey the size exactly on the next commit with a wl_buffer.
+
+ This event shall be sent once as a response to creating a
+ weston_touch_calibrator object.
+ </description>
+ <arg name="width" type="int" summary="surface width"/>
+ <arg name="height" type="int" summary="surface height"/>
+ </event>
+
+ <event name="cancel_calibration">
+ <description summary="cancel the calibration procedure">
+ This is sent when the server wants to cancel the calibration and
+ drop the touch device grab. The server unmaps the surface, if
+ it was mapped.
+
+ The weston_touch_calibrator object will not send any more events. The
+ client should destroy it.
+ </description>
+ </event>
+
+ <event name="wrong_touch">
"invalid_device_touch" maybe? 'wrong' feels.... wrong :)
also, while explained in the description, wrong_touch could also refer to a
touch that's outside the boundary, at the wrong time, etc. All these things
one could assume if one doesn't read the docs.
Yeah, this is for any touch event (most likely touch-down event) that
cannot be used for the on-going calibration. "unusable_touch"?
"bad_touch"? :-D
"incorrect_touch"?
Post by Peter Hutterer
Post by Pekka Paalanen
+ <description summary="user touched a wrong touchscreen">
+ Calibration can only be done on one touch input device at a time. If
+ the user touches another touch device during calibration, the server
+ sends this event to notify the calibration client. The client should
+ show feedback to the user that he touched a wrong screen. This is
+ useful particularly when it is impossible for a user to tell which
+ screen he should touch (when the screens are cloned).
+ </description>
+ </event>
Thanks,
pq
Peter Hutterer
2018-04-11 00:16:46 UTC
Permalink
Post by Pekka Paalanen
On Mon, 9 Apr 2018 12:54:43 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
This is a Wayland protocol extension to allow the calibration of
touchscreens in Weston.
See: https://phabricator.freedesktop.org/T7868
---
Makefile.am | 5 +-
protocol/weston-touch-calibration.xml | 320 ++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 protocol/weston-touch-calibration.xml
+ <interface name="weston_touch_calibration" version="1">
+ <description summary="weston touchscreen calibration interface">
+ This is the global interface for calibrating a touchscreen input
+ coordinate transformation. It is recommended to make this interface
+ privileged.
+
+ This interface can be used by a client to show a calibration pattern and
+ receive uncalibrated touch coordinates, facilitating the computation of
+ a calibration transformation that will align actual touch positions
+ on screen with their expected coordinates.
+
+ Immediately after being bound by a client, the server sends the
+ touch_device events.
s/server/compositor/, in a few more places
I'm a bit mixed there. I tried to be consistent with "server", but one
"compositor" still remained. There are a couple dozen "server".
Is your point that all Wayland spec language should be using
"compositor" to talk about the server-side?
Yeah, sorry, should've made this clear. 30 years of confusion about X'
client and server model suggests using 'compositor' and 'client' is
superior, simply because at least you know one side for sure now :)
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ <request name="create_calibrator">
+ <description summary="give the calibrator role to a surface">
+ This gives the calibrator role to the surface and ties it with the
+ given touch input device.
+
+ The surface must not already have a role, otherwise invalid_surface
+ error is raised.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ There must not exist a weston_touch_calibrator protocol object in the
+ server already, otherwise already_exists error is raised. This
+ limitation is server-wide and not specific to any particular client.
Personal preference: I find it easier to understand when a sentence is "if
blah then error E". As opposed to "if not blah, otherwise E". It also
narrows down the error cases a bit better too because you're only describing
the error case rather than the not-error case.
Yup.
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface to give the role to"/>
+ <arg name="device" type="string" summary="the touch device to calibrate"/>
+ <arg name="cal" type="new_id" interface="weston_touch_calibrator"
+ summary="a new calibrator object"/>
+ </request>
+
+ <request name="save">
+ <description summary="save calibration for a touch device">
+ This request asks the server to save the calibration data for the
+ given touch input device. The server may ignore this request.
+
+ The device string must be one advertised with touch_device event's
+ device argument, otherwise invalid_device error is raised.
+
+ The array must contain exactly six 'float' (the 32-bit floating
+ point format used by the C language on the system) numbers. The numbers
+ form a libinput style 2-by-3 calibration matrix in row-major order.
'libinput-style', i.e. hyphenated. but tbh, no need to mention libinput
"""
For a 3D matrix in the form
( a b c )
( d e f )
( 0 0 1 )
the array must contain the values [a, b, c, d, e, f] with a at index 0.
"""
Not only that, but it also needs to define the coordinate spaces where
it applies, and probably also the ordering with other transformations
as well. Hence I just punted to "how libinput uses it", because
everything here is specifically designed for libinput.
I'm also not sure I can actually lay out a matrix so that it will still
look ok in generated output and doxygen docs generated from that.
@code and @endcode in doxygen map to <pre> (you can use the html tag
directly too). Or there's @verbatim and @endverbatim. You can do fancy
things with latex like libinput does, but it makes the source pretty awful
to look at (and introduces extra build dependencies).
Post by Pekka Paalanen
Post by Peter Hutterer
Much easier than figuring out what "row-major order" means.
It's a standard term.
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string" summary="the target touch device"/>
+ <arg name="matrix" type="array" summary="the new calibration matrix"/>
+ </request>
+
+ <event name="touch_device">
+ <description summary="advertise a touchscreen input device">
+ When a client binds to weston_touch_calibration, one touch_device
+ event is sent for each touchscreen that is available to be calibrated.
+ These events are not sent later even if new touch devices appear.
Unclear what "later" means, use "Touch devices added after the initial burst
of events will not generate a touch_device event".
Touch devices added in the server will not generate touch_device events
for existing weston_touch_calibration objects.
This is the only time when touch_device events are sent.
works, but I'd even use both ("this is the only time... Touch devices added
.."). language is ambiguous, reducing ambiguity is always good.
Post by Pekka Paalanen
Post by Peter Hutterer
Though arguably - why not? Because it makes things easier to implement?
Yes. There is no event to remove a touch device, and there is no need
for a client to maintain an accurate list of touch devices.
wfm
Post by Pekka Paalanen
Since there is no event to remove a touch device, there also is no
protocol race between server removing and the client using one.
There is still the race of server internally removing a device and a
client starting to use it after touch_device event, but that could
be fixed by having the server remember what devices it advertised and
if the client uses a deleted device, just ignore the saving or
immediately cancel the calibration. This is not implemented in this
patch series, though.
Post by Peter Hutterer
Post by Pekka Paalanen
+
+ An event carries the touch device identification and the associated
+ output or head (display connector) name.
+
+ On platforms using udev, the device identification is the udev DEVPATH.
do we want to really set this in stone in the protocol? Also, the DEVPATH
is without /sys, right? Why not make this an open format and let the client
figure it out. It's not hard to strcmp for /sys or /dev/input/event in the
client and iirc we have precedence for that in the tablet protocol. or in
libinput for tablets, can't remember now :)
Yes, I wanted to have a clear, unambgious definition. What udev calls
the DEVPATH is the path without /sys. We can revise this decision
if there ever arises a need for anything else, but it's still a Weston
private protocol for now.
When would one use /dev paths?
*shrug*, not sure in this case, just used that as an example here. Point was
more: prefixing with /sys makes it a lot more identifiable than just using
the DEVPATH, not least because you can immediately stat that path (or ls, important
for bug reporters).

also, udev_device_new_from_syspath() seems to error out if it doesn't start
with "/sys", so any programmatic user would have to pre-pend /sys anway.
Post by Pekka Paalanen
Btw. are device node and DEVPATH equally racy wrt. device getting
replaced with a different one?
not 100% sure. Theory is that if you have the DEVPATH you can get the
devnode through udev and by then all races should've been resolved. If you
get the device node independently, then you're bound to run into races.
Post by Pekka Paalanen
We could have rules like if it starts with /sys, remove /sys and you
have the DEVPATH; if it starts with /dev, it's the device node, and so
on, but I don't see the point. So rather than pretending that I cater
for other environments, I just rule them out for now.
sure, it's fine to just support udev. I just question the utility of DEVPATH
in particular. I mean DEVPATH=/devices/rmi4-00/input/input25/event17 on my
touchpad, but /sys/class/input/event17 gives me the same udev device. Or
/sys/dev/char/.../
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string"
+ summary="the touch device identification"/>
+ <arg name="head" type="string"
+ summary="name of the head or display connector"/>
+ </event>
+ </interface>
+
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
I'm not sure. The underlying assumption is that there is a finite and
closed range of values a sensor can output, and that is mapped to [0,
1]. I don't know what nominal width and height are or can the values
extend to negative. It might be easier to define the calibration units
without defining those first.
sorry, I used "nominal" before I rewrote to use "sensor", probably made it
more confusing. On some devices, the sensor is larger than the screen so you
can get coordinates outside the screen area. This is not a technical
problem, just a user perception mismatch that doens't need to be exposed and
after all that's what calibration is about.

Once you apply calibration though you can get real negative coordinates,
especially if the device advertises a [n:m] range but then sends events less
than n. For example, all synaptics touchpads (pre 2014, some after that) do
that.

These devices are a problem because we rely on the coordinate range for the
size and then on the calibration on top to normalize the input. So they may
need two fixes, one for the range adjustment, one for calibration.
Post by Pekka Paalanen
It doesn't really matter, the important point is that when the
resulting matrix is loaded into libinput and then libinput is used
according to its API, the result is as expected. I'd be tempted to just
punt all this to libinput.
libinput will sort-of punt it back :)
https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tablet-bounds
Post by Pekka Paalanen
"The translation component (c, f) is expected to be normalized
to the device coordinate range."
I suppose rephrasing with "the device coordinate range" would be ok?
But is "device coordinates" well defined here?
The calibration units cover the device coordinate range exactly.
yep, works for me.
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+
+ <event name="configure">
+ <description summary="surface size">
+ This event tells the client what size to make the surface. The client
+ must obey the size exactly on the next commit with a wl_buffer.
+
+ This event shall be sent once as a response to creating a
+ weston_touch_calibrator object.
+ </description>
+ <arg name="width" type="int" summary="surface width"/>
+ <arg name="height" type="int" summary="surface height"/>
+ </event>
+
+ <event name="cancel_calibration">
+ <description summary="cancel the calibration procedure">
+ This is sent when the server wants to cancel the calibration and
+ drop the touch device grab. The server unmaps the surface, if
+ it was mapped.
+
+ The weston_touch_calibrator object will not send any more events. The
+ client should destroy it.
+ </description>
+ </event>
+
+ <event name="wrong_touch">
"invalid_device_touch" maybe? 'wrong' feels.... wrong :)
also, while explained in the description, wrong_touch could also refer to a
touch that's outside the boundary, at the wrong time, etc. All these things
one could assume if one doesn't read the docs.
Yeah, this is for any touch event (most likely touch-down event) that
cannot be used for the on-going calibration. "unusable_touch"?
"bad_touch"? :-D
"incorrect_touch"?
"invalid_touch" then, I'd say. Simply because it only specifies that it
can't be used for whatever reason. Separating invalid from incorrect device
might be good for user interface purposes, but whether it's worth the
effort is a different matter.

Cheers,
Peter
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ <description summary="user touched a wrong touchscreen">
+ Calibration can only be done on one touch input device at a time. If
+ the user touches another touch device during calibration, the server
+ sends this event to notify the calibration client. The client should
+ show feedback to the user that he touched a wrong screen. This is
+ useful particularly when it is impossible for a user to tell which
+ screen he should touch (when the screens are cloned).
+ </description>
+ </event>
Thanks,
pq
Pekka Paalanen
2018-04-11 11:00:49 UTC
Permalink
On Wed, 11 Apr 2018 10:16:46 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
On Mon, 9 Apr 2018 12:54:43 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
This is a Wayland protocol extension to allow the calibration of
touchscreens in Weston.
See: https://phabricator.freedesktop.org/T7868
---
Makefile.am | 5 +-
protocol/weston-touch-calibration.xml | 320 ++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 protocol/weston-touch-calibration.xml
+ <interface name="weston_touch_calibration" version="1">
+ <description summary="weston touchscreen calibration interface">
+ This is the global interface for calibrating a touchscreen input
+ coordinate transformation. It is recommended to make this interface
+ privileged.
+
+ This interface can be used by a client to show a calibration pattern and
+ receive uncalibrated touch coordinates, facilitating the computation of
+ a calibration transformation that will align actual touch positions
+ on screen with their expected coordinates.
+
+ Immediately after being bound by a client, the server sends the
+ touch_device events.
s/server/compositor/, in a few more places
I'm a bit mixed there. I tried to be consistent with "server", but one
"compositor" still remained. There are a couple dozen "server".
Is your point that all Wayland spec language should be using
"compositor" to talk about the server-side?
Yeah, sorry, should've made this clear. 30 years of confusion about X'
client and server model suggests using 'compositor' and 'client' is
superior, simply because at least you know one side for sure now :)
I wouldn't be so sure, the misconception that Wayland is a display
server, desktop projects just write their compositors and without
understanding that server=compositor was pretty wide-spread. :-)

At least the client and server model is still the same. Compositor used
to be a client or neither really, and now it is a server, so I could
argue that "compositor" is more confusing than "server". Hence I tend
to talk about a display server nowadays.

But ok, I can follow the precedent and call it "compositor".
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ An event carries the touch device identification and the associated
+ output or head (display connector) name.
+
+ On platforms using udev, the device identification is the udev DEVPATH.
do we want to really set this in stone in the protocol? Also, the DEVPATH
is without /sys, right? Why not make this an open format and let the client
figure it out. It's not hard to strcmp for /sys or /dev/input/event in the
client and iirc we have precedence for that in the tablet protocol. or in
libinput for tablets, can't remember now :)
Yes, I wanted to have a clear, unambgious definition. What udev calls
the DEVPATH is the path without /sys. We can revise this decision
if there ever arises a need for anything else, but it's still a Weston
private protocol for now.
When would one use /dev paths?
*shrug*, not sure in this case, just used that as an example here. Point was
more: prefixing with /sys makes it a lot more identifiable than just using
the DEVPATH, not least because you can immediately stat that path (or ls, important
for bug reporters).
also, udev_device_new_from_syspath() seems to error out if it doesn't start
with "/sys", so any programmatic user would have to pre-pend /sys anway.
Ok, I can change it to be "DEVPATH prefixed with /sys". Or is SYSPATH a
thing defined by udev? There is udev_device_get_syspath() but I cannot
get 'udevadm info' to print that. Originally I wanted to stick to what
I can see with 'udevadm info'.
Post by Peter Hutterer
Post by Pekka Paalanen
Btw. are device node and DEVPATH equally racy wrt. device getting
replaced with a different one?
not 100% sure. Theory is that if you have the DEVPATH you can get the
devnode through udev and by then all races should've been resolved. If you
get the device node independently, then you're bound to run into races.
Alright, I'll pay no further thought to device nodes.
Post by Peter Hutterer
Post by Pekka Paalanen
We could have rules like if it starts with /sys, remove /sys and you
have the DEVPATH; if it starts with /dev, it's the device node, and so
on, but I don't see the point. So rather than pretending that I cater
for other environments, I just rule them out for now.
sure, it's fine to just support udev. I just question the utility of DEVPATH
in particular. I mean DEVPATH=/devices/rmi4-00/input/input25/event17 on my
touchpad, but /sys/class/input/event17 gives me the same udev device. Or
/sys/dev/char/.../
You cannot use any alias in the 'save' or 'create_calibrator' requests
in any case, Weston does not create a new udev_device from it. It
simply compares the given device string to what it advertised.

DEVPATH seemed to be all of unique, well-defined, well-known, and
easily available and usable. Otherwise I don't care what the device
string is.
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string"
+ summary="the touch device identification"/>
+ <arg name="head" type="string"
+ summary="name of the head or display connector"/>
+ </event>
+ </interface>
+
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
I'm not sure. The underlying assumption is that there is a finite and
closed range of values a sensor can output, and that is mapped to [0,
1]. I don't know what nominal width and height are or can the values
extend to negative. It might be easier to define the calibration units
without defining those first.
sorry, I used "nominal" before I rewrote to use "sensor", probably made it
more confusing. On some devices, the sensor is larger than the screen so you
can get coordinates outside the screen area. This is not a technical
problem, just a user perception mismatch that doens't need to be exposed and
after all that's what calibration is about.
Once you apply calibration though you can get real negative coordinates,
especially if the device advertises a [n:m] range but then sends events less
than n. For example, all synaptics touchpads (pre 2014, some after that) do
that.
But wait, if there are actually devices that report the range [n:m] but
can still send values less than n or greater than m, it means I cannot
transmit those with the encoding set up here, because
libinput_event_touch_get_x_transformed(touch_event, 1) can return
values outside of [0, 1] even when the loaded calibration matrix is
identity. Is that right?

Do I need to find an encoding to cope with that, or can I just reject
such touches?

I mean, the range [n:m] is in raw input coordinates, before any
normalization or calibration in userspace, right?
Post by Peter Hutterer
These devices are a problem because we rely on the coordinate range for the
size and then on the calibration on top to normalize the input. So they may
need two fixes, one for the range adjustment, one for calibration.
If I can just transmit whatever values I get for "normalized" with
identity in the above, then the calibration matrix will automatically
account for any out-of-range values - I mean they become a non-issue,
because the compositor just tranforms them to the output or a global
coordinate system first and then clips to the actual output area. Or
doesn't clip, makes no difference.
Post by Peter Hutterer
Post by Pekka Paalanen
It doesn't really matter, the important point is that when the
resulting matrix is loaded into libinput and then libinput is used
according to its API, the result is as expected. I'd be tempted to just
punt all this to libinput.
libinput will sort-of punt it back :)
https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tablet-bounds
I don't see a problem in that section. The goal of this calibration is
to match input event coordinates to output pixel coordinates in the
complete pipeline by adjusting the normalized calibration matrix in
libinput. It's ok for calibrated input coordinates go beyond any
limits, the input and output coordinate planes can be thought as
infinite.

Or do you refer to the functions returning positions in millimeters? I
never looked at those, they're not used with touchscreens. I don't even
know if a calibration matrix affects them.
Post by Peter Hutterer
Post by Pekka Paalanen
"The translation component (c, f) is expected to be normalized
to the device coordinate range."
I suppose rephrasing with "the device coordinate range" would be ok?
But is "device coordinates" well defined here?
The calibration units cover the device coordinate range exactly.
yep, works for me.
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+
+ <event name="configure">
+ <description summary="surface size">
+ This event tells the client what size to make the surface. The client
+ must obey the size exactly on the next commit with a wl_buffer.
+
+ This event shall be sent once as a response to creating a
+ weston_touch_calibrator object.
+ </description>
+ <arg name="width" type="int" summary="surface width"/>
+ <arg name="height" type="int" summary="surface height"/>
+ </event>
+
+ <event name="cancel_calibration">
+ <description summary="cancel the calibration procedure">
+ This is sent when the server wants to cancel the calibration and
+ drop the touch device grab. The server unmaps the surface, if
+ it was mapped.
+
+ The weston_touch_calibrator object will not send any more events. The
+ client should destroy it.
+ </description>
+ </event>
+
+ <event name="wrong_touch">
"invalid_device_touch" maybe? 'wrong' feels.... wrong :)
also, while explained in the description, wrong_touch could also refer to a
touch that's outside the boundary, at the wrong time, etc. All these things
one could assume if one doesn't read the docs.
Yeah, this is for any touch event (most likely touch-down event) that
cannot be used for the on-going calibration. "unusable_touch"?
"bad_touch"? :-D
"incorrect_touch"?
"invalid_touch" then, I'd say. Simply because it only specifies that it
can't be used for whatever reason. Separating invalid from incorrect device
might be good for user interface purposes, but whether it's worth the
effort is a different matter.
I don't have a use for invalid other than an incorrect device, unless
input coordinates out of the advertised range are possible to get and
impossible to transmit.


Thanks,
pq
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ <description summary="user touched a wrong touchscreen">
+ Calibration can only be done on one touch input device at a time. If
+ the user touches another touch device during calibration, the server
+ sends this event to notify the calibration client. The client should
+ show feedback to the user that he touched a wrong screen. This is
+ useful particularly when it is impossible for a user to tell which
+ screen he should touch (when the screens are cloned).
+ </description>
+ </event>
Peter Hutterer
2018-04-13 04:31:39 UTC
Permalink
Post by Pekka Paalanen
On Wed, 11 Apr 2018 10:16:46 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ An event carries the touch device identification and the associated
+ output or head (display connector) name.
+
+ On platforms using udev, the device identification is the udev DEVPATH.
do we want to really set this in stone in the protocol? Also, the DEVPATH
is without /sys, right? Why not make this an open format and let the client
figure it out. It's not hard to strcmp for /sys or /dev/input/event in the
client and iirc we have precedence for that in the tablet protocol. or in
libinput for tablets, can't remember now :)
Yes, I wanted to have a clear, unambgious definition. What udev calls
the DEVPATH is the path without /sys. We can revise this decision
if there ever arises a need for anything else, but it's still a Weston
private protocol for now.
When would one use /dev paths?
*shrug*, not sure in this case, just used that as an example here. Point was
more: prefixing with /sys makes it a lot more identifiable than just using
the DEVPATH, not least because you can immediately stat that path (or ls, important
for bug reporters).
also, udev_device_new_from_syspath() seems to error out if it doesn't start
with "/sys", so any programmatic user would have to pre-pend /sys anway.
Ok, I can change it to be "DEVPATH prefixed with /sys". Or is SYSPATH a
thing defined by udev? There is udev_device_get_syspath() but I cannot
get 'udevadm info' to print that. Originally I wanted to stick to what
I can see with 'udevadm info'.
I checked the libudev sources and it has:
src/libsystemd/sd-device/sd-device.c:device_set_syspath()

devpath = syspath + STRLEN("/sys");

so DEVPATH is literally the substring of the syspath.
Post by Pekka Paalanen
but I cannot get 'udevadm info' to print that.
this is because udevadm info takes the syspath as input argument to figure
out which device it needs to print. So I suspect it decided not to print it
because you've already given it on the commandline anyway :) Even though any
single device has multiple syspaths that are all symlinked to the
/sys/$DEVPATH node.

Ironically, when you use udevadm info -p $DEVPATH, it also needs to prefix
'/sys' to be able to do anything with the device (see
src/udev/udevadm-info.c)
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
Btw. are device node and DEVPATH equally racy wrt. device getting
replaced with a different one?
not 100% sure. Theory is that if you have the DEVPATH you can get the
devnode through udev and by then all races should've been resolved. If you
get the device node independently, then you're bound to run into races.
Alright, I'll pay no further thought to device nodes.
Reading this again and thinking about it some more: if you replace a device
node quickly enough, you'll lag behind and run into this issue. So by the
time you get the udev device's devnode the device may have been removed and
the kernel replaced it with another device, re-using the event node. You'll
eventually get the device-removed event from udev but...

In the libinput test suite I work around this by comparing the syspaths, in
the form of:
devnode = udev_device_get_devnode(dev);
fd = open(devnode)
new_dev = udev_device_new_from_devnum(fd)

strcmp(udev_device_get_syspath(dev), udev_device_get_syspath(new_dev))
if not equal:
oops, we've raced ourselves into a corner


I need this because I create and remove hundreds of devices per minute.
I doubt it's needed in a real-world situation though.
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
We could have rules like if it starts with /sys, remove /sys and you
have the DEVPATH; if it starts with /dev, it's the device node, and so
on, but I don't see the point. So rather than pretending that I cater
for other environments, I just rule them out for now.
sure, it's fine to just support udev. I just question the utility of DEVPATH
in particular. I mean DEVPATH=/devices/rmi4-00/input/input25/event17 on my
touchpad, but /sys/class/input/event17 gives me the same udev device. Or
/sys/dev/char/.../
You cannot use any alias in the 'save' or 'create_calibrator' requests
in any case, Weston does not create a new udev_device from it. It
simply compares the given device string to what it advertised.
DEVPATH seemed to be all of unique, well-defined, well-known, and
easily available and usable. Otherwise I don't care what the device
string is.
sure, my argument was that using the syspath gives you the same but it's
more flexible when used with other tools.
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ </description>
+ <arg name="device" type="string"
+ summary="the touch device identification"/>
+ <arg name="head" type="string"
+ summary="name of the head or display connector"/>
+ </event>
+ </interface>
+
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
I'm not sure. The underlying assumption is that there is a finite and
closed range of values a sensor can output, and that is mapped to [0,
1]. I don't know what nominal width and height are or can the values
extend to negative. It might be easier to define the calibration units
without defining those first.
sorry, I used "nominal" before I rewrote to use "sensor", probably made it
more confusing. On some devices, the sensor is larger than the screen so you
can get coordinates outside the screen area. This is not a technical
problem, just a user perception mismatch that doens't need to be exposed and
after all that's what calibration is about.
Once you apply calibration though you can get real negative coordinates,
especially if the device advertises a [n:m] range but then sends events less
than n. For example, all synaptics touchpads (pre 2014, some after that) do
that.
But wait, if there are actually devices that report the range [n:m] but
can still send values less than n or greater than m, it means I cannot
transmit those with the encoding set up here, because
libinput_event_touch_get_x_transformed(touch_event, 1) can return
values outside of [0, 1] even when the loaded calibration matrix is
identity. Is that right?
correct, that can happen.
Post by Pekka Paalanen
Do I need to find an encoding to cope with that, or can I just reject
such touches?
I mean, the range [n:m] is in raw input coordinates, before any
normalization or calibration in userspace, right?
uhm. let me just detail this and you can pick which one was the answer to
that question:
- the kernel exposes [n:m] axis ranges for ABS_X/Y and the MT equivalents
- libinput converts this into mm by default, see libinput_event_touch_get_x()
- libinput converts this to a ranged value [0:N] when calling
libnput_event_touch_get_x_transformed(N). This is what weston uses to get
the screen coordinates.

Both libinput values have the calibration factored in already.

Weston doesn't ever see raw input coordinates. But if you use
get_x_transformed, you can get values outside [0:N]. This happens when
the [n:m] range is incorrect. Accommodating for some error margin, libinput
will print a warning to the log when this happens.

The correct solution would be to warn the user that the device is garbage
and make them put a 60-evdev.hwdb quirk in to fix the axis ranges.
Depending on the device, this could be a generic solution but let's not bet
on that...

The user-friendly solution would be to have two layers of mapping. By
default, you map the libinput range into [0, 1]. Whenever you see negative
values or values > 1 you notify the user that this screen is garbage. The
only option you have now is to get the user to touch the furthest extents of
all axes to get the lowest and highest values the axis provides (similar to
the touchpad-edge-detector tool). Then you can map those into the libinput
provided range so that e.g. libinput's -0.25 is normalized 0 and 1.25 is
normalized 1.0.

Now you can restart calibration with that extra mapping on top.

A few caveats, beyond the effort required:
- even with the calibration matrix applied, the libinput ranges will still
be outside the [0:N] range. Only a real axis fix can fix that
- you cannot know the true min/max until you get respective event from the
screen

So yeah, the situation is less than ideal...

I've considered auto-scaling libinput into the new axis range but this is
likely going to hurt even more because it's undiscoverable and for most
devices, a quirk in the hwdb file can solve this problem properly.
Post by Pekka Paalanen
Post by Peter Hutterer
These devices are a problem because we rely on the coordinate range for the
size and then on the calibration on top to normalize the input. So they may
need two fixes, one for the range adjustment, one for calibration.
If I can just transmit whatever values I get for "normalized" with
identity in the above, then the calibration matrix will automatically
account for any out-of-range values - I mean they become a non-issue,
because the compositor just tranforms them to the output or a global
coordinate system first and then clips to the actual output area. Or
doesn't clip, makes no difference.
Post by Peter Hutterer
Post by Pekka Paalanen
It doesn't really matter, the important point is that when the
resulting matrix is loaded into libinput and then libinput is used
according to its API, the result is as expected. I'd be tempted to just
punt all this to libinput.
libinput will sort-of punt it back :)
https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tablet-bounds
I don't see a problem in that section. The goal of this calibration is
to match input event coordinates to output pixel coordinates in the
complete pipeline by adjusting the normalized calibration matrix in
libinput. It's ok for calibrated input coordinates go beyond any
limits, the input and output coordinate planes can be thought as
infinite.
Or do you refer to the functions returning positions in millimeters? I
never looked at those, they're not used with touchscreens. I don't even
know if a calibration matrix affects them.
There's a corner-case with touchscreens and mm. Plenty of cheap touchscreens
don't have a resolution set, so the mm value is calculated using the default
resolution of 1.

The calibration matrix is applied to the mm as well. That's largely a
side-effect for the matrix originally being used for rotation rather than
scaling or translation.

Cheers,
Peter
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
"The translation component (c, f) is expected to be normalized
to the device coordinate range."
I suppose rephrasing with "the device coordinate range" would be ok?
But is "device coordinates" well defined here?
The calibration units cover the device coordinate range exactly.
yep, works for me.
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+
+ <event name="configure">
+ <description summary="surface size">
+ This event tells the client what size to make the surface. The client
+ must obey the size exactly on the next commit with a wl_buffer.
+
+ This event shall be sent once as a response to creating a
+ weston_touch_calibrator object.
+ </description>
+ <arg name="width" type="int" summary="surface width"/>
+ <arg name="height" type="int" summary="surface height"/>
+ </event>
+
+ <event name="cancel_calibration">
+ <description summary="cancel the calibration procedure">
+ This is sent when the server wants to cancel the calibration and
+ drop the touch device grab. The server unmaps the surface, if
+ it was mapped.
+
+ The weston_touch_calibrator object will not send any more events. The
+ client should destroy it.
+ </description>
+ </event>
+
+ <event name="wrong_touch">
"invalid_device_touch" maybe? 'wrong' feels.... wrong :)
also, while explained in the description, wrong_touch could also refer to a
touch that's outside the boundary, at the wrong time, etc. All these things
one could assume if one doesn't read the docs.
Yeah, this is for any touch event (most likely touch-down event) that
cannot be used for the on-going calibration. "unusable_touch"?
"bad_touch"? :-D
"incorrect_touch"?
"invalid_touch" then, I'd say. Simply because it only specifies that it
can't be used for whatever reason. Separating invalid from incorrect device
might be good for user interface purposes, but whether it's worth the
effort is a different matter.
I don't have a use for invalid other than an incorrect device, unless
input coordinates out of the advertised range are possible to get and
impossible to transmit.
Thanks,
pq
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ <description summary="user touched a wrong touchscreen">
+ Calibration can only be done on one touch input device at a time. If
+ the user touches another touch device during calibration, the server
+ sends this event to notify the calibration client. The client should
+ show feedback to the user that he touched a wrong screen. This is
+ useful particularly when it is impossible for a user to tell which
+ screen he should touch (when the screens are cloned).
+ </description>
+ </event>
Pekka Paalanen
2018-04-13 07:51:37 UTC
Permalink
On Fri, 13 Apr 2018 14:31:39 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
On Wed, 11 Apr 2018 10:16:46 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
I'm not sure. The underlying assumption is that there is a finite and
closed range of values a sensor can output, and that is mapped to [0,
1]. I don't know what nominal width and height are or can the values
extend to negative. It might be easier to define the calibration units
without defining those first.
sorry, I used "nominal" before I rewrote to use "sensor", probably made it
more confusing. On some devices, the sensor is larger than the screen so you
can get coordinates outside the screen area. This is not a technical
problem, just a user perception mismatch that doens't need to be exposed and
after all that's what calibration is about.
Once you apply calibration though you can get real negative coordinates,
especially if the device advertises a [n:m] range but then sends events less
than n. For example, all synaptics touchpads (pre 2014, some after that) do
that.
But wait, if there are actually devices that report the range [n:m] but
can still send values less than n or greater than m, it means I cannot
transmit those with the encoding set up here, because
libinput_event_touch_get_x_transformed(touch_event, 1) can return
values outside of [0, 1] even when the loaded calibration matrix is
identity. Is that right?
correct, that can happen.
Post by Pekka Paalanen
Do I need to find an encoding to cope with that, or can I just reject
such touches?
I mean, the range [n:m] is in raw input coordinates, before any
normalization or calibration in userspace, right?
uhm. let me just detail this and you can pick which one was the answer to
- the kernel exposes [n:m] axis ranges for ABS_X/Y and the MT equivalents
- libinput converts this into mm by default, see libinput_event_touch_get_x()
- libinput converts this to a ranged value [0:N] when calling
libnput_event_touch_get_x_transformed(N). This is what weston uses to get
the screen coordinates.
Both libinput values have the calibration factored in already.
Weston doesn't ever see raw input coordinates. But if you use
get_x_transformed, you can get values outside [0:N]. This happens when
the [n:m] range is incorrect. Accommodating for some error margin, libinput
will print a warning to the log when this happens.
The correct solution would be to warn the user that the device is garbage
and make them put a 60-evdev.hwdb quirk in to fix the axis ranges.
Depending on the device, this could be a generic solution but let's not bet
on that...
The user-friendly solution would be to have two layers of mapping. By
default, you map the libinput range into [0, 1]. Whenever you see negative
values or values > 1 you notify the user that this screen is garbage. The
only option you have now is to get the user to touch the furthest extents of
all axes to get the lowest and highest values the axis provides (similar to
the touchpad-edge-detector tool). Then you can map those into the libinput
provided range so that e.g. libinput's -0.25 is normalized 0 and 1.25 is
normalized 1.0.
Now you can restart calibration with that extra mapping on top.
- even with the calibration matrix applied, the libinput ranges will still
be outside the [0:N] range. Only a real axis fix can fix that
- you cannot know the true min/max until you get respective event from the
screen
So yeah, the situation is less than ideal...
I've considered auto-scaling libinput into the new axis range but this is
likely going to hurt even more because it's undiscoverable and for most
devices, a quirk in the hwdb file can solve this problem properly.
Thank you very much for the thorough explanation.

I think the appropriate solution for me here is to check whether the
normalized value I expect to be in [0, 1] falls outside of the range,
and if so, I will a) print a warning in Weston's log that the device
may need a quirk like you explained, and b) send the "invalid touch"
event instead of a touch event to the calibrator app.

This makes the calibrator app show a big red cross, so the user will
know something is strange if he thinks he did touch the correct spot.

Well, it does touch-downs at least. I think if motion event would end
up out of range, I'll send cancel event followed by invalid touch.
Whee, I found use for the cancel event!
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
These devices are a problem because we rely on the coordinate range for the
size and then on the calibration on top to normalize the input. So they may
need two fixes, one for the range adjustment, one for calibration.
If I can just transmit whatever values I get for "normalized" with
identity in the above, then the calibration matrix will automatically
account for any out-of-range values - I mean they become a non-issue,
because the compositor just tranforms them to the output or a global
coordinate system first and then clips to the actual output area. Or
doesn't clip, makes no difference.
Post by Peter Hutterer
Post by Pekka Paalanen
It doesn't really matter, the important point is that when the
resulting matrix is loaded into libinput and then libinput is used
according to its API, the result is as expected. I'd be tempted to just
punt all this to libinput.
libinput will sort-of punt it back :)
https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tablet-bounds
I don't see a problem in that section. The goal of this calibration is
to match input event coordinates to output pixel coordinates in the
complete pipeline by adjusting the normalized calibration matrix in
libinput. It's ok for calibrated input coordinates go beyond any
limits, the input and output coordinate planes can be thought as
infinite.
Or do you refer to the functions returning positions in millimeters? I
never looked at those, they're not used with touchscreens. I don't even
know if a calibration matrix affects them.
There's a corner-case with touchscreens and mm. Plenty of cheap touchscreens
don't have a resolution set, so the mm value is calculated using the default
resolution of 1.
The calibration matrix is applied to the mm as well. That's largely a
side-effect for the matrix originally being used for rotation rather than
scaling or translation.
Right. If someone needs this calibration method to work on devices that
user by millimeter units, I'll let them figure out what to fix if
anything. :-)


Thanks,
pq
Peter Hutterer
2018-04-16 03:40:44 UTC
Permalink
Post by Pekka Paalanen
On Fri, 13 Apr 2018 14:31:39 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
On Wed, 11 Apr 2018 10:16:46 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
+ <interface name="weston_touch_calibrator" version="1">
+ <description summary="calibrator surface for a specific touch device">
+ On creation, this object is tied to a specific touch device. The
+ server sends a configure event which the client must obey with the
+ associated wl_surface.
+
+ Once the client has committed content to the surface, the server can
+ grab the touch input device, prevent it from emitting normal touch events,
+ show the surface on the correct output, and relay input events from the
+ touch device via this protocol object.
+
+ Touch events from other touch devices than the one tied to this object
+ must generate wrong_touch events on at least touch-down and must not
+ generate normal or calibration touch events.
+
+ At any time, the server can choose to cancel the calibration procedure by
+ sending the cancel_calibration event. This should also be used if the
+ touch device disappears or anything else prevents the calibration from
+ continuing on the server side.
+
+ If the wl_surface is destroyed, the server must cancel the calibration.
+
+ The touch event coordinates and conversion results are delivered in
+ calibration units. Calibration units are in the closed interval
+ [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be
should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
width and height of the device's sensor. Or is this something we need to
hide?
I'm not sure. The underlying assumption is that there is a finite and
closed range of values a sensor can output, and that is mapped to [0,
1]. I don't know what nominal width and height are or can the values
extend to negative. It might be easier to define the calibration units
without defining those first.
sorry, I used "nominal" before I rewrote to use "sensor", probably made it
more confusing. On some devices, the sensor is larger than the screen so you
can get coordinates outside the screen area. This is not a technical
problem, just a user perception mismatch that doens't need to be exposed and
after all that's what calibration is about.
Once you apply calibration though you can get real negative coordinates,
especially if the device advertises a [n:m] range but then sends events less
than n. For example, all synaptics touchpads (pre 2014, some after that) do
that.
But wait, if there are actually devices that report the range [n:m] but
can still send values less than n or greater than m, it means I cannot
transmit those with the encoding set up here, because
libinput_event_touch_get_x_transformed(touch_event, 1) can return
values outside of [0, 1] even when the loaded calibration matrix is
identity. Is that right?
correct, that can happen.
Post by Pekka Paalanen
Do I need to find an encoding to cope with that, or can I just reject
such touches?
I mean, the range [n:m] is in raw input coordinates, before any
normalization or calibration in userspace, right?
uhm. let me just detail this and you can pick which one was the answer to
- the kernel exposes [n:m] axis ranges for ABS_X/Y and the MT equivalents
- libinput converts this into mm by default, see libinput_event_touch_get_x()
- libinput converts this to a ranged value [0:N] when calling
libnput_event_touch_get_x_transformed(N). This is what weston uses to get
the screen coordinates.
Both libinput values have the calibration factored in already.
Weston doesn't ever see raw input coordinates. But if you use
get_x_transformed, you can get values outside [0:N]. This happens when
the [n:m] range is incorrect. Accommodating for some error margin, libinput
will print a warning to the log when this happens.
The correct solution would be to warn the user that the device is garbage
and make them put a 60-evdev.hwdb quirk in to fix the axis ranges.
Depending on the device, this could be a generic solution but let's not bet
on that...
The user-friendly solution would be to have two layers of mapping. By
default, you map the libinput range into [0, 1]. Whenever you see negative
values or values > 1 you notify the user that this screen is garbage. The
only option you have now is to get the user to touch the furthest extents of
all axes to get the lowest and highest values the axis provides (similar to
the touchpad-edge-detector tool). Then you can map those into the libinput
provided range so that e.g. libinput's -0.25 is normalized 0 and 1.25 is
normalized 1.0.
Now you can restart calibration with that extra mapping on top.
- even with the calibration matrix applied, the libinput ranges will still
be outside the [0:N] range. Only a real axis fix can fix that
- you cannot know the true min/max until you get respective event from the
screen
So yeah, the situation is less than ideal...
I've considered auto-scaling libinput into the new axis range but this is
likely going to hurt even more because it's undiscoverable and for most
devices, a quirk in the hwdb file can solve this problem properly.
Thank you very much for the thorough explanation.
I think the appropriate solution for me here is to check whether the
normalized value I expect to be in [0, 1] falls outside of the range,
and if so, I will a) print a warning in Weston's log that the device
may need a quirk like you explained, and b) send the "invalid touch"
event instead of a touch event to the calibrator app.
This makes the calibrator app show a big red cross, so the user will
know something is strange if he thinks he did touch the correct spot.
I'd argue that this is worthy of a custom event, given that the client will
have to provide a specific UI for this case. But it's also not worth
worrying about in the first iteration of the protocol.
Post by Pekka Paalanen
Well, it does touch-downs at least. I think if motion event would end
up out of range, I'll send cancel event followed by invalid touch.
Whee, I found use for the cancel event!
The cancel event would also be used for the multitouch-case, right?
If a second touch appears before the first one is fully processed, you also
need to cancel the first touch. Doesn't even need to be intentional by the
user, could be a palm touch or an accidental touch where they're holding the
screen on an edge.

Cheers,
Peter
Post by Pekka Paalanen
Post by Peter Hutterer
Post by Pekka Paalanen
Post by Peter Hutterer
These devices are a problem because we rely on the coordinate range for the
size and then on the calibration on top to normalize the input. So they may
need two fixes, one for the range adjustment, one for calibration.
If I can just transmit whatever values I get for "normalized" with
identity in the above, then the calibration matrix will automatically
account for any out-of-range values - I mean they become a non-issue,
because the compositor just tranforms them to the output or a global
coordinate system first and then clips to the actual output area. Or
doesn't clip, makes no difference.
Post by Peter Hutterer
Post by Pekka Paalanen
It doesn't really matter, the important point is that when the
resulting matrix is loaded into libinput and then libinput is used
according to its API, the result is as expected. I'd be tempted to just
punt all this to libinput.
libinput will sort-of punt it back :)
https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tablet-bounds
I don't see a problem in that section. The goal of this calibration is
to match input event coordinates to output pixel coordinates in the
complete pipeline by adjusting the normalized calibration matrix in
libinput. It's ok for calibrated input coordinates go beyond any
limits, the input and output coordinate planes can be thought as
infinite.
Or do you refer to the functions returning positions in millimeters? I
never looked at those, they're not used with touchscreens. I don't even
know if a calibration matrix affects them.
There's a corner-case with touchscreens and mm. Plenty of cheap touchscreens
don't have a resolution set, so the mm value is calculated using the default
resolution of 1.
The calibration matrix is applied to the mm as well. That's largely a
side-effect for the matrix originally being used for rotation rather than
scaling or translation.
Right. If someone needs this calibration method to work on devices that
user by millimeter units, I'll let them figure out what to fix if
anything. :-)
Thanks,
pq
Pekka Paalanen
2018-04-18 08:30:42 UTC
Permalink
On Mon, 16 Apr 2018 13:40:44 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
I think the appropriate solution for me here is to check whether the
normalized value I expect to be in [0, 1] falls outside of the range,
and if so, I will a) print a warning in Weston's log that the device
may need a quirk like you explained, and b) send the "invalid touch"
event instead of a touch event to the calibrator app.
This makes the calibrator app show a big red cross, so the user will
know something is strange if he thinks he did touch the correct spot.
I'd argue that this is worthy of a custom event, given that the client will
have to provide a specific UI for this case. But it's also not worth
worrying about in the first iteration of the protocol.
Right, I won't add another event for now.
Post by Peter Hutterer
Post by Pekka Paalanen
Well, it does touch-downs at least. I think if motion event would end
up out of range, I'll send cancel event followed by invalid touch.
Whee, I found use for the cancel event!
The cancel event would also be used for the multitouch-case, right?
If a second touch appears before the first one is fully processed, you also
need to cancel the first touch. Doesn't even need to be intentional by the
user, could be a palm touch or an accidental touch where they're holding the
screen on an edge.
I hadn't even thought of that, sounds good.


Thanks,
pq
Pekka Paalanen
2018-04-24 11:03:41 UTC
Permalink
On Wed, 18 Apr 2018 11:30:42 +0300
Post by Pekka Paalanen
On Mon, 16 Apr 2018 13:40:44 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Well, it does touch-downs at least. I think if motion event would end
up out of range, I'll send cancel event followed by invalid touch.
Whee, I found use for the cancel event!
The cancel event would also be used for the multitouch-case, right?
If a second touch appears before the first one is fully processed, you also
need to cancel the first touch. Doesn't even need to be intentional by the
user, could be a palm touch or an accidental touch where they're holding the
screen on an edge.
I hadn't even thought of that, sounds good.
Actually, I don't have to filter multiple touchpoints in the server.
The protocol is copied from the wl_touch interface and can handle
multiple touch points just fine. It can be up to the client to enter a
denial mode on a second touch down until they are all up.

Likewise, if the user puts first touch down on a right device, and a
second touch down on a wrong device which results in a invalid_touch
event, it can be up to the client to decide whether it accepts the
first touch or not.

I'll just have to make the client deal with those.

Do you see any problem with this?


Thanks,
pq
Peter Hutterer
2018-04-26 04:37:02 UTC
Permalink
Post by Pekka Paalanen
On Wed, 18 Apr 2018 11:30:42 +0300
Post by Pekka Paalanen
On Mon, 16 Apr 2018 13:40:44 +1000
Post by Peter Hutterer
Post by Pekka Paalanen
Well, it does touch-downs at least. I think if motion event would end
up out of range, I'll send cancel event followed by invalid touch.
Whee, I found use for the cancel event!
The cancel event would also be used for the multitouch-case, right?
If a second touch appears before the first one is fully processed, you also
need to cancel the first touch. Doesn't even need to be intentional by the
user, could be a palm touch or an accidental touch where they're holding the
screen on an edge.
I hadn't even thought of that, sounds good.
Actually, I don't have to filter multiple touchpoints in the server.
The protocol is copied from the wl_touch interface and can handle
multiple touch points just fine. It can be up to the client to enter a
denial mode on a second touch down until they are all up.
Likewise, if the user puts first touch down on a right device, and a
second touch down on a wrong device which results in a invalid_touch
event, it can be up to the client to decide whether it accepts the
first touch or not.
I'll just have to make the client deal with those.
Do you see any problem with this?
nope, all good. Sorry, I forgot about the touch ID being in the events...

Cheers,
Peter
Pekka Paalanen
2018-03-23 12:01:04 UTC
Permalink
This post might be inappropriate. Click to display it.
Pekka Paalanen
2018-03-23 12:01:05 UTC
Permalink
This post might be inappropriate. Click to display it.
Pekka Paalanen
2018-03-23 12:01:03 UTC
Permalink
From: Louis-Francis Ratté-Boulianne <***@collabora.com>

Add an option to enable the touchscreen calibrator interface. This is a
global on/off toggle, in lack of more fine-grained access restrictions.

As Weston should not hardcode system specifics, the actual permanent
saving of a new calibration is left for a user supplied script or a
program. Usually this script would write an appropriate udev rule to set
LIBINPUT_CALIBRATION_MATRIX for the touch device.

Co-developed by Louis-Francis and Pekka.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
---
compositor/main.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
man/weston.ini.man | 36 +++++++++++++++++++++++++++++
2 files changed, 104 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index 6650676a..caea25b9 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -792,6 +792,64 @@ load_modules(struct weston_compositor *ec, const char *modules,
return 0;
}

+static int
+save_touch_device_calibration(struct weston_compositor *compositor,
+ struct weston_touch_device *device,
+ const struct weston_touch_device_matrix *calibration)
+{
+ struct weston_config_section *s;
+ struct weston_config *config = wet_get_config(compositor);
+ char *helper = NULL;
+ char *helper_cmd = NULL;
+ int ret = -1;
+ int status;
+ const float *m = calibration->m;
+
+ s = weston_config_get_section(config,
+ "libinput", NULL, NULL);
+
+ weston_config_section_get_string(s, "calibration_helper",
+ &helper, NULL);
+
+ if (!helper || strlen(helper) == 0) {
+ ret = 0;
+ goto out;
+ }
+
+ if (asprintf(&helper_cmd, "\"%s\" '%s' %f %f %f %f %f %f",
+ helper, device->devpath,
+ m[0], m[1], m[2],
+ m[3], m[4], m[5]) < 0)
+ goto out;
+
+ status = system(helper_cmd);
+ free(helper_cmd);
+
+ if (status < 0) {
+ weston_log("Error: failed to run calibration helper '%s'.\n",
+ helper);
+ goto out;
+ }
+
+ if (!WIFEXITED(status)) {
+ weston_log("Error: calibration helper '%s' possibly killed.\n",
+ helper);
+ goto out;
+ }
+
+ if (WEXITSTATUS(status) == 0) {
+ ret = 0;
+ } else {
+ weston_log("Calibration helper '%s' exited with status %d.\n",
+ helper, WEXITSTATUS(status));
+ }
+
+out:
+ free(helper);
+
+ return ret;
+}
+
static int
weston_compositor_init_config(struct weston_compositor *ec,
struct weston_config *config)
@@ -800,7 +858,9 @@ weston_compositor_init_config(struct weston_compositor *ec,
struct weston_config_section *s;
int repaint_msec;
int vt_switching;
+ int cal;

+ /* weston.ini [keyboard] */
s = weston_config_get_section(config, "keyboard", NULL, NULL);
weston_config_section_get_string(s, "keymap_rules",
(char **) &xkb_names.rules, NULL);
@@ -825,6 +885,7 @@ weston_compositor_init_config(struct weston_compositor *ec,
&vt_switching, true);
ec->vt_switching = vt_switching;

+ /* weston.ini [core] */
s = weston_config_get_section(config, "core", NULL, NULL);
weston_config_section_get_int(s, "repaint-window", &repaint_msec,
ec->repaint_msec);
@@ -837,6 +898,13 @@ weston_compositor_init_config(struct weston_compositor *ec,
weston_log("Output repaint window is %d ms maximum.\n",
ec->repaint_msec);

+ /* weston.ini [libinput] */
+ s = weston_config_get_section(config, "libinput", NULL, NULL);
+ weston_config_section_get_bool(s, "touchscreen_calibrator", &cal, 0);
+ if (cal)
+ weston_compositor_enable_touch_calibrator(ec,
+ save_touch_device_calibration);
+
return 0;
}

diff --git a/man/weston.ini.man b/man/weston.ini.man
index f237fd60..2d261fb9 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -200,8 +200,44 @@ Available configuration are:
.TP 7
.BI "enable_tap=" true
enables tap to click on touchpad devices
+.TP 7
+.BI "touchscreen_calibrator=" true
+Advertise the touchscreen calibrator interface to all clients. This is a
+potential denial-of-service attack vector, so it should only be enabled on
+trusted userspace. Boolean, defaults to
+.BR false .
+
+The interface is required for running touchscreen calibrator applications. It
+provides the application raw touch events, bypassing the normal touch handling.
+It also allows the application to upload a new calibration into the compositor.
+
+Even though this option is listed in the libinput section, it does affect all
+Weston configurations regardless of the used backend. If the backend does not
+use libinput, the interface can still be advertised, but it will not list any
+devices.
+.TP 7
+.BI "calibration_helper=" /bin/echo
+An optional calibration helper program to permanently save a new touchscreen
+calibration. String, defaults to unset.
+
+The given program will be executed with seven arguments when a calibrator
+application requests the server to take a new calibration matrix into use.
+The program is executed synchronously and will therefore block Weston for its
+duration. If the program exit status is non-zero, Weston will not apply the
+new calibration. If the helper is unset or the program exit status is zero,
+Weston will use the new calibration immediately.
+
+The program is invoked as:
+.PP
+.RS 10
+.I calibration_helper devpath m1 m2 m3 m4 m5 m6
+.RE
.RS
.PP
+.RI "where " devpath " is the udev devpath and " m1 " through " m6
+are the calibration matrix elements in libinput's
+.BR LIBINPUT_CALIBRATION_MATRIX " udev property format."
+.RE

.SH "SHELL SECTION"
The
--
2.16.1
Matt Hoosier
2018-03-23 13:46:46 UTC
Permalink
I am very much in favor of the overall approach on this patch series.
I've experienced every single one of the problems described in this
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
- You can unambiguously pick a physical touch device to calibrate.
- You can be sure your touch events come only from that particular
device, and that you cannot miss touch events even if the current
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on the right
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens that are
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without playing
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
This series contains many patches that could be landed separately, but I
have not pulled them out at this point.
A notable single patch is patch 3 which deprecates the udev property
WL_CALIBRATION. It is completely already superseded by
LIBINPUT_CALIBRATION_MATRIX property. I would hope to remove
WL_CALIBRATION support after few releases and the old calibrator tool
with it.
Patch 21 adds the touchscreen calibration protocol, patch 24 adds the
new calibrator tool, and patch 25 gives suggestions on how to let the
server permanently store a new calibration into udev rules.
As LIBINPUT_CALIBRATION_MATRIX is automatically handled by libinput,
being a description of the physical input device (like e.g. mouse dpi),
it perfectly possible to run Weston and the calibrator once to calibrate
your touchscreens, and then run whatever compositor you want.
One major thing this patch series does not address is how a Wayland
client gets authorized to use the touchscreen calibration interface. The
interface offers two privileged actions: the ability to grab all touch
input (but with provision for the server to cancel at will - not
implemented in weston so far), and the ability to upload a new
calibration for a touch device. Obviously these should not be free for
all. The method implemented here is a simple global configuration
setting to advertise or not the touchscreen calibration interface. If
advertised, it is free for all. Therefore it is not advertised by
default.
Thanks,
pq
input: introduce weston_touch_device
libweston: fix weston_touch_start_grab() arg name
input: move touchpoint counting up
input: introduce touch event mode for calibrator
libweston: implement touch calibration protocol
weston: add touchscreen_calibrator option
clients: add a new touchscreen calibrator
libinput: remove evdev_device::devnode
libinput: note if calibrating without an output
libinput: deprecate WL_CALIBRATION
libinput: log input device to output associations
libinput: make setting the same output a no-op
libinput: allow evdev_device_set_output(dev, NULL)
libinput: use head names for output matching
libweston: require connected heads for input devices
libinput: do not switch output associations on disable
man: document WESTON_LIBINPUT_LOG_PRIORITY env
tests: add test_seat_release() for symmetry
libinput: move calibration printing into do_set_calibration()
libweston: notify_touch API to use weston_touch_device
libweston: unexport weston_{pointer,keyboard,touch}_{create,destroy}()
libweston: introduce notify_touch_cal() and doc
input: do not forward unmatched touch-ups
protocol: add weston_touch_calibration
doc: add example calibration-helper script
.gitignore | 1 +
Makefile.am | 21 +-
clients/touch-calibrator.c | 774 ++++++++++++++++++++++++++++++++++
clients/window.c | 4 +-
clients/window.h | 4 +
compositor/main.c | 68 +++
doc/calibration-helper.bash | 44 ++
libweston/compositor-wayland.c | 31 +-
libweston/compositor.c | 32 ++
libweston/compositor.h | 174 +++++++-
libweston/input.c | 330 +++++++++++++--
libweston/libinput-device.c | 238 ++++++++---
libweston/libinput-device.h | 4 +-
libweston/libinput-seat.c | 88 +++-
libweston/libinput-seat.h | 1 +
libweston/touch-calibration.c | 667 +++++++++++++++++++++++++++++
man/weston-drm.man | 4 +
man/weston.ini.man | 36 ++
protocol/weston-touch-calibration.xml | 320 ++++++++++++++
tests/weston-test.c | 64 ++-
20 files changed, 2771 insertions(+), 134 deletions(-)
create mode 100644 clients/touch-calibrator.c
create mode 100755 doc/calibration-helper.bash
create mode 100644 libweston/touch-calibration.c
create mode 100644 protocol/weston-touch-calibration.xml
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-03-23 14:30:12 UTC
Permalink
On Fri, 23 Mar 2018 08:46:46 -0500
Post by Matt Hoosier
I am very much in favor of the overall approach on this patch series.
I've experienced every single one of the problems described in this
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Hi Matt,

that is very heartwarming to hear. Is your tool specifically for Weston
too?

I would be very happy if this proposal fits your needs, and certainly
interested in hearing where it falls short.


Thanks,
pq
Post by Matt Hoosier
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
- You can unambiguously pick a physical touch device to calibrate.
- You can be sure your touch events come only from that particular
device, and that you cannot miss touch events even if the current
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on the right
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens that are
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without playing
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
Matt Hoosier
2018-03-23 14:38:27 UTC
Permalink
Post by Pekka Paalanen
On Fri, 23 Mar 2018 08:46:46 -0500
Post by Matt Hoosier
I am very much in favor of the overall approach on this patch series.
I've experienced every single one of the problems described in this
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Hi Matt,
that is very heartwarming to hear. Is your tool specifically for Weston
too?
Yes and no. It's not phrased as a patch against the Weston source
code, but it uses heuristics for determining which output the raw
/dev/input/* events should be correlated against, and those heuristics
probably would fail if some different compositor happened to be
running.
Post by Pekka Paalanen
I would be very happy if this proposal fits your needs, and certainly
interested in hearing where it falls short.
Thanks,
pq
Post by Matt Hoosier
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
- You can unambiguously pick a physical touch device to calibrate.
- You can be sure your touch events come only from that particular
device, and that you cannot miss touch events even if the current
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on the right
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens that are
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without playing
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
Matt Hoosier
2018-04-02 13:11:24 UTC
Permalink
Do you happen to know of a good readymade program that fakes the presence
of a touch input device with uinput? I'd love to test this series, but
current Weston is far ahead of what my embedded devices will do; so I'm in
the position of mostly relying on the desktop for testing.
Post by Matt Hoosier
Post by Pekka Paalanen
On Fri, 23 Mar 2018 08:46:46 -0500
Post by Matt Hoosier
I am very much in favor of the overall approach on this patch series.
I've experienced every single one of the problems described in this
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Hi Matt,
that is very heartwarming to hear. Is your tool specifically for Weston
too?
Yes and no. It's not phrased as a patch against the Weston source
code, but it uses heuristics for determining which output the raw
/dev/input/* events should be correlated against, and those heuristics
probably would fail if some different compositor happened to be
running.
Post by Pekka Paalanen
I would be very happy if this proposal fits your needs, and certainly
interested in hearing where it falls short.
Thanks,
pq
Post by Matt Hoosier
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems.
This
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
- You can unambiguously pick a physical touch device to calibrate.
- You can be sure your touch events come only from that particular
device, and that you cannot miss touch events even if the current
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on the
right
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens that are
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without playing
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
Pekka Paalanen
2018-04-03 09:20:08 UTC
Permalink
On Mon, 02 Apr 2018 13:11:24 +0000
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the presence
of a touch input device with uinput? I'd love to test this series, but
current Weston is far ahead of what my embedded devices will do; so I'm in
the position of mostly relying on the desktop for testing.
Hi Matt,

sorry, I have no knowledge of such tool, and the lack of it has
prevented me from testing things before as well.

Right now I have two borrowed touchscreens to work with, but I need to
send them back one day.


Thanks,
pq
Peter Hutterer
2018-04-04 01:47:37 UTC
Permalink
Post by Pekka Paalanen
On Mon, 02 Apr 2018 13:11:24 +0000
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the presence
of a touch input device with uinput? I'd love to test this series, but
current Weston is far ahead of what my embedded devices will do; so I'm in
the position of mostly relying on the desktop for testing.
Hi Matt,
sorry, I have no knowledge of such tool, and the lack of it has
prevented me from testing things before as well.
Right now I have two borrowed touchscreens to work with, but I need to
send them back one day.
$ cat /etc/udev/rules.d/99-touchpads-are-touchscreens.rules
ACTION!="add|change", GOTO="_end"
ENV{ID_INPUT_TOUCHPAD}=="1", ENV{ID_INPUT_TOUCHPAD}="", ENV{ID_INPUT_TOUCHSCREEN}="1"
LABEL="_end"

This turns your touchpad into a touchscreen, should be good enough for
testing. Afterwards, reboot or run

sudo udevadm test /sys/class/input/event17
sudo udevadm test /sys/class/input/event17/device

with your event node. Note that you need both the event node and the parent
input device to avoid both flags being set.

Cheers,
Peter
Philipp Kerling
2018-04-03 10:35:13 UTC
Permalink
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the
presence of a touch input device with uinput? I'd love to test this
series, but current Weston is far ahead of what my embedded devices
will do; so I'm in the position of mostly relying on the desktop for
testing.
I'm not sure whether it fits your use case, but you can give mtemu a
spin.

https://gitlab.com/shul/mtemu
Post by Matt Hoosier
Post by Matt Hoosier
Post by Pekka Paalanen
On Fri, 23 Mar 2018 08:46:46 -0500
Post by Matt Hoosier
I am very much in favor of the overall approach on this patch
series.
Post by Pekka Paalanen
Post by Matt Hoosier
I've experienced every single one of the problems described in
this
Post by Pekka Paalanen
Post by Matt Hoosier
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Hi Matt,
that is very heartwarming to hear. Is your tool specifically for
Weston
Post by Pekka Paalanen
too?
Yes and no. It's not phrased as a patch against the Weston source
code, but it uses heuristics for determining which output the raw
/dev/input/* events should be correlated against, and those
heuristics
probably would fail if some different compositor happened to be
running.
Post by Pekka Paalanen
I would be very happy if this proposal fits your needs, and
certainly
Post by Pekka Paalanen
interested in hearing where it falls short.
Thanks,
pq
Post by Matt Hoosier
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several
problems. This
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
proposal intends to solve them all by introducing a new
protocol
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
extension for touchscreen calibration and a new calibrator
tool.
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
- You can unambiguously pick a physical touch device to
calibrate.
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
- You can be sure your touch events come only from that
particular
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
device, and that you cannot miss touch events even if the
current
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on
the right
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens
that are
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without
playing
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Matt Hoosier
2018-04-03 13:10:41 UTC
Permalink
Post by Philipp Kerling
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the
presence of a touch input device with uinput? I'd love to test this
series, but current Weston is far ahead of what my embedded devices
will do; so I'm in the position of mostly relying on the desktop for
testing.
I'm not sure whether it fits your use case, but you can give mtemu a
spin.
https://gitlab.com/shul/mtemu


Thanks. That looks like a really useful program. As it turns out, I ended
up being able to find some real touch hardware to use. It was ancient stuff
that required writing a uinput program to translate the old ABS_X/Y-style
input into modern mtdev representation, but it eventually worked.

The calibration functionality in Pekka's patch series seems to work for me.
I didn't test out any of the mechanism to spawn an auxiliary program that
saves the results to disk, but the hot-installed copy of the calibration
worked.

I did have a little trouble understanding how I can pick-and-choose from
the client side which touch device should be associated with which output.
The new weston-touch-calibrator program just prints out a flat list of
input devices and head names, but doesn't seem to let you do permutations
of that.

So I ended up having to use a 'mode=off' directive in weston.ini to
temporarily turn off the main display of my laptop so that the touchscreen
got pegged to the correct physical output. I have the feeling that I just
overlooked some aspect of configuration about this.

-Matt
Post by Philipp Kerling
Post by Matt Hoosier
Post by Matt Hoosier
Post by Pekka Paalanen
On Fri, 23 Mar 2018 08:46:46 -0500
Post by Matt Hoosier
I am very much in favor of the overall approach on this patch
series.
Post by Pekka Paalanen
Post by Matt Hoosier
I've experienced every single one of the problems described in
this
Post by Pekka Paalanen
Post by Matt Hoosier
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Hi Matt,
that is very heartwarming to hear. Is your tool specifically for
Weston
Post by Pekka Paalanen
too?
Yes and no. It's not phrased as a patch against the Weston source
code, but it uses heuristics for determining which output the raw
/dev/input/* events should be correlated against, and those heuristics
probably would fail if some different compositor happened to be
running.
Post by Pekka Paalanen
I would be very happy if this proposal fits your needs, and
certainly
Post by Pekka Paalanen
interested in hearing where it falls short.
Thanks,
pq
Post by Matt Hoosier
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several
problems. This
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
proposal intends to solve them all by introducing a new
protocol
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
extension for touchscreen calibration and a new calibrator
tool.
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
- You can unambiguously pick a physical touch device to
calibrate.
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
- You can be sure your touch events come only from that
particular
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
device, and that you cannot miss touch events even if the
current
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on
the right
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens
that are
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without
playing
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Matt Hoosier
2018-04-04 00:37:58 UTC
Permalink
Post by Philipp Kerling
Post by Philipp Kerling
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the
presence of a touch input device with uinput? I'd love to test this
series, but current Weston is far ahead of what my embedded devices
will do; so I'm in the position of mostly relying on the desktop for
testing.
I'm not sure whether it fits your use case, but you can give mtemu a
spin.
https://gitlab.com/shul/mtemu
Thanks. That looks like a really useful program. As it turns out, I ended
up being able to find some real touch hardware to use. It was ancient stuff
that required writing a uinput program to translate the old ABS_X/Y-style
input into modern mtdev representation, but it eventually worked.
The calibration functionality in Pekka's patch series seems to work for
me. I didn't test out any of the mechanism to spawn an auxiliary program
that saves the results to disk, but the hot-installed copy of the
calibration worked.
I did have a little trouble understanding how I can pick-and-choose from
the client side which touch device should be associated with which output.
The new weston-touch-calibrator program just prints out a flat list of
input devices and head names, but doesn't seem to let you do permutations
of that.
So I ended up having to use a 'mode=off' directive in weston.ini to
temporarily turn off the main display of my laptop so that the touchscreen
got pegged to the correct physical output. I have the feeling that I just
overlooked some aspect of configuration about this.
... which is of course the statically set UDev properties that associate an
input device with the Wayland output. Nevermind about this question.
Post by Philipp Kerling
-Matt
Post by Philipp Kerling
Post by Matt Hoosier
Post by Matt Hoosier
Post by Pekka Paalanen
On Fri, 23 Mar 2018 08:46:46 -0500
Post by Matt Hoosier
I am very much in favor of the overall approach on this patch
series.
Post by Pekka Paalanen
Post by Matt Hoosier
I've experienced every single one of the problems described in
this
Post by Pekka Paalanen
Post by Matt Hoosier
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.
Hi Matt,
that is very heartwarming to hear. Is your tool specifically for
Weston
Post by Pekka Paalanen
too?
Yes and no. It's not phrased as a patch against the Weston source
code, but it uses heuristics for determining which output the raw
/dev/input/* events should be correlated against, and those heuristics
probably would fail if some different compositor happened to be
running.
Post by Pekka Paalanen
I would be very happy if this proposal fits your needs, and
certainly
Post by Pekka Paalanen
interested in hearing where it falls short.
Thanks,
pq
Post by Matt Hoosier
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several
problems. This
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
proposal intends to solve them all by introducing a new
protocol
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
extension for touchscreen calibration and a new calibrator
tool.
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
- You can unambiguously pick a physical touch device to
calibrate.
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
- You can be sure your touch events come only from that
particular
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
device, and that you cannot miss touch events even if the
current
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on
the right
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens
that are
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without
playing
Post by Pekka Paalanen
Post by Matt Hoosier
Post by Pekka Paalanen
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-04-04 07:27:57 UTC
Permalink
On Wed, 04 Apr 2018 00:37:58 +0000
Post by Matt Hoosier
Post by Philipp Kerling
Post by Philipp Kerling
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the
presence of a touch input device with uinput? I'd love to test this
series, but current Weston is far ahead of what my embedded devices
will do; so I'm in the position of mostly relying on the desktop for
testing.
I'm not sure whether it fits your use case, but you can give mtemu a
spin.
https://gitlab.com/shul/mtemu
Thanks. That looks like a really useful program. As it turns out, I ended
up being able to find some real touch hardware to use. It was ancient stuff
that required writing a uinput program to translate the old ABS_X/Y-style
input into modern mtdev representation, but it eventually worked.
The calibration functionality in Pekka's patch series seems to work for
me. I didn't test out any of the mechanism to spawn an auxiliary program
that saves the results to disk, but the hot-installed copy of the
calibration worked.
I did have a little trouble understanding how I can pick-and-choose from
the client side which touch device should be associated with which output.
The new weston-touch-calibrator program just prints out a flat list of
input devices and head names, but doesn't seem to let you do permutations
of that.
So I ended up having to use a 'mode=off' directive in weston.ini to
temporarily turn off the main display of my laptop so that the touchscreen
got pegged to the correct physical output. I have the feeling that I just
overlooked some aspect of configuration about this.
... which is of course the statically set UDev properties that associate an
input device with the Wayland output. Nevermind about this question.
Indeed. There is no runtime way to change the touchscreen/output
associations in Weston, it picks it up from the udev properties on the
touch device. Other DEs may have their ways, but I considered it out of
scope for this protocol extension.

Thank you for testing! I wonder if I can out your Tested-by tag in all
the patches, or maybe some specific patch... like the "clients: add a
new touchscreen calibrator"?


Thanks,
pq
Matt Hoosier
2018-04-04 12:59:31 UTC
Permalink
Post by Pekka Paalanen
On Wed, 04 Apr 2018 00:37:58 +0000
Post by Matt Hoosier
Post by Philipp Kerling
Post by Philipp Kerling
Post by Matt Hoosier
Do you happen to know of a good readymade program that fakes the
presence of a touch input device with uinput? I'd love to test this
series, but current Weston is far ahead of what my embedded devices
will do; so I'm in the position of mostly relying on the desktop for
testing.
I'm not sure whether it fits your use case, but you can give mtemu a
spin.
https://gitlab.com/shul/mtemu
Thanks. That looks like a really useful program. As it turns out, I
ended
Post by Matt Hoosier
Post by Philipp Kerling
up being able to find some real touch hardware to use. It was ancient
stuff
Post by Matt Hoosier
Post by Philipp Kerling
that required writing a uinput program to translate the old
ABS_X/Y-style
Post by Matt Hoosier
Post by Philipp Kerling
input into modern mtdev representation, but it eventually worked.
The calibration functionality in Pekka's patch series seems to work for
me. I didn't test out any of the mechanism to spawn an auxiliary
program
Post by Matt Hoosier
Post by Philipp Kerling
that saves the results to disk, but the hot-installed copy of the
calibration worked.
I did have a little trouble understanding how I can pick-and-choose
from
Post by Matt Hoosier
Post by Philipp Kerling
the client side which touch device should be associated with which
output.
Post by Matt Hoosier
Post by Philipp Kerling
The new weston-touch-calibrator program just prints out a flat list of
input devices and head names, but doesn't seem to let you do
permutations
Post by Matt Hoosier
Post by Philipp Kerling
of that.
So I ended up having to use a 'mode=off' directive in weston.ini to
temporarily turn off the main display of my laptop so that the
touchscreen
Post by Matt Hoosier
Post by Philipp Kerling
got pegged to the correct physical output. I have the feeling that I
just
Post by Matt Hoosier
Post by Philipp Kerling
overlooked some aspect of configuration about this.
... which is of course the statically set UDev properties that associate
an
Post by Matt Hoosier
input device with the Wayland output. Nevermind about this question.
Indeed. There is no runtime way to change the touchscreen/output
associations in Weston, it picks it up from the udev properties on the
touch device. Other DEs may have their ways, but I considered it out of
scope for this protocol extension.
Thank you for testing! I wonder if I can out your Tested-by tag in all
the patches, or maybe some specific patch... like the "clients: add a
new touchscreen calibrator"?
Sure. Let's say that all the patches in this series except "doc: add
example calibration-helper script" are:

Tested-by: Matt Hoosier <***@gmail.com>

You're welcome to apply the my T-B to the final patch too, but since it's
only documentation, I'm not sure the testing is very relevant there.
Post by Pekka Paalanen
Thanks,
pq
Jason Gerecke
2018-03-23 16:33:10 UTC
Permalink
Nice! Do you think its reasonable to extend the allowed use of this
protocol to other kinds of direct-input devices? I don't see anything
which would prevent this protocol from working equally well to
calibrate the pen input for a tablet PC or Cintiq for example. The
compositor would need to obviously have pen support and notify the
client of the device in a "touch_device" event, but otherwise it looks
like the client doesn't really have to care about the underlying
device -- the protocol provides its own definition of down/up/motion
events. Just the documentation would need to be tweaked.

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
- You can unambiguously pick a physical touch device to calibrate.
- You can be sure your touch events come only from that particular
device, and that you cannot miss touch events even if the current
calibration is horribly wrong.
- You can be sure the calibration window (pattern) is shown on the right
output with the right coordinates.
- You can unambiguously calibrate even multiple touchscreens that are
all cloned (showing the same image).
- You get a libinput style calibation matrix instead of the
WL_CALIBRATION format which depends on output resolution.
- You can load a new calibration into the compositor without playing
tricks with udev or restarting the compositor.
https://phabricator.freedesktop.org/T7868
https://patchwork.freedesktop.org/series/32898/
https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
This series contains many patches that could be landed separately, but I
have not pulled them out at this point.
A notable single patch is patch 3 which deprecates the udev property
WL_CALIBRATION. It is completely already superseded by
LIBINPUT_CALIBRATION_MATRIX property. I would hope to remove
WL_CALIBRATION support after few releases and the old calibrator tool
with it.
Patch 21 adds the touchscreen calibration protocol, patch 24 adds the
new calibrator tool, and patch 25 gives suggestions on how to let the
server permanently store a new calibration into udev rules.
As LIBINPUT_CALIBRATION_MATRIX is automatically handled by libinput,
being a description of the physical input device (like e.g. mouse dpi),
it perfectly possible to run Weston and the calibrator once to calibrate
your touchscreens, and then run whatever compositor you want.
One major thing this patch series does not address is how a Wayland
client gets authorized to use the touchscreen calibration interface. The
interface offers two privileged actions: the ability to grab all touch
input (but with provision for the server to cancel at will - not
implemented in weston so far), and the ability to upload a new
calibration for a touch device. Obviously these should not be free for
all. The method implemented here is a simple global configuration
setting to advertise or not the touchscreen calibration interface. If
advertised, it is free for all. Therefore it is not advertised by
default.
Thanks,
pq
input: introduce weston_touch_device
libweston: fix weston_touch_start_grab() arg name
input: move touchpoint counting up
input: introduce touch event mode for calibrator
libweston: implement touch calibration protocol
weston: add touchscreen_calibrator option
clients: add a new touchscreen calibrator
libinput: remove evdev_device::devnode
libinput: note if calibrating without an output
libinput: deprecate WL_CALIBRATION
libinput: log input device to output associations
libinput: make setting the same output a no-op
libinput: allow evdev_device_set_output(dev, NULL)
libinput: use head names for output matching
libweston: require connected heads for input devices
libinput: do not switch output associations on disable
man: document WESTON_LIBINPUT_LOG_PRIORITY env
tests: add test_seat_release() for symmetry
libinput: move calibration printing into do_set_calibration()
libweston: notify_touch API to use weston_touch_device
libweston: unexport weston_{pointer,keyboard,touch}_{create,destroy}()
libweston: introduce notify_touch_cal() and doc
input: do not forward unmatched touch-ups
protocol: add weston_touch_calibration
doc: add example calibration-helper script
.gitignore | 1 +
Makefile.am | 21 +-
clients/touch-calibrator.c | 774 ++++++++++++++++++++++++++++++++++
clients/window.c | 4 +-
clients/window.h | 4 +
compositor/main.c | 68 +++
doc/calibration-helper.bash | 44 ++
libweston/compositor-wayland.c | 31 +-
libweston/compositor.c | 32 ++
libweston/compositor.h | 174 +++++++-
libweston/input.c | 330 +++++++++++++--
libweston/libinput-device.c | 238 ++++++++---
libweston/libinput-device.h | 4 +-
libweston/libinput-seat.c | 88 +++-
libweston/libinput-seat.h | 1 +
libweston/touch-calibration.c | 667 +++++++++++++++++++++++++++++
man/weston-drm.man | 4 +
man/weston.ini.man | 36 ++
protocol/weston-touch-calibration.xml | 320 ++++++++++++++
tests/weston-test.c | 64 ++-
20 files changed, 2771 insertions(+), 134 deletions(-)
create mode 100644 clients/touch-calibrator.c
create mode 100755 doc/calibration-helper.bash
create mode 100644 libweston/touch-calibration.c
create mode 100644 protocol/weston-touch-calibration.xml
--
2.16.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-03-26 09:36:32 UTC
Permalink
On Fri, 23 Mar 2018 09:33:10 -0700
Post by Jason Gerecke
Nice! Do you think its reasonable to extend the allowed use of this
protocol to other kinds of direct-input devices? I don't see anything
which would prevent this protocol from working equally well to
calibrate the pen input for a tablet PC or Cintiq for example. The
compositor would need to obviously have pen support and notify the
client of the device in a "touch_device" event, but otherwise it looks
like the client doesn't really have to care about the underlying
device -- the protocol provides its own definition of down/up/motion
events. Just the documentation would need to be tweaked.
Hi Jason,

I suppose, but I am not at all familiar with such devices, so I cannot
make the call. I have never even used one.

If the design is a good match for other devices, I would be happy to
accept spec changes and renaming interfaces to be more generic, but
only if someone is working on an implementation somewhere as well.

Currently the protocol is being proposed to be Weston private. We might
land it as is, and then talk about how it should be generalized and
which package should install it for public use. We can have
libweston-dev install the XML file, or it could be proposed to be
included in wayland-protocols if it satisfies the general inclusion
requirements there. Or somewhere else.


Thanks,
pq
Post by Jason Gerecke
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
Jason Gerecke
2018-03-26 14:58:47 UTC
Permalink
Post by Pekka Paalanen
On Fri, 23 Mar 2018 09:33:10 -0700
Post by Jason Gerecke
Nice! Do you think its reasonable to extend the allowed use of this
protocol to other kinds of direct-input devices? I don't see anything
which would prevent this protocol from working equally well to
calibrate the pen input for a tablet PC or Cintiq for example. The
compositor would need to obviously have pen support and notify the
client of the device in a "touch_device" event, but otherwise it looks
like the client doesn't really have to care about the underlying
device -- the protocol provides its own definition of down/up/motion
events. Just the documentation would need to be tweaked.
Hi Jason,
I suppose, but I am not at all familiar with such devices, so I cannot
make the call. I have never even used one.
If the design is a good match for other devices, I would be happy to
accept spec changes and renaming interfaces to be more generic, but
only if someone is working on an implementation somewhere as well.
Currently the protocol is being proposed to be Weston private. We might
land it as is, and then talk about how it should be generalized and
which package should install it for public use. We can have
libweston-dev install the XML file, or it could be proposed to be
included in wayland-protocols if it satisfies the general inclusion
requirements there. Or somewhere else.
Thanks,
pq
Good to know. It certainly seems like a good match at least for this
second class of devices, so I would be very interested in seeing the
wording made a bit more generic.

Weston's tablet support is still in the works (I believe Lyude was
last working on it?) so there's not an urgent need to make these
changes, but it would be good to think about them if/when the protocol
goes public. I imagine GNOME would be remotely interested in this
protocol... Their current tablet calibration tool has to deal with
exactly the same issues that this protocol very nicely addresses (e.g.
un-transforming coordinates, screen mapping, grabbing input).

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
Post by Pekka Paalanen
Post by Jason Gerecke
Post by Pekka Paalanen
Hi all,
the existing touchscreen calibrator in Weston has several problems. This
proposal intends to solve them all by introducing a new protocol
extension for touchscreen calibration and a new calibrator tool.
Pekka Paalanen
2018-03-27 09:00:15 UTC
Permalink
On Mon, 26 Mar 2018 07:58:47 -0700
Post by Jason Gerecke
Post by Pekka Paalanen
On Fri, 23 Mar 2018 09:33:10 -0700
Post by Jason Gerecke
Nice! Do you think its reasonable to extend the allowed use of this
protocol to other kinds of direct-input devices? I don't see anything
which would prevent this protocol from working equally well to
calibrate the pen input for a tablet PC or Cintiq for example. The
compositor would need to obviously have pen support and notify the
client of the device in a "touch_device" event, but otherwise it looks
like the client doesn't really have to care about the underlying
device -- the protocol provides its own definition of down/up/motion
events. Just the documentation would need to be tweaked.
Hi Jason,
I suppose, but I am not at all familiar with such devices, so I cannot
make the call. I have never even used one.
If the design is a good match for other devices, I would be happy to
accept spec changes and renaming interfaces to be more generic, but
only if someone is working on an implementation somewhere as well.
Currently the protocol is being proposed to be Weston private. We might
land it as is, and then talk about how it should be generalized and
which package should install it for public use. We can have
libweston-dev install the XML file, or it could be proposed to be
included in wayland-protocols if it satisfies the general inclusion
requirements there. Or somewhere else.
Thanks,
pq
Good to know. It certainly seems like a good match at least for this
second class of devices, so I would be very interested in seeing the
wording made a bit more generic.
Weston's tablet support is still in the works (I believe Lyude was
last working on it?) so there's not an urgent need to make these
changes, but it would be good to think about them if/when the protocol
goes public. I imagine GNOME would be remotely interested in this
protocol... Their current tablet calibration tool has to deal with
exactly the same issues that this protocol very nicely addresses (e.g.
un-transforming coordinates, screen mapping, grabbing input).
Ok, very good. Personally I won't worry about the wording until someone
proposes a patch to install the XML file for public consumption.

The implementation I referred to does not need to be in Weston by the
way. Any serious implementation would be good enough for me to accept
spec changes.


Thanks,
pq
Loading...