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

[Jonas Ådahl]

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.


Jonas

> 
> come to think of it, the best approach to fully split this out into
> independent code would be to have a custom struct pointer_accelerator_x230
> and not rely on any shared code.
> 
> Cheers,
>    Peter
> 
> > > +
> > > +	filter->dpi_factor = 1; /* unused for this accel method */
> > >  
> > >  	return &filter->base;
> > >  }
> > > -- 
> > > 2.4.3
> > > 
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel