[PATCH 2/2] fb: Adjust transform or composite coordinates for pixman operations
Jeremy Huddleston
jeremyhu at freedesktop.org
Fri Jan 8 12:31:35 PST 2010
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 <keithp at keithp.com>
> ---
> 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
> xorg-devel at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5820 bytes
Desc: not available
Url : http://lists.x.org/archives/xorg-devel/attachments/20100108/7e9954fd/attachment.bin
More information about the xorg-devel
mailing list