Comment 62 for bug 28052

Revision history for this message
In , Peter Hutterer (peter-hutterer) wrote :

(In reply to comment #47)
> > I'd be better if DEVICE_PTRACCEL only takes up one spot and then you have a
> sub-variable for the scheme in the struct.
>
> > The range from DEVICE_PTRACCEL to DEVICE_PTRACCEL_MAX is a bit messy.
>
> Agreed. It was like this in the first place. I changed the design in order to
> support getting parameters (GetDeviceControl doesn't upload a struct to the
> server) plus keeping the possibility to get/set (rather) arbitrary parameters.
>
> I thought it to be better than a loaded struct from which something will always
> be missing, especially if someone draws up additional schemes or the algorithm
> gets new tweaks. Maybe there is another way to do it?

there is a simple reason why the current approach is unsuitable. you're essentially making anybody else who may need to add a device ctrl dependent on your DEVICE_PTRACCEL_MAX.
Now in the worst case s.b. adds a DevCtl as MAX + 1. Clients depend on it and then when you add an accel scheme suddenly the absolute value of MAX + 1 has changed. This is bad!
Remember, #defines are compiled in so once they are in binary you can't change them anymore w/o recompilation.

I'm also not sure why the current approach gets around the problem of an "incomplete" struct. Maybe I'm misunderstanding you here?
What is wrong with

typedef struct {
    XID control; /* DEVIC_PTRACCEL */
    int length;
    int type;
} XDeviceAccelerationControl, XDeviceAccelerationState;

for the default, and then depending on the type you have a struct for each accel scheme that needs it. type determines which struct, and since length is in bytes anyway, nothing breaks by adding new schemes.

I'll have a look at the server patch but the inputproto stuff in its current state is a dealbreaker.