[PATCH v2 xf86-input-libinput] Add tablet tool area ratio property
Jason Gerecke
killertofu at gmail.com
Fri Jan 6 19:49:52 UTC 2017
On 01/03/2017 08:38 PM, Peter Hutterer wrote:
> By default, the X server maps the tablet axes to the available screen area.
> When a tablet is mapped to the screen but has a different aspect ratio than
> the screen, input data is skewed. Expose an area ratio property to map the
> a subsection of the available tablet area into the desired ratio.
>
> Differences to the wacom driver: there the x/y min/max values must be
> specified manually and in device coordinates. For this driver we merely
> provide the area ratio (e.g. 4:3) and let the driver work out the rest.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Changes to v1:
> - link up the tools so the ratio applies to all tools
> - only init the area property for non-builtin tablets, for the built-in ones
> we don't use the area but the calibration matrix in case they're off
>
> include/libinput-properties.h | 3 +
> man/libinput.man | 30 +++++++
> src/xf86libinput.c | 188 ++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 215 insertions(+), 6 deletions(-)
>
> diff --git a/include/libinput-properties.h b/include/libinput-properties.h
> index f76500f..a701316 100644
> --- a/include/libinput-properties.h
> +++ b/include/libinput-properties.h
> @@ -190,4 +190,7 @@
> */
> #define LIBINPUT_PROP_TABLET_TOOL_PRESSURECURVE "libinput Tablet Tool Pressurecurve"
>
> +/* Tablet tool area ratio: CARD32, 2 values, w and h */
> +#define LIBINPUT_PROP_TABLET_TOOL_AREA_RATIO "libinput Tablet Tool Area Ratio"
> +
> #endif /* _LIBINPUT_PROPERTIES_H_ */
> diff --git a/man/libinput.man b/man/libinput.man
> index 88a0428..d717ff7 100644
> --- a/man/libinput.man
> +++ b/man/libinput.man
> @@ -161,6 +161,14 @@ points. The respective x/y coordinate must be in the [0.0, 1.0] range. For
> more information see section
> .B TABLET STYLUS PRESSURE CURVE.
> .TP 7
> +.BI "Option \*qTabletToolAreaRatio\*q \*q" "w:h" \*q
> +Sets the area ratio for a tablet tool. The area always starts at the
> +origin (0/0) and expands to the largest available area with the specified
> +aspect ratio. Events outside this area are cropped to the area. The special
> +value "default" is used for the default mapping (i.e. the device-native
> +mapping). For more information see section
> +.B TABLET TOOL AREA RATIO.
> +.TP 7
> .BI "Option \*qTapping\*q \*q" bool \*q
> Enables or disables tap-to-click behavior.
> .TP 7
> @@ -261,6 +269,11 @@ enabled on this device.
> .BI "libinput Tablet Tool Pressurecurve"
> 4 32-bit float values [0.0 to 1.0]. See section
> .B TABLET TOOL PRESSURE CURVE
> +.TP7
> +.BI "libinput Tablet Tool Area Ratio"
> +2 32-bit values, corresponding to width and height. Special value 0, 0
> +resets to the default ratio. See section
> +.B TABLET TOOL AREA RATIO
> for more information.
> .TP 7
> .BI "libinput Tapping Enabled"
> @@ -343,6 +356,23 @@ curve (softer) might be "0.0/0.0 0.0/0.05 0.95/1.0 1.0/1.0".
> .TP
> This feature is provided by this driver, not by libinput.
>
> +.SH TABLET TOOL AREA RATIO
> +By default, a tablet tool can access the whole sensor area and the tablet
> +area is mapped to the available screen area. For external tablets like
> +the Wacom Intuos series, the height:width ratio of the tablet may be
> +different to that of the monitor, causing the skew of input data.
> +.PP
> +To avoid this skew of input data, an area ratio may be set to match the
> +ratio of the screen device. For example, a ratio of 4:3 will reduce the
> +available area of the tablet to the largest available area with a ratio of
> +4:3. Events within this area will scale to the tablet's announced axis
> +range, the area ratio is thus transparent to the X server. Any events
> +outside this area will send events equal to the maximum value of that axis.
> +The area always starts at the device's origin in it's current rotation, i.e.
> +it takes left-handed-ness into account.
> +.TP
> +This feature is provided by this driver, not by libinput.
> +
> .SH AUTHORS
> Peter Hutterer
> .SH "SEE ALSO"
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index d43f67f..1ff0622 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -167,6 +167,9 @@ struct xf86libinput {
>
> float rotation_angle;
> struct bezier_control_point pressurecurve[4];
> + struct ratio {
> + int x, y;
> + } area;
> } options;
>
> struct draglock draglock;
> @@ -184,6 +187,10 @@ struct xf86libinput {
> int *values;
> size_t sz;
> } pressurecurve;
> +
> + struct scale_factor {
> + double x, y;
> + } area_scale_factor;
> };
>
> enum event_handling {
> @@ -418,6 +425,29 @@ xf86libinput_set_pressurecurve(struct xf86libinput *driver_data,
> driver_data->pressurecurve.sz);
> }
>
> +static inline void
> +xf86libinput_set_area_ratio(struct xf86libinput *driver_data,
> + const struct ratio *ratio)
> +{
> + double f;
> + double w, h;
> +
> + if (libinput_device_get_size(driver_data->shared_device->device, &w, &h) != 0)
> + return;
> +
> + f = 1.0 * (ratio->x * h)/(ratio->y * w);
> +
> + if (f <= 1.0) {
> + driver_data->area_scale_factor.x = 1.0/f;
> + driver_data->area_scale_factor.y = 1.0;
> + } else {
> + driver_data->area_scale_factor.x = 1.0;
> + driver_data->area_scale_factor.y = f;
> + }
> +
> + driver_data->options.area = *ratio;
> +}
This function sets my Spidey-sense tingling. I don't like how
ratio={0,0} is treated like any other valid value, causing the function
to store an NaN in area_scale_factor. I want to say that we should
explicitly handle the {0,0} case and set area_scale_factor to {1.0,1.0}
so that other functions don't have to tiptoe around bogus values that
may appear. However, area_scale_factor doesn't always have a valid value
anyway (its initial value is {0,0} until a ratio is set) so maybe it
doesn't really matter.
I'll leave the decision up to you.
> +
> static int
> LibinputSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val,
> BOOL checkonly);
> @@ -1717,6 +1747,26 @@ xf86libinput_handle_tablet_button(InputInfoPtr pInfo,
> return EVENT_HANDLED;
> }
>
> +static void
> +xf86libinput_apply_area(InputInfoPtr pInfo, double *x, double *y)
> +{
> + struct xf86libinput *driver_data = pInfo->private;
> + const struct scale_factor *f = &driver_data->area_scale_factor;
> + double sx, sy;
> +
> + if (driver_data->options.area.x == 0)
> + return;
> +
> + /* In left-handed mode, libinput already gives us transformed
> + * coordinates, so we can clip the same way. */
> +
> + sx = min(*x * f->x, TABLET_AXIS_MAX);
> + sy = min(*y * f->y, TABLET_AXIS_MAX);
> +
> + *x = sx;
> + *y = sy;
> +}
> +
> static enum event_handling
> xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
> struct libinput_event_tablet_tool *event)
> @@ -1726,16 +1776,18 @@ xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
> ValuatorMask *mask = driver_data->valuators;
> struct libinput_tablet_tool *tool;
> double value;
> + double x, y;
>
> if (xf86libinput_tool_queue_event(event))
> return EVENT_QUEUED;
>
> - value = libinput_event_tablet_tool_get_x_transformed(event,
> - TABLET_AXIS_MAX);
> - valuator_mask_set_double(mask, 0, value);
> - value = libinput_event_tablet_tool_get_y_transformed(event,
> - TABLET_AXIS_MAX);
> - valuator_mask_set_double(mask, 1, value);
> + x = libinput_event_tablet_tool_get_x_transformed(event,
> + TABLET_AXIS_MAX);
> + y = libinput_event_tablet_tool_get_y_transformed(event,
> + TABLET_AXIS_MAX);
> + xf86libinput_apply_area(pInfo, &x, &y);
> + valuator_mask_set_double(mask, 0, x);
> + valuator_mask_set_double(mask, 1, y);
>
> tool = libinput_event_tablet_tool_get_tool(event);
>
> @@ -2786,6 +2838,48 @@ out:
> xf86libinput_set_pressurecurve(driver_data, controls);
> }
>
> +static inline bool
> +want_area_handling(struct xf86libinput *driver_data)
> +{
> + struct libinput_device *device = driver_data->shared_device->device;
> +
> + if ((driver_data->capabilities & CAP_TABLET_TOOL) == 0)
> + return false;
> +
> + /* If we have a calibration matrix, it's a built-in tablet and we
> + * don't need to set the area ratio on those */
> + return !libinput_device_config_calibration_has_matrix(device);
> +}
> +
> +static void
> +xf86libinput_parse_tablet_area_option(InputInfoPtr pInfo,
> + struct xf86libinput *driver_data,
> + struct ratio *area_out)
> +{
> + char *str;
> + int rc;
> + struct ratio area;
> +
> + if (!want_area_handling(driver_data))
> + return;
> +
> + str = xf86SetStrOption(pInfo->options,
> + "TabletToolAreaRatio",
> + NULL);
> + if (!str || strcmp(str, "default") == 0)
> + goto out;
> +
> + rc = sscanf(str, "%d:%d", &area.x, &area.y);
> + if (rc != 2 || area.x <= 0 || area.y <= 0) {
> + xf86IDrvMsg(pInfo, X_ERROR, "Invalid tablet tool area ratio: %s\n", str);
> + } else {
> + *area_out = area;
> + }
> +
> +out:
> + free(str);
> +}
> +
> static void
> xf86libinput_parse_options(InputInfoPtr pInfo,
> struct xf86libinput *driver_data,
> @@ -2823,6 +2917,9 @@ xf86libinput_parse_options(InputInfoPtr pInfo,
> xf86libinput_parse_pressurecurve_option(pInfo,
> driver_data,
> options->pressurecurve);
> + xf86libinput_parse_tablet_area_option(pInfo,
> + driver_data,
> + &options->area);
> }
>
> static const char*
> @@ -3282,6 +3379,7 @@ static Atom prop_rotation_angle_default;
> static Atom prop_draglock;
> static Atom prop_horiz_scroll;
> static Atom prop_pressurecurve;
> +static Atom prop_area_ratio;
>
> /* general properties */
> static Atom prop_float;
> @@ -4078,6 +4176,62 @@ LibinputSetPropertyPressureCurve(DeviceIntPtr dev,
> return Success;
> }
>
> +static inline int
> +LibinputSetPropertyAreaRatio(DeviceIntPtr dev,
> + Atom atom,
> + XIPropertyValuePtr val,
> + BOOL checkonly)
> +{
> + InputInfoPtr pInfo = dev->public.devicePrivate;
> + struct xf86libinput *driver_data = pInfo->private;
> + uint32_t *vals;
> + struct ratio area = { 0, 0 };
> +
> + if (val->format != 32 || val->size != 2 || val->type != XA_CARDINAL)
> + return BadMatch;
I was tripped up by this in testing so I might as dig out my newbie hat
and ask... Why XA_CARDINAL? What's the difference between XA_CARDINAL
and XA_INTEGER? Are cardinals related to the CARD{n} types, or is that
just a naming coincidence? Is there any way to set cardinal properties
with xinput set-prop?
> +
> + vals = val->data;
> + area.x = vals[0];
> + area.y = vals[1];
> +
> + if (checkonly) {
> + if (area.x < 0 || area.y < 0)
> + return BadValue;
> +
> + if ((area.x != 0 && area.y == 0) ||
> + (area.x == 0 && area.y != 0))
> + return BadValue;
> +
> + if (!xf86libinput_check_device (dev, atom))
> + return BadMatch;
> + } else {
> + struct xf86libinput *other;
> +
> + xf86libinput_set_area_ratio(driver_data, &area);
> +
> + xorg_list_for_each_entry(other,
> + &driver_data->shared_device->device_list,
> + shared_device_link) {
> + DeviceIntPtr other_device = other->pInfo->dev;
> +
> + if (other->options.area.x == area.x &&
> + other->options.area.y == area.y)
> + continue;
> +
> + XIChangeDeviceProperty(other_device,
> + atom,
> + val->type,
> + val->format,
> + PropModeReplace,
> + val->size,
> + val->data,
> + TRUE);
> + }
> + }
> +
> + return Success;
> +}
> +
> static int
> LibinputSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val,
> BOOL checkonly)
> @@ -4132,6 +4286,8 @@ LibinputSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val,
> rc = LibinputSetPropertyRotationAngle(dev, atom, val, checkonly);
> else if (atom == prop_pressurecurve)
> rc = LibinputSetPropertyPressureCurve(dev, atom, val, checkonly);
> + else if (atom == prop_area_ratio)
> + rc = LibinputSetPropertyAreaRatio(dev, atom, val, checkonly);
> else if (atom == prop_device || atom == prop_product_id ||
> atom == prop_tap_default ||
> atom == prop_tap_drag_default ||
> @@ -4986,6 +5142,25 @@ LibinputInitPressureCurveProperty(DeviceIntPtr dev,
> }
>
> static void
> +LibinputInitTabletAreaRatioProperty(DeviceIntPtr dev,
> + struct xf86libinput *driver_data)
> +{
> + const struct ratio *ratio = &driver_data->options.area;
> + uint32_t data[2];
> +
> + if (!want_area_handling(driver_data))
> + return;
> +
> + data[0] = ratio->x;
> + data[1] = ratio->y;
> +
> + prop_area_ratio = LibinputMakeProperty(dev,
> + LIBINPUT_PROP_TABLET_TOOL_AREA_RATIO,
> + XA_CARDINAL, 32,
> + 2, data);
> +}
> +
> +static void
> LibinputInitProperty(DeviceIntPtr dev)
> {
> InputInfoPtr pInfo = dev->public.devicePrivate;
> @@ -5042,4 +5217,5 @@ LibinputInitProperty(DeviceIntPtr dev)
> LibinputInitDragLockProperty(dev, driver_data);
> LibinputInitHorizScrollProperty(dev, driver_data);
> LibinputInitPressureCurveProperty(dev, driver_data);
> + LibinputInitTabletAreaRatioProperty(dev, driver_data);
> }
>
Everything but the NaN looks fine to me.
Reviewed-by: Jason Gerecke <jason.gerecke at wacom.com>
--
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....
More information about the xorg-devel
mailing list