[v2] st/nine: skip position checks in SetCursorPosition()

Submitted by Andre Heider on April 8, 2019, 7:10 a.m.

Details

Message ID 20190408071009.18102-1-a.heider@gmail.com
State New
Headers show
Series "st/nine: skip position checks in SetCursorPosition()" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Andre Heider April 8, 2019, 7:10 a.m.
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@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(-)

Patch hide | download patch | download mbox

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
+ */
 
 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;

Comments

Fine by me, but please improve the drm.h description, see below.
With that changed, this is:
Reviewed-by: Axel Davy <davyaxel0@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@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;