[PATCH v5 xserver 3/7] xf86Cursor: Add xf86CheckHWCursor() helper function

[Hans de Goede]

HI,

On 06-09-16 14:35, Daniel Martin wrote:
> Sorry for jumping in that late ...
>
> On 6 September 2016 at 13:31, Hans de Goede <hdegoede at redhat.com> wrote:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> This is a preparation patch for adding prime hw-cursor support.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  hw/xfree86/ramdac/xf86Cursor.c     | 11 ++---------
>>  hw/xfree86/ramdac/xf86CursorPriv.h |  1 +
>>  hw/xfree86/ramdac/xf86HWCurs.c     | 12 ++++++++++++
>>  3 files changed, 15 insertions(+), 9 deletions(-)
>>
> ...
>> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
>> index 458781c..0f6990a 100644
>> --- a/hw/xfree86/ramdac/xf86HWCurs.c
>> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
>> @@ -130,6 +130,18 @@ xf86ShowCursor(xf86CursorInfoPtr infoPtr)
>>  }
>>
>>  Bool
>> +xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr)
>> +{
>> +    return
>> +        (cursor->bits->argb && infoPtr->UseHWCursorARGB &&
>> +         infoPtr->UseHWCursorARGB(pScreen, cursor)) ||
>> +        (cursor->bits->argb == 0 &&
>> +         cursor->bits->height <= infoPtr->MaxHeight &&
>> +         cursor->bits->width <= infoPtr->MaxWidth &&
>> +         (!infoPtr->UseHWCursor || infoPtr->UseHWCursor(pScreen, cursor)));
>> +}
>
> You just moved the code, but do you mind splitting the if-clause to
> make it more pleasant for the eyes? I.e.
>
>     if (cursor->bits->argb) {
>         if (infoPtr->UseHWCursorARGB)
>             return infoPtr->UseHWCursorARGB(pScreen, cursor);
>     } else
>     if (cursor->bits->width <= infoPtr->MaxWidth &&
>         cursor->bits->height <= infoPtr->MaxHeight) {
>         if (infoPtr->UseHWCursor)
>             return infoPtr->UseHWCursor(pScreen, cursor)
>         else
>             return TRUE;
>     }
>
> As my comment came late and is beautifying only, feel free to ignore it.

I'm not sure I find that more readable. Point of proof:

You're missing a return FALSE when cursor->bits->argb != NULL and
infoPtr->UseHWCursorARGB == NULL

Regards,

Hans