From ab0b853e754fa11485963de2bec6f07e117f07cd Mon Sep 17 00:00:00 2001 From: Peter Harris Date: Fri, 8 Mar 2019 09:55:57 -0500 Subject: [PATCH] fb: Fix fbComposite source and mask clipping The render specification says that the user's clip is applied to both input and output. Other backends (eg. glamor and exa) clip the source and mask via miComputeCompositeRegion. Instead, fb relies on pixman to do all of the clipping, but pixman cannot clip the source unless we tell it the clip that should be applied to the source. fbComposite originally set the clip parameter of image_from_pict to TRUE before commit a72c65e9176c51de95db2fdbf4c5d946a4911695 "fb: Adjust transform or composite coordinates for pixman operations". That commit talks about origin adjustments, but makes no mention of clipping, which leads me to believe that the change from TRUE to FALSE was unintentional. Unfortunately, we can't revert this small change because the code now uses pCompositeClip, which is incorrect when there is a transform applied. I noticed this due to a massive performance regression when running x11perf under marco (the window manager of the MATE desktop environment). marco sets a clip on the source picture before calling RenderComposite to update the screen, rather than the destination picture. Signed-off-by: Peter Harris --- fb/fbpict.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/fb/fbpict.c b/fb/fbpict.c index 9797447b4..0c08e6cf5 100644 --- a/fb/fbpict.c +++ b/fb/fbpict.c @@ -286,6 +286,7 @@ create_bits_picture(PicturePtr pict, Bool has_clip, int *xoff, int *yoff) FbStride stride; int bpp; pixman_image_t *image; + RegionPtr clip; fbGetDrawablePixmap(pict->pDrawable, pixmap, *xoff, *yoff); fbGetPixmapBitsData(pixmap, bits, stride, bpp); @@ -304,20 +305,26 @@ create_bits_picture(PicturePtr pict, Bool has_clip, int *xoff, int *yoff) (pixman_write_memory_func_t) wfbWriteMemory); #endif - /* pCompositeClip is undefined for source pictures, so - * only set the clip region for pictures with drawables - */ - if (has_clip) { + clip = pict->pCompositeClip; + + if (!has_clip) { + /* pCompositeClip is undefined for source pictures, so + * use the client clip region directly + */ + clip = pict->clientClip; + } + + if (clip) { if (pict->clientClip) pixman_image_set_has_client_clip(image, TRUE); if (*xoff || *yoff) - pixman_region_translate(pict->pCompositeClip, *xoff, *yoff); + pixman_region_translate(clip, *xoff, *yoff); - pixman_image_set_clip_region(image, pict->pCompositeClip); + pixman_image_set_clip_region(image, clip); if (*xoff || *yoff) - pixman_region_translate(pict->pCompositeClip, -*xoff, -*yoff); + pixman_region_translate(clip, -*xoff, -*yoff); } /* Indexed table */