[PATCH] xfree86: Fix rotation of 2-color non-interleaved cursor images

Alex Deucher alexdeucher at gmail.com
Tue Nov 30 07:04:39 PST 2010


On Mon, Nov 29, 2010 at 9:03 PM, Aaron Plattner <aplattner at nvidia.com> wrote:
> Does anyone else want to review this, or are Robert's (@nvidia.com)
> Reviewed-by and Cyril's Tested-by sufficient?

Looks good to me.

Reviewed-by: Alex Deucher <alexdeucher at gmail.com>


>
> 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
>>
> _______________________________________________
> 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