Closed
Bug 690937
Opened 13 years ago
Closed 11 years ago
Add an XInput backend for Windows gamepad support
Categories
(Core :: Hardware Abstraction Layer (HAL), defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: ted, Assigned: ted)
References
(Blocks 1 open bug)
Details
(Whiteboard: [paladin])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
XInput is the shiny new API that replaces DirectInput. It has much better support for XBox 360 controllers and other new gamepads. We should be able to use both XI and DI to support the widest range of devices, but we'll get much better results out of 360 controllers using XI.
Keywords: dev-doc-needed
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → jon
Assignee | ||
Comment 1•13 years ago
|
||
I'm not sure why you think this is dev-doc-needed? Doesn't seem to fit the criteria to me. In any event, it hasn't been implemented yet, so it's a bit early for that.
Assignee | ||
Updated•13 years ago
|
Keywords: dev-doc-needed
(In reply to Ted Mielczarek [:ted, :luser] from comment #1)
> Doesn't seem to fit the criteria to me.
True, sorry. This was a quick scan thrugh bugs which might need that keyword.
> In any event, it hasn't been implemented yet, so it's a bit early for that.
It's never to early (it doesn't hurt) but somtimes to late (if forgotten). IMHO it should be added when the bug is filed.
http://www.bitstampede.com/?s=dev-doc-needed
"First off, my traditional plea: if you’ve got code you’re working on that even might impact documentation, please add the “dev-doc-needed” keyword to the relevant bug"
"First, if there’s a bug related to your work, make sure the “dev-doc-needed” keyword is added to the bug in Bugzilla. It doesn’t matter if you’ve actually made the change or not. The writers only apply changes to the documentation once the bug is both tagged as “dev-doc-needed” and the bug is marked as fixed."
Updated•13 years ago
|
Whiteboard: [paladin]
Assignee | ||
Updated•12 years ago
|
Component: DOM → Hardware Abstraction Layer (HAL)
Updated•12 years ago
|
Assignee: jon → nobody
Comment 3•12 years ago
|
||
Jesse, do you have any interest in working on this for your final releases, since your layout stuff for webvtt is done? I know Ted's looking for some help.
Assignee | ||
Comment 4•12 years ago
|
||
This is a work in progress. With this patch applied my demos work with an xbox 360 controller on Windows. The patch disables scanning for DirectInput devices, since I haven't added code to check if a DI device is handled using XInput.
As a bonus, gamepads exposed via XInput get mapped to the same buttons/axes as they do in Chrome.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → ted
Assignee | ||
Comment 5•11 years ago
|
||
This is written on top of the patch in bug 996078, and adds support for XInput controllers.
The only downside here is that XInput is a polling API, so this adds a repeating timer to do the polling. We could probably be a little smarter about disabling the timer when no visible page is using the gamepad.
Assignee | ||
Updated•11 years ago
|
Attachment #731140 -
Attachment is obsolete: true
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 8406260 [details] [diff] [review]
Add XInput support to the Windows gamepad backend
Jim: since you so graciously reviewed my Raw Input patch, this patch goes on top of that one.
The XInput docs are here:
http://msdn.microsoft.com/en-us/library/windows/desktop/ee417001%28v=vs.85%29.aspx
It's a pretty simple API.
Attachment #8406260 -
Flags: review?(jmathies)
Comment 7•11 years ago
|
||
rats, I only have wireless controllers. I'll order one on amazon but won't have it to test this patch.
Comment 8•11 years ago
|
||
Comment on attachment 8406260 [details] [diff] [review]
Add XInput support to the Windows gamepad backend
Review of attachment 8406260 [details] [diff] [review]:
-----------------------------------------------------------------
::: hal/windows/WindowsGamepad.cpp
@@ +45,5 @@
> // Therefore, we wait a bit after receiving one before looking for
> // device changes.
> const uint32_t kDevicesChangedStableDelay = 200;
> +// Arbitrary.
> +const uint32_t kXInputPollInterval = 50;
nit - maybe be a bit more descriptive here. :) What's this poll value for?
@@ +122,5 @@
> // Used during rescan to find devices that were disconnected.
> bool present;
> };
>
> +// If we dropped support for VC++ < 2010 I would use decltype in the
I'd suggest rewording this - "when we drop support for.." "..switch to decltype for GetProcAddress calls below."
@@ +369,5 @@
> + return false;
> +}
> +
> +bool
> +WindowsGamepadService::ScanForXInputDevices()
nit - add a debug assert that checks for a valid mXInput here.
@@ +448,5 @@
> + self->PollXInput();
> +}
> +
> +void
> +WindowsGamepadService::PollXInput()
nit - debug assert on mXInput.
@@ +496,5 @@
> + }
> +
> + // Finally deal with analog sticks
> + if (state.Gamepad.sThumbLX != gamepad.state.Gamepad.sThumbLX) {
> + //TODO: deadzone
nit - how bout we skip adding the todos and just file a bug?
@@ +792,5 @@
>
> void
> WindowsGamepadService::Cleanup()
> {
> + mXInputPollTimer->Cancel();
any chance this could be null here? If so please wrap in a null check.
Attachment #8406260 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #7)
> rats, I only have wireless controllers. I'll order one on amazon but won't
> have it to test this patch.
FYI you can use one of these to use wireless controllers on a PC:
http://www.amazon.com/Xbox-360-Wireless-Gaming-Receiver-Windows/dp/B000HZFCT2
Assignee | ||
Comment 10•11 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #8)
> > +// If we dropped support for VC++ < 2010 I would use decltype in the
>
> I'd suggest rewording this - "when we drop support for.." "..switch to
> decltype for GetProcAddress calls below."
As it turns out we did already drop support for VC < 2010, so I can do this. Unfortunately the definition of XInputEnable isn't in the SDK I have, so I think I need to leave the hacky bit in for that. I'll fix the one declaration and the comment, at least.
Assignee | ||
Comment 11•11 years ago
|
||
Try run is green:
https://tbpl.mozilla.org/?tree=Try&rev=4299de24b9db
Assignee | ||
Comment 12•11 years ago
|
||
Comment 13•11 years ago
|
||
Backed out for B2G desktop Windows build failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f99cf3aee13f
Assignee | ||
Comment 14•11 years ago
|
||
Fixed and re-pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2462b87f45bb
Comment 15•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 16•11 years ago
|
||
Landed trivial mingw fixes:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ead17eacbb43
Comment 17•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•