Re: [PATCH synaptics] Fix st-mt scaling
On Thu, Apr 12, 2012 at 7:57 AM, Peter Hutterer
peter.hutte...@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.hutte...@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.l...@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.l...@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,
Re: [PATCH synaptics] Fix st-mt scaling
On 03/29/2012 05:12 AM, Leon Shaw wrote:
From: Leon Shaw shaw.l...@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?
Signed-off-by: Leon Shaw shaw.l...@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:
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;
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH synaptics] Fix st-mt scaling
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.l...@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.
Signed-off-by: Leon Shaw shaw.l...@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.
Cheers,
Peter
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;
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
