merged 1/4 and 3/4 into -next, skipping them in this reply. It'd be really helpful if you could send the messages one-by-one though.
patch 4/4 still has a comment typo I've pointed out before - should the valuator mask really be called first_valuator?? I'm not sure what has changed why in the other patches, please always send a changelog of what changed over the last version of the patch. There are a few comments on the patches in http://lists.freedesktop.org/archives/xorg-devel/2011-February/019033.html maybe that email disappeared or got swamped out of your inbox? Either way, I don't know whether the comments have been addressed or if not, why not. The code looks better though, still some comments inside. On Tue, Feb 15, 2011 at 12:40:35AM +0100, Simon Thum wrote: > From e8758c7e77c4d5e501b4bcaf0622a4f3c9824253 Mon Sep 17 00:00:00 2001 > From: Simon Thum <simon.t...@gmx.de> > Date: Sun, 5 Sep 2010 18:10:42 +0200 > Subject: [PATCH 2/4] dix: refactor predictable scheme initialization > > This intends to clean up the predictable accel struct > from purely scheme-related things like input properties, > as they would be useless in other use cases such > as wheel acceleration. > > Signed-off-by: Simon Thum <simon.t...@gmx.de> > --- > dix/ptrveloc.c | 75 ++++++++++++++++++++++++++++++++++----------------- > include/ptrveloc.h | 13 ++++++--- > 2 files changed, 59 insertions(+), 29 deletions(-) > > diff --git a/dix/ptrveloc.c b/dix/ptrveloc.c > index 1b9c81b..5416ef7 100644 > --- a/dix/ptrveloc.c > +++ b/dix/ptrveloc.c > @@ -30,6 +30,7 @@ > #include <ptrveloc.h> > #include <exevents.h> > #include <X11/Xatom.h> > +#include <os.h> > > #include <xserver-properties.h> > > @@ -68,9 +69,9 @@ SimpleSmoothProfile(DeviceIntPtr dev, DeviceVelocityPtr > vel, float velocity, > static PointerAccelerationProfileFunc > GetAccelerationProfile(DeviceVelocityPtr vel, int profile_num); > static BOOL > -InitializePredictableAccelerationProperties(DeviceIntPtr dev); > +InitializePredictableAccelerationProperties(DeviceIntPtr, long*); > static BOOL > -DeletePredictableAccelerationProperties(DeviceIntPtr dev); > +DeletePredictableAccelerationProperties(DeviceIntPtr, long*); > > /*#define PTRACCEL_DEBUGGING*/ > > @@ -87,6 +88,9 @@ DeletePredictableAccelerationProperties(DeviceIntPtr dev); > /* some int which is not a profile number */ > #define PROFILE_UNINITIALIZE (-100) > > +/* number of properties for predictable acceleration */ > +#define NPROPS_PREDICTABLE_ACCEL 4 > + > > /** > * Init DeviceVelocity struct so it should match the average case > @@ -125,17 +129,24 @@ FreeVelocityData(DeviceVelocityPtr vel){ > */ > Bool > InitPredictableAccelerationScheme(DeviceIntPtr dev, > - ValuatorAccelerationPtr protoScheme) { > + ValuatorAccelerationPtr protoScheme) { > DeviceVelocityPtr vel; > ValuatorAccelerationRec scheme; > + PredictableAccelSchemePtr schemeData; > scheme = *protoScheme; > vel = calloc(1, sizeof(DeviceVelocityRec)); > - if (!vel) > - return FALSE; > + schemeData = calloc(1, sizeof(PredictableAccelSchemeRec)); > + if (!vel || !schemeData) > + return FALSE; > InitVelocityData(vel); > - scheme.accelData = vel; > + schemeData->vel = vel; > + schemeData->prop_handlers = calloc(NPROPS_PREDICTABLE_ACCEL, > + sizeof(long)); please no magic lengths arrays, schemeData should have length field for prop_handlers. this goes for all APIs related to that, they should take a length field for sanity checks. > + if (!schemeData->prop_handlers) > + return FALSE; > + scheme.accelData = schemeData; > dev->valuator->accelScheme = scheme; > - InitializePredictableAccelerationProperties(dev); > + InitializePredictableAccelerationProperties(dev, > schemeData->prop_handlers); > return TRUE; > } > > @@ -146,14 +157,24 @@ InitPredictableAccelerationScheme(DeviceIntPtr dev, > void > AccelerationDefaultCleanup(DeviceIntPtr dev) > { > - /*sanity check*/ > - if( dev->valuator->accelScheme.AccelSchemeProc == > acceleratePointerPredictable > - && dev->valuator->accelScheme.accelData != NULL){ > + DeviceVelocityPtr vel = GetDevicePredictableAccelData(dev); > + long* prop_handlers; > + if (vel) { > + /* the proper guarantee would be that we're not inside of > + * AccelSchemeProc(), but that seems impossible. Schemes don't get > + * schwitched often anyway. > + */ > + OsBlockSignals(); > dev->valuator->accelScheme.AccelSchemeProc = NULL; > - FreeVelocityData(dev->valuator->accelScheme.accelData); > + FreeVelocityData(vel); > + free(vel); > + prop_handlers = ((PredictableAccelSchemePtr) > + dev->valuator->accelScheme.accelData)->prop_handlers; > + DeletePredictableAccelerationProperties(dev, prop_handlers); > + free(prop_handlers); > free(dev->valuator->accelScheme.accelData); > dev->valuator->accelScheme.accelData = NULL; > - DeletePredictableAccelerationProperties(dev); > + OsReleaseSignals(); > } > } > > @@ -345,26 +366,30 @@ AccelInitScaleProperty(DeviceIntPtr dev, > DeviceVelocityPtr vel) > return XIRegisterPropertyHandler(dev, AccelSetScaleProperty, NULL, NULL); > } > > -BOOL > -InitializePredictableAccelerationProperties(DeviceIntPtr dev) > +static BOOL > +InitializePredictableAccelerationProperties( > + DeviceIntPtr dev, > + long* prop_handlers) > { > DeviceVelocityPtr vel = GetDevicePredictableAccelData(dev); > > if(!vel) > return FALSE; > > - vel->prop_handlers[0] = AccelInitProfileProperty(dev, vel); > - vel->prop_handlers[1] = AccelInitDecelProperty(dev, vel); > - vel->prop_handlers[2] = AccelInitAdaptDecelProperty(dev, vel); > - vel->prop_handlers[3] = AccelInitScaleProperty(dev, vel); > + prop_handlers[0] = AccelInitProfileProperty(dev, vel); > + prop_handlers[1] = AccelInitDecelProperty(dev, vel); > + prop_handlers[2] = AccelInitAdaptDecelProperty(dev, vel); > + prop_handlers[3] = AccelInitScaleProperty(dev, vel); > > return TRUE; > } > > BOOL > -DeletePredictableAccelerationProperties(DeviceIntPtr dev) > +DeletePredictableAccelerationProperties( > + DeviceIntPtr dev, > + long* prop_handlers) > { > - DeviceVelocityPtr vel; > + DeviceVelocityPtr vel; > Atom prop; > int i; > > @@ -379,8 +404,8 @@ DeletePredictableAccelerationProperties(DeviceIntPtr dev) > > vel = GetDevicePredictableAccelData(dev); > for (i = 0; vel && i < NPROPS_PREDICTABLE_ACCEL; i++) > - if (vel->prop_handlers[i]) > - XIUnregisterPropertyHandler(dev, vel->prop_handlers[i]); > + if (prop_handlers[i]) > + XIUnregisterPropertyHandler(dev, prop_handlers[i]); reset the prop handlers to 0? not needed afaict, but good form nonetheless. > return TRUE; > } > @@ -397,8 +422,7 @@ InitTrackers(DeviceVelocityPtr vel, int ntracker) > return; > } > free(vel->tracker); > - vel->tracker = (MotionTrackerPtr)malloc(ntracker * > sizeof(MotionTracker)); > - memset(vel->tracker, 0, ntracker * sizeof(MotionTracker)); > + vel->tracker = (MotionTrackerPtr)calloc(ntracker, sizeof(MotionTracker)); > vel->num_tracker = ntracker; > } > > @@ -1026,7 +1050,8 @@ GetDevicePredictableAccelData( > acceleratePointerPredictable && > dev->valuator->accelScheme.accelData != NULL){ > > - return (DeviceVelocityPtr)dev->valuator->accelScheme.accelData; > + return ((PredictableAccelSchemePtr) > + dev->valuator->accelScheme.accelData)->vel; > } > return NULL; > } > diff --git a/include/ptrveloc.h b/include/ptrveloc.h > index 8c59c03..94cf8c9 100644 > --- a/include/ptrveloc.h > +++ b/include/ptrveloc.h > @@ -62,9 +62,6 @@ typedef struct _MotionTracker { > int dir; /* initial direction bitfield */ > } MotionTracker, *MotionTrackerPtr; > > -/* number of properties for predictable acceleration */ > -#define NPROPS_PREDICTABLE_ACCEL 4 > - > /** > * Contains all data needed to implement mouse ballistics > */ > @@ -91,9 +88,17 @@ typedef struct _DeviceVelocityRec { > struct { /* to be able to query this information */ > int profile_number; > } statistics; > - long prop_handlers[NPROPS_PREDICTABLE_ACCEL]; > } DeviceVelocityRec, *DeviceVelocityPtr; > > +/** > + * contains the run-time data for the predictable scheme, that is, a > + * DeviceVelocityPtr and the property handlers. > + */ > +typedef struct _PredictableAccelSchemeRec { > + DeviceVelocityPtr vel; > + long* prop_handlers; > +} PredictableAccelSchemeRec, *PredictableAccelSchemePtr; > + > extern _X_EXPORT void > InitVelocityData(DeviceVelocityPtr vel); > > -- > 1.7.3.4 > > From 53ccbcb1052754ea85b667d4ef7197dccffca621 Mon Sep 17 00:00:00 2001 > From: Simon Thum <simon.t...@gmx.de> > Date: Thu, 3 Feb 2011 21:52:24 +0100 > Subject: [PATCH 4/4] dix: update pointer acceleration code to use ValuatorMask > > Signed-off-by: Simon Thum <simon.t...@gmx.de> > --- > dix/getevents.c | 24 ++--------- > dix/ptrveloc.c | 111 > ++++++++++++++++++++++++---------------------------- > include/input.h | 10 ++--- > include/ptrveloc.h | 12 +++--- > 4 files changed, 65 insertions(+), 92 deletions(-) > > diff --git a/dix/getevents.c b/dix/getevents.c > index 794df42..1ccddfa 100644 > --- a/dix/getevents.c > +++ b/dix/getevents.c > @@ -791,17 +791,14 @@ moveRelative(DeviceIntPtr dev, int *x, int *y, > ValuatorMask *mask) > * Accelerate the data in valuators based on the device's acceleration > scheme. > * > * @param dev The device which's pointer is to be moved. > - * @param first The first valuator in @valuators > - * @param num Total number of valuators in @valuators. > - * @param valuators Valuator data for each axis between @first and > - * @first+@num. > + * @param valuators Valuator mask > * @param ms Current time. > */ > static void > -accelPointer(DeviceIntPtr dev, int first, int num, int *valuators, CARD32 ms) > +accelPointer(DeviceIntPtr dev, ValuatorMask* valuators, CARD32 ms) > { > if (dev->valuator->accelScheme.AccelSchemeProc) > - dev->valuator->accelScheme.AccelSchemeProc(dev, first, num, > valuators, ms); > + dev->valuator->accelScheme.AccelSchemeProc(dev, valuators, ms); > } > > /** > @@ -1169,20 +1166,7 @@ GetPointerEvents(EventList *events, DeviceIntPtr pDev, > int type, int buttons, > moveAbsolute(pDev, &x, &y, &mask); > } else { > if (flags & POINTER_ACCELERATE) { > - /* FIXME: Pointer acceleration only requires X and Y values. This > - * should be converted to masked valuators. */ > - int vals[2]; > - vals[0] = valuator_mask_isset(&mask, 0) ? > - valuator_mask_get(&mask, 0) : 0; > - vals[1] = valuator_mask_isset(&mask, 1) ? > - valuator_mask_get(&mask, 1) : 0; > - accelPointer(pDev, 0, 2, vals, ms); > - > - if (valuator_mask_isset(&mask, 0)) > - valuator_mask_set(&mask, 0, vals[0]); > - if (valuator_mask_isset(&mask, 1)) > - valuator_mask_set(&mask, 1, vals[1]); > - > + accelPointer(pDev, &mask, ms); > /* The pointer acceleration code modifies the fractional part > * in-place, so we need to extract this information first */ > x_frac = pDev->last.remainder[0]; > diff --git a/dix/ptrveloc.c b/dix/ptrveloc.c > index 56ea809..0313061 100644 > --- a/dix/ptrveloc.c > +++ b/dix/ptrveloc.c > @@ -1068,18 +1068,15 @@ GetDevicePredictableAccelData( > void > acceleratePointerPredictable( > DeviceIntPtr dev, > - int first_valuator, > - int num_valuators, > - int *valuators, > - int evtime) > + ValuatorMask* val, > + CARD32 evtime) whoah. how did the int → CARD32 change sneak into this patch. Should be a separate patch, IMO. though tbh I don't see the point, if we have different sizes for ints here then we are really in trouble. > { > float fdx, fdy, tmp, mult; /* no need to init */ > - int dx = 0, dy = 0; > - int *px = NULL, *py = NULL; > + int dx = 0, dy = 0, tmpi; > DeviceVelocityPtr velocitydata = GetDevicePredictableAccelData(dev); > Bool soften = TRUE; > > - if (!num_valuators || !valuators || !velocitydata) > + if (!velocitydata) > return; > > if (velocitydata->statistics.profile_number == AccelProfileNone && > @@ -1087,13 +1084,12 @@ acceleratePointerPredictable( > return; /*we're inactive anyway, so skip the whole thing.*/ > } > > - if (first_valuator == 0) { > - dx = valuators[0]; > - px = &valuators[0]; > + if (valuator_mask_isset(val, 0)) { > + dx = valuator_mask_get(val, 0); > } > - if (first_valuator <= 1 && num_valuators >= (2 - first_valuator)) { > - dy = valuators[1 - first_valuator]; > - py = &valuators[1 - first_valuator]; > + > + if (valuator_mask_isset(val, 1)) { > + dy = valuator_mask_get(val, 1); > } > > if (dx || dy){ > @@ -1107,7 +1103,7 @@ acceleratePointerPredictable( > mult = ComputeAcceleration (dev, velocitydata, > dev->ptrfeed->ctrl.threshold, > (float)dev->ptrfeed->ctrl.num / > - (float)dev->ptrfeed->ctrl.den); > + (float)dev->ptrfeed->ctrl.den); > > if(mult != 1.0f || velocitydata->const_acceleration != 1.0f) { > ApplySofteningAndConstantDeceleration( velocitydata, > @@ -1122,13 +1118,15 @@ acceleratePointerPredictable( > * process each axis conditionally, there's no danger > * of a toggling remainder. Its lack of guarantees likely > * makes it faster on the average target. */ > - *px = lrintf(tmp); > - dev->last.remainder[0] = tmp - (float)*px; > + tmpi = lrintf(tmp); > + valuator_mask_set(val, 0, tmpi); > + dev->last.remainder[0] = tmp - (float)tmpi; > } > if (dy) { > tmp = mult * fdy + dev->last.remainder[1]; > - *py = lrintf(tmp); > - dev->last.remainder[1] = tmp - (float)*py; > + tmpi = lrintf(tmp); > + valuator_mask_set(val, 1, tmpi); > + dev->last.remainder[1] = tmp - (float)tmpi; > } > DebugAccelF("pos (%i | %i) remainders x: %.3f y: %.3f delta > x:%.3f y:%.3f\n", > *px, *py, dev->last.remainder[0], > dev->last.remainder[1], fdx, fdy); > @@ -1149,25 +1147,18 @@ acceleratePointerPredictable( > void > acceleratePointerLightweight( > DeviceIntPtr dev, > - int first_valuator, > - int num_valuators, > - int *valuators, > - int ignored) > + ValuatorMask* val, > + CARD32 ignored) > { > - float mult = 0.0; > - int dx = 0, dy = 0; > - int *px = NULL, *py = NULL; > - > - if (!num_valuators || !valuators) > - return; > + float mult = 0.0, tmpf; > + int dx = 0, dy = 0, tmpi; > > - if (first_valuator == 0) { > - dx = valuators[0]; > - px = &valuators[0]; > + if (valuator_mask_isset(val, 0)) { > + dx = valuator_mask_get(val, 0); > } > - if (first_valuator <= 1 && num_valuators >= (2 - first_valuator)) { > - dy = valuators[1 - first_valuator]; > - py = &valuators[1 - first_valuator]; > + > + if (valuator_mask_isset(val, 1)) { > + dy = valuator_mask_get(val, 1); > } > > if (!dx && !dy) > @@ -1177,24 +1168,24 @@ acceleratePointerLightweight( > /* modeled from xf86Events.c */ > if (dev->ptrfeed->ctrl.threshold) { > if ((abs(dx) + abs(dy)) >= dev->ptrfeed->ctrl.threshold) { > - dev->last.remainder[0] = ((float)dx * > - > (float)(dev->ptrfeed->ctrl.num)) / > - (float)(dev->ptrfeed->ctrl.den) > + > - dev->last.remainder[0]; > - if (px) { > - *px = (int)dev->last.remainder[0]; > - dev->last.remainder[0] = dev->last.remainder[0] - > - (float)(*px); > + tmpf = ((float)dx * > + (float)(dev->ptrfeed->ctrl.num)) / > + (float)(dev->ptrfeed->ctrl.den) + > + dev->last.remainder[0]; > + if (dx) { > + tmpi = (int) tmpf; > + valuator_mask_set(val, 0, tmpi); > + dev->last.remainder[0] = tmpf - (float)tmpi; > } > > - dev->last.remainder[1] = ((float)dy * > - > (float)(dev->ptrfeed->ctrl.num)) / > - (float)(dev->ptrfeed->ctrl.den) > + > - dev->last.remainder[1]; > - if (py) { > - *py = (int)dev->last.remainder[1]; > - dev->last.remainder[1] = dev->last.remainder[1] - > - (float)(*py); > + tmpf = ((float)dy * > + (float)(dev->ptrfeed->ctrl.num)) / > + (float)(dev->ptrfeed->ctrl.den) + > + dev->last.remainder[1]; > + if (dy) { > + tmpi = (int) tmpf; > + valuator_mask_set(val, 1, tmpi); > + dev->last.remainder[1] = tmpf - (float)tmpi; tabs → spaces please if using vim, set listchars=tab:>-,trail:~ set list helps tremendously with this > } > } > } > @@ -1204,18 +1195,18 @@ acceleratePointerLightweight( > (float)(dev->ptrfeed->ctrl.den) - 1.0) / > 2.0) / 2.0; > if (dx) { > - dev->last.remainder[0] = mult * (float)dx + > - dev->last.remainder[0]; > - *px = (int)dev->last.remainder[0]; > - dev->last.remainder[0] = dev->last.remainder[0] - > - (float)(*px); > + tmpf = mult * (float)dx + > + dev->last.remainder[0]; > + tmpi = (int) tmpf; > + valuator_mask_set(val, 0, tmpi); > + dev->last.remainder[0] = tmpf - (float)tmpi; > } > if (dy) { > - dev->last.remainder[1] = mult * (float)dy + > - dev->last.remainder[1]; > - *py = (int)dev->last.remainder[1]; > - dev->last.remainder[1] = dev->last.remainder[1] - > - (float)(*py); > + tmpf = mult * (float)dy + > + dev->last.remainder[1]; > + tmpi = (int)tmpf; > + valuator_mask_set(val, 1, tmpi); > + dev->last.remainder[1] = tmpf - (float)tmpi; > } > } > } > diff --git a/include/input.h b/include/input.h > index ab58a15..3b2652c 100644 > --- a/include/input.h > +++ b/include/input.h > @@ -106,6 +106,8 @@ typedef struct _ClassesRec *ClassesPtr; > typedef struct _SpriteRec *SpritePtr; > typedef union _GrabMask GrabMask; > > +typedef struct _ValuatorMask ValuatorMask; > + > typedef struct _EventList { > xEvent* event; > int evlen; /* length of allocated memory for event in bytes. This is not > @@ -142,10 +144,8 @@ typedef void (*DeviceUnwrapProc)( > /* pointer acceleration handling */ > typedef void (*PointerAccelSchemeProc)( > DeviceIntPtr /*pDev*/, > - int /*first_valuator*/, > - int /*num_valuators*/, > - int* /*valuators*/, > - int /*evtime*/); > + ValuatorMask* /*first_valuator*/, first_valuator? Cheers, Peter > + CARD32 /*evtime*/); > > typedef void (*DeviceCallbackProc)( > DeviceIntPtr /*pDev*/); > @@ -163,8 +163,6 @@ typedef struct _DeviceRec { > Bool on; /* used by DDX to keep state */ > } DeviceRec, *DevicePtr; > > -typedef struct _ValuatorMask ValuatorMask; > - > typedef struct { > int click, bell, bell_pitch, bell_duration; > Bool autoRepeat; > diff --git a/include/ptrveloc.h b/include/ptrveloc.h > index 94cf8c9..6411a2c 100644 > --- a/include/ptrveloc.h > +++ b/include/ptrveloc.h > @@ -1,6 +1,6 @@ > /* > * > - * Copyright ?? 2006-2009 Simon Thum simon dot thum at gmx dot de > + * Copyright ?? 2006-2011 Simon Thum simon dot thum at gmx dot de > * > * Permission is hereby granted, free of charge, to any person obtaining a > * copy of this software and associated documentation files (the "Software"), > @@ -25,7 +25,7 @@ > #ifndef POINTERVELOCITY_H > #define POINTERVELOCITY_H > > -#include <input.h> /* DeviceIntPtr */ > +#include <input.h> > > /* constants for acceleration profiles */ > > @@ -133,11 +133,11 @@ InitPredictableAccelerationScheme(DeviceIntPtr dev, > struct _ValuatorAccelerationRec* protoScheme); > > extern _X_INTERNAL void > -acceleratePointerPredictable(DeviceIntPtr dev, int first_valuator, > - int num_valuators, int *valuators, int evtime); > +acceleratePointerPredictable(DeviceIntPtr dev, ValuatorMask* val, > + CARD32 evtime); > > extern _X_INTERNAL void > -acceleratePointerLightweight(DeviceIntPtr dev, int first_valuator, > - int num_valuators, int *valuators, int ignored); > +acceleratePointerLightweight(DeviceIntPtr dev, ValuatorMask* val, > + CARD32 evtime); > > #endif /* POINTERVELOCITY_H */ > -- > 1.7.3.4 > _______________________________________________ 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
