[PATCH] dix: fix scale_to_desktop for edge ABS events

[Daniel Kurtz]

On Wed, Jul 25, 2012 at 2:04 AM, Jeremy Huddleston Sequoia
<jeremyhu at apple.com> wrote:
> Yick.  This is why origin/size are better descriptors =(
>
> Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
>
> On Jul 20, 2012, at 16:41, Yufeng Shen <miletus at chromium.org> wrote:
>
>> Scale_to_desktop() converts ABS events from device coordinates
>> to screen coordinates:
>> [dev_X_min, dev_X_max]  -> [screen_X_min, screen_X_max]
>> [dev_Y_min, dev_Y_max]  -> [screen_Y_min, screen_Y_max]
>>
>> An edge ABS event with X = dev_X_max (e.g., generated from the
>> edge of a touchscreen) will be converted to have screen X value
>> = screen_X_max, which, however, will be filterd out when xserver
>> tries to find proper Window to receive the event, because the
>> range check for a Window to receive events is
>>       window_X_min <= event_screen_X < window_X_max
>> Events with event_screen_X = screen_X_max will fail the test get
>> and rejected by the Window.
>>
>> To fix this, we change the device to screen coordinates mapping to
>> [dev_X_min, dev_X_max]  -> [screen_X_min, screen_X_max-1]
>> [dev_Y_min, dev_Y_max]  -> [screen_Y_min, screen_Y_max-1]
>>
>> Signed-off-by: Yufeng Shen <miletus at chromium.org>
>> ---
>> dix/getevents.c |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/dix/getevents.c b/dix/getevents.c
>> index b78d5ce..9898c6a 100644
>> --- a/dix/getevents.c
>> +++ b/dix/getevents.c
>> @@ -890,9 +890,9 @@ scale_to_desktop(DeviceIntPtr dev, ValuatorMask *mask,
>>
>>     /* scale x&y to desktop coordinates */
>>     *screenx = rescaleValuatorAxis(x, dev->valuator->axes + 0, NULL,
>> -                                   screenInfo.x, screenInfo.width);
>> +                                   screenInfo.x, screenInfo.width - 1);

Hi Yufeng,

rescaleValuatorAxis() expects min/max for its last two arguments.

Technically, this patch (and the original code) looks like it is
assuming that screenInfo.x == 0.  This is probably a good assumption
(since this is the global origin
for all screens together).  However, I think something like the
following is probably safer:

  *screenx = rescaleValuatorAxis(x, dev->valuator->axes + 0, NULL,
screenInfo.x, screenInfo.x + screenInfo.width - 1);
  *screeny = rescaleValuatorAxis(y, dev->valuator->axes + 0, NULL,
screenInfo.y, screenInfo.y + screenInfo.height - 1);

Also, can you double check to make sure that the arguments to
rescaleValuatorAxis() are set correctly elsewhere as well?  A quick
glance shows that (a) positionSprite() has the exact same issue, and
(b) other callers seem to be treating them as "origin / width", and
not "min / max".

Thanks,
-Daniel

>>     *screeny = rescaleValuatorAxis(y, dev->valuator->axes + 1, NULL,
>> -                                   screenInfo.y, screenInfo.height);
>> +                                   screenInfo.y, screenInfo.height - 1);
>>
>>     *devx = x;
>>     *devy = y;
>> --
>> 1.7.7.3
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel