[PATCH synaptics] Fix st->mt scaling

[Leon Shaw]

On Thu, Apr 12, 2012 at 7:57 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Fri, Mar 30, 2012 at 02:58:51PM +0800, Leon Shaw wrote:
>> On Fri, Mar 30, 2012 at 6:56 AM, Peter Hutterer
>> <peter.hutterer at who-t.net> wrote:
>> > On Thu, Mar 29, 2012 at 10:21:09AM -0700, Chase Douglas wrote:
>> >> On 03/29/2012 05:12 AM, Leon Shaw wrote:
>> >> > From: Leon Shaw <shaw.leon at gmail.com>
>> >>
>> >> Hi Leon,
>> >>
>> >> Please include a description of the problem and how this fix addresses
>> >> it in the commit message. If there is a bug in the bug tracker, please
>> >> include the url too.
>> >>
>> >> What device are you seeing this issue on?
>> >
>> > unless my maths yesterday was out, any device where st_min != mt_min could
>> > trigger this bug. try it with st ranges [2...8], mt ranges [-1...11] before
>> > and after.
>> >
>> > either way, Leon, it should be easy to write up something in
>> > test/eventcomm-test.c that tests a number of ranges before/after for
>> > correctness.
>>
>> The min and max values are fetched through ioctl syscall during init.
>> To test this, we need either simulate ioctl, or directly access the
>> proto_data, which breaks the test principles. So shall we wrap the
>> ioctl into a macro?
>
> I think here you can just redefine the SYSCALL macro and make it fill in the
> test data instead. does that sound alright?
>
> Cheers,
>  Peter
>

Well, I just don't know how to do this. SYSCALL is defined and used in
src/eventcomm.c, test code doesn't have a hook to redefine it (and
it's not only for getting axis ranges).
So I just skip the test of initialization, fill in st_min and st_scale
directly and test only the scaling?

Thanks,
Leon

>>
>> >> > Signed-off-by: Leon Shaw <shaw.leon at gmail.com>
>> >> > ---
>> >> > ásrc/eventcomm.c | á 16 +++++++++-------
>> >> > á1 file changed, 9 insertions(+), 7 deletions(-)
>> >> >
>> >> > diff --git a/src/eventcomm.c b/src/eventcomm.c
>> >> > index 28d034f..2d03743 100644
>> >> > --- a/src/eventcomm.c
>> >> > +++ b/src/eventcomm.c
>> >> > @@ -71,7 +71,7 @@ struct eventcomm_proto_data
>> >> > á á á * exists for readability of the code.
>> >> > á á á */
>> >> > á á áBOOL need_grab;
>> >> > - á áint st_to_mt_offset[2];
>> >> > + á áint st_min[2];
>> >> > á á ádouble st_to_mt_scale[2];
>> >> > á#ifdef HAVE_MULTITOUCH
>> >> > á á ástruct mtdev *mtdev;
>> >> > @@ -396,6 +396,8 @@ event_query_axis_ranges(InputInfoPtr pInfo)
>> >> > á á áevent_get_abs(pInfo, pInfo->fd, ABS_Y, &priv->miny, &priv->maxy,
>> >> > á á á á á á á &priv->synpara.hyst_y, &priv->resy);
>> >> >
>> >> > + á áproto_data->st_min[0] = priv->minx;
>> >> > + á áproto_data->st_min[1] = priv->miny;
>> >> > á á ápriv->has_pressure = FALSE;
>> >> > á á ápriv->has_width = FALSE;
>> >> > á á áSYSCALL(rc = ioctl(pInfo->fd, EVIOCGBIT(EV_ABS, sizeof(absbits)), absbits));
>> >> > @@ -429,10 +431,8 @@ event_query_axis_ranges(InputInfoPtr pInfo)
>> >> > á á á á áevent_get_abs(pInfo, pInfo->fd, ABS_MT_POSITION_Y, &priv->miny,
>> >> > á á á á á á á á á á á á&priv->maxy, &priv->synpara.hyst_y, &priv->resy);
>> >> >
>> >> > - á á á áproto_data->st_to_mt_offset[0] = priv->minx - st_minx;
>> >> > á á á á áproto_data->st_to_mt_scale[0] =
>> >> > á á á á á á á(priv->maxx - priv->minx) / (st_maxx - st_minx);
>> >> > - á á á áproto_data->st_to_mt_offset[1] = priv->miny - st_miny;
>> >> > á á á á áproto_data->st_to_mt_scale[1] =
>> >> > á á á á á á á(priv->maxy - priv->miny) / (st_maxy - st_miny);
>> >> > á á á}
>> >> > @@ -641,9 +641,11 @@ static int count_fingers(InputInfoPtr pInfo, const struct CommData *comm)
>> >> >
>> >> >
>> >> > ástatic inline double
>> >> > -apply_st_scaling(struct eventcomm_proto_data *proto_data, int value, int axis)
>> >> > +apply_st_scaling(SynapticsPrivate *priv, int value, int axis)
>> >> > á{
>> >> > - á áreturn value * proto_data->st_to_mt_scale[axis] + proto_data->st_to_mt_offset[axis];
>> >> > + á ástruct eventcomm_proto_data *proto_data = priv->proto_data;
>> >> > + á áreturn (value - proto_data->st_min[axis]) * proto_data->st_to_mt_scale[axis] +
>> >> > + á á á á á á á á(axis ? priv->miny : priv->minx);
>> >>
>> >> proto_data->st_min[0] == priv->minx, so we can get rid of
>> >> proto_daata->st_min. This would then become:
>> >
>> > Check the previous hunk, it overwrites priv->minx with the
>> > ABS_MT_POSITION_X data.
>>
>> Yes, overwritten in MT case.
>>
>> Thanks,
>> Leon
>>
>> >
>> >> if (axis == 0)
>> >> á á return (value - priv->minx) * proto_data->st_to_mt_scale[axis] +
>> >> á á á á á ápriv->minx;
>> >> else
>> >> á á return (value - priv->miny) * proto_data->st_to_mt_scale[axis] +
>> >> á á á á á ápriv->miny;
>> >>
>> >> I don't think this is correct. priv->min* are in mt coordinates, but
>> >> value is in st coordinates. You're subtracting two values in different
>> >> coordinate systems, which won't work.
>> >>
>> >> I'm not real sure what the bug you're seeing is, please provide more
>> >> information so we can tell what's wrong.
>> >>
>> >> > á}
>> >> >
>> >> > áBool
>> >> > @@ -738,10 +740,10 @@ EventReadHwState(InputInfoPtr pInfo,
>> >> > á á á á if (ev.code < ABS_MT_SLOT) {
>> >> > á á á á á á switch (ev.code) {
>> >> > á á á á á á case ABS_X:
>> >> > - á á á á á á á hw->x = apply_st_scaling(proto_data, ev.value, 0);
>> >> > + á á á á á á á hw->x = apply_st_scaling(priv, ev.value, 0);
>> >> > á á á á á á á á break;
>> >> > á á á á á á case ABS_Y:
>> >> > - á á á á á á á hw->y = apply_st_scaling(proto_data, ev.value, 1);
>> >> > + á á á á á á á hw->y = apply_st_scaling(priv, ev.value, 1);
>> >> > á á á á á á á á break;
>> >> > á á á á á á case ABS_PRESSURE:
>> >> > á á á á á á á á hw->z = ev.value;
>>