[X2Go-Dev] Patch to Fix x2goagent / nxagent With New LibCairo

[Mike Gabriel]

Hi Jim, hi all,

NoMachine upstream of NX has become aware of the issue (reported by me)...
http://www.nomachine.com/tr/view.php?id=TR05J02703

Mike

On Do 10 Mai 2012 19:21:18 CEST Jim Burnes wrote:

> 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.
>
> Let me know.
>
> eris0xff
>
> *** x2go/nx-libs-3.5.0.12/nx-X11/programs/Xserver/hw/nxagent/Render.c
> 2012-03-07 14:04:02.000000000 -0700
> ---
> x2go-new/nx-libs-3.5.0.12/nx-X11/programs/Xserver/hw/nxagent/Render.c
> 2012-05-10 11:09:39.631786853 -0600
> ***************
> *** 995,1000 ****
> --- 995,1030 ----
>     #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,1043 ****
>     }
>
>     #endif
> !
> !   if (NXAGENT_SHOULD_DEFER_COMPOSITE(pSrc, pMask, pDst))
>     {
>       pDstRegion = nxagentCreateRegion(pDst -> pDrawable, NULL, xDst, yDst,
> width, height);
>
> --- 1066,1073 ----
>     }
>
>     #endif
> !   /* if (NXAGENT_SHOULD_DEFER_COMPOSITE(pSrc, pMask, pDst)) */
> !   if (nxagentShouldDeferComposite(pSrc, pMask, pDst))
>     {
>       pDstRegion = nxagentCreateRegion(pDst -> pDrawable, NULL, xDst, yDst,
> width, height);
>
> ***************
> *** 1095,1101 ****
>       }
>     }
>
> !   if (pMask != NULL && pMask -> pDrawable != pSrc -> pDrawable &&
>             pMask -> pDrawable != pDst -> pDrawable)
>     {
>       nxagentSynchronizeShmPixmap(pMask -> pDrawable, xMask, yMask, width,
> height);
> --- 1125,1132 ----
>       }
>     }
>
> !   if ((pMask) && (pMask->pDrawable) &&
> !           pMask -> pDrawable != pSrc -> pDrawable &&
>             pMask -> pDrawable != pDst -> pDrawable)
>     {
>       nxagentSynchronizeShmPixmap(pMask -> pDrawable, xMask, yMask, width,
> height);
> ***************
> *** 1259,1265 ****
>      * on the real X server.
>      */
>
> !   if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentGlyphs: Synchronizing source [%s] at [%p].\n",
> --- 1290,1296 ----
>      * on the real X server.
>      */
>
> !   if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) ==
> NotSynchronized))
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentGlyphs: Synchronizing source [%s] at [%p].\n",
> ***************
> *** 1302,1315 ****
>         nxagentSynchronizeBox(pSrc -> pDrawable, &glyphBox, NEVER_BREAK);
>       }
>
> !     if (pSrc -> pDrawable -> type == DRAWABLE_PIXMAP)
>       {
>         nxagentIncreasePixmapUsageCounter((PixmapPtr) pSrc -> pDrawable);
>       }
>     }
>
> !   if (pSrc -> pDrawable != pDst -> pDrawable &&
> !           nxagentDrawableStatus(pDst -> pDrawable) == NotSynchronized)
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentGlyphs: Synchronizing destination [%s] at
> [%p].\n",
> --- 1333,1347 ----
>         nxagentSynchronizeBox(pSrc -> pDrawable, &glyphBox, NEVER_BREAK);
>       }
>
> !     if (pSrc -> pDrawable && (pSrc -> pDrawable -> type ==
> DRAWABLE_PIXMAP))
>       {
>         nxagentIncreasePixmapUsageCounter((PixmapPtr) pSrc -> pDrawable);
>       }
>     }
>
> !
> !   if (pSrc -> pDrawable && (pSrc -> pDrawable != pDst -> pDrawable &&
> !           nxagentDrawableStatus(pDst -> pDrawable) == NotSynchronized))
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentGlyphs: Synchronizing destination [%s] at
> [%p].\n",
> ***************
> *** 1749,1755 ****
>       return;
>     }
>
> !   if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTrapezoids: Going to synchronize the source
> drawable at [%p].\n",
> --- 1781,1789 ----
>       return;
>     }
>
> !   /* 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,1849 ****
>      * operation like nxagentTrapezoids() does.
>      */
>
> !   if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTriangles: Going to synchronize the source
> drawable at [%p].\n",
> --- 1877,1885 ----
>      * operation like nxagentTrapezoids() does.
>      */
>
> !
> !
> !   if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) ==
> NotSynchronized))
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTriangles: Going to synchronize the source
> drawable at [%p].\n",
> ***************
> *** 1920,1926 ****
>      * operation like nxagentTrapezoids() does.
>      */
>
> !   if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTriStrip: Going to synchronize the source
> drawable at [%p].\n",
> --- 1956,1963 ----
>      * operation like nxagentTrapezoids() does.
>      */
>
> !
> !   if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) ==
> NotSynchronized))
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTriStrip: Going to synchronize the source
> drawable at [%p].\n",
> ***************
> *** 1997,2003 ****
>      * operation like nxagentTrapezoids() does.
>      */
>
> !   if (nxagentDrawableStatus(pSrc -> pDrawable) == NotSynchronized)
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTriFan: Going to synchronize the source
> drawable at [%p].\n",
> --- 2034,2041 ----
>      * operation like nxagentTrapezoids() does.
>      */
>
> !
> !   if (pSrc -> pDrawable && (nxagentDrawableStatus(pSrc -> pDrawable) ==
> NotSynchronized))
>     {
>       #ifdef TEST
>       fprintf(stderr, "nxagentTriFan: Going to synchronize the source
> drawable at [%p].\n",
>



-- 

DAS-NETZWERKTEAM
mike gabriel, dorfstr. 27, 24245 barmissen
fon: +49 (4302) 281418, fax: +49 (4302) 281419

GnuPG Key ID 0xB588399B
mail: mike.gabriel at das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digitale PGP-Unterschrift
URL: <http://lists.x2go.org/pipermail/x2go-dev/attachments/20120514/1d5527ef/attachment.pgp>