On Tue, Jul 24, 2012 at 10:44 PM, Daniel Kurtz <[email protected]> wrote:
> On Wed, Jul 25, 2012 at 2:04 AM, Jeremy Huddleston Sequoia > <[email protected]> wrote: > > Yick. This is why origin/size are better descriptors =( > > > > Reviewed-by: Jeremy Huddleston Sequoia <[email protected]> > > > > On Jul 20, 2012, at 16:41, Yufeng Shen <[email protected]> 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 <[email protected]> > >> --- > >> 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); > > I think you are correct at pointing out using [screenInfo.x, screenInfo.x + screenInfo.width) as [screen_min, screen_max) But I would like to have another patch for correcting this since this seems a different bug than the patch intends to fix (off-by-one when scaling ABS event) and it is seen elsewhere through getevents.c (also in updateSlaveDeviceCoords() and positionSprite() ) so it needs different tests. 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". > > I checked that updateSlaveDeviceCoords() and positionSprite() also use rescaleValuatorAxis() to map coordinates from [screenInfo.x, screenInfo.width] to [device_min, device_max] (which is opposite to scale_to_desktop()) Actually I don't know in this case should we use [screenInfo.x, screenInfo.width] --> [device_min, device_max] or [screenInfo.x, screenInfo.width) --> [device_min, device_max] or does it matter (depending on how the device coordinates are used in the following code path) --- Yufeng Shen > 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 > >> > >> _______________________________________________ > >> [email protected]: X.Org development > >> Archives: http://lists.x.org/archives/xorg-devel > >> Info: http://lists.x.org/mailman/listinfo/xorg-devel > >> > > > > _______________________________________________ > > [email protected]: X.Org development > > Archives: http://lists.x.org/archives/xorg-devel > > Info: http://lists.x.org/mailman/listinfo/xorg-devel >
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
