[PATCH v2 0/3] MR2573: dinput: Set guidType = GUID_Unknown for HID collections.
I'm not sure if I'm using the dinput HID test infrastructure correctly. Also, an existing test could be extended instead of adding a whole new test function, saving a couple of source code lines and test execution time, but I wasn't sure which one to extend - feel free to suggest one and I will update the test. Also, maybe the suggested fix is not a great idea. Maybe there are actually some collection objects that are supposed to have a guidType that is not GUID_Unknown? I couldn't find any in the limited number of Windows environments I can access, so I wanted to put it out there for discussion. The issue I'm trying to solve is that Zusi 3 does not appear to check the DIDFT_NODATA bit in dwType, so it just adds most of the objects listed by EnumObjects() to the SetDataFormat() call, maybe filtering out only those that have guidType = GUID_Unknown, or maybe there's an allow-list of GUIDs to add, and that list includes GUID_ZAxis. I haven't seen complaints about joysticks not working in Zusi 3 on Windows, so I guess collections (and DIDFT_NODATA objects in general) rarely/never have a non-Unknown GUID, or maybe Windows simply accepts these collections in SetDataFormat() and that only fails in Wine. -- v2: dinput/tests: Remove unmatched winetest_pop_context(). dinput: Set guidType = GUID_Unknown for HID collections. dinput/tests: Add guidType test for collection objects. https://gitlab.winehq.org/wine/wine/-/merge_requests/2573
From: Florian Will <florian.will(a)gmail.com> --- dlls/dinput/tests/joystick8.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index 3ab6f3c26d4..a5c0b3958a4 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -1450,6 +1450,11 @@ static void test_simple_joystick( DWORD version ) REPORT_COUNT(1, 4), INPUT(1, Data|Var|Abs), END_COLLECTION, + + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), + USAGE(1, HID_USAGE_GENERIC_RZ), + COLLECTION(1, Physical), + END_COLLECTION, END_COLLECTION, }; C_ASSERT(sizeof(report_desc) < MAX_HID_DESCRIPTOR_LEN); @@ -1679,6 +1684,14 @@ static void test_simple_joystick( DWORD version ) .wUsagePage = HID_USAGE_PAGE_GENERIC, .wUsage = HID_USAGE_GENERIC_JOYSTICK, }, + { + .dwSize = sizeof(DIDEVICEOBJECTINSTANCEW), + .guidType = GUID_Unknown, + .dwType = DIDFT_COLLECTION|DIDFT_NODATA|DIDFT_MAKEINSTANCE(2), + .tszName = L"Collection 2 - Z Rotation", + .wUsagePage = HID_USAGE_PAGE_GENERIC, + .wUsage = HID_USAGE_GENERIC_RZ, + }, }; const DIDEVICEOBJECTINSTANCEW expect_objects_5[] = { @@ -1764,6 +1777,21 @@ static void test_simple_joystick( DWORD version ) .wReportId = 1, }, }; + struct check_object_todo todo_objects[ARRAY_SIZE(expect_objects)] = + { + {0}, + {0}, + {0}, + {0}, + {0}, + {0}, + {0}, + {0}, + {0}, + {0}, + {0}, + { .guid = TRUE }, + }; struct check_object_todo todo_objects_5[ARRAY_SIZE(expect_objects_5)] = { {.guid = TRUE, .type = TRUE, .flags = TRUE, .usage = TRUE, .usage_page = TRUE, .name = TRUE}, @@ -1780,7 +1808,7 @@ static void test_simple_joystick( DWORD version ) .version = version, .expect_count = version < 0x700 ? ARRAY_SIZE(expect_objects_5) : ARRAY_SIZE(expect_objects), .expect_objs = version < 0x700 ? expect_objects_5 : expect_objects, - .todo_objs = version < 0x700 ? todo_objects_5 : NULL, + .todo_objs = version < 0x700 ? todo_objects_5 : todo_objects, .todo_extra = version < 0x700, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2573
From: Florian Will <florian.will(a)gmail.com> Fixes an issue in Zusi 3 where DIDFT_COLLECTION objects would be passed to SetDataFormat. --- dlls/dinput/joystick_hid.c | 2 +- dlls/dinput/tests/joystick8.c | 17 +---------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index 42f58c52202..3320ac92a1c 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -728,7 +728,7 @@ static BOOL enum_objects( struct hid_joystick *impl, const DIPROPHEADER *filter, instance.dwFlags = 0; instance.wUsagePage = node->usage_page; instance.wUsage = node->usage; - instance.guidType = *object_usage_to_guid( instance.wUsagePage, instance.wUsage ); + instance.guidType = GUID_Unknown; instance.wReportId = 0; instance.wCollectionNumber = node->parent; instance.dwDimension = 0; diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index a5c0b3958a4..2eb12bd3ba2 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -1777,21 +1777,6 @@ static void test_simple_joystick( DWORD version ) .wReportId = 1, }, }; - struct check_object_todo todo_objects[ARRAY_SIZE(expect_objects)] = - { - {0}, - {0}, - {0}, - {0}, - {0}, - {0}, - {0}, - {0}, - {0}, - {0}, - {0}, - { .guid = TRUE }, - }; struct check_object_todo todo_objects_5[ARRAY_SIZE(expect_objects_5)] = { {.guid = TRUE, .type = TRUE, .flags = TRUE, .usage = TRUE, .usage_page = TRUE, .name = TRUE}, @@ -1808,7 +1793,7 @@ static void test_simple_joystick( DWORD version ) .version = version, .expect_count = version < 0x700 ? ARRAY_SIZE(expect_objects_5) : ARRAY_SIZE(expect_objects), .expect_objs = version < 0x700 ? expect_objects_5 : expect_objects, - .todo_objs = version < 0x700 ? todo_objects_5 : todo_objects, + .todo_objs = version < 0x700 ? todo_objects_5 : NULL, .todo_extra = version < 0x700, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2573
From: Florian Will <florian.will(a)gmail.com> --- dlls/dinput/tests/joystick8.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index 2eb12bd3ba2..b6a627d6d25 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -4196,7 +4196,6 @@ static void test_many_axes_joystick(void) done: hid_device_stop( &desc, 1 ); cleanup_registry_keys(); - winetest_pop_context(); } static void test_driving_wheel_axes(void) @@ -4416,7 +4415,6 @@ static void test_driving_wheel_axes(void) done: hid_device_stop( &desc, 1 ); cleanup_registry_keys(); - winetest_pop_context(); } static BOOL test_winmm_joystick(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2573
On Tue Apr 4 07:45:13 2023 +0000, Rémi Bernon wrote:
The fix seems to make sense, I don't think any test showed that collections are supposed to have an actual GUID, and it was set with `object_usage_to_guid` mostly because the other objects are. Regarding the test I think you can add an empty collection in `test_simple_joystick` instead, though you might have to adjust the expected descs accordingly. Thank you, Rémi! Pushed v2 that extends `test_simple_joystick` instead of adding a new test function, and removes both unmatched `winetest_pop_context` calls in a third commit.
Splitting test&fix in this MR might be a bit silly as it's a bit more than just one "todo_wine" macro call… anyway. It works, as far as I can tell. :-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2573#note_28815
On Tue Apr 4 07:46:58 2023 +0000, Florian Will wrote:
Thank you, Rémi! Pushed v2 that extends `test_simple_joystick` instead of adding a new test function, and removes both unmatched `winetest_pop_context` calls in a third commit. Edit: I also switched to a "Z Rotation" collection instead of "Z Axis", because the test assumes further down that no "Z Axis" exists. My actual hardware has both, a "Z Axis" and a "Z Rotation" collection. Splitting test&fix in this MR might be a bit silly as it's a bit more than just one "todo_wine" macro call… anyway. It works, as far as I can tell. :-) Thanks! I really appreciate the tests and fixes being split, even if it means adding then removing a bunch of lines, it makes the changes more obvious.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2573#note_28816
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2573
participants (3)
-
Florian Will -
Florian Will (＠w-flo)
-
Rémi Bernon