On Fri, 2015-06-05 at 19:14 +0200, Marc-André Lureau wrote:
> XPutImage requires to copy the images around, and the request may be
> split over several chunks. Using XShm may improve performance.
Almost certainly will, the XPutImage implementation is fairly comic.
> +static volatile int XErrorFlag = 0;
> +
> +/**
> + * Catches potential Xlib errors.
> + */
> +static int
> +handle_xerror(Display *dpy, XErrorEvent *event)
> +{
> + (void) dpy;
> + (void) event;
> + XErrorFlag = 1;
> + return 0;
> +}
>
This should only set the error flag if the error really did come from
XShmAttach, and (ideally) should call through to the old handler
otherwise. X is async, there might be requests before the ShmAttach in
the queue that fail.
> +static Bool
> +XCreateDrawable(struct drisw_drawable * pdp, int shmid, Display * dpy)
> +{
> + if (pdp->ximage)
> + XDestroyImage(pdp->ximage);
> +
> + if (shmid >= 0) {
> + int (*old_handler)(Display *, XErrorEvent *);
> +
> + pdp->shminfo.shmid = shmid;
> + pdp->ximage = XShmCreateImage(dpy,
> + pdp->visinfo->visual,
> + pdp->visinfo->depth,
> + ZPixmap,
> + NULL,
> + &pdp->shminfo,
> + 0, 0);
> + if (pdp->ximage == NULL)
> + goto ximage;
> +
> + XErrorFlag = 0;
> + old_handler = XSetErrorHandler(handle_xerror);
> + /* This may trigger the X protocol error we're ready to catch: */
> + XShmAttach(dpy, &pdp->shminfo);
> + XSync(dpy, False);
> +
> + if (!XErrorFlag)
> + goto end;
> +
> + /* we are on a remote display, this error is normal, don't print it */
> + XFlush(dpy);
XFlush after XSync is a no-op. (Well, unless other threads have
touched the display, but even if they have there's no reason to flush
here.)
> + XErrorFlag = 0;
> + XDestroyImage(pdp->ximage);
> + pdp->ximage = NULL;
> + (void) XSetErrorHandler(old_handler);
> + }
This is the non-shm fallback path; it's also the only place where
you're restoring old_handler. Should do it right after XSync.
> @@ -144,6 +203,11 @@ swrastPutImage2(__DRIdrawable * draw, int op,
> XImage *ximage;
> GC gc;
>
> + if (!pdp->ximage || shmid != pdp->shminfo.shmid) {
> + if (!XCreateDrawable(pdp, shmid, dpy))
> + return;
> + }
> +
Can you explain why you're doing image realization lazily instead of at
createDrawable time?
Also, you're punishing remote clients pretty harshly here. You've got
dri_sw_displaytarget_display calling put_image_shm if the storage was
allocated as a shm segment (shmid != -1), but pdp->shminfo.shmid is
only set to shmid if ShmAttach succeeds. On remote connections it
won't, but you take an XSync on every SwapBuffers to find that out.
> @@ -156,24 +220,51 @@ swrastPutImage2(__DRIdrawable * draw, int op,
> }
>
> drawable = pdraw->xDrawable;
> -
> ximage = pdp->ximage;
> - ximage->data = data;
> - ximage->width = w;
> - ximage->height = h;
> ximage->bytes_per_line = stride ? stride : bytes_per_line(w *
> ximage->bits_per_pixel, 32);
> + ximage->data = data;
>
> - XPutImage(dpy, drawable, gc, ximage, 0, 0, x, y, w, h);
> -
> + if (pdp->shminfo.shmid >= 0) {
> + ximage->width = ximage->bytes_per_line / 4;
> + ximage->height = h;
> + XShmPutImage(dpy, drawable, gc, ximage, 0, 0, x, y, w, h, False);
Pretty sure this is broken for non-32bpp.
- ajax
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
