Description: Fix nxagent/x2goagent With New LibCairo (>1.12.1) Quoting two postings of Jim Burnes on x2go-dev ML: I don't know what the current patch status is for fixing nxagent with the new libcairo (1.12.1+ I believe), but eventually I got tired of waiting and created my own patches for nxagent/x2goagent. Most of the fixes were required because the render extension now allows (and libcairo uses) null source drawables (for gradients etc), null masks and null mask drawables. This change creates a bit of a logic mess in the code. Previous patches to the code tried to account for all of the possibilities, but fell a little short. Consider this an alpha-quality patch. I've only tested it in KDE while running GTK applications. All my favorite GTK apps like Firefox, Emacs, rox-filer and all my other GTK apps that were broken are now working just fine. (Though I'm getting only the standard GTK look and feel - don't know if that's caused by anything I've done.) Could someone test this under Gnome? Also, since I'm not primarily an X software engineer I'd like a specialist to take a look at it. The fix is a little crude. I just attached to the x2goagent process and fixed the lines that caused segfaults. (About 10 of them). I also rewrote one of the macros in Pixels.h into a local subroutine in Render.c. It had a bug in it and complex macro bugs are a PITA to debug in gdb (or anything else really). The macro is only used in one place and although the code in the macro is called pretty often, it's very likely that the compiler would inline it anyway. The rewrite increases readability by a large factor. A better patch could be created by someone that understands nxagent and X much better. The render extension code receives render ops from X client programs. The render ops can contain any combination of picture source, picture destination and picture mask. It's apparently legal to send render ops with combinations of null picture source drawables, picture masks and picture mask drawables. A better way to patch this would be to simply perform a return on all the illegal combinations of null parameters for the render ops. That way you wouldn't have to keep re-checking the parameter values. So anyway, here it is. I appreciate it if someone out there would test it and let me know. Also if anyone knows of the X docs which discuss null picture sources and masks in the render extension I'd be glad to create a cleaner patch that conforms to the stands. You can reproduce the issue by running any recent copy of x2go/nxagent and start any program that uses very recent versions of libCairo. Things started breaking for both ArchLinux and Debian SID users about 3 weeks ago. The issues started with versions of libCairo >= libcairo2_1.12.0-2_amd64 (debian packages of course). These versions of Cairo seem to use null parameters in render ops a lot. Users of recent GTK environments would have the startup process just crash. KDE sessions start and run fine until you start a gtk app. Forwarded: pending Author: Jim Burnes Last-Update: 2012-05-11 --- a/nx-X11/programs/Xserver/hw/nxagent/Render.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Render.c @@ -995,6 +995,36 @@ #endif } + +int nxagentShouldDeferComposite(PicturePtr pSrc, PicturePtr pMask, PicturePtr pDst) +{ + +int drawableDst; +int linkDeferred; +int unSyncedSrcMask; + + drawableDst = ( nxagentRenderVersionMajor == 0 && + nxagentRenderVersionMinor == 8 && + (pDst) -> pDrawable -> type == DRAWABLE_PIXMAP + ); + + linkDeferred = ( nxagentOption(DeferLevel) >= 2 && + nxagentOption(LinkType) < LINK_TYPE_ADSL + ); + + unSyncedSrcMask = ( nxagentOption(DeferLevel) == 1 && + (pDst) -> pDrawable -> type == DRAWABLE_PIXMAP && + ( + (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)) || + ((pMask) && pMask -> pDrawable && (nxagentDrawableStatus((pMask) -> pDrawable) == NotSynchronized)) + ) + ); + + + return drawableDst || linkDeferred || unSyncedSrcMask; +} + + void nxagentComposite(CARD8 op, PicturePtr pSrc, PicturePtr pMask, PicturePtr pDst, INT16 xSrc, INT16 ySrc, INT16 xMask, INT16 yMask, INT16 xDst, INT16 yDst, CARD16 width, CARD16 height) @@ -1036,8 +1066,8 @@ } #endif - - if (NXAGENT_SHOULD_DEFER_COMPOSITE(pSrc, pMask, pDst)) + /* if (NXAGENT_SHOULD_DEFER_COMPOSITE(pSrc, pMask, pDst)) */ + if (nxagentShouldDeferComposite(pSrc, pMask, pDst)) { pDstRegion = nxagentCreateRegion(pDst -> pDrawable, NULL, xDst, yDst, width, height); @@ -1095,7 +1125,8 @@ } } - if (pMask != NULL && pMask -> pDrawable != pSrc -> pDrawable && + if ((pMask) && (pMask->pDrawable) && + pMask -> pDrawable != pSrc -> pDrawable && pMask -> pDrawable != pDst -> pDrawable) { nxagentSynchronizeShmPixmap(pMask -> pDrawable, xMask, yMask, width, height); @@ -1259,7 +1290,7 @@ * on the real X server. */ - if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized) + if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)) { #ifdef TEST fprintf(stderr, "nxagentGlyphs: Synchronizing source [%s] at [%p].\n", @@ -1302,14 +1333,15 @@ nxagentSynchronizeBox(pSrc -> pDrawable, &glyphBox, NEVER_BREAK); } - if (pSrc -> pDrawable -> type == DRAWABLE_PIXMAP) + if (pSrc -> pDrawable && (pSrc -> pDrawable -> type == DRAWABLE_PIXMAP)) { nxagentIncreasePixmapUsageCounter((PixmapPtr) pSrc -> pDrawable); } } - if (pSrc -> pDrawable != pDst -> pDrawable && - nxagentDrawableStatus(pDst -> pDrawable) == NotSynchronized) + + if (pSrc -> pDrawable && (pSrc -> pDrawable != pDst -> pDrawable && + nxagentDrawableStatus(pDst -> pDrawable) == NotSynchronized)) { #ifdef TEST fprintf(stderr, "nxagentGlyphs: Synchronizing destination [%s] at [%p].\n", @@ -1749,7 +1781,9 @@ return; } - if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized) + /* the following blocks need fixing to ignore null values of pDrawable */ + + if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)) { #ifdef TEST fprintf(stderr, "nxagentTrapezoids: Going to synchronize the source drawable at [%p].\n", @@ -1843,7 +1877,9 @@ * operation like nxagentTrapezoids() does. */ - if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized) + + + if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)) { #ifdef TEST fprintf(stderr, "nxagentTriangles: Going to synchronize the source drawable at [%p].\n", @@ -1920,7 +1956,8 @@ * operation like nxagentTrapezoids() does. */ - if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized) + + if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)) { #ifdef TEST fprintf(stderr, "nxagentTriStrip: Going to synchronize the source drawable at [%p].\n", @@ -1997,7 +2034,8 @@ * operation like nxagentTrapezoids() does. */ - if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized) + + if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)) { #ifdef TEST fprintf(stderr, "nxagentTriFan: Going to synchronize the source drawable at [%p].\n",