From 1f945fcabe2c99e4637add34103de58c4b6d6477 Mon Sep 17 00:00:00 2001 From: Ben Byer Date: Fri, 2 Nov 2007 15:19:39 -0700 Subject: [PATCH] Imported a fix from xserver/master which was part of a larger refactoring of fbComposite code. This part fixes a logic error in SafeAlphaComposite where some return paths resulted in the server changing the color depth of a Drawable out from under the client. This caused random crashes in many cairo/pango/gtk apps. Props to Guy Harris for tracking this down in WireShark, but it will probably solve problems in other apps (Inkscape, Gimp?) --- fb/fbpict.c | 117 +++- fb/fbpict.h | 31 ++ miext/rootless/safeAlpha/safeAlphaPicture.c | 559 +++----------------- 3 files changed, 197 insertions(+), 510 deletions(-) diff --git a/fb/fbpict.c b/fb/fbpict.c index 613e652e9..5f7289ad2 100644 --- a/fb/fbpict.c +++ b/fb/fbpict.c @@ -37,19 +37,6 @@ #include "fbpict.h" #include "fbmmx.h" -typedef void (*CompositeFunc) (CARD8 op, - PicturePtr pSrc, - PicturePtr pMask, - PicturePtr pDst, - INT16 xSrc, - INT16 ySrc, - INT16 xMask, - INT16 yMask, - INT16 xDst, - INT16 yDst, - CARD16 width, - CARD16 height); - CARD32 fbOver (CARD32 x, CARD32 y) { @@ -825,6 +812,110 @@ fbCompositeSolidMask_nx1xn (CARD8 op, # define mod(a,b) ((b) == 1 ? 0 : (a) >= 0 ? (a) % (b) : (b) - (-a) % (b)) +void +fbWalkCompositeRegion (CARD8 op, + PicturePtr pSrc, + PicturePtr pMask, + PicturePtr pDst, + INT16 xSrc, + INT16 ySrc, + INT16 xMask, + INT16 yMask, + INT16 xDst, + INT16 yDst, + CARD16 width, + CARD16 height, + Bool srcRepeat, + Bool maskRepeat, + CompositeFunc compositeRect) +{ + RegionRec region; + int n; + BoxPtr pbox; + int w, h, w_this, h_this; + int x_msk, y_msk, x_src, y_src, x_dst, y_dst; + + xDst += pDst->pDrawable->x; + yDst += pDst->pDrawable->y; + if (pSrc->pDrawable) + { + xSrc += pSrc->pDrawable->x; + ySrc += pSrc->pDrawable->y; + } + if (pMask && pMask->pDrawable) + { + xMask += pMask->pDrawable->x; + yMask += pMask->pDrawable->y; + } + + if (!miComputeCompositeRegion (®ion, pSrc, pMask, pDst, xSrc, ySrc, + xMask, yMask, xDst, yDst, width, height)) + return; + + n = REGION_NUM_RECTS (®ion); + pbox = REGION_RECTS (®ion); + while (n--) + { + h = pbox->y2 - pbox->y1; + y_src = pbox->y1 - yDst + ySrc; + y_msk = pbox->y1 - yDst + yMask; + y_dst = pbox->y1; + while (h) + { + h_this = h; + w = pbox->x2 - pbox->x1; + x_src = pbox->x1 - xDst + xSrc; + x_msk = pbox->x1 - xDst + xMask; + x_dst = pbox->x1; + if (maskRepeat) + { + y_msk = mod (y_msk - pMask->pDrawable->y, pMask->pDrawable->height); + if (h_this > pMask->pDrawable->height - y_msk) + h_this = pMask->pDrawable->height - y_msk; + y_msk += pMask->pDrawable->y; + } + if (srcRepeat) + { + y_src = mod (y_src - pSrc->pDrawable->y, pSrc->pDrawable->height); + if (h_this > pSrc->pDrawable->height - y_src) + h_this = pSrc->pDrawable->height - y_src; + y_src += pSrc->pDrawable->y; + } + while (w) + { + w_this = w; + if (maskRepeat) + { + x_msk = mod (x_msk - pMask->pDrawable->x, pMask->pDrawable->width); + if (w_this > pMask->pDrawable->width - x_msk) + w_this = pMask->pDrawable->width - x_msk; + x_msk += pMask->pDrawable->x; + } + if (srcRepeat) + { + x_src = mod (x_src - pSrc->pDrawable->x, pSrc->pDrawable->width); + if (w_this > pSrc->pDrawable->width - x_src) + w_this = pSrc->pDrawable->width - x_src; + x_src += pSrc->pDrawable->x; + } + (*compositeRect) (op, pSrc, pMask, pDst, + x_src, y_src, x_msk, y_msk, x_dst, y_dst, + w_this, h_this); + w -= w_this; + x_src += w_this; + x_msk += w_this; + x_dst += w_this; + } + h -= h_this; + y_src += h_this; + y_msk += h_this; + y_dst += h_this; + } + pbox++; + } + REGION_UNINIT (pDst->pDrawable->pScreen, ®ion); +} + void fbComposite (CARD8 op, PicturePtr pSrc, diff --git a/fb/fbpict.h b/fb/fbpict.h index bfcb38e58..b90863e67 100644 --- a/fb/fbpict.h +++ b/fb/fbpict.h @@ -618,6 +618,37 @@ fbComposite (CARD8 op, CARD16 width, CARD16 height); + +typedef void (*CompositeFunc) (CARD8 op, + PicturePtr pSrc, + PicturePtr pMask, + PicturePtr pDst, + INT16 xSrc, + INT16 ySrc, + INT16 xMask, + INT16 yMask, + INT16 xDst, + INT16 yDst, + CARD16 width, + CARD16 height); + +void +fbWalkCompositeRegion (CARD8 op, + PicturePtr pSrc, + PicturePtr pMask, + PicturePtr pDst, + INT16 xSrc, + INT16 ySrc, + INT16 xMask, + INT16 yMask, + INT16 xDst, + INT16 yDst, + CARD16 width, + CARD16 height, + Bool srcRepeat, + Bool maskRepeat, + CompositeFunc compositeRect); + /* fbtrap.c */ void diff --git a/miext/rootless/safeAlpha/safeAlphaPicture.c b/miext/rootless/safeAlpha/safeAlphaPicture.c index 6ccc05a27..8f6631531 100644 --- a/miext/rootless/safeAlpha/safeAlphaPicture.c +++ b/miext/rootless/safeAlpha/safeAlphaPicture.c @@ -48,21 +48,6 @@ #include "rootlessCommon.h" # define mod(a,b) ((b) == 1 ? 0 : (a) >= 0 ? (a) % (b) : (b) - (-a) % (b)) - -typedef void (*CompositeFunc) (CARD8 op, - PicturePtr pSrc, - PicturePtr pMask, - PicturePtr pDst, - INT16 xSrc, - INT16 ySrc, - INT16 xMask, - INT16 yMask, - INT16 xDst, - INT16 yDst, - CARD16 width, - CARD16 height); - - /* Optimized version of fbCompositeSolidMask_nx8x8888 */ void SafeAlphaCompositeSolidMask_nx8x8888( @@ -148,499 +133,79 @@ SafeAlphaCompositeSolidMask_nx8x8888( } void -SafeAlphaComposite (CARD8 op, - PicturePtr pSrc, - PicturePtr pMask, - PicturePtr pDst, - INT16 xSrc, - INT16 ySrc, - INT16 xMask, - INT16 yMask, - INT16 xDst, - INT16 yDst, - CARD16 width, - CARD16 height) +SafeAlphaComposite (CARD8 op, + PicturePtr pSrc, + PicturePtr pMask, + PicturePtr pDst, + INT16 xSrc, + INT16 ySrc, + INT16 xMask, + INT16 yMask, + INT16 xDst, + INT16 yDst, + CARD16 width, + CARD16 height) { - RegionRec region; - int n; - BoxPtr pbox; - CompositeFunc func = 0; - Bool srcRepeat = pSrc->repeat; - Bool maskRepeat = FALSE; - Bool srcAlphaMap = pSrc->alphaMap != 0; - Bool maskAlphaMap = FALSE; - Bool dstAlphaMap = pDst->alphaMap != 0; - int x_msk, y_msk, x_src, y_src, x_dst, y_dst; - int w, h, w_this, h_this; - int dstDepth = pDst->pDrawable->depth; - int oldFormat = pDst->format; + if (!pSrc) { + ErrorF("SafeAlphaComposite: pSrc must not be null!\n"); + return; + } - xDst += pDst->pDrawable->x; - yDst += pDst->pDrawable->y; - xSrc += pSrc->pDrawable->x; - ySrc += pSrc->pDrawable->y; - if (pMask) + if (!pDst) { + ErrorF("SafeAlphaComposite: pDst must not be null!\n"); + return; + } + + int oldDepth = pDst->pDrawable->depth; + int oldFormat = pDst->format; + + /* + * We can use the more optimized fbpict code, but it sets bits above + * the depth to zero. Temporarily adjust destination depth if needed. + */ + if (pDst->pDrawable->type == DRAWABLE_WINDOW + && pDst->pDrawable->depth == 24 + && pDst->pDrawable->bitsPerPixel == 32) { - xMask += pMask->pDrawable->x; - yMask += pMask->pDrawable->y; - maskRepeat = pMask->repeat; - maskAlphaMap = pMask->alphaMap != 0; + pDst->pDrawable->depth = 32; } - - - /* - * We can use the more optimized fbpict code, but it sets bits above - * the depth to zero. Temporarily adjust destination depth if needed. - */ - if (pDst->pDrawable->type == DRAWABLE_WINDOW - && pDst->pDrawable->depth == 24 - && pDst->pDrawable->bitsPerPixel == 32) + + /* For rootless preserve the alpha in x8r8g8b8 which really is + * a8r8g8b8 + */ + if (oldFormat == PICT_x8r8g8b8) { - pDst->pDrawable->depth = 32; + pDst->format = PICT_a8r8g8b8; } - /* For rootless preserve the alpha in x8r8g8b8 which really is - * a8r8g8b8 - */ - if (oldFormat == PICT_x8r8g8b8) + + if (pSrc->pDrawable && pMask && pMask->pDrawable && + !pSrc->transform && !pMask->transform && + !pSrc->alphaMap && !pMask->alphaMap && + !pMask->repeat && !pMask->componentAlpha && !pDst->alphaMap && + pMask->format == PICT_a8 && + pSrc->repeatType == RepeatNormal && + pSrc->pDrawable->width == 1 && + pSrc->pDrawable->height == 1 && + (pDst->format == PICT_a8r8g8b8 || + pDst->format == PICT_x8r8g8b8 || + pDst->format == PICT_a8b8g8r8 || + pDst->format == PICT_x8b8g8r8)) { - pDst->format = PICT_a8r8g8b8; + fbWalkCompositeRegion (op, pSrc, pMask, pDst, + xSrc, ySrc, xMask, yMask, xDst, yDst, + width, height, + TRUE /* srcRepeat */, + FALSE /* maskRepeat */, + SafeAlphaCompositeSolidMask_nx8x8888); } - - - - if (!pSrc->transform && !(pMask && pMask->transform)) - if (!maskAlphaMap && !srcAlphaMap && !dstAlphaMap) - switch (op) { - case PictOpSrc: -#ifdef USE_MMX - if (!pMask && pSrc->format == pDst->format && - pSrc->pDrawable != pDst->pDrawable) - { - func = fbCompositeCopyAreammx; - } -#endif - break; - case PictOpOver: - if (pMask) - { - if (srcRepeat && - pSrc->pDrawable->width == 1 && - pSrc->pDrawable->height == 1) - { - srcRepeat = FALSE; - if (PICT_FORMAT_COLOR(pSrc->format)) { - switch (pMask->format) { - case PICT_a8: - switch (pDst->format) { - case PICT_r5g6b5: - case PICT_b5g6r5: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSolidMask_nx8x0565mmx; - else -#endif - func = fbCompositeSolidMask_nx8x0565; - break; - case PICT_r8g8b8: - case PICT_b8g8r8: - func = fbCompositeSolidMask_nx8x0888; - break; - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: - case PICT_a8b8g8r8: - case PICT_x8b8g8r8: - func = SafeAlphaCompositeSolidMask_nx8x8888; - break; - } - break; - case PICT_a8r8g8b8: - if (pMask->componentAlpha) { - switch (pDst->format) { - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSolidMask_nx8888x8888Cmmx; - else -#endif - func = fbCompositeSolidMask_nx8888x8888C; - break; - case PICT_r5g6b5: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSolidMask_nx8888x0565Cmmx; - else -#endif - func = fbCompositeSolidMask_nx8888x0565C; - break; - } - } - break; - case PICT_a8b8g8r8: - if (pMask->componentAlpha) { - switch (pDst->format) { - case PICT_a8b8g8r8: - case PICT_x8b8g8r8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSolidMask_nx8888x8888Cmmx; - else -#endif - func = fbCompositeSolidMask_nx8888x8888C; - break; - case PICT_b5g6r5: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSolidMask_nx8888x0565Cmmx; - else -#endif - func = fbCompositeSolidMask_nx8888x0565C; - break; - } - } - break; - case PICT_a1: - switch (pDst->format) { - case PICT_r5g6b5: - case PICT_b5g6r5: - case PICT_r8g8b8: - case PICT_b8g8r8: - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: - case PICT_a8b8g8r8: - case PICT_x8b8g8r8: - func = fbCompositeSolidMask_nx1xn; - break; - } - break; - } - } - } - else /* has mask and non-repeating source */ - { - if (pSrc->pDrawable == pMask->pDrawable && - xSrc == xMask && ySrc == yMask && - !pMask->componentAlpha) - { - /* source == mask: non-premultiplied data */ - switch (pSrc->format) { - case PICT_x8b8g8r8: - switch (pMask->format) { - case PICT_a8r8g8b8: - case PICT_a8b8g8r8: - switch (pDst->format) { - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888RevNPx8888mmx; -#endif - break; - case PICT_r5g6b5: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888RevNPx0565mmx; -#endif - break; - } - break; - } - break; - case PICT_x8r8g8b8: - switch (pMask->format) { - case PICT_a8r8g8b8: - case PICT_a8b8g8r8: - switch (pDst->format) { - case PICT_a8b8g8r8: - case PICT_x8b8g8r8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888RevNPx8888mmx; -#endif - break; - case PICT_r5g6b5: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888RevNPx0565mmx; -#endif - break; - } - break; - } - break; - } - break; - } - else - { - /* non-repeating source, repeating mask => translucent window */ - if (maskRepeat && - pMask->pDrawable->width == 1 && - pMask->pDrawable->height == 1) - { - if (pSrc->format == PICT_x8r8g8b8 && - pDst->format == PICT_x8r8g8b8 && - pMask->format == PICT_a8) - { -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888x8x8888mmx; -#endif - } - } - } - } - } - else /* no mask */ - { - if (srcRepeat && - pSrc->pDrawable->width == 1 && - pSrc->pDrawable->height == 1) - { - /* no mask and repeating source */ - switch (pSrc->format) { - case PICT_a8r8g8b8: - switch (pDst->format) { - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: -#ifdef USE_MMX - if (fbHaveMMX()) - { - srcRepeat = FALSE; - func = fbCompositeSolid_nx8888mmx; - } -#endif - break; - case PICT_r5g6b5: -#ifdef USE_MMX - if (fbHaveMMX()) - { - srcRepeat = FALSE; - func = fbCompositeSolid_nx0565mmx; - } -#endif - break; - } - break; - } - } - else - { - switch (pSrc->format) { - case PICT_a8r8g8b8: - switch (pDst->format) { - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888x8888mmx; - else -#endif - func = fbCompositeSrc_8888x8888; - break; - case PICT_r8g8b8: - func = fbCompositeSrc_8888x0888; - break; - case PICT_r5g6b5: - func = fbCompositeSrc_8888x0565; - break; - } - break; - case PICT_x8r8g8b8: - switch (pDst->format) { - case PICT_a8r8g8b8: - case PICT_x8r8g8b8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeCopyAreammx; -#endif - break; - } - case PICT_x8b8g8r8: - switch (pDst->format) { - case PICT_a8b8g8r8: - case PICT_x8b8g8r8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeCopyAreammx; -#endif - break; - } - break; - case PICT_a8b8g8r8: - switch (pDst->format) { - case PICT_a8b8g8r8: - case PICT_x8b8g8r8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrc_8888x8888mmx; - else -#endif - func = fbCompositeSrc_8888x8888; - break; - case PICT_b8g8r8: - func = fbCompositeSrc_8888x0888; - break; - case PICT_b5g6r5: - func = fbCompositeSrc_8888x0565; - break; - } - break; - case PICT_r5g6b5: - switch (pDst->format) { - case PICT_r5g6b5: - func = fbCompositeSrc_0565x0565; - break; - } - break; - case PICT_b5g6r5: - switch (pDst->format) { - case PICT_b5g6r5: - func = fbCompositeSrc_0565x0565; - break; - } - break; - } - } - } - break; - case PictOpAdd: - if (pMask == 0) - { - switch (pSrc->format) { - case PICT_a8r8g8b8: - switch (pDst->format) { - case PICT_a8r8g8b8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrcAdd_8888x8888mmx; - else -#endif - func = fbCompositeSrcAdd_8888x8888; - break; - } - break; - case PICT_a8b8g8r8: - switch (pDst->format) { - case PICT_a8b8g8r8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrcAdd_8888x8888mmx; - else -#endif - func = fbCompositeSrcAdd_8888x8888; - break; - } - break; - case PICT_a8: - switch (pDst->format) { - case PICT_a8: -#ifdef USE_MMX - if (fbHaveMMX()) - func = fbCompositeSrcAdd_8000x8000mmx; - else -#endif - func = fbCompositeSrcAdd_8000x8000; - break; - } - break; - case PICT_a1: - switch (pDst->format) { - case PICT_a1: - func = fbCompositeSrcAdd_1000x1000; - break; - } - break; - } - } - break; - } - - if (!func) { - /* no fast path, use the general code */ - fbCompositeGeneral(op, pSrc, pMask, pDst, xSrc, ySrc, xMask, yMask, xDst, yDst, width, height); - // Reset destination depth and format to their true value - pDst->pDrawable->depth = dstDepth; - pDst->format = oldFormat; - return; - } - - if (!miComputeCompositeRegion (®ion, - pSrc, - pMask, - pDst, - xSrc, - ySrc, - xMask, - yMask, - xDst, - yDst, - width, - height)) - return; - - n = REGION_NUM_RECTS (®ion); - pbox = REGION_RECTS (®ion); - while (n--) + else { - h = pbox->y2 - pbox->y1; - y_src = pbox->y1 - yDst + ySrc; - y_msk = pbox->y1 - yDst + yMask; - y_dst = pbox->y1; - while (h) - { - h_this = h; - w = pbox->x2 - pbox->x1; - x_src = pbox->x1 - xDst + xSrc; - x_msk = pbox->x1 - xDst + xMask; - x_dst = pbox->x1; - if (maskRepeat) - { - y_msk = mod (y_msk, pMask->pDrawable->height); - if (h_this > pMask->pDrawable->height - y_msk) - h_this = pMask->pDrawable->height - y_msk; - } - if (srcRepeat) - { - y_src = mod (y_src, pSrc->pDrawable->height); - if (h_this > pSrc->pDrawable->height - y_src) - h_this = pSrc->pDrawable->height - y_src; - } - while (w) - { - w_this = w; - if (maskRepeat) - { - x_msk = mod (x_msk, pMask->pDrawable->width); - if (w_this > pMask->pDrawable->width - x_msk) - w_this = pMask->pDrawable->width - x_msk; - } - if (srcRepeat) - { - x_src = mod (x_src, pSrc->pDrawable->width); - if (w_this > pSrc->pDrawable->width - x_src) - w_this = pSrc->pDrawable->width - x_src; - } - (*func) (op, pSrc, pMask, pDst, - x_src, y_src, x_msk, y_msk, x_dst, y_dst, - w_this, h_this); - w -= w_this; - x_src += w_this; - x_msk += w_this; - x_dst += w_this; - } - h -= h_this; - y_src += h_this; - y_msk += h_this; - y_dst += h_this; - } - pbox++; + fbComposite (op, pSrc, pMask, pDst, + xSrc, ySrc, xMask, yMask, xDst, yDst, width, height); } - REGION_UNINIT (pDst->pDrawable->pScreen, ®ion); - // Reset destination depth/format to its true value - pDst->pDrawable->depth = dstDepth; - pDst->format = oldFormat; + pDst->pDrawable->depth = oldDepth; + pDst->format = oldFormat; } #endif /* RENDER */