X server testing

[Simon Thum]

Peter Hutterer wrote:
> On Mon, Jun 22, 2009 at 07:13:12PM -0400, Thomas Jaeger wrote:
>>>> * Subpixel coordinates are not working for me.  Glancing through the
>>>> code, I think this is to be expected for devices like my tablet whose
>>>> device resolution is higher than the screen resolution, but it seems
>>>> that this should work for accelerated devices like my trackpoint.  Once
>>>> again, not really a big deal.
>>> yes, those bits are missing. sorry. subpixels right now are only for accel
>>> code. feel free to send me a patch for this though, rescaleValuatorAxis
>>> should be it.
>> I've attached a series of patches enabling subpixel coordinates for
>> high-resolution devices along with a few minor refactorings and a
>> bugfix.  I haven't noticed any additional issues running this code for
>> the last few days.
>>
>> I don't like how the pointer acceleration code currently handles
>> subpixel coordinates:  I think it'd be much cleaner if it didn't modify
>> pDev->last.remainder[...] directly, but instead returned the new
>> subpixel value, for example using the following interface.  I'd be happy
>> to supply a patch.
>>
>> /* pointer acceleration handling */
>> typedef void (*PointerAccelSchemeProc)(
>>     DeviceIntPtr /*pDev*/,
>>     int /*first_valuator*/,
>>     int /*num_valuators*/,
>>     int* /*valuators*/,
>>     float* /*x_frac*/,
>>     float* /*y_frac*/,
>>     int /*evtime*/
>> );
> 
> punting this to simon (cc'd)
A good idea at first glance; I aligned to the scheme modifying valuators 
and remainder in-place simply because that is how it was done. But, 
aside for being cleaner in some way, what does it buy us? It's also 
harder to optimize the no-op case since you always need to copy back.

BTW, I've got API changes (mailed to list+peter) which would allow 
multiple accel contexts anyway (in case that's the targeted use case).

>>  static int
>> -rescaleValuatorAxis(int coord, AxisInfoPtr from, AxisInfoPtr to,
>> +rescaleValuatorAxis(int coord, float remainder, float *remainder_return, AxisInfoPtr from, AxisInfoPtr to,
> 
> at this point I wonder whether we should just give in and work with floats.
> the reason why floats are separate are mainly because subpixels were
> tacked onto the current system, but keeping them separate everywhere seems
> to overly complicate things.
> would it make sense to just change the return value and coord to a float?
> 
> this has a larger fallout, including switching dev->last.remainder over as
> well. The question is, is it worth the effort?
> AIUI floats are a problem on embedded platforms where they carry a
> significant performance hit. I'm not sure if the current implementation
> really avoids this issue. Most of the event processing deals with integers
> only, but during event generation where we do the pointer accel anyway I
> think splitting it up doesn't give us much benefit, does it?
I'd stick to integers + remainder. If only because of this:
http://www.mega-nerd.com/FPcast/

Of course, lrintf() and friends are POSIX.1-2001, which we can use. But 
we really should avoid not using them, then.

The float-only is tempting of course, and if done carefully it may be 
superior. But really, as long as there is no 100% nailed-down 
explanation for bug#21456, it'd prefer not to.

Also, for large ranges, floats may not have sufficient precision to 
represent adjacent integers without 'holes'. The int+remainder approach 
gets around this, because we know the int range well and can assume all 
ints are in fact representable.

 > the approach of the patch looks alright to me, but carrying that many
 > parameters around makes me cringe (as said above).

Same here. Why not represent the value in a
struct ValuatorAxisValueCoordinateTypeThing {
     int value;
     float remainder;
}

and ease the function signature bloat this way? (Name should be shorter 
to reach that goal :) One could use NaN as a marker for paths where 
remainder processing is not wanted.

Cheers,

Simon