https://bugs.winehq.org/show_bug.cgi?id=52062
Bug ID: 52062 Summary: dinput HID joystick doesn't implement effect state reports. Product: Wine Version: 6.21 Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: directx-dinput Assignee: wine-bugs@winehq.org Reporter: rbernon@codeweavers.com Distribution: ---
Split from https://bugs.winehq.org/show_bug.cgi?id=51873, IDirectInputDevice8_GetForceFeedbackState and IDirectInputEffect_GetEffectStatus are both not implemented and returning DIERR_UNSUPPORTED.
Some games are then unable to tell the current status of created effects and may try to create them again or just consider force feedback as not working.
This will require PID effect state reports, and possible use of HidD_GetInputReport / IOCTL_HID_GET_INPUT_REPORT, as some device only support these reports through explicit requests and aren't sending them with the device state reports.
https://bugs.winehq.org/show_bug.cgi?id=52062
Rémi Bernon rbernon@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=51873
https://bugs.winehq.org/show_bug.cgi?id=52062
Ivo Ivanov logos128@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |logos128@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #1 from Rémi Bernon rbernon@codeweavers.com --- I wrote (and sent upstream) a few tests for these functions and as far as I can see, Microsoft PID driver isn't trying to read the effect state reports using IOCTL_HID_GET_INPUT_REPORT at all, but expects the reports to come alongside the device state input reports.
In any case, not having the reports implemented doesn't seem to be much of an issue, and DInput returns some default flags depending on the operations and the effects is knows have been created and started.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #2 from Ivo Ivanov logos128@gmail.com --- (In reply to Rémi Bernon from comment #1)
I wrote (and sent upstream) a few tests for these functions and as far as I can see, Microsoft PID driver isn't trying to read the effect state reports using IOCTL_HID_GET_INPUT_REPORT at all, but expects the reports to come alongside the device state input reports.
In any case, not having the reports implemented doesn't seem to be much of an issue, and DInput returns some default flags depending on the operations and the effects is knows have been created and started.
I thought the same. There is no sign of any Get_Input_Report requests in the captured usb traffic from Windows, either.
IMO it would need the start time (GetTickCount64(), etc.) and loop count from Start(), and duration from SetParameters(). Also if start delay is confirmed by a conformance test to be separate from the duration, it should be added to the equation as well. Probably would need a flag for the paused state too. Then there is the Solo mode in Start(), and the various FFB commands in SendForceFeedbackCommand(), which would need enumeration of the created effects and resetting (zeroing, etc.) their start times and probably the other state parameters.
Slightly more complicated look the Pause and Continue commands from SendForceFeedbackCommand(), which by documentation should pause the effect timers by their current step, and then continue playing for the remaining time. They would need enumeration of the created effects on Pause and clearing the start time, start delay, and reducing the duration to the remaining time, while flagging them as paused. Then Continue would set the start time of the paused effects, etc. All of this while also considering the Infinity duration and loop count.
This could be implemented as backup for the other method.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #3 from Ivo Ivanov logos128@gmail.com --- Created attachment 71166 --> https://bugs.winehq.org/attachment.cgi?id=71166 dinput-some-fixes-and-tests.patch
Attached a patch with more tests for the effect status and the FFB state. As you said, on the driver side they seem to implement very simple logic. Mark an effect as playing on Start, and as non playing on Stop. No other FFB commands as stop-all, or any parameters as duration / start delay have any influence on the effect playing status. Then they change the status according to the PID State Reports received from the device.
Similar is the situation on the FFB state. On the driver side MS seems to update only the empty and stopped states. Interesting is that commands send through SendForceFeedbackCommand, as set actuators on/off or pause/continue doesn't influence the FFB state on the driver side. Those states and all others are updated only from the received PID State Reports.
Added also a patch to fix the SendForceFeedbackCommand sending Device Gain Reports on every command. While it is visible from the tests only DISFFC_RESET sends such reports.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #4 from Rémi Bernon rbernon@codeweavers.com --- Thanks, I upstreamed your patches and sent some more today to, I think, fully implement support of PID effect state reports and the corresponding status updates.
I also sent the SDL and evdev patches, although I believe SDL_HapticGetEffectStatus doesn't do anything on Linux at the moment.
The only remaining issue we know about, is then with hidraw backend (and maybe iohid but I know nothing about it), which may require actively polling for the input reports with HIDIOCGINPUT, instead of just reading them. We could perhaps do that in the same way as SDL is implemented, although it'd just be easier if the device were sending the input report on the interrupt channel and maybe we can consider that it should.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #5 from Ivo Ivanov logos128@gmail.com --- Thanks. I'll test it with the device, when today's patches get committed. I had something very similar to your implementation in GetEffectStatus (just to pass the tests), and all affected games and apps worked as expected. BeamNG had FFB and felt exactly like Windows. WheelCheck started showing properly the status of all created effects, etc. With WheelCheck, though, found another small issue caused by GetParameters. I have a patch for it, but want to confirm it is working properly with the device, and will attach it here.
For the hidraw backend, at least we have the PID state reports implemented now on par with the Windows PID driver. I plan to contact Granite Devices for this and the other issue with the device descriptor, but wanted the "black Friday" craze to pass a little :)
The PID state report could be quite useful for the GetForceFeedbackState, though, where we can retrieve the report on each call. I believe this function is not used frequently.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #6 from Ivo Ivanov logos128@gmail.com --- Created attachment 71200 --> https://bugs.winehq.org/attachment.cgi?id=71200 dinput-additions-and-fixes-2.patch
I tested the device with the latest changes and can confirm that everything works properly regarding this issue. BeamNG has FFB which is equal to the Windows version. WheelCheck shows the status of the effects in accordance with the simple logic without PID State reports. The same as the Windows version.
The other issue with WheelCheck is that, it doesn't show detailed info about the constant and periodic effects, while the Windows version does. The attached patch corrects that, and can confirm that it works properly on Wine.
There is another patch, which corrects an issue with the ramp force type specific parameters (Get|Set)Parameters using the constant_force struct instead of ramp_force.
The patchset is rebased to 27584c14497163dc2a15a9e8db9ddd40d3a516f8.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #7 from Rémi Bernon rbernon@codeweavers.com --- Thanks a lot for spotting this (and for your other contributions, it's been really helpful), I also took the opportunity to refactor that area, which was unnecessarily split after all.
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #8 from Rémi Bernon rbernon@codeweavers.com --- I believe this is already fixed in Wine 7.0?
https://bugs.winehq.org/show_bug.cgi?id=52062
--- Comment #9 from Ivo Ivanov logos128@gmail.com --- Yes, my device (Simucube 2) is working well with all backends.I still don't have definitive answer whether Granite Devices would implement the PID State reports, as per specification.
I think the bug report can be closed as fixed, and if they decide to implement the functionality and there are issues, I'll post back here.
https://bugs.winehq.org/show_bug.cgi?id=52062
Rémi Bernon rbernon@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #10 from Rémi Bernon rbernon@codeweavers.com --- Marking as fixed.
https://bugs.winehq.org/show_bug.cgi?id=52062
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #11 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 7.1.
-
WineHQ Bugzilla