[PATCH 0/6] MR53: windows.gaming.input: Test fixes and being implementing FF classes.
From: Rémi Bernon <rbernon(a)codeweavers.com> There is a race otherwise where we try to complete a pending IRP but because the async is writing the report from another thread we didn't find it and instead ignored it. Instead we need to atomically check if there was a pending IRP, and if the queue is empty, or queue the wait. Later, when a report is going to be marked as pending, and if there's someone waiting for it already, we instead complete it immediately. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/tests/dinput_test.h | 5 ++-- dlls/dinput/tests/driver_bus.c | 45 ++++++++++++++++++++++-------- dlls/dinput/tests/driver_hid.h | 7 ++++- dlls/dinput/tests/force_feedback.c | 4 +-- dlls/dinput/tests/hid.c | 6 ++-- 5 files changed, 48 insertions(+), 19 deletions(-) diff --git a/dlls/dinput/tests/dinput_test.h b/dlls/dinput/tests/dinput_test.h index 0adbd0b1ac8..c4ee42ca1c8 100644 --- a/dlls/dinput/tests/dinput_test.h +++ b/dlls/dinput/tests/dinput_test.h @@ -103,8 +103,9 @@ BOOL sync_ioctl_( const char *file, int line, HANDLE device, DWORD code, void *i #define set_hid_expect( a, b, c ) set_hid_expect_( __FILE__, __LINE__, a, b, c ) void set_hid_expect_( const char *file, int line, HANDLE device, struct hid_expect *expect, DWORD expect_size ); -#define wait_hid_expect( a, b ) wait_hid_expect_( __FILE__, __LINE__, a, b, FALSE ) -void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL todo ); +#define wait_hid_expect( a, b ) wait_hid_expect_( __FILE__, __LINE__, a, b, FALSE, FALSE ) +#define wait_hid_pending( a, b ) wait_hid_expect_( __FILE__, __LINE__, a, b, TRUE, FALSE ) +void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL wait_pending, BOOL todo ); #define send_hid_input( a, b, c ) send_hid_input_( __FILE__, __LINE__, a, b, c ) void send_hid_input_( const char *file, int line, HANDLE device, struct hid_expect *expect, DWORD expect_size ); diff --git a/dlls/dinput/tests/driver_bus.c b/dlls/dinput/tests/driver_bus.c index 6554d50c14b..6a1994e509c 100644 --- a/dlls/dinput/tests/driver_bus.c +++ b/dlls/dinput/tests/driver_bus.c @@ -188,26 +188,34 @@ static NTSTATUS expect_queue_add_pending( struct expect_queue *queue, IRP *irp ) return status; } -static void expect_queue_clear_pending( struct expect_queue *queue ) +/* complete an expect report previously marked as pending, or wait for one and then for the queue to empty */ +static NTSTATUS expect_queue_wait_pending( struct expect_queue *queue, IRP *irp ) { + NTSTATUS status; + IRP *pending; KIRQL irql; - IRP *irp; KeAcquireSpinLock( &queue->lock, &irql ); - if ((irp = queue->pending_wait)) + if ((pending = queue->pending_wait)) { queue->pending_wait = NULL; - if (!IoSetCancelRoutine( irp, NULL )) irp = NULL; + if (!IoSetCancelRoutine( pending, NULL )) pending = NULL; } + + if (pending && queue->pos == queue->end) status = STATUS_SUCCESS; + else status = expect_queue_add_pending_locked( queue, irp ); KeReleaseSpinLock( &queue->lock, irql ); - if (irp) + if (pending) { - irp->IoStatus.Status = STATUS_SUCCESS; - IoCompleteRequest( irp, IO_NO_INCREMENT ); + pending->IoStatus.Status = STATUS_SUCCESS; + IoCompleteRequest( pending, IO_NO_INCREMENT ); } + + return status; } +/* wait for the expect queue to empty */ static NTSTATUS expect_queue_wait( struct expect_queue *queue, IRP *irp ) { NTSTATUS status; @@ -257,11 +265,21 @@ static void expect_queue_next( struct expect_queue *queue, ULONG code, HID_XFER_ if (running_under_wine || !queue->pos->wine_only) break; queue->pos++; } - if (queue->pos == queue->end && (irp = queue->pending_wait)) + + if ((irp = queue->pending_wait)) { - queue->pending_wait = NULL; - if (!IoSetCancelRoutine( irp, NULL )) irp = NULL; + /* don't mark the IRP as pending if someone's already waiting */ + if (expect->ret_status == STATUS_PENDING) expect->ret_status = STATUS_SUCCESS; + + /* complete the pending wait IRP if the queue is now empty */ + if (queue->pos != queue->end) irp = NULL; + else + { + queue->pending_wait = NULL; + if (!IoSetCancelRoutine( irp, NULL )) irp = NULL; + } } + memcpy( context, queue->context, context_size ); KeReleaseSpinLock( &queue->lock, irql ); @@ -1244,9 +1262,12 @@ static NTSTATUS WINAPI pdo_ioctl( DEVICE_OBJECT *device, IRP *irp ) status = STATUS_SUCCESS; break; case IOCTL_WINETEST_HID_WAIT_EXPECT: - expect_queue_clear_pending( &impl->expect_queue ); - status = expect_queue_wait( &impl->expect_queue, irp ); + { + struct wait_expect_params wait_params = *(struct wait_expect_params *)irp->AssociatedIrp.SystemBuffer; + if (!wait_params.wait_pending) status = expect_queue_wait( &impl->expect_queue, irp ); + else status = expect_queue_wait_pending( &impl->expect_queue, irp ); break; + } case IOCTL_WINETEST_HID_SEND_INPUT: input_queue_reset( &impl->input_queue, irp->AssociatedIrp.SystemBuffer, in_size ); status = STATUS_SUCCESS; diff --git a/dlls/dinput/tests/driver_hid.h b/dlls/dinput/tests/driver_hid.h index 4711d391bfe..c5641097df3 100644 --- a/dlls/dinput/tests/driver_hid.h +++ b/dlls/dinput/tests/driver_hid.h @@ -42,7 +42,7 @@ DEFINE_GUID(control_class,0xdeadbeef,0x29ef,0x4538,0xa5,0xfd,0xb6,0x95,0x73,0xa3,0x62,0xc0); #define IOCTL_WINETEST_HID_SET_EXPECT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x800, METHOD_IN_DIRECT, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_HID_WAIT_EXPECT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x801, METHOD_NEITHER, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_HID_WAIT_EXPECT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x801, METHOD_IN_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_HID_SEND_INPUT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x802, METHOD_IN_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_HID_SET_CONTEXT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x803, METHOD_IN_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_CREATE_DEVICE CTL_CODE(FILE_DEVICE_KEYBOARD, 0x804, METHOD_IN_DIRECT, FILE_ANY_ACCESS) @@ -61,6 +61,11 @@ struct hid_expect BYTE report_buf[128]; }; +struct wait_expect_params +{ + BOOL wait_pending; +}; + /* create/remove device */ #define MAX_HID_DESCRIPTOR_LEN 2048 diff --git a/dlls/dinput/tests/force_feedback.c b/dlls/dinput/tests/force_feedback.c index f5b387f3566..976b1b93863 100644 --- a/dlls/dinput/tests/force_feedback.c +++ b/dlls/dinput/tests/force_feedback.c @@ -5536,7 +5536,7 @@ static void test_windows_gaming_input(void) ok( !bool_async_handler.invoked, "handler invoked\n" ); IAsyncInfo_Release( async_info ); - wait_hid_expect( file, 100 ); + wait_hid_pending( file, 100 ); ret = WaitForSingleObject( bool_async_handler.event, 100 ); ok( ret == 0, "WaitForSingleObject returned %#lx\n", ret ); CloseHandle( bool_async_handler.event ); @@ -5581,7 +5581,7 @@ static void test_windows_gaming_input(void) ok( !bool_async_handler.invoked, "handler invoked\n" ); IAsyncInfo_Release( async_info ); - wait_hid_expect( file, 500 ); + wait_hid_pending( file, 100 ); ret = WaitForSingleObject( bool_async_handler.event, 100 ); ok( ret == 0, "WaitForSingleObject returned %#lx\n", ret ); CloseHandle( bool_async_handler.event ); diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index 7e2d9cf86cf..00a3d65fe4d 100644 --- a/dlls/dinput/tests/hid.c +++ b/dlls/dinput/tests/hid.c @@ -911,10 +911,12 @@ void set_hid_expect_( const char *file, int line, HANDLE device, struct hid_expe ok_(file, line)( ret, "IOCTL_WINETEST_HID_SET_EXPECT failed, last error %lu\n", GetLastError() ); } -void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL todo ) +void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL wait_pending, BOOL todo ) { + struct wait_expect_params params = {.wait_pending = wait_pending}; + todo_wine_if(todo) { - BOOL ret = sync_ioctl_( file, line, device, IOCTL_WINETEST_HID_WAIT_EXPECT, NULL, 0, NULL, 0, timeout ); + BOOL ret = sync_ioctl_( file, line, device, IOCTL_WINETEST_HID_WAIT_EXPECT, ¶ms, sizeof(params), NULL, 0, timeout ); ok_(file, line)( ret, "IOCTL_WINETEST_HID_WAIT_EXPECT failed, last error %lu\n", GetLastError() ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/53
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/windows.gaming.input/async.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/windows.gaming.input/async.c b/dlls/windows.gaming.input/async.c index 1426b3d8481..dfcb4144e74 100644 --- a/dlls/windows.gaming.input/async.c +++ b/dlls/windows.gaming.input/async.c @@ -146,7 +146,8 @@ static HRESULT WINAPI async_impl_get_Completed( IWineAsyncInfoImpl *iface, IWine EnterCriticalSection( &impl->cs ); if (impl->status == Closed) hr = E_ILLEGAL_METHOD_CALL; - *handler = (impl->handler != HANDLER_NOT_SET) ? impl->handler : NULL; + if (impl->handler == NULL || impl->handler == HANDLER_NOT_SET) *handler = NULL; + else IWineAsyncOperationCompletedHandler_AddRef( (*handler = impl->handler) ); LeaveCriticalSection( &impl->cs ); return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/53
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/windows.gaming.input/async.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/windows.gaming.input/async.c b/dlls/windows.gaming.input/async.c index dfcb4144e74..fc593bc6121 100644 --- a/dlls/windows.gaming.input/async.c +++ b/dlls/windows.gaming.input/async.c @@ -98,6 +98,7 @@ static ULONG WINAPI async_impl_Release( IWineAsyncInfoImpl *iface ) IAsyncInfo_Close( &impl->IAsyncInfo_iface ); if (impl->param) IUnknown_Release( impl->param ); if (impl->invoker) IUnknown_Release( impl->invoker ); + impl->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection( &impl->cs ); free( impl ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/53
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/windows.gaming.input/provider.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/windows.gaming.input/provider.c b/dlls/windows.gaming.input/provider.c index 70900000eef..1f3f4e047f5 100644 --- a/dlls/windows.gaming.input/provider.c +++ b/dlls/windows.gaming.input/provider.c @@ -572,7 +572,7 @@ void provider_create( const WCHAR *device_path ) EnterCriticalSection( &provider_cs ); LIST_FOR_EACH_ENTRY( entry, &provider_list, struct provider, entry ) - if ((found = !wcscmp( entry->device_path, device_path ))) break; + if ((found = !wcsicmp( entry->device_path, device_path ))) break; if (!found) list_add_tail( &provider_list, &impl->entry ); LeaveCriticalSection( &provider_cs ); @@ -592,11 +592,12 @@ void provider_remove( const WCHAR *device_path ) EnterCriticalSection( &provider_cs ); LIST_FOR_EACH_ENTRY( entry, &provider_list, struct provider, entry ) - if ((found = !wcscmp( entry->device_path, device_path ))) break; + if ((found = !wcsicmp( entry->device_path, device_path ))) break; if (found) list_remove( &entry->entry ); LeaveCriticalSection( &provider_cs ); - if (found) + if (!found) WARN( "provider not found for device %s\n", debugstr_w( device_path ) ); + else { provider = &entry->IGameControllerProvider_iface; manager_on_provider_removed( provider ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/53
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/windows.gaming.input/classes.idl | 3 ++- include/windows.gaming.input.forcefeedback.idl | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/windows.gaming.input/classes.idl b/dlls/windows.gaming.input/classes.idl index ca3bd5d8dd7..2adf43cdbb0 100644 --- a/dlls/windows.gaming.input/classes.idl +++ b/dlls/windows.gaming.input/classes.idl @@ -29,11 +29,12 @@ import "asyncinfo.idl"; import "eventtoken.idl"; import "windowscontracts.idl"; import "windows.foundation.idl"; +import "windows.foundation.numerics.idl"; import "windows.devices.haptics.idl"; -import "windows.gaming.input.forcefeedback.idl"; import "windows.system.idl"; import "windows.devices.power.idl"; #define DO_NO_IMPORTS +#include "windows.gaming.input.forcefeedback.idl" #include "windows.gaming.input.idl" #include "windows.gaming.input.custom.idl" diff --git a/include/windows.gaming.input.forcefeedback.idl b/include/windows.gaming.input.forcefeedback.idl index 290d1f62971..82fb083b34b 100644 --- a/include/windows.gaming.input.forcefeedback.idl +++ b/include/windows.gaming.input.forcefeedback.idl @@ -20,12 +20,14 @@ #pragma winrt ns_prefix #endif +#ifndef DO_NO_IMPORTS import "inspectable.idl"; import "asyncinfo.idl"; import "eventtoken.idl"; import "windowscontracts.idl"; import "windows.foundation.idl"; import "windows.foundation.numerics.idl"; +#endif namespace Windows.Gaming.Input.ForceFeedback { typedef enum ForceFeedbackEffectAxes ForceFeedbackEffectAxes; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/53
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/tests/force_feedback.c | 3 +- dlls/windows.gaming.input/Makefile.in | 1 + dlls/windows.gaming.input/constant_effect.c | 117 ++++++++++++++++++++ dlls/windows.gaming.input/main.c | 3 + dlls/windows.gaming.input/private.h | 1 + 5 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 dlls/windows.gaming.input/constant_effect.c diff --git a/dlls/dinput/tests/force_feedback.c b/dlls/dinput/tests/force_feedback.c index 976b1b93863..a1e41dedf44 100644 --- a/dlls/dinput/tests/force_feedback.c +++ b/dlls/dinput/tests/force_feedback.c @@ -5620,15 +5620,14 @@ static void test_windows_gaming_input(void) hr = pWindowsCreateString( constant_effect_class_name, wcslen( constant_effect_class_name ), &str ); ok( hr == S_OK, "WindowsCreateString returned %#lx\n", hr ); hr = pRoGetActivationFactory( str, &IID_IActivationFactory, (void **)&activation_factory ); - todo_wine ok( hr == S_OK, "RoGetActivationFactory returned %#lx\n", hr ); pWindowsDeleteString( str ); - if (hr != S_OK) goto skip_tests; hr = IActivationFactory_ActivateInstance( activation_factory, &tmp_inspectable ); todo_wine ok( hr == S_OK, "QueryInterface returned %#lx\n", hr ); IActivationFactory_Release( activation_factory ); + if (hr != S_OK) goto skip_tests; hr = IInspectable_QueryInterface( tmp_inspectable, &IID_IForceFeedbackEffect, (void **)&effect ); todo_wine diff --git a/dlls/windows.gaming.input/Makefile.in b/dlls/windows.gaming.input/Makefile.in index 463f6b97f74..8f3af4fec62 100644 --- a/dlls/windows.gaming.input/Makefile.in +++ b/dlls/windows.gaming.input/Makefile.in @@ -3,6 +3,7 @@ IMPORTS = combase uuid user32 dinput8 setupapi hid C_SRCS = \ async.c \ + constant_effect.c \ controller.c \ event_handlers.c \ force_feedback.c \ diff --git a/dlls/windows.gaming.input/constant_effect.c b/dlls/windows.gaming.input/constant_effect.c new file mode 100644 index 00000000000..a27258dd9bf --- /dev/null +++ b/dlls/windows.gaming.input/constant_effect.c @@ -0,0 +1,117 @@ +/* WinRT Windows.Gaming.Input implementation + * + * Copyright 2022 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "private.h" +#include "provider.h" + +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(input); + +struct constant_factory +{ + IActivationFactory IActivationFactory_iface; + LONG ref; +}; + +static inline struct constant_factory *impl_from_IActivationFactory( IActivationFactory *iface ) +{ + return CONTAINING_RECORD( iface, struct constant_factory, IActivationFactory_iface ); +} + +static HRESULT WINAPI activation_QueryInterface( IActivationFactory *iface, REFIID iid, void **out ) +{ + struct constant_factory *impl = impl_from_IActivationFactory( iface ); + + TRACE( "iface %p, iid %s, out %p.\n", iface, debugstr_guid( iid ), out ); + + if (IsEqualGUID( iid, &IID_IUnknown ) || + IsEqualGUID( iid, &IID_IInspectable ) || + IsEqualGUID( iid, &IID_IAgileObject ) || + IsEqualGUID( iid, &IID_IActivationFactory )) + { + IInspectable_AddRef( (*out = &impl->IActivationFactory_iface) ); + return S_OK; + } + + FIXME( "%s not implemented, returning E_NOINTERFACE.\n", debugstr_guid( iid ) ); + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI activation_AddRef( IActivationFactory *iface ) +{ + struct constant_factory *impl = impl_from_IActivationFactory( iface ); + ULONG ref = InterlockedIncrement( &impl->ref ); + TRACE( "iface %p increasing refcount to %lu.\n", iface, ref ); + return ref; +} + +static ULONG WINAPI activation_Release( IActivationFactory *iface ) +{ + struct constant_factory *impl = impl_from_IActivationFactory( iface ); + ULONG ref = InterlockedDecrement( &impl->ref ); + TRACE( "iface %p decreasing refcount to %lu.\n", iface, ref ); + return ref; +} + +static HRESULT WINAPI activation_GetIids( IActivationFactory *iface, ULONG *iid_count, IID **iids ) +{ + FIXME( "iface %p, iid_count %p, iids %p stub!\n", iface, iid_count, iids ); + return E_NOTIMPL; +} + +static HRESULT WINAPI activation_GetRuntimeClassName( IActivationFactory *iface, HSTRING *class_name ) +{ + FIXME( "iface %p, class_name %p stub!\n", iface, class_name ); + return E_NOTIMPL; +} + +static HRESULT WINAPI activation_GetTrustLevel( IActivationFactory *iface, TrustLevel *trust_level ) +{ + FIXME( "iface %p, trust_level %p stub!\n", iface, trust_level ); + return E_NOTIMPL; +} + +static HRESULT WINAPI activation_ActivateInstance( IActivationFactory *iface, IInspectable **instance ) +{ + FIXME( "iface %p, instance %p stub!\n", iface, instance ); + return E_NOTIMPL; +} + +static const struct IActivationFactoryVtbl activation_vtbl = +{ + activation_QueryInterface, + activation_AddRef, + activation_Release, + /* IInspectable methods */ + activation_GetIids, + activation_GetRuntimeClassName, + activation_GetTrustLevel, + /* IActivationFactory methods */ + activation_ActivateInstance, +}; + +static struct constant_factory constant_statics = +{ + {&activation_vtbl}, + 1, +}; + +IInspectable *constant_effect_factory = (IInspectable *)&constant_statics.IActivationFactory_iface; diff --git a/dlls/windows.gaming.input/main.c b/dlls/windows.gaming.input/main.c index 21808d9c2ad..aeb870b039b 100644 --- a/dlls/windows.gaming.input/main.c +++ b/dlls/windows.gaming.input/main.c @@ -185,6 +185,9 @@ HRESULT WINAPI DllGetActivationFactory( HSTRING class_str, IActivationFactory ** if (!wcscmp( buffer, RuntimeClass_Windows_Gaming_Input_Custom_GameControllerFactoryManager )) IGameControllerFactoryManagerStatics2_QueryInterface( manager_factory, &IID_IActivationFactory, (void **)factory ); + if (!wcscmp( buffer, RuntimeClass_Windows_Gaming_Input_ForceFeedback_ConstantForceEffect )) + IInspectable_QueryInterface( constant_effect_factory, &IID_IActivationFactory, (void **)factory ); + if (*factory) return S_OK; return CLASS_E_CLASSNOTAVAILABLE; } diff --git a/dlls/windows.gaming.input/private.h b/dlls/windows.gaming.input/private.h index 68f6bdcf5b6..2e97f182e78 100644 --- a/dlls/windows.gaming.input/private.h +++ b/dlls/windows.gaming.input/private.h @@ -45,6 +45,7 @@ extern ICustomGameControllerFactory *controller_factory; extern ICustomGameControllerFactory *gamepad_factory; extern ICustomGameControllerFactory *racing_wheel_factory; extern IGameControllerFactoryManagerStatics2 *manager_factory; +extern IInspectable *constant_effect_factory; struct vector_iids { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/53
participants (1)
-
Rémi Bernon