On Sun, Nov 18, 2012 at 11:29:41PM -0500, Yuly Novikov wrote: > Well then, out of the 3 possible options, which one do you prefer: > 1. Storing X and Y before transform in addition to valuators. > 2. Storing only the valuators, but pre-transform.
this one ^^ so essentially your patch, but without the removal of the valuator mask Cheers, Peter > 3. Storing the post transform valuators as now, but inverse transform > them and transform again each time. > I don't really like option 3, since not every transformation is > invertible and there will be an error if transformation is changed. > My preference is option 2. > What do you think? Am I right in interpreting your comment that your > preference is option 1? > > Regards, > Yuly > > On Sun, Nov 18, 2012 at 10:32 PM, Peter Hutterer > <peter.hutte...@who-t.net> wrote: > > sorry about the delay, I wanted to get a test-case for this first to > > reliably reproduce it and that took me a while. > > > > On Thu, Nov 08, 2012 at 05:13:39PM -0500, Yuly Novikov wrote: > >> A touchpoint used to store the entire valuators structure, > >> and coordinates there were stored post-transformation, > >> which resulted in Coordinate Transformation Matrix > >> being applied multiple times to the last coordinates, > >> in the case when only pressure changes in the last touch event. > >> > >> This commit gets rid of the valuators, as only X and Y coordinates > >> are actually used, and stores only the relevant values. > >> The variable name was changed to indicate > >> that the values are stored before transformation, > >> to avoid confusion with last.valuators, which are after transformation. > > > > NACK to that part, the reason we use the ValuatorMask here is because this > > way we always deal with valuator masks when dealing with input coordinates. > > last.valuator not being a VM yet is a leftover from the time before the VMs > > were introduced. > > > > please leave that as-is, even if it feels clunky. > > > > Cheers, > > Peter > > > >> > >> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=49347 > >> > >> Signed-off-by: Yuly Novikov <ynovi...@chromium.org> > > > >> --- > >> dix/devices.c | 2 -- > >> dix/getevents.c | 19 ++++++------------- > >> dix/touch.c | 1 - > >> include/inputstr.h | 3 +-- > >> 4 files changed, 7 insertions(+), 18 deletions(-) > >> > >> diff --git a/dix/devices.c b/dix/devices.c > >> index 613323f..d23b1b4 100644 > >> --- a/dix/devices.c > >> +++ b/dix/devices.c > >> @@ -967,8 +967,6 @@ CloseDevice(DeviceIntPtr dev) > >> free(dev->deviceGrab.sync.event); > >> free(dev->config_info); /* Allocated in xf86ActivateDevice. */ > >> free(dev->last.scroll); > >> - for (j = 0; j < dev->last.num_touches; j++) > >> - free(dev->last.touches[j].valuators); > >> free(dev->last.touches); > >> dev->config_info = NULL; > >> dixFreePrivates(dev->devPrivates, PRIVATE_DEVICE); > >> diff --git a/dix/getevents.c b/dix/getevents.c > >> index 8b4379d..fe60ac3 100644 > >> --- a/dix/getevents.c > >> +++ b/dix/getevents.c > >> @@ -1951,32 +1951,25 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr > >> dev, uint32_t ddx_touchid, > >> default: > >> return 0; > >> } > >> - if (t->mode == XIDirectTouch && !(flags & TOUCH_CLIENT_ID)) { > >> - if (!valuator_mask_isset(&mask, 0)) > >> - valuator_mask_set_double(&mask, 0, > >> - > >> valuator_mask_get_double(touchpoint.ti-> > >> - valuators, > >> 0)); > >> - if (!valuator_mask_isset(&mask, 1)) > >> - valuator_mask_set_double(&mask, 1, > >> - > >> valuator_mask_get_double(touchpoint.ti-> > >> - valuators, > >> 1)); > >> - } > >> > >> /* Get our screen event co-ordinates (root_x/root_y/event_x/event_y): > >> * these come from the touchpoint in Absolute mode, or the sprite in > >> * Relative. */ > >> if (t->mode == XIDirectTouch) { > >> - transformAbsolute(dev, &mask); > >> > >> if (!(flags & TOUCH_CLIENT_ID)) { > >> - for (i = 0; i < valuator_mask_size(&mask); i++) { > >> + for (i = 0; i < 2; i++) { > >> double val; > >> > >> if (valuator_mask_fetch_double(&mask, i, &val)) > >> - valuator_mask_set_double(touchpoint.ti->valuators, i, > >> val); > >> + touchpoint.ti->last_raw_axes[i] = val; > >> + else > >> + valuator_mask_set_double(&mask, i, > >> + > >> touchpoint.ti->last_raw_axes[i]); > >> } > >> } > >> > >> + transformAbsolute(dev, &mask); > >> clipAbsolute(dev, &mask); > >> } > >> else { > >> diff --git a/dix/touch.c b/dix/touch.c > >> index 5f77be5..5074ec7 100644 > >> --- a/dix/touch.c > >> +++ b/dix/touch.c > >> @@ -226,7 +226,6 @@ void > >> TouchInitDDXTouchPoint(DeviceIntPtr dev, DDXTouchPointInfoPtr ddxtouch) > >> { > >> memset(ddxtouch, 0, sizeof(*ddxtouch)); > >> - ddxtouch->valuators = valuator_mask_new(dev->valuator->numAxes); > >> } > >> > >> Bool > >> diff --git a/include/inputstr.h b/include/inputstr.h > >> index 5a38924..c06f9fe 100644 > >> --- a/include/inputstr.h > >> +++ b/include/inputstr.h > >> @@ -330,8 +330,7 @@ typedef struct _DDXTouchPointInfo { > >> Bool active; /* whether or not the touch is active */ > >> uint32_t ddx_id; /* touch ID given by the DDX */ > >> Bool emulate_pointer; > >> - > >> - ValuatorMask *valuators; /* last recorded axis values */ > >> + double last_raw_axes[2]; /* last X/Y axis valuator data as posted > >> */ > >> } DDXTouchPointInfoRec; > >> > >> typedef struct _TouchClassRec { > >> -- > >> 1.7.7.3 > >> _______________________________________________ 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