On complexifigurability

[Peter Hutterer]

On Tue, Apr 05, 2011 at 06:37:47PM +0100, Daniel Stone wrote:
> > It's about organization of the code really, which leads to correct
> > driver API usage, so we could talk about deprecation and proper
> > versioning of unused/old/unmaintained drivers - we would be
> > enforcing ourselves to use correct interfaces. Privately. So, I'm
> > sure if we pick 90% of the code inside Xorg DDX we will see that
> > they are used only for old drivers. IOW, why a newer server needs to
> > support a very old driver?
> 
> Sure, but how does #ifdef'ing the pointer acceleration code help here?
> For the most part[0], it's completely invisible to drivers, hidden
> behind GetPointerEvents().  And if you want to talk about cleaning up
> our interfaces so that drivers don't have to care about pointless and
> esoteric server implementation details, how about fixing the
> GetPointerEvents API? At the moment, basically every single GPE user
> does this:
>     static EventList *events = InitEventList(GetMaximumEventsNum());
>     int nev;
>     OsBlockSignals();
>     nev = GetPointerEvents(dev, MotionNotify, ...);
>     for (i = 0; i < nev; i++)
>         mieqEnqueue(dev, (InternalEvent *)((events + i)->event));
>     OsReleaseSignals();
> 
> This could just turn into:
>     PostPointerEvents(dev, MotionNotify, ...);
> 
> And the server could take care of these implementation details itself,
> and no-one except myself and Peter would have to care anymore.  And I
> wouldn't have to have a patch that bumps GetMaximumEventsNum()'s return
> up to 49 or possibly 97 for pathological cases when using smooth
> scrolling, because we can enqueue an unlimited number of events.

note that no driver cares about the enqueuing. they just call
xf86Post{Button|Motion|...}Event and be done with it. the enqueuing is
in-server only and we could change this without an abi bump.
With the ABI 12 changes to our input drivers, no driver does more about the
history size than passing it into InitValuatorClassDeviceStruct().
So we could even safely ignore this right, let alone just removing it from
the next ABI.

the EventList stuff is also confined to the server only (and mostly a legacy
of MPX pre 1.6), so could be cleaned quite simply if someone cares enough.

as for defining an actual input API - yeah, other problem, partly made
harder by the need to support multiple server versions in most drivers. It's
a bit more difficult (not impossible though) to just create a
xf86InputDriverAPI.h and have that as the single include in all drivers
(minus misc.h, list.h, etc.).

Of course, if you go down that path you'd probably want to make DeviceIntRec
opaque to the drivers and have functions defined to access everything. Not
screwing those up is the bulk of the work, really.

Cheers,
  Peter

> Now _that's_ an API cleanup! It simplifies things, makes things a lot
> easier for drivers, and gets a lot of stuff no-one should ever have to
> care about out of the way.  And removes opportunities for getting things
> wrong.  Hell, it even brings us closer to having an actual API rather
> than a set of random headers that we export for external modules to use,
> with some rough versioning.
> 
> If you do that, it means the entire mieq, EventList and Get*Events API
> can become deprecated for driver usage, giving us the opportunity in
> future to remove a whole lot of driver API.
> 
> When you say API cleanup, this is the kind of thing I think about.  Not
> just another untested codepath that seems to just be randomly added for
> its own sake, not providing anyone anywhere any benefit.  And doesn't
> change the API at all.
> 
> Also, in fairness, can I just add that the pointer acceleration code is
> currently quite well organised and very well segregated?
> GetPointerEvents calls accelPointer() which, well, accelerates the
> pointer, and all the actual acceleration code is hidden in ptrveloc.c.
> So if your goal is modularisation and organisation -- we're already
> there.
> 
> Cheers,
> Daniel
> 
> [0]: Unless you're really cool like Synaptics and want to implement your
>      own acceleration profile.