[cairo] freedesktop.org links in configure.in are wrong

[Owen Taylor]

On Sat, 2005-05-14 at 19:37 -0700, Jamey Sharp wrote:

> Since you've put in all that effort, I figure the least I could do is
> review the patch. ;-) Everything looks fine, but there were some
> questions in comments that I can answer.
> 
> > +    geomrep = XCBGetGeometryReply(surface->dpy,
> > +                                 XCBGetGeometry(surface->dpy, surface->drawable),
> > +                                 0);
> > +    /* XXX: I haven't looked closely at XCB yet to know why this might
> > +     * fail. */
> > +    if(!geomrep)
> > +       return CAIRO_STATUS_NO_MEMORY;
> 
> Memory allocation failure is one possibility. More likely is an X error
> being returned instead of the desired reply. Also, if the X server
> connection closes, you should get a failure return from functions trying
> to access server data that hadn't arrived before the loss of the
> connection.
> 
> If you can usefully handle X errors from this request, then you should
> pass a reference to a XCBGenericError* as the third argument to the
> XCB...Reply function, and it will be filled in if an error occurs. By
> passing a null pointer as above, any error will be inserted into the
> event queue instead for the application to handle.

Thinking about some hypothetical future where Cairo is using XCB
internally in an application that is using Xlib, it seems like Cairo can
never rely on the "error event" approach, because the app at best will
just ignore the unknown event.

Usually X events an app can receive fall into three categories:

 - Predictable errors that the app should handle. If I want to 
   SendEvent to another apps window, that can always fail. If I
   call SetInputFocus on my own window, that can fail if the window
   manager unmaps my window.
   
   (a fairly common situation ... SetInputFocus, say, is where you
   need to ignore the error but don't care about whether it occurred
   or not.)

 - Unpredictable errors that indicate more-or-less unrecoverable
   global failure modes. (connection disconnection and out-of-memory)

 - Programming errors. (Mismatched depths, or say, you don't handle one
   of the errors in the first category.)

Preferably, cairo would handle all errors in the first category,
set a cairo status on the second class, and die a noisy death on
the third.

> > +    /* XXX: This should try to use the XShm extension if available */
> 
> You may want to be using Vincent Torri's XCBImage library, which is a
> port of XImage to XCB. The code is available from the xcb-util module of
> XCB CVS. It includes the XImage functions from Xlib as well as the XShm
> functions.
> 
> I seem to recall the XYPixmap support doesn't work and we don't
> understand why, but the library is probably worth using regardless.
> XCBImage should let you eliminate a fair amount of code from the Cairo
> XCB backend, while more closely reflecting the Xlib backend; and Cairo
> really shouldn't use XYPixmaps anyway. ;-)

I can't imagine us ever wanting to use XYPixmaps.

But I don't really don't think Cairo needs anything much from images
beyond for raw X protocol wrappers ...  we don't need the imutil
stuff .... GetPixel/PutPixel, because we have libpixman.

Do you need XCBImage to get protocol wrappers for MIT-SHM ?

(But since MIT-SHM isn't used in the Xlib backend either ... there are
some questions that would have to be answered there. Creating 
and destroying SHM images on the fly for each cairo_surface_t 
may be prohibitively expensive, and wipe out the marginal performance
gains that it gives you. GTK+ actually allocates a big SHM scratch area,
but there are distinct problems with that as well.)

> > +       /* XCBGetImage from a window is dangerous because it can
> > +        * produce errors if the window is unmapped or partially
> > +        * outside the screen. We could check for errors and
> > +        * retry, but to keep things simple, we just create a
> > +        * temporary pixmap
> > +        */
> 
> Again, XCB makes it easy to trap errors returned from particular
> requests. I'm not sure how the "retry" aspect would work here, but if
> the error-checking was the hard part under Xlib, then under XCB you
> should just do the GetImage.

The reason why the retry is necessary is that you get an X error if
you try to GetImage from an off-screen area of a Window. (protocol
bug, really.) So, a failure from GetImage can't just be ignored.

The current code in CVS actually has a hybrid approach ... when 
copying from a drawable of unknown type it traps errors:

        cairo_xlib_error_func_t old_handler;

        old_handler = XSetErrorHandler (_noop_error_handler);

        ximage = XGetImage (surface->dpy,
                            surface->drawable,
                            x1, y1,
                            x2 - x1, y2 - y1,
                            AllPlanes, ZPixmap);

        XSetErrorHandler (old_handler);

[ quoting code just to indicate that trapping errors in Xlib isn't
  *that* bad, except for thread-safety issues ]

But once it first gets an error it goes through a safe CopyArea/GetImage
code path. 

There's a nice opportunity for XCB usage in 
_cairo_xlib_surface_get_size() ...what it want to do is to determine
A) whether a drawable is a Window B) the current size of a drawable
in a single round trip. 

Using Xlib (and not using Xlibint.h) you can do that in a single 
roundtrip for a Window ... if XGetWindowAttributes() succeeds 
you've gotten both. But if XGetWindowAttributes gives you BadWindow,
you need to retry with a separate GetGeometry... two roundtrips.

Regards,
						Owen

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050515/73f20802/attachment.pgp