There are some extra changes in this patch, which are probably fine, but maybe you meant to separate them? So I'll mention them anyway. :-)
On Wed, Apr 20, 2011 at 04:28:19PM +1000, Peter Hutterer wrote: > @@ -611,15 +612,15 @@ QueryTrackers(DeviceVelocityPtr vel, int cur_t){ > * even more precision we could subdivide as a final step, so possible > * non-linearities are accounted for. > */ > - dir &= vel->tracker[n].dir; > - if(dir == 0){ > + dir &= tracker->dir; > + if(dir == 0){ /* we've changed octant of movement (e.g. NE → NW) */ This (helpful!) comment is unrelated. > @@ -648,17 +649,17 @@ QueryTrackers(DeviceVelocityPtr vel, int cur_t){ > i = vel->num_tracker-1; > } > if(i>=0){ > - n = TRACKER_INDEX(vel, i); > +#ifdef PTRACCEL_DEBUGGING > + MotionTracker *tracker = TRACKER(vel, i); > DebugAccelF("(dix prtacc) result: offset %i [dx: %i dy: %i diff: %i]\n", I was confused by the introduction of an ifdef here. I see now that DebugAccelF is a no-op unless PTRACCEL_DEBUGGING, and I imagine you just wanted to suppress an unused variable warning on "tracker". I'd suggest a quick comment in the commit message, and surrounding the entire "if(i>=0)" block with the ifdef, since there's no point in an empty if. On a related note, I learned a trick some years ago that might be an improvement for xserver source. If you make symbols like PTRACCEL_DEBUGGING be boolean-valued (0 or non-zero), you can write the above like this instead: if(PTRACCEL_DEBUGGING && i >= 0) { ... } Then the compiler can check that the debugging code is syntactically correct and issue sensible warnings or errors for it, but because the symbol is constant, the optimizer will discard the implementation when debugging is turned off. Jamey
signature.asc
Description: Digital signature
_______________________________________________ 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