[PATCH libinput 12/16] filter: duplicate the code for the Lenovo x230 accel method

[Peter Hutterer]

On Tue, Aug 11, 2015 at 04:13:16PM +0800, Jonas Ådahl wrote:
> On Tue, Aug 11, 2015 at 08:52:46AM +1000, Peter Hutterer wrote:
> > On Mon, Aug 10, 2015 at 04:22:40PM +0800, Jonas Ådahl wrote:
> > > On Wed, Aug 05, 2015 at 04:32:41PM +1000, Peter Hutterer wrote:
> > > > This is "once-tested, don't touch it again" code. The quirks on the touchpad
> > > > means we'd have to find that specific device again and re-test everything if
> > > > we change anything elsewhere in the code. So duplicate it properly, so that we
> > > > don't have to touch it again.
> > > > 
> > > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > > ---
> > > >  src/filter.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> > > >  1 file changed, 51 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/src/filter.c b/src/filter.c
> > > > index 16dedb4..6e20069 100644
> > > > --- a/src/filter.c
> > > > +++ b/src/filter.c
> > > > @@ -337,6 +337,32 @@ accelerator_filter_low_dpi(struct motion_filter *filter,
> > > >  	return accelerated;
> > > >  }
> > > >  
> > > > +static struct normalized_coords
> > > > +accelerator_filter_x230(struct motion_filter *filter,
> > > > +			const struct normalized_coords *unaccelerated,
> > > > +			void *data, uint64_t time)
> > > > +{
> > > > +	struct pointer_accelerator *accel =
> > > > +		(struct pointer_accelerator *) filter;
> > > > +	double accel_factor; /* unitless factor */
> > > > +	struct normalized_coords accelerated;
> > > > +	double velocity; /* units/us */
> > > > +
> > > > +	feed_trackers(accel, unaccelerated, time);
> > > > +	velocity = calculate_velocity(accel, time);
> > > > +	accel_factor = calculate_acceleration(accel,
> > > > +					      data,
> > > > +					      velocity,
> > > > +					      accel->last_velocity,
> > > > +					      time);
> > > > +	accel->last_velocity = velocity;
> > > > +
> > > > +	accelerated.x = accel_factor * unaccelerated->x;
> > > > +	accelerated.y = accel_factor * unaccelerated->y;
> > > > +
> > > > +	return accelerated;
> > > > +}
> > > > +
> > > >  static void
> > > >  accelerator_restart(struct motion_filter *filter,
> > > >  		    void *data,
> > > > @@ -661,17 +687,39 @@ create_pointer_accelerator_filter_touchpad(int dpi)
> > > >  	return &filter->base;
> > > >  }
> > > >  
> > > > +struct motion_filter_interface accelerator_interface_x230 = {
> > > > +	accelerator_filter_x230,
> > > > +	accelerator_restart,
> > > > +	accelerator_destroy,
> > > > +	accelerator_set_speed,
> > > > +};
> > > > +
> > > > +/* The Lenovo x230 has a bad touchpad. This accel method has been
> > > > + * trial-and-error'd, any changes to it will require re-testing everything.
> > > > + * Don't touch this.
> > > > + */
> > > >  struct motion_filter *
> > > >  create_pointer_accelerator_filter_lenovo_x230(int dpi)
> > > >  {
> > > >  	struct pointer_accelerator *filter;
> > > >  
> > > > -	filter = create_default_filter(dpi);
> > > > -	if (!filter)
> > > > +	filter = zalloc(sizeof *filter);
> > > > +	if (filter == NULL)
> > > >  		return NULL;
> > > >  
> > > > -	filter->base.interface = &accelerator_interface;
> > > > +	filter->base.interface = &accelerator_interface_x230;
> > > >  	filter->profile = touchpad_lenovo_x230_accel_profile;
> > > > +	filter->last_velocity = 0.0;
> > > > +
> > > > +	filter->trackers =
> > > > +		calloc(NUM_POINTER_TRACKERS, sizeof *filter->trackers);
> > > > +	filter->cur_tracker = 0;
> > > > +
> > > > +	filter->threshold = v_ms2us(0.4);
> > > > +	filter->accel = 2.0;
> > > > +	filter->incline = 1.1;
> > > 
> > > Should we make these macros just with a _X230 postfix or something? So
> > > even though they are magic (as the other) we'd keep the magic in the
> > > same place. If you disagree with that, at least make the same comments
> > > regarding unitless and units (for the second two at least).
> > > 
> > > In any way, Reviewed-by: Jonas Ådahl <jadahl at gmail.com>
> > 
> > thanks. I've added comments to accel/incline but I'm not sure what you mean
> > with having specific macros here. the helpers we currently use are to
> > convert between physical measurements - I hope we won't need device-specific
> > instances here :)
> 
> With macros i ment #define X230_THRESHOLD next to DEFAULT_THRESHOLD to
> make all the constants that are derived from trial and error in one
> place.

ah, right, that make sense. I squashed this in now:

diff --git a/src/filter.c b/src/filter.c
index c3dc820..7054faf 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -96,6 +96,11 @@ filter_get_speed(struct motion_filter *filter)
 #define DEFAULT_ACCELERATION 2.0		/* unitless factor */
 #define DEFAULT_INCLINE 1.1			/* unitless factor */
 
+/* for the Lenovo x230 custom accel. do not touch */
+#define X230_THRESHOLD v_ms2us(0.4)		/* in units/us */
+#define X230_ACCELERATION 2.0			/* unitless factor */
+#define X230_INCLINE 1.1			/* unitless factor */
+
 /*
  * Pointer acceleration filter constants
  */
@@ -777,9 +782,9 @@ create_pointer_accelerator_filter_lenovo_x230(int dpi)
 		calloc(NUM_POINTER_TRACKERS, sizeof *filter->trackers);
 	filter->cur_tracker = 0;
 
-	filter->threshold = v_ms2us(0.4);
-	filter->accel = 2.0; /* unitless factor */
-	filter->incline = 1.1; /* incline of the acceleration function */
+	filter->threshold = X230_THRESHOLD;
+	filter->accel = X230_ACCELERATION; /* unitless factor */
+	filter->incline = X230_INCLINE; /* incline of the acceleration function */
 
 	filter->dpi_factor = 1; /* unused for this accel method */