[xserver][PATCH] Tablet to screen coordinate translation

[Magnus Vigerlöf]

On Friday 09 March 2007 02:56, Peter Hutterer wrote:
[...]
> yes. we really want to have it in one function only. otherwise we end
> up with the same nightmare we had not too long ago...

Oh... You wouldn't possibly have a link where this was discussed? (if it was) 
Keywords that I could use for searching the bugzilla and/or mail archives 
would work as well. It would be interesting to read so I know what was before 
the current code and what I should avoid.

> > So.. Adding two function pointers to DeviceIntRec is the preferred
> > way? [...]
> > But that would force us to use the same interface as the old
> > functions, maybe
> > we should update the parameters to these two functions while we're
> > at it?
>
> If you're breaking one struct, you might as well break the other one.

I really don't have a good reason for changing the parameters for these 
functions. The first argument has to change from LocalDevicePtr to 
DeviceIntPtr though, so we might as well change the others as well.

Making it as simple as possible (the current functions I've looked at only 
looks at the valuators for x&y anyway) I would suggest something like this:

Bool convert(DeviceIntPtr dDev, int x, int y, *convx, int *convy)

I don't see a problem for the two conversion functions to have the same 
parameters.

> > In that case the first function which could update the function
> > pointers seems
> > to be the DEVICE_INIT call for the device. We'll spread out the
> > code for
> > filling in the struct, but for initialization it should be safe
> > enough. A bit
> > confusing for the driver designer perhaps...
>
> apologize my ignorance, having never written a driver I don't quite
> understand what you mean.
> At the moment all you have to do is get the conversion_proc and
> reverse_conversion_proc into the DeviceIntRec, right? This can be
> done in xf86ActivateDevice.

The initialization/opening of each input device is made by three calls 
(DEVICE_INSTALL, DEVICE_INIT, DEVICE_ON). See also [1] for a few more 
details. The call to xf86ActivateDevice is made between the two first ones, 
outside the driver code.

The current function pointers are assigned in the struct allocated and 
returned by the first call, so if we remove the current function pointers in 
LocalDeviceRec and let the driver set the conversion function pointers in the 
second call directly in the DeviceIntRec struct we'll end up with 
initialization of the device in two different calls to the driver.

Otoh we'll end up with these functions in two places if we want to set it in 
the first call, and maybe also have ambigous errors if we're trying to reuse 
the current function pointers. As the first parameter has to change to make 
it possible to use the functions from GPE, I'll have to change my preferred 
choice to split the initialization instead (previous paragraph)..

Did that make more sense?

> Once they are in there, you can call them from GPE. The only
> additional thing I see right now is that you have to pass the screen
> limits in somewhere to adjust the scaling properly. So if you change
> the parameters to the conversion proc to take in another two ints,
> you can basically use the existing code (provided you adjust the
> functions).

The screen resolution is already available through some global variables, and 
prior to calling xf86PostMotionEvent the device must define the screen which 
it will be using (this is probably not the exact truth, but I hope it's not 
that far from it). So this is already solved in the current code.

> Am I missing something?

I think we're getting closer to the point where I can make an updated version 
of the patch. So, no I don't think we're missing that much at the moment.

Thanks for taking the time to talk through this, I really appreciate it!

Cheers
  Magnus V

[1] http://xorg.freedesktop.org/wiki/XOrgInputDriverSpec