On complexifigurability

[Daniel Stone]

Hi,

On Tue, Apr 05, 2011 at 08:21:43PM +0300, Tiago Vignatti wrote:
> On 04/05/2011 05:51 PM, ext Daniel Stone wrote:
> >Sure, but at some stage there has to be a limit.  Every configuration
> >option has a cost: in making the source files larger by putting #ifdefs
> >everywhere - and you could argue we already have enough of those, by
> >implementing the alternate codepath, by testing that both codepaths
> >build, by testing that both codepaths work, by documenting it, by
> >supporting users who use it, etc, etc.  The server is not exactly a
> >simple place as is, so adding additional complexity should be carefully
> >considered[0].
> >
> >So, if there's a demonstratable benefit, then cool! By all means.  But
> >if we're doing it either just for the sake of it, or to provide an
> >absolutely trivial savings in binary size which could be much better
> >accomplished elsewhere[1], then I don't see that the tradeoff makes
> >sense.  Even so, is the result really usable? No-one using relative
> >devices wants no acceleration.
> 
> it's not about binary size, nor memory savings. They are not the
> main benefits we should be targeting when talking about the server
> modularization - or did you care about it as a motivation for
> xkbcommon?

The motivation for xkbcommon was to cut down on duplication of code
between X server, X client, xkbcomp, Wayland compositor and Wayland
client.  Also one factor was to try to build a coherent XKB API
organically based on the needs of all five groups, which will help us
work towards a future version of XKB (I've stopped calling it XKB2 now,
it's not even funny).

So yeah, I think that does actually solve some problems, not least the
current insanity of forking xkbcomp.

> 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.

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110405/101e6b59/attachment-0001.pgp>