[Xcb] Some comments on XCB

[Vincent Torri]

On Mon, 16 May 2005, Carl Worth wrote:

> I've spent a (very small) bit of time in the last week helping to
> bring the cairo XCB backend up to date with the latest changes to
> cairo. In doing so, I noticed a few things in the XCB
> API/implementation that I thought worth comment.
>
> The first thing I ran across was an inconsistent case convention in
> XCB types. I found the following in the code I was using:
>
> 	XCBConnection
> 	XCBGCONTEXT
> 	XCBDRAWABLE
> 	XCBVISUALTYPE
> 	XCBRenderPICTURE
>
> The result is that it's impossible for me to discern a consistent rule
> for capitalization. I imagine there might actually be a consistent
> rule, but I would argue it's not helpful to the user, as it's not
> discernible and therefore likely to just be a source of confusion and
> consternation. (Not to mention the fact that three words strung
> together in ALLCAPS as in XCBVISUALTYPE makes it REALLYHARDTOREAD even
> when ThisIsBadEnough.)

it should not be difficult to change. But we must agree on what convention
to use.

>
> The second thing I ran into is that the useful prototypes I needed to
> consult while coding are in XCB/xproto.h but this file has a reckless
> disregard for helpful whitespace, containing things like:
>
> XCBGetFontPathRep *XCBGetFontPathReply(XCBConnection *c, XCBGetFontPathCookie cookie, XCBGenericError **e);
> XCBVoidCookie XCBCreatePixmap(XCBConnection *c, CARD8 depth, XCBPIXMAP pid, XCBDRAWABLE drawable, CARD16 width, CARD16 height);
> XCBVoidCookie XCBFreePixmap(XCBConnection *c, XCBPIXMAP pixmap);
>
> which makes it really hard to visually extract the information I
> need. I'd really prefer to find:
>
> XCBGetFontPathRep *
> XCBGetFontPathReply (XCBConnection	  *c,
> 		     XCBGetFontPathCookie  cookie,
> 		     XCBGenericError	 **e);
>
> XCBVoidCookie
> XCBCreatePixmap (XCBConnection *c,
> 		 CARD8		depth,
> 		 XCBPIXMAP	pid,
> 		 XCBDRAWABLE	drawable,
> 		 CARD16		width,
> 		 CARD16		height);
>
> XCBVoidCookie
> XCBFreePixmap (XCBConnection *c,
> 	       XCBPIXMAP      pixmap);
>
> I recognized that there might be some difficulty in this area due to
> XCB's use of code generation, but anything that could be done to
> improve the current situation would be appreciated.

I completely agree with you, the API is very difficult to read.

I have already done such thing with html output, but I used html table to
do that, so it was simpler that just writing in a text file. Nevertheles,
puttin a \n after the return type is easy. Making the parameters aligned
is quite more complicated.

Josh, if you want my xslt code, tell me. Maybe it could give you ideas.

But, are the headers the good thing to read ? I think that we also could
add documentation. Say, something like using Doxygen. We put coments in
the .c files. I don't know if putting he documentation in the xml files is
usefull, as it's only the protocol.

Carl, there's tutorial for xcb. It's in cvs (xcb/doc/tutorial) but it is
not complete yet. You could look at it, maybe.

> Another minor snag I hit was with specifically with the
> XCBCreatePixmap call. If I have the pid of my new pixmap in
> "drawable.pixmap", it's tempting to look at the prototype above and
> think that I might need to pass "drawable" in for the drawable
> parameter. But, in fact, I need to pass in a drawable which serves as
> a marker for the screen on which I want to create a pixmap.
>
> I recognize that XCB closely follows the protocol description,
> (something from which I would not encourage deviation), so I'm not
> sure what to recommend here. It is interesting to note that
> XCreatePixmap doesn't suffer from the same potential for misuse as it
> returns the Pixmap being created so in that case it cannot possibly be
> confused with the screen-indication drawable parameter.
>
> Maybe the "drawable" parameter just needs a more descriptive name to
> indicate its purpose, (along similar lines as "XCBWINDOW parent" in
> XCBCreateWindow). But what? "screen_drawable" ? I'm not sure.
>
> OK, so finally, let's look at the code I came up with to get an
> XCBDRAWABLE from a screen, (let's assume screen 0), to pass into this
> parameter for XCBCreatePixmap. I wrote:
>
> 	XCBSCREEN *root;
> 	XCBDRAWABLE root_drawable;
> 	root = XCBConnSetupSuccessRepRootsIter(XCBGetSetup(c)).data;
> 	root_drawable.window = root->root;
>
> Ouch. Am I bringing pain upon myself, or is there no more convenient
> way to pull this off in XCB? Note that the equivalent code from Xlib
> is:
>
> 	DefaultRootWindow (dpy)
>

I could add such function in aux. Look at xcb-util/convenient/xcb_aux.h. I
already have written some (few) convenient functions. I could add more. In
that case, I would not used the same name (DefaultRootWindow) though.

> I certainly won't recommend Xlib's namespace polluting naming
> convention, but it is nice that there is a single call to get the job
> done here.
>
> Let me mention a few specifics about the block of XCB code above:
>
> "XCBConnection" seems a fine name for the primary opaque handle.
>
> "XCBConnSetupSuccessRep" seems rather unwieldy, but I can imagine the
> name choice might be somewhat constrained by naming conventions. But,
> assuming this name is as commonly used as I suspect, it would be
> really nice to make it shorter if possible.
>
> Is there a one-to-one mapping between XCBConnection and
> XCBConnSetupSuccessRep? (I'm guessing so since one is opaque and one
> is not). If so, is there really a benefit to passing the
> XCBConnSetupSuccessRep* rather than the XCBConnection* to the iterator
> fetch functions like XCBConnSetupSeccussRepRootsIter ? It leads to the
> nesting in the calls above and also more use of the longer name. Could
> something like this work instead (or in addition):
>
> 	XCBConnectionRootsIter (c).data;
>
> Finally, once I do get an XCBSCREEN* through the above, I can find a
> root->root XCBWINDOW but XCBCreatePixmap needs an XCBDRAWABLE
> instead. Hence, I'm forced to do the root_drawable wrapping
> above. This is a rather annoying side effect of the otherwise very
> useful feature of extra type checking. Anyone have a suggestion for
> simplifying this "casting" operation which is known to be safe in this
> direction?

I let the dev answer, here.

>
> It occurs to me that if we were just inventing a new API here, it's
> the screen that is really relevant to XCBCreatePixmap, so passing an
> XCBSCREEN* rather than an XCBDRAWABLE, (which the server uses to
> identify the screen), would work fine and would be simpler. But, since
> XCB is intended to be a tight binding to the actual protocol, that
> would not be a good idea.

i think it should remain as closest as posible to the protocol, imho.

Vincent