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

[Jamey Sharp]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110420/a2c2c6e6/attachment.pgp>