Re: Staging dinput-scancode fix

Rémi Bernon <[email protected]>
Newsgroups gmane.comp.emulators.wine.devel
Message-ID <[email protected]> 
On 8/22/24 23:17, Elizabeth Figura wrote:
> On Thursday, 22 August 2024 14:23:13 CDT DodoGTA GT wrote:
>> Hello
>>
>> Here's a better regression fix for the dinput-scancode
>> patchset (the 0001 patch of that patchset caused NFS
>> Underground to segfault since Wine 9.15 with the addition
>> of a keycode loop in keyboard_create_device() because of
>> DIPROP_SCANCODE stuff); I modified the existing 0001
>> patch to solve this by copying the duplicate object checks
>> to that new loop (maybe I should create a separate 0003
>> patch to show the changes better?)
> 
> Rémi, can you comment on this? Since you're the author of the original
> patch, I'd appreciate your review before adding this one into
> Wine-Staging.
> 
> And, if correct, does patch 0002 need a similar fix?
> 
> --Zeb
> 

Yes, the Aidas patch looks correct, and yes probably 0002 will need the 
same kind of thing. Both loops should be pretty much identical. I'm 
attaching a change that factors them together first, with the other 
patches applied on top.

-- 
Rémi Bernon <[email protected]>
0003-dinput-Enumerate-lower-keyboard-scancodes-values-fir.patch (text/x-patch, 11.2 KB) 
From eafa0a1d0881fa0b77310b2a0428bdd5a02cd44f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Bernon?= <[email protected]>
Date: Fri, 23 Aug 2024 08:49:13 +0200
Subject: [PATCH 1/3] dinput: Factor keyboard initialization and enumeration
 loops.

---
 dlls/dinput/keyboard.c | 129 +++++++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 57 deletions(-)

diff --git a/dlls/dinput/keyboard.c b/dlls/dinput/keyboard.c
index 8ec9dd47150..d1fa242574e 100644
--- a/dlls/dinput/keyboard.c
+++ b/dlls/dinput/keyboard.c
@@ -185,12 +185,79 @@ HRESULT keyboard_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *instan
     return DI_OK;
 }
 
+static BOOL enum_object( struct keyboard *impl, const DIPROPHEADER *filter, DWORD flags, enum_object_callback callback,
+                         UINT index, DIDEVICEOBJECTINSTANCEW *instance, void *data )
+{
+    if (flags != DIDFT_ALL && !(flags & DIDFT_GETTYPE( instance->dwType ))) return DIENUM_CONTINUE;
+
+    switch (filter->dwHow)
+    {
+    case DIPH_DEVICE:
+        return callback( &impl->base, index, NULL, instance, data );
+    case DIPH_BYOFFSET:
+        if (filter->dwObj != instance->dwOfs) return DIENUM_CONTINUE;
+        return callback( &impl->base, index, NULL, instance, data );
+    case DIPH_BYID:
+        if ((filter->dwObj & 0x00ffffff) != (instance->dwType & 0x00ffffff)) return DIENUM_CONTINUE;
+        return callback( &impl->base, index, NULL, instance, data );
+    }
+
+    return DIENUM_CONTINUE;
+}
+
+static HRESULT enum_objects( struct keyboard *impl, const DIPROPHEADER *filter,
+                             DWORD flags, enum_object_callback callback, void *data )
+{
+    BYTE subtype = GET_DIDEVICE_SUBTYPE( impl->base.instance.dwDevType );
+    DIDEVICEOBJECTINSTANCEW instance =
+    {
+        .dwSize = sizeof(DIDEVICEOBJECTINSTANCEW),
+        .guidType = GUID_Key,
+        .dwOfs = DIK_ESCAPE,
+        .dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( DIK_ESCAPE ),
+    };
+    DWORD index, i, dik;
+    BOOL ret;
+
+    for (i = 0, index = 0; i < 512; ++i)
+    {
+        if (!GetKeyNameTextW( i << 16, instance.tszName, ARRAY_SIZE(instance.tszName) )) continue;
+        if (!(dik = map_dik_code( i, 0, subtype, impl->base.dinput->dwVersion ))) continue;
+        instance.dwOfs = dik;
+        instance.dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( dik );
+        ret = enum_object( impl, filter, flags, callback, index++, &instance, data );
+        if (ret != DIENUM_CONTINUE) return DIENUM_STOP;
+    }
+
+    return DIENUM_CONTINUE;
+}
+
+static BOOL init_object_properties( struct dinput_device *device, UINT index, struct hid_value_caps *caps,
+                                    const DIDEVICEOBJECTINSTANCEW *instance, void *data )
+{
+    struct object_properties *properties;
+    UINT dik = instance->dwOfs;
+
+    if (index == -1) return DIENUM_STOP;
+    properties = device->object_properties + index;
+
+    if (dik == DIK_NUMLOCK) properties->scan_code = 0x451de1;
+    else if (dik == DIK_PAUSE) properties->scan_code = 0x45;
+    else if (dik < 0x80) properties->scan_code = dik;
+    else properties->scan_code = (dik - 0x80) << 8 | 0x00e0;
+
+    return DIENUM_CONTINUE;
+}
+
 HRESULT keyboard_create_device( struct dinput *dinput, const GUID *guid, IDirectInputDevice8W **out )
 {
-    DIDEVICEOBJECTINSTANCEW instance;
+    static const DIPROPHEADER filter =
+    {
+        .dwSize = sizeof(filter),
+        .dwHeaderSize = sizeof(filter),
+        .dwHow = DIPH_DEVICE,
+    };
     struct keyboard *impl;
-    DWORD i, index, dik;
-    BYTE subtype;
     HRESULT hr;
 
     TRACE( "dinput %p, guid %s, out %p.\n", dinput, debugstr_guid( guid ), out );
@@ -207,20 +274,9 @@ HRESULT keyboard_create_device( struct dinput *dinput, const GUID *guid, IDirect
     impl->base.caps.dwFirmwareRevision = 100;
     impl->base.caps.dwHardwareRevision = 100;
     if (dinput->dwVersion >= 0x0800) impl->base.use_raw_input = TRUE;
-    subtype = GET_DIDEVICE_SUBTYPE( impl->base.instance.dwDevType );
 
     if (FAILED(hr = dinput_device_init_device_format( &impl->base.IDirectInputDevice8W_iface ))) goto failed;
-
-    for (i = 0, index = 0; i < 512; ++i)
-    {
-        if (!GetKeyNameTextW( i << 16, instance.tszName, ARRAY_SIZE(instance.tszName) )) continue;
-        if (!(dik = map_dik_code( i, 0, subtype, impl->base.dinput->dwVersion ))) continue;
-
-        if (dik == DIK_NUMLOCK) impl->base.object_properties[index++].scan_code = 0x451de1;
-        else if (dik == DIK_PAUSE) impl->base.object_properties[index++].scan_code = 0x45;
-        else if (dik < 0x80) impl->base.object_properties[index++].scan_code = dik;
-        else impl->base.object_properties[index++].scan_code = (dik - 0x80) << 8 | 0x00e0;
-    }
+    enum_objects( impl, &filter, DIDFT_BUTTON, init_object_properties, NULL );
 
     *out = &impl->base.IDirectInputDevice8W_iface;
     return DI_OK;
@@ -248,52 +304,11 @@ static HRESULT keyboard_unacquire( IDirectInputDevice8W *iface )
     return DI_OK;
 }
 
-static BOOL try_enum_object( struct dinput_device *impl, const DIPROPHEADER *filter, DWORD flags, enum_object_callback callback,
-                             UINT index, DIDEVICEOBJECTINSTANCEW *instance, void *data )
-{
-    if (flags != DIDFT_ALL && !(flags & DIDFT_GETTYPE( instance->dwType ))) return DIENUM_CONTINUE;
-
-    switch (filter->dwHow)
-    {
-    case DIPH_DEVICE:
-        return callback( impl, index, NULL, instance, data );
-    case DIPH_BYOFFSET:
-        if (filter->dwObj != instance->dwOfs) return DIENUM_CONTINUE;
-        return callback( impl, index, NULL, instance, data );
-    case DIPH_BYID:
-        if ((filter->dwObj & 0x00ffffff) != (instance->dwType & 0x00ffffff)) return DIENUM_CONTINUE;
-        return callback( impl, index, NULL, instance, data );
-    }
-
-    return DIENUM_CONTINUE;
-}
-
 static HRESULT keyboard_enum_objects( IDirectInputDevice8W *iface, const DIPROPHEADER *filter,
                                       DWORD flags, enum_object_callback callback, void *context )
 {
     struct keyboard *impl = impl_from_IDirectInputDevice8W( iface );
-    BYTE subtype = GET_DIDEVICE_SUBTYPE( impl->base.instance.dwDevType );
-    DIDEVICEOBJECTINSTANCEW instance =
-    {
-        .dwSize = sizeof(DIDEVICEOBJECTINSTANCEW),
-        .guidType = GUID_Key,
-        .dwOfs = DIK_ESCAPE,
-        .dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( DIK_ESCAPE ),
-    };
-    DWORD index, i, dik;
-    BOOL ret;
-
-    for (i = 0, index = 0; i < 512; ++i)
-    {
-        if (!GetKeyNameTextW( i << 16, instance.tszName, ARRAY_SIZE(instance.tszName) )) continue;
-        if (!(dik = map_dik_code( i, 0, subtype, impl->base.dinput->dwVersion ))) continue;
-        instance.dwOfs = dik;
-        instance.dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( dik );
-        ret = try_enum_object( &impl->base, filter, flags, callback, index++, &instance, context );
-        if (ret != DIENUM_CONTINUE) return DIENUM_STOP;
-    }
-
-    return DIENUM_CONTINUE;
+    return enum_objects( impl, filter, flags, callback, context );
 }
 
 static HRESULT keyboard_get_property( IDirectInputDevice8W *iface, DWORD property,
-- 
2.45.2

From 82a7acbe05537f7766b78f17b0767e7aea2d6884 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Bernon?= <[email protected]>
Date: Fri, 23 Aug 2024 08:51:44 +0200
Subject: [PATCH 2/3] dinput: Avoid duplicated objects in keyboard devices.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55205
---
 dlls/dinput/keyboard.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dlls/dinput/keyboard.c b/dlls/dinput/keyboard.c
index d1fa242574e..d9c73cd7367 100644
--- a/dlls/dinput/keyboard.c
+++ b/dlls/dinput/keyboard.c
@@ -216,13 +216,15 @@ static HRESULT enum_objects( struct keyboard *impl, const DIPROPHEADER *filter,
         .dwOfs = DIK_ESCAPE,
         .dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( DIK_ESCAPE ),
     };
+    BOOL ret, mapped[0x100] = {0};
     DWORD index, i, dik;
-    BOOL ret;
 
     for (i = 0, index = 0; i < 512; ++i)
     {
         if (!GetKeyNameTextW( i << 16, instance.tszName, ARRAY_SIZE(instance.tszName) )) continue;
         if (!(dik = map_dik_code( i, 0, subtype, impl->base.dinput->dwVersion ))) continue;
+        if (mapped[dik]) continue;
+        mapped[dik] = TRUE;
         instance.dwOfs = dik;
         instance.dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( dik );
         ret = enum_object( impl, filter, flags, callback, index++, &instance, data );
-- 
2.45.2

From 4cc789281ce997379fe6617f075c2ee624a3b070 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20Bernon?= <[email protected]>
Date: Fri, 23 Aug 2024 08:52:34 +0200
Subject: [PATCH 3/3] dinput: Enumerate lower keyboard scancodes values first.

Windows usually doesn't have scancodes higher than 0x7f, or extended
scancodes higher than 0x17f, but X11 does for several XF86 keys.

We want to enumerate the basic keys first including in the extended
scancode range, so they appear before the XF86 keys in the dinput
device object list.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55205
---
 dlls/dinput/keyboard.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/dlls/dinput/keyboard.c b/dlls/dinput/keyboard.c
index d9c73cd7367..407634967de 100644
--- a/dlls/dinput/keyboard.c
+++ b/dlls/dinput/keyboard.c
@@ -208,6 +208,7 @@ static BOOL enum_object( struct keyboard *impl, const DIPROPHEADER *filter, DWOR
 static HRESULT enum_objects( struct keyboard *impl, const DIPROPHEADER *filter,
                              DWORD flags, enum_object_callback callback, void *data )
 {
+    static const UINT vsc_base[] = {0, 0x100, 0x80, 0x180};
     BYTE subtype = GET_DIDEVICE_SUBTYPE( impl->base.instance.dwDevType );
     DIDEVICEOBJECTINSTANCEW instance =
     {
@@ -217,18 +218,21 @@ static HRESULT enum_objects( struct keyboard *impl, const DIPROPHEADER *filter,
         .dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( DIK_ESCAPE ),
     };
     BOOL ret, mapped[0x100] = {0};
-    DWORD index, i, dik;
+    DWORD index, i, dik, vsc;
 
-    for (i = 0, index = 0; i < 512; ++i)
+    for (i = 0, index = 0; i < ARRAY_SIZE(vsc_base); ++i)
     {
-        if (!GetKeyNameTextW( i << 16, instance.tszName, ARRAY_SIZE(instance.tszName) )) continue;
-        if (!(dik = map_dik_code( i, 0, subtype, impl->base.dinput->dwVersion ))) continue;
-        if (mapped[dik]) continue;
-        mapped[dik] = TRUE;
-        instance.dwOfs = dik;
-        instance.dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( dik );
-        ret = enum_object( impl, filter, flags, callback, index++, &instance, data );
-        if (ret != DIENUM_CONTINUE) return DIENUM_STOP;
+        for (vsc = vsc_base[i]; vsc < vsc_base[i] + 0x80; vsc++)
+        {
+            if (!GetKeyNameTextW( vsc << 16, instance.tszName, ARRAY_SIZE(instance.tszName) )) continue;
+            if (!(dik = map_dik_code( vsc, 0, subtype, impl->base.dinput->dwVersion ))) continue;
+            if (mapped[dik]) continue;
+            mapped[dik] = TRUE;
+            instance.dwOfs = dik;
+            instance.dwType = DIDFT_PSHBUTTON | DIDFT_MAKEINSTANCE( dik );
+            ret = enum_object( impl, filter, flags, callback, index++, &instance, data );
+            if (ret != DIENUM_CONTINUE) return DIENUM_STOP;
+        }
     }
 
     return DIENUM_CONTINUE;
-- 
2.45.2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.