[PATCH] xfree86: Fix rotation of 2-color non-interleaved cursor images
Aaron Plattner
aplattner at nvidia.com
Mon Nov 29 18:03:53 PST 2010
Does anyone else want to review this, or are Robert's (@nvidia.com)
Reviewed-by and Cyril's Tested-by sufficient?
On Tue, Nov 16, 2010 at 12:17:33PM -0800, Aaron Plattner wrote:
> When RandR 1.2's transformation code is enabled, it rotates the cursor
> image so that it appears upright on a rotated screen. This code
> completely mangles 2-color cursors on hardware where the the mask and
> source images are not interleaved due to two problems:
>
> 1. stride is calculated as (width / 4) rather than (width / 8), so the
> expression (y * stride) skips two lines instead of one for every
> time y is incremented.
> 2. cursor_bitpos ignores the 'mask' parameter if the hardware doesn't
> specify any of the HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_* flags.
>
> To fix this, refactor the code to pass the whole xf86CursorInfoPtr
> through to cursor_bitpos and compute the correct stride there based on
> the flags. If none of the SOURCE_MASK_INTERLEAVE flags are set, use
> the total cursor size to move the 'image' variable into the mask part
> of the image before computing the desired byte pointer.
>
> Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
> Reviewed-by: Robert Morell <rmorell at nvidia.com>
> ---
> Could somebody with hardware that sets SOURCE_MASK_INTERLEAVE (e.g.
> Intel) please give this patch a try and verify that it doesn't break
> rotated cursors?
>
> hw/xfree86/modes/xf86Cursors.c | 62 +++++++++++++++++++++++++++------------
> 1 files changed, 43 insertions(+), 19 deletions(-)
>
> diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
> index ab07b60..0667447 100644
> --- a/hw/xfree86/modes/xf86Cursors.c
> +++ b/hw/xfree86/modes/xf86Cursors.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright © 2007 Keith Packard
> + * Copyright © 2010 Aaron Plattner
> *
> * Permission to use, copy, modify, distribute, and sell this software and its
> * documentation for any purpose is hereby granted without fee, provided that
> @@ -126,12 +127,33 @@ xf86_crtc_rotate_coord_back (Rotation rotation,
> *y_src = y_dst;
> }
>
> +struct cursor_bit {
> + CARD8 *byte;
> + char bitpos;
> +};
> +
> /*
> * Convert an x coordinate to a position within the cursor bitmap
> */
> -static int
> -cursor_bitpos (int flags, int x, Bool mask)
> +static struct cursor_bit
> +cursor_bitpos (CARD8 *image, xf86CursorInfoPtr cursor_info, int x, int y,
> + Bool mask)
> {
> + const int flags = cursor_info->Flags;
> + const Bool interleaved =
> + !!(flags & (HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_1 |
> + HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_8 |
> + HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_16 |
> + HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_32 |
> + HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_64));
> + const int width = cursor_info->MaxWidth;
> + const int height = cursor_info->MaxHeight;
> + const int stride = interleaved ? width / 4 : width / 8;
> +
> + struct cursor_bit ret;
> +
> + image += y * stride;
> +
> if (flags & HARDWARE_CURSOR_SWAP_SOURCE_AND_MASK)
> mask = !mask;
> if (flags & HARDWARE_CURSOR_NIBBLE_SWAPPED)
> @@ -149,29 +171,33 @@ cursor_bitpos (int flags, int x, Bool mask)
> x = ((x & ~31) << 1) | (mask << 5) | (x & 31);
> else if (flags & HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_64)
> x = ((x & ~63) << 1) | (mask << 6) | (x & 63);
> - return x;
> + else if (mask)
> + image += stride * height;
> +
> + ret.byte = image + (x / 8);
> + ret.bitpos = x & 7;
> +
> + return ret;
> }
>
> /*
> * Fetch one bit from a cursor bitmap
> */
> static CARD8
> -get_bit (CARD8 *image, int stride, int flags, int x, int y, Bool mask)
> +get_bit (CARD8 *image, xf86CursorInfoPtr cursor_info, int x, int y, Bool mask)
> {
> - x = cursor_bitpos (flags, x, mask);
> - image += y * stride;
> - return (image[(x >> 3)] >> (x & 7)) & 1;
> + struct cursor_bit bit = cursor_bitpos(image, cursor_info, x, y, mask);
> + return (*bit.byte >> bit.bitpos) & 1;
> }
>
> /*
> * Set one bit in a cursor bitmap
> */
> static void
> -set_bit (CARD8 *image, int stride, int flags, int x, int y, Bool mask)
> +set_bit (CARD8 *image, xf86CursorInfoPtr cursor_info, int x, int y, Bool mask)
> {
> - x = cursor_bitpos (flags, x, mask);
> - image += y * stride;
> - image[(x >> 3)] |= 1 << (x & 7);
> + struct cursor_bit bit = cursor_bitpos(image, cursor_info, x, y, mask);
> + *bit.byte |= 1 << bit.bitpos;
> }
>
> /*
> @@ -186,7 +212,6 @@ xf86_crtc_convert_cursor_to_argb (xf86CrtcPtr crtc, unsigned char *src)
> CARD32 *cursor_image = (CARD32 *) xf86_config->cursor_image;
> int x, y;
> int xin, yin;
> - int stride = cursor_info->MaxWidth >> 2;
> int flags = cursor_info->Flags;
> CARD32 bits;
>
> @@ -201,10 +226,10 @@ xf86_crtc_convert_cursor_to_argb (xf86CrtcPtr crtc, unsigned char *src)
> cursor_info->MaxWidth,
> cursor_info->MaxHeight,
> x, y, &xin, &yin);
> - if (get_bit (src, stride, flags, xin, yin, TRUE) ==
> + if (get_bit (src, cursor_info, xin, yin, TRUE) ==
> ((flags & HARDWARE_CURSOR_INVERT_MASK) == 0))
> {
> - if (get_bit (src, stride, flags, xin, yin, FALSE))
> + if (get_bit (src, cursor_info, xin, yin, FALSE))
> bits = xf86_config->cursor_fg;
> else
> bits = xf86_config->cursor_bg;
> @@ -407,7 +432,6 @@ xf86_crtc_load_cursor_image (xf86CrtcPtr crtc, CARD8 *src)
> int x, y;
> int xin, yin;
> int stride = cursor_info->MaxWidth >> 2;
> - int flags = cursor_info->Flags;
>
> cursor_image = xf86_config->cursor_image;
> memset(cursor_image, 0, cursor_info->MaxHeight * stride);
> @@ -419,10 +443,10 @@ xf86_crtc_load_cursor_image (xf86CrtcPtr crtc, CARD8 *src)
> cursor_info->MaxWidth,
> cursor_info->MaxHeight,
> x, y, &xin, &yin);
> - if (get_bit(src, stride, flags, xin, yin, FALSE))
> - set_bit(cursor_image, stride, flags, x, y, FALSE);
> - if (get_bit(src, stride, flags, xin, yin, TRUE))
> - set_bit(cursor_image, stride, flags, x, y, TRUE);
> + if (get_bit(src, cursor_info, xin, yin, FALSE))
> + set_bit(cursor_image, cursor_info, x, y, FALSE);
> + if (get_bit(src, cursor_info, xin, yin, TRUE))
> + set_bit(cursor_image, cursor_info, x, y, TRUE);
> }
> }
> crtc->funcs->load_cursor_image (crtc, cursor_image);
> --
> 1.7.1
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
More information about the xorg-devel
mailing list