I believe you don't want to call XSync after XGetImage returns, since by the time that call returns you're guaranteed that any error it generated has been processed. Therefore calling XSync there reduces performance with no benefit. (Yes, GetImage is slow anyway, but why make it worse?)
Calling XSync before changing the error handler the first time is correct, though, as there may be async errors waiting to be read off the wire, or even reply-free requests waiting in the client's output queue that could trigger errors once they're flushed. Or, of course, you could switch to XCB for this one call, and avoid the extra round-trip due to XSync entirely. ;-) (If you only change this one call to use XCB then the resulting patch is a fair bit bigger than this one, and performance wins in Xnest's GetImage are pretty worthless, so that isn't a very serious suggestion.) Jamey On Sat, Oct 05, 2013 at 07:43:21AM +0200, Egbert Eich wrote: > From: Radek Doulik <[email protected]> > > When an Xnest instance is not viewable it will crash when a client in > that instance calls GetImage. This is because the Xnest server will > itself receives a BadMatch error. > This patch ignores the error. The application which has requested the > image will receive garbage - this however is fully legal according > to the specs as obscured areas will always contain garbage if there > isn't some sort of backing store as discussed in > https://bugs.freedesktop.org/show_bug.cgi?id=9488 > The applied patch is a version from Dadek Doulik. > > v2: Call XSync() before changing error handlers as suggested by > Daniel Stone <[email protected]>. > > Signed-off-by: Egbert Eich <[email protected]> > --- > hw/xnest/GCOps.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/hw/xnest/GCOps.c b/hw/xnest/GCOps.c > index e26a136..7b1956d 100644 > --- a/hw/xnest/GCOps.c > +++ b/hw/xnest/GCOps.c > @@ -94,15 +94,29 @@ xnestPutImage(DrawablePtr pDrawable, GCPtr pGC, int > depth, int x, int y, > } > } > > +static int > +xnestIgnoreErrorHandler (Display *display, > + XErrorEvent *event) > +{ > + return False; /* return value is ignored */ > +} > + > void > xnestGetImage(DrawablePtr pDrawable, int x, int y, int w, int h, > unsigned int format, unsigned long planeMask, char *pImage) > { > XImage *ximage; > int length; > + int (*old_handler)(Display*, XErrorEvent*); > + > + /* we may get BadMatch error when xnest window is minimized */ > + XSync(xnestDisplay, False); > + old_handler = XSetErrorHandler (xnestIgnoreErrorHandler); > > ximage = XGetImage(xnestDisplay, xnestDrawable(pDrawable), > x, y, w, h, planeMask, format); > + XSync(xnestDisplay, False); > + XSetErrorHandler(old_handler); > > if (ximage) { > length = ximage->bytes_per_line * ximage->height; > -- > 1.8.1.4 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
