[Mesa-dev] [PATCH v2] st/nine: skip position checks in SetCursorPosition()

[Axel Davy]

Fine by me, but please improve the drm.h description, see below.
With that changed, this is:
Reviewed-by: Axel Davy <davyaxel0 at gmail.com>

Axel

On 08/04/2019 09:10, Andre Heider wrote:
> For HW cursors, "cursor.pos" doesn't hold the current position of the
> pointer, just the position of the last call to SetCursorPosition().
>
> Skip the check against stale values and bump the d3dadapter9 drm version
> to expose this change of behaviour.
>
> Signed-off-by: Andre Heider <a.heider at gmail.com>
> ---
> V2: don't introduce SetVersion(), bump D3DADAPTER9DRM_MINOR instead
>
> Corresponding d3d9-nine.dll patch:
> https://github.com/iXit/wine-nine-standalone/commit/e09fcbbad4efd481833df1123de0cb690e1b2860
>
>   include/d3dadapter/drm.h                  | 7 +++++--
>   src/gallium/state_trackers/nine/device9.c | 8 +++++---
>   2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/include/d3dadapter/drm.h b/include/d3dadapter/drm.h
> index 647f017fc7f..210e2395669 100644
> --- a/include/d3dadapter/drm.h
> +++ b/include/d3dadapter/drm.h
> @@ -29,11 +29,14 @@
>   #define D3DADAPTER9DRM_NAME "drm"
>   /* current version */
>   #define D3DADAPTER9DRM_MAJOR 0
> -#define D3DADAPTER9DRM_MINOR 1
> +#define D3DADAPTER9DRM_MINOR 2
>   
>   /* version 0.0: Initial release
>    *         0.1: All IDirect3D objects can be assumed to have a pointer to the
> - *              internal vtable in second position of the structure */
> + *              internal vtable in second position of the structure
> + *         0.2: IDirect3DDevice9_SetCursorPosition doesn't check the cursor
> + *              position anymore
> + */

This is ambiguous (we could be checking bounds, or whatever). I think 
being more explicit should be preferred.

For example: doesn't filter out redundant cursor position settings anymore.

or: all IDirect3DDevice9_SetCursorPosition call with hardware cursor 
results in a call to ID3DPresent_SetCursorPos.


>   
>   struct D3DAdapter9DRM
>   {
> diff --git a/src/gallium/state_trackers/nine/device9.c b/src/gallium/state_trackers/nine/device9.c
> index db1c3a1d23d..0b1fe59cb70 100644
> --- a/src/gallium/state_trackers/nine/device9.c
> +++ b/src/gallium/state_trackers/nine/device9.c
> @@ -793,9 +793,11 @@ NineDevice9_SetCursorPosition( struct NineDevice9 *This,
>   
>       DBG("This=%p X=%d Y=%d Flags=%d\n", This, X, Y, Flags);
>   
> -    if (This->cursor.pos.x == X &&
> -        This->cursor.pos.y == Y)
> -        return;
> +    /* present >= v1.4 handles this itself */
> +    if (This->minor_version_num < 4) {
> +        if (This->cursor.pos.x == X && This->cursor.pos.y == Y)
> +            return;
> +    }
>   
>       This->cursor.pos.x = X;
>       This->cursor.pos.y = Y;