X server testing

[Thomas Jaeger]

Peter Hutterer wrote:
> On Mon, Jun 22, 2009 at 07:13:12PM -0400, Thomas Jaeger wrote:
> 
>> From 67076a1404cad4042e5cfc667d28f150e1663115 Mon Sep 17 00:00:00 2001
>> From: Thomas Jaeger <ThJaeger at gmail.com>
>> Date: Sat, 20 Jun 2009 20:17:41 -0400
>> Subject: [PATCH 4/4] dix: report subpixel coordinates for high-resolution devices
>>
>> ---
>>  dix/getevents.c |   93 +++++++++++++++++++++++++++++++++++++-----------------
>>  1 files changed, 64 insertions(+), 29 deletions(-)
>>
>> diff --git a/dix/getevents.c b/dix/getevents.c
>> index 25dd187..a64a0c7 100644
>> --- a/dix/getevents.c
>> +++ b/dix/getevents.c
>> @@ -240,10 +240,11 @@ CreateClassesChangedEvent(EventList* event,
>>   * Rescale the coord between the two axis ranges.
>>   */
>>  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've been playing with the idea of changing the types of various
parameter, but it never seems to work out very well:  The X server uses
many different formats to represent valuator information internally, so
you end up converting back and forth no matter what format you choose.
The fact that every other function in dix/getevents.c modifiers the
valuator array and/or dev->last.valuator[i] in-place doesn't exactly
help either, this code could use a good refactoring.  So I think dealing
with long parameter lists is the lesser evil, and I'm resubmitting the
patch with a few minor style cleanups and a fixed rescaleValuatorEvents
that actually takes the fractional value into account.

>> @@ -745,26 +759,36 @@ accelPointer(DeviceIntPtr dev, int first, int num, int *valuators, CARD32 ms)
>>   * @param dev The device to be moved.
>>   * @param x Pointer to current x-axis value, may be modified.
>>   * @param y Pointer to current y-axis value, may be modified.
>> + * @param x_frac
>> + * @param y_frac
>         ^^ missing doc. I know it's just a few words, but..
>>   * @param scr Screen the device's sprite is currently on.
>>   * @param screenx Screen x coordinate the sprite is on after the update.
>>   * @param screeny Screen y coordinate the sprite is on after the update.
>> + * @param screenx_frac
>> + * @param screeny_frac
>         ^^ same thing here
This is what happens when you decide to come back later to fill in the
details and then never do, sorry.

>>      } else {
>> -        if (flags & POINTER_ACCELERATE)
>> +        if (flags & POINTER_ACCELERATE) {
>>              accelPointer(pDev, first_valuator, num_valuators, valuators, ms);
>> +            /* The pointer acceleration code modifies the fractional part
>> +	     * in-place, so we need to extract this information first */
>    ^^^ stray tab
Sorry, I haven't managed to configure vim in such a way that it adopts
the indenting and space conventions of the code that it is looking at...

Thanks,
Tom