[PATCH 10/20] dix: don't pass the index for a tracker around, pass the tracker

[Peter Hutterer]

On Wed, Apr 20, 2011 at 11:07:50AM -0700, Jamey Sharp wrote:
> 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.

doh, I was hoping no-one would notice ;)
I'll squashed this in with the "add some more documentation" patch.

> > @@ -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.

nice one. Will keep that in mind for next time, but this one I'll try to get
out as-is. ifdef moved up arount the if condition though.

Cheers,
  Peter