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

[Peter Hutterer]

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 :)

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