I'm trying to disect this commit to see why it's causing problems for Peter Dyballa (his emacs buffers are getting "cut off" ... see his original "xorg-server 1.7.99.2 is faulty" thread). I'm not too familiar with pixman and this area of the server, so hopefully someone else (Keith?) can respond to this...
I think the problem may lie in the fact that some variables are uninitialized: eg: > + int image_xoff, image_yoff; > + pixman_image_t *image = image_from_pict (pPicture, FALSE, &image_xoff, > &image_yoff); which then does: > image = create_bits_picture (pict, has_clip, xoff, yoff); which then does: > fbGetDrawablePixmap (pict->pDrawable, pixmap, *xoff, *yoff); even though *xoff and *yoff are garbage values. On Dec 2, 2009, at 16:25, Keith Packard wrote: > > Sorry for replying to myself again -- I meant to describe the effect of > this patch on source operands. > > What this does is use the entire pixmap as the source operand and then > adjust either the source transform (if present) or source coordinates to > fetch pixels from the right part of that pixmap. > > So, this gets the right pixels for source coordinates within the source > window. What it doesn't do is get the right pixels for anything > else. None of the repeat modes work, and pixels falling outside of the > window, but inside the pixmap will get whatever was in the pixmap at > that location. > > To make the repeat modes work, we'd need to provide a rectangle > describing the projection of the window into the pixman_image, then > replace all of the existing pixman_image-based clipping with clipping > based on the window. This can be done in a backwards compatible > fashion by setting the clipping to the bounds of the pixman_image by > default and exposing a new API for source clipping. > > We could also make source clipping work by performing point-in-region > computations for every source pixel fetch; one suspects that might be a > bit too expensive for most applications though. > On Dec 2, 2009, at 16:05, Keith Packard wrote: > Windows (or even pixmaps, in some cases) may not sit at the origin of > the containing pixmap, so any coordinates relative to the drawable > must be adjusted. For destinations and untransformed sources, the > operation coordinates are adjusted. For transformed sources, the > transform matrix is adjusted. > > Signed-off-by: Keith Packard <[email protected]> > --- > fb/fb.h | 7 ++++- > fb/fbpict.c | 71 ++++++++++++++++++++++++++++++++++++++++------------------ > fb/fbtrap.c | 6 +++- > 3 files changed, 58 insertions(+), 26 deletions(-) > > diff --git a/fb/fb.h b/fb/fb.h > index ed21f9e..02d6c03 100644 > --- a/fb/fb.h > +++ b/fb/fb.h > @@ -2082,8 +2082,11 @@ fbFillRegionSolid (DrawablePtr pDrawable, > FbBits xor); > > extern _X_EXPORT pixman_image_t * > -image_from_pict (PicturePtr pict, > - Bool has_clip); > +image_from_pict (PicturePtr pict, > + Bool has_clip, > + int *xoff, > + int *yoff); > + > extern _X_EXPORT void free_pixman_pict (PicturePtr, pixman_image_t *); > > #endif /* _FB_H_ */ > diff --git a/fb/fbpict.c b/fb/fbpict.c > index 07a2286..251754b 100644 > --- a/fb/fbpict.c > +++ b/fb/fbpict.c > @@ -158,19 +158,24 @@ fbComposite (CARD8 op, > CARD16 height) > { > pixman_image_t *src, *mask, *dest; > + int src_xoff, src_yoff; > + int msk_xoff, msk_yoff; > + int dst_xoff, dst_yoff; > > miCompositeSourceValidate (pSrc, xSrc - xDst, ySrc - yDst, width, height); > if (pMask) > miCompositeSourceValidate (pMask, xMask - xDst, yMask - yDst, width, > height); > > - src = image_from_pict (pSrc, TRUE); > - mask = image_from_pict (pMask, TRUE); > - dest = image_from_pict (pDst, TRUE); > + src = image_from_pict (pSrc, FALSE, &src_xoff, &src_yoff); > + mask = image_from_pict (pMask, FALSE, &msk_xoff, &msk_yoff); > + dest = image_from_pict (pDst, TRUE, &dst_xoff, &dst_yoff); > > if (src && dest && !(pMask && !mask)) > { > pixman_image_composite (op, src, mask, dest, > - xSrc, ySrc, xMask, yMask, xDst, yDst, > + xSrc + src_xoff, ySrc + src_yoff, > + xMask + msk_xoff, yMask + msk_yoff, > + xDst + dst_xoff, yDst + dst_yoff, > width, height); > } > > @@ -270,22 +275,22 @@ create_conical_gradient_image (PictGradient *gradient) > > static pixman_image_t * > create_bits_picture (PicturePtr pict, > - Bool has_clip) > + Bool has_clip, > + int *xoff, > + int *yoff) > { > + PixmapPtr pixmap; > FbBits *bits; > FbStride stride; > - int bpp, xoff, yoff; > + int bpp; > pixman_image_t *image; > > - fbGetDrawable (pict->pDrawable, bits, stride, bpp, xoff, yoff); > - > - bits = (FbBits*)((CARD8*)bits + > - (pict->pDrawable->y + yoff) * stride * sizeof(FbBits) + > - (pict->pDrawable->x + xoff) * (bpp / 8)); > + fbGetDrawablePixmap (pict->pDrawable, pixmap, *xoff, *yoff); > + fbGetPixmapBitsData(pixmap, bits, stride, bpp); > > image = pixman_image_create_bits ( > pict->format, > - pict->pDrawable->width, pict->pDrawable->height, > + pixmap->drawable.width, pixmap->drawable.height, > (uint32_t *)bits, stride * sizeof (FbStride)); > > > @@ -311,30 +316,52 @@ create_bits_picture (PicturePtr pict, > if (pict->clientClipType != CT_NONE) > pixman_image_set_has_client_clip (image, TRUE); > > - pixman_region_translate (pict->pCompositeClip, - pict->pDrawable->x, - > pict->pDrawable->y); > + if (*xoff || *yoff) > + pixman_region_translate (pict->pCompositeClip, *xoff, *yoff); > > pixman_image_set_clip_region (image, pict->pCompositeClip); > > - pixman_region_translate (pict->pCompositeClip, pict->pDrawable->x, > pict->pDrawable->y); > + if (*xoff || *yoff) > + pixman_region_translate (pict->pCompositeClip, -*xoff, -*yoff); > } > > /* Indexed table */ > if (pict->pFormat->index.devPrivate) > pixman_image_set_indexed (image, pict->pFormat->index.devPrivate); > > + /* Add in drawable origin to position within the image */ > + *xoff += pict->pDrawable->x; > + *yoff += pict->pDrawable->y; > + > return image; > } > > static void > -set_image_properties (pixman_image_t *image, PicturePtr pict) > +set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, > int *xoff, int *yoff) > { > pixman_repeat_t repeat; > pixman_filter_t filter; > > if (pict->transform) > { > - pixman_image_set_transform ( > - image, (pixman_transform_t *)pict->transform); > + /* For source images, adjust the transform to account > + * for the drawable offset within the pixman image, > + * then set the offset to 0 as it will be used > + * to compute positions within the transformed image. > + */ > + if (!has_clip) { > + struct pixman_transform adjusted; > + > + adjusted = *pict->transform; > + pixman_transform_translate(&adjusted, > + NULL, > + pixman_int_to_fixed(*xoff), > + pixman_int_to_fixed(*yoff)); > + pixman_image_set_transform (image, &adjusted); > + *xoff = 0; > + *yoff = 0; > + } else > + pixman_image_set_transform (image, pict->transform); > } > > switch (pict->repeatType) > @@ -361,7 +388,8 @@ set_image_properties (pixman_image_t *image, PicturePtr > pict) > > if (pict->alphaMap) > { > - pixman_image_t *alpha_map = image_from_pict (pict->alphaMap, TRUE); > + int alpha_xoff, alpha_yoff; > + pixman_image_t *alpha_map = image_from_pict (pict->alphaMap, FALSE, > &alpha_xoff, &alpha_yoff); > > pixman_image_set_alpha_map ( > image, alpha_map, pict->alphaOrigin.x, pict->alphaOrigin.y); > @@ -394,8 +422,7 @@ set_image_properties (pixman_image_t *image, PicturePtr > pict) > } > > pixman_image_t * > -image_from_pict (PicturePtr pict, > - Bool has_clip) > +image_from_pict (PicturePtr pict, Bool has_clip, int *xoff, int *yoff) > { > pixman_image_t *image = NULL; > > @@ -404,7 +431,7 @@ image_from_pict (PicturePtr pict, > > if (pict->pDrawable) > { > - image = create_bits_picture (pict, has_clip); > + image = create_bits_picture (pict, has_clip, xoff, yoff); > } > else if (pict->pSourcePict) > { > @@ -428,7 +455,7 @@ image_from_pict (PicturePtr pict, > } > > if (image) > - set_image_properties (image, pict); > + set_image_properties (image, pict, has_clip, xoff, yoff); > > return image; > } > diff --git a/fb/fbtrap.c b/fb/fbtrap.c > index 830603a..515e2e1 100644 > --- a/fb/fbtrap.c > +++ b/fb/fbtrap.c > @@ -40,7 +40,8 @@ fbAddTraps (PicturePtr pPicture, > int ntrap, > xTrap *traps) > { > - pixman_image_t *image = image_from_pict (pPicture, FALSE); > + int image_xoff, image_yoff; > + pixman_image_t *image = image_from_pict (pPicture, FALSE, &image_xoff, > &image_yoff); > > if (!image) > return; > @@ -56,7 +57,8 @@ fbRasterizeTrapezoid (PicturePtr pPicture, > int x_off, > int y_off) > { > - pixman_image_t *image = image_from_pict (pPicture, FALSE); > + int mask_xoff, mask_yoff; > + pixman_image_t *image = image_from_pict (pPicture, FALSE, &mask_xoff, > &mask_yoff); > > if (!image) > return; > -- > 1.6.5.3 > > _______________________________________________ > xorg-devel mailing list > [email protected] > http://lists.x.org/mailman/listinfo/xorg-devel
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
