Apologies for the delay in replying. I've been busy with a few other
tasks and haven't had the time to sit down and think through the
behavior like I wanted.
On 9/7/2015 9:00 AM, Mario Geiger wrote:
> Dear Jason,
>
>> Have you considered allowing a variable length? Changing the functions
>> to support polynomials of arbitrary degree would be trivial and could
>> potentially allow for more accurate calibration (obviously at the cost
>> of increased CPU use).
> It's would be great but I have no idea how to pass these parameters
> because the number of coefficients can vary.
>
>> Do you think it would be reasonable/better/worse to work with the
> actual
>> coordinates instead of a fraction? I'm not sure which would be better,
>> myself.
>>
>> Also, my gut says that the correction should be done in the tablet's
>> native coordindate space (axis_x->min_value; axis_x->max_value) rather
>> than the currently-set area (priv->topX, priv->topY). I'm not 100%
> sure
> I continue the think that it's better to use the currently-set area. My
> idea is that the currently-set area is set such that in the center of
> the screen the stylus works perfectly (because I assume that everything
> is linear in the center). When the currently-set area is well configured
> we can work on the borders without touching the central part.
That's a reasonable assumption to make, but I don't see why you'd make
it in the first place. It implies that you either reset the Area to its
default value and hope its linear or have the user go through a two-step
calibration where the first step calculates/applies an appropriate Area.
I think it should be possible to calculate both from a single set of
calibration samples, allowing them to be applied in whatever order makes
the most sense.
>> though given that it might make it quite difficult for programs to
>> calculate the polynomial coefficients. I'll have to think a little
> harder...
> Yes we need to have an input close to 1 to avoid arithmetic overflow.
>
That shouldn't be a problem either way :)
>> The typical idiom is to use something like:
>> float_atom = XInternAtom(dpy, "FLOAT", False);
>> which will does the same thing as these two lines and just uses a
>> hardcoded string.
> I don't know what to take as dpy.
>
Ah, indeed. Looks like there's no sensible place to get 'dpy' here.
> For the other comments I think I fix them.
> There is the new patch :
Looks pretty good aside from copy/paste-induced whitespace issues and
one little nitpick below:
>
> diff --git include/wacom-properties.h include/wacom-properties.h
> index b845083..54e7793 100644
> --- include/wacom-properties.h
> +++ include/wacom-properties.h
> @@ -27,6 +27,9 @@
> /* 32 bit, 4 values, top x, top y, bottom x, bottom y */
> #define WACOM_PROP_TABLET_AREA "Wacom Tablet Area"
>
> +/* 32 bit, 4x5=20 values, 4x[border width, polynomial coefficient x^3,
> x^2, x, 1] */
> +#define WACOM_PROP_TABLET_DISTORTION "Wacom Border Distortion"
> +
> /* 8 bit, 1 value, [0 - 3] (NONE, CW, CCW, HALF) */
> #define WACOM_PROP_ROTATION "Wacom Rotation"
>
> diff --git src/wcmCommon.c src/wcmCommon.c
> index 9408f42..06315cb 100644
> --- src/wcmCommon.c
> +++ src/wcmCommon.c
> @@ -438,6 +438,26 @@ static void sendCommonEvents(InputInfoPtr pInfo,
> const WacomDeviceState* ds,
> sendWheelStripEvents(pInfo, ds, first_val, num_vals, valuators);
> }
>
> +/* compute the polynomial of order /order
> + * /polynomial must contain coefficients from the higher order to the
> constant
> + * { /in if /in < /limit
> + * result = {
> + * { Poly(/in) if /in >= /limit
> + */
> +static float wcmComputePolynomial(float in, float limit, float*
> polynomial, int order)
> +{
> + if (in < limit) {
> + int i;
> + float out = polynomial[0];
> + for (i = 1; i <= order; ++i) {
> + out *= in;
> + out += polynomial[i];
> + }
> + return out;
> + }
> + return in;
> +}
> +
> /* rotate x and y before post X inout events */
> void wcmRotateAndScaleCoordinates(InputInfoPtr pInfo, int* x, int* y)
> {
> @@ -446,19 +466,38 @@ void wcmRotateAndScaleCoordinates(InputInfoPtr
> pInfo, int* x, int* y)
> DeviceIntPtr dev = pInfo->dev;
> AxisInfoPtr axis_x, axis_y;
> int tmp_coord;
> + float f;
>
> /* scale into on topX/topY area */
> axis_x = &dev->valuator->axes[0];
> axis_y = &dev->valuator->axes[1];
>
> /* Don't try to scale relative axes */
> - if (axis_x->max_value > axis_x->min_value)
> - *x = xf86ScaleAxis(*x, axis_x->max_value, axis_x->min_value,
> - priv->bottomX, priv->topX);
> + if (axis_x->max_value > axis_x->min_value) {
> + f = (*x - priv->topX) / (float)(priv->bottomX - priv->topX); //
> f is
> approximatively in [0,1]
>
> - if (axis_y->max_value > axis_y->min_value)
> - *y = xf86ScaleAxis(*y, axis_y->max_value, axis_y->min_value,
> - priv->bottomY, priv->topY);
> + // fix the topX border distortion with a polynomial
> + f = wcmComputePolynomial(f, priv->distortion_topX_border,
> priv->distortion_topX_poly, 3);
> +
> + // fix the bottomX border distortion with a polynomial
> + f = 1.0f - wcmComputePolynomial(1.0f - f,
> priv->distortion_bottomX_border, priv->distortion_bottomX_poly, 3);
> +
> + *x = roundf(f * (axis_x->max_value - axis_x->min_value) +
> axis_x->min_value);
> +
> + if (*x < axis_x->min_value) *x = axis_x->min_value;
> + if (*x > axis_x->max_value) *x = axis_x->max_value;
> + /* In the case of the two last if, the stylus is out of the
> screen
> and no events should be sent */
> + }
> +
> + if (axis_y->max_value > axis_y->min_value) {
> + f = (*y - priv->topY) / (float)(priv->bottomY - priv->topY);
> + f = wcmComputePolynomial(f, priv->distortion_topY_border,
> priv->distortion_topY_poly, 3);
> + f = 1.0f - wcmComputePolynomial(1.0f - f,
> priv->distortion_bottomY_border, priv->distortion_bottomY_poly, 3);
> +
> + *y = roundf(f * (axis_y->max_value - axis_y->min_value) +
> axis_y->min_value);
> + if (*y < axis_y->min_value) *y = axis_y->min_value;
> + if (*y > axis_y->max_value) *y = axis_y->max_value;
> + }
>
> /* coordinates are now in the axis rage we advertise for the device */
>
> diff --git src/wcmXCommand.c src/wcmXCommand.c
> index 346ff61..b2ef4e2 100644
> --- src/wcmXCommand.c
> +++ src/wcmXCommand.c
> @@ -33,6 +33,8 @@
> #define XI_PROP_PRODUCT_ID "Device Product ID"
> #endif
>
> +static Atom float_atom;
> +
> static void wcmBindToSerial(InputInfoPtr pInfo, unsigned int serial);
>
>
> /*****************************************************************************
> @@ -82,6 +84,7 @@ int wcmDevSwitchMode(ClientPtr client, DeviceIntPtr
> dev, int mode)
> static Atom prop_devnode;
> static Atom prop_rotation;
> static Atom prop_tablet_area;
> +static Atom prop_distortion;
> static Atom prop_pressurecurve;
> static Atom prop_serials;
> static Atom prop_serial_binding;
> @@ -204,11 +207,23 @@ static Atom InitWcmAtom(DeviceIntPtr dev, const
> char *name, Atom type, int forma
> return atom;
> }
>
> +static Atom InitFloatAtom(DeviceIntPtr dev, const char *name, int
> nvalues, float *values)
> +{
> + Atom atom;
> +
> + atom = MakeAtom(name, strlen(name), TRUE);
> + XIChangeDeviceProperty(dev, atom, float_atom, 32,
> + PropModeReplace, nvalues, values, FALSE);
> + XISetDevicePropertyDeletable(dev, atom, FALSE);
> + return atom;
> +}
> +
> void InitWcmDeviceProperties(InputInfoPtr pInfo)
> {
> WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
> WacomCommonPtr common = priv->common;
> int values[WCM_MAX_BUTTONS];
> + float fvalues[20];
> int i;
>
> DBG(10, priv, "\n");
> @@ -227,6 +242,24 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
> prop_tablet_area = InitWcmAtom(pInfo->dev,
> WACOM_PROP_TABLET_AREA,
> XA_INTEGER, 32, 4, values);
> }
>
> + if (!IsPad(priv)) {
> + //float_atom = XInternAtom(dpy, "FLOAT", FALSE);
> + float_atom = XIGetKnownProperty("FLOAT");
> + if (!float_atom) float_atom = MakeAtom("FLOAT", 5, TRUE);
Please place the 'if (!float_atom)' on its own line.
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....
> +
> + if (float_atom) {
> + // topX, topY, bottomX, bottomY
> + for (i = 0; i < 4; ++i) {
> + fvalues[i*5+0] = 0.0; // border
> + fvalues[i*5+1] = 0.0; // x^3
> + fvalues[i*5+2] = 0.0; // x^2
> + fvalues[i*5+3] = 1.0; // x
> + fvalues[i*5+4] = 0.0; // 1
> + }
> + prop_distortion = InitFloatAtom(pInfo->dev,
> WACOM_PROP_TABLET_DISTORTION, 20, fvalues);
> + }
> + }
> +
> values[0] = common->wcmRotate;
> if (!IsPad(priv)) {
> prop_rotation = InitWcmAtom(pInfo->dev, WACOM_PROP_ROTATION,
> XA_INTEGER, 8, 1, values);
> @@ -683,6 +716,20 @@ int wcmDeleteProperty(DeviceIntPtr dev, Atom
> property)
> return (i >= 0) ? BadAccess : Success;
> }
>
> +/* help to copy the values from the parameters into the WacomDevice
> structure
> + * values[0] is the width of the distoation on a border
> + * values[1], values[2], ... are coefficients of the polynomials of
> x^3, x^2, x and constant
> + * all these values in units (WacomDevice::top, WacomDevice::bottom) ->
> (0,1) where 0 is mapped to the nearest border
> + */
> +static void setDistortionProperty(float* values, float *border, float
> *polynomial)
> +{
> + *border = values[0];
> + polynomial[0] = values[1];
> + polynomial[1] = values[2];
> + polynomial[2] = values[3];
> + polynomial[3] = values[4];
> +}
> +
> int wcmSetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr
> prop,
> BOOL checkonly)
> {
> @@ -717,6 +764,20 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property,
> XIPropertyValuePtr prop,
> priv->bottomX = values[2];
> priv->bottomY = values[3];
> }
> + } else if (property == prop_distortion)
> + {
> + float *values = (float*)prop->data;
> +
> + if (prop->size != 20 || prop->format != 32 || prop->type !=
> float_atom)
> + return BadValue;
> +
> + if (!checkonly)
> + {
> + setDistortionProperty(values,
> &priv->distortion_topX_border,
> priv->distortion_topX_poly);
> + setDistortionProperty(values+5,
> &priv->distortion_topY_border,
> priv->distortion_topY_poly);
> + setDistortionProperty(values+10,
> &priv->distortion_bottomX_border,
> priv->distortion_bottomX_poly);
> + setDistortionProperty(values+15,
> &priv->distortion_bottomY_border,
> priv->distortion_bottomY_poly);
> + }
> } else if (property == prop_pressurecurve)
> {
> INT32 *pcurve;
> diff --git src/xf86WacomDefs.h src/xf86WacomDefs.h
> index 1575960..2d76514 100644
> --- src/xf86WacomDefs.h
> +++ src/xf86WacomDefs.h
> @@ -262,6 +262,16 @@ struct _WacomDeviceRec
> unsigned int cur_serial; /* current serial in prox */
> int cur_device_id; /* current device ID in prox */
>
> + /* distortion */
> + float distortion_topX_border;
> + float distortion_topY_border;
> + float distortion_bottomX_border;
> + float distortion_bottomY_border;
> + float distortion_topX_poly[4];
> + float distortion_topY_poly[4];
> + float distortion_bottomX_poly[4];
> + float distortion_bottomY_poly[4];
> +
> /* button mapping information
> *
> * 'button' variables are indexed by physical button number
> (0..nbuttons)
>
>
------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
