[Xcb] About next release of xcb/util

[Barton C Massey]

In message <sa3d3yvas5u.fsf at cigue.easter-eggs.fr> you wrote:
> Barton C Massey <bart at cs.pdx.edu> writes:
> > Huh?  The image library uses it quite a bit. What's the win
> > in reimplementing things like the visual searches?
> 
> IMHO, that's just add overhead for maybe nothing. If I take
> xcb_aux_get_visualtype() (guessing it's the one your
> talking about),

I was actually talking about the iterators you discuss
below.

> the first lines are:
> 
>   screen = xcb_aux_get_screen (c, scr);
>   if (!screen) return NULL;
> 
> Just 2 lines to do nothing because with a bit of luck I may have already
> add the xcb_screen_t laying around.

These lines don't cost anything, I think?  They're like
three instructions in the normal case, and it's much more
convenient than digging out the scr I had lying around and
passing it in. :-)

> Here's my review of the rest of aux:

BTW, thank you for doing the detailed review.  Greatly
appreciated.

> xcb_aux_get_depth:
>   Is ridiculously synchronous, for god sake!

True.  This one should just die.  I can't think of any
useful replacement for it.

> pack_list and all other stuff after:
>   Xlib hiding of proto stuff

Rather, I would characterize this as a convenience layer on
top of XCB to provide a more sane C interface.  Making the
caller allocate and fill an array is just kind of
programmer-hostile IMHO, so I tend to use these interfaces
instead.

OTOH, this code could definitely use some love.
xcb_params_cw_t should arguably be called
xcb_aux_params_change_window_t but isn't for some reason,
and similarly for the other types.  The hard-coded "16" in
the array dimension is just scary and gross.

> xcb_aux_sync:
>   One-liner to send a get_input_focus(). That's hide people how X11 is
>   crap. But well, that may be useful.

Certainly everybody is going to re-implement it if we don't
provide it.  I think we could actually argue that this one
is ubiquitous enough to just be put into core XCB.  But if
not, then it needs to be here.

> xcb_aux_get_depth_of_visual:
> xcb_aux_get_screen:
> xcb_aux_find_visual_by_id:
> xcb_aux_find_visual_by_attrs:
>   Iterators. Might be needed somehow, but again loading a full library
>   just to have handy iterators to browse some struct seems to me totally
>   overkill.

I think you'll find that a lot of programs should be using
these iterators.  Right now, I think lots of programs make a
lot of assumptions about this stuff that they don't actually
check, or don't check properly.

> That makes me wonder if we should not make xcb_aux a
> header file of some inlined iterator-lookup functions. :)

Most of the stuff in this file could be inlined, certainly.
I'm not sure anyone would care, though: there aren't any
obviously performance-critical bits in here.  I guess we'd
avoid having to link against the library.  Meh.

> Note that I really do not care about what's done to
> xcb-aux[1], but if not dropped it should be at least
> merged in a xcb-util.so library, and not managed as it is
> now, i.e. in a separate .so files, as said.

I'd be fine with piling it in with the event and atom stuff.
IMHO this would be a good idea; these are also chunks of
code that most programs may want to use.

> I've already been hit by a GNOME guy because when adding
> XCB support to libstartup-notification I add to link this
> last one to 3 xcb-util libs (event, atom and aux). And
> they say it slows down the application startup (no clue
> about that, I'm not an expert in this area).

The dynamic loader has been really broken in this area in
the past.  IIRC it's mostly fixed now.  But there is a point
beyond which more tiny interdependent libraries is just
obnoxious, in any case.

> > The stuff that auto-creates valueparam structs also
> > seems like a win for normal use?
> 
> For people coming from Xlib, probably. I spent most time
> using XCB than Xlib and I never had to use this stuff that
> just hides the protocol, making people more stupid.  They
> are interesting for documentation purpose as I said, but
> to use them, no way IMHO.

I think we can agree to disagree on this one. :-) I can't
imagine what I'd be learning by hand-building the weird
value lists that the protocol wants, once I got the basic
message that the protocol wants weird value lists sometimes.

> > Indeed.  The question is whether it makes more sense to
> > think of it [keyboard] as core functionality that almost
> > everyone will want, or as something that only some
> > people will use.  I don't know the answer to this.
> 
> I assure you that this is something every-damn-one will
> use. Keyboard is just unusable without the current 2 or 3
> functions in this lib, and it misses *a lot* of thing I
> really hope Arnaud will add ASAP. :-(

I think I agree, in which case I'd throw keyboard in with
event, aux and atom also.

This leads to the interesting question of whether it's
reasonable to have several Git repos for parts of the same
shared library.  Thoughts, anyone?

At any rate, I think there's maybe a difference in
philosophy about what xcb-aux is?  It was initially created
precisely to be a library of convenience wrappers over core
xcb, to put commonly-used and idiomatic code in one place.
We certainly don't want to make anyone use it, but it's way
easier when writing standalone xcb programs to just link
against this thing than to rewrite a bunch of stuff or call
things the hard way all over the place.  I hardly think
there's any danger of it becoming Xlib, in any case. :-)

    Bart