Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XInput: Apply Rumble/Motor output only on changes (again) #3790

Merged

Conversation

mimimi085181
Copy link
Contributor

@mimimi085181 mimimi085181 commented Apr 21, 2016

Disclaimer: I can't test if this works on xbox one controllers, i don't have one. But i have confirmed that this UpdateMotors() is related to rumble for emulated wiimotes.

This partially reverts commit "XInput: Apply immediately as well" (1958a10) from pr # #1560

Hopefully this fixes the xbox one controller rumble issue:
https://bugs.dolphin-emu.org/issues/9071

And in theory it might reduce the used usb bandwidth, as it was originally intended before pr 1560.

@JMC47: Please do a good amount of testing, to see if this breaks rumble for wiimotes or gamecube controllers emulated with xinput devices.


This change is Reviewable

@phire
Copy link
Member

phire commented Apr 21, 2016

I tested this, fixes the xbox one controller rumble (as wiimote)

I also tested a 360 controller and that still works.

@delroth delroth added this to the Dolphin Release 5.0 milestone Apr 21, 2016
@delroth
Copy link
Member

delroth commented Apr 21, 2016

@mimimi085181: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


@mimimi085181: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


@mimimi085181: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


Amazing, thanks for tracking this down!

Feel free to merge whenever you think enough testing has been done. @dolphin-emu-bot allowmerge

@@ -90,7 +90,7 @@ class Device : public Core::Device

private:
XINPUT_STATE m_state_in;
XINPUT_VIBRATION m_state_out;
XINPUT_VIBRATION m_state_out, m_current_state_out;

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@RisingFog
Copy link
Member

:lgtm:


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

Disclaimer: I can't test if this works on xbox one controllers, i don't have one. But i have conformed that this UpdateMotors() is related to rumble for emulated wiimotes.

This partially reverts commit "XInput: Apply immediately as well" (1958a10) from pr # dolphin-emu#1560

Hopefully this fixes the xbox one controller rumble issue:
https://bugs.dolphin-emu.org/issues/9071

And in theory it might reduce the used usb bandwidth, as it was originally intended before pr 1560.

@JMC47: Please do a good amount of testing, to see if this breaks rumble for wiimotes or gamecube controllers emulated with xinput devices.
@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • nfsu-purplerect on ogl-lin-mesa: diff
  • pm-hc-jp on ogl-lin-mesa: diff
  • rs2-skybox on ogl-lin-mesa: diff
  • super-sluggers-white-out on ogl-lin-mesa: diff
  • DKCR-fast-depth on sw-lin-mesa: diff
  • ed-updated on sw-lin-mesa: diff
  • fortune-street-white-box on sw-lin-mesa: diff

automated-fifoci-reporter

@mimimi085181
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 1 unresolved discussion.


Source/Core/InputCommon/ControllerInterface/XInput/XInput.h, line 93 [r1] (raw file):
Done.


Comments from Reviewable

@dolphin-emu-bot dolphin-emu-bot merged commit 21e56b5 into dolphin-emu:master Apr 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants