Bug 69451

Summary: Adding gamepad support
Product: WebKit Reporter: Scott Graham <scottmg>
Component: PlatformAssignee: Scott Graham <scottmg>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, adman.com, ap, dglazkov, donggwan.kim, eoconnor, fishd, ojan, scheib, ted, tkent, webkit.arunp, webkit.review.bot, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://dvcs.w3.org/hg/webevents/raw-file/default/gamepad.html
Bug Depends on:    
Bug Blocks: 84386    
Attachments:
Description Flags
Patch
none
plumb through platform rather than client
none
remove unnecessary include
none
remove custom binding code, add DRT hooks and tests
none
update Skipped for various ports
none
remove everything except platform plumbing and DRT/testing
none
update WebKit/chromium/DEPS
none
try a different chromium rev
none
add apparently newly-needed chromium deps
none
fix layering violations
none
no change, just to EWS again with DEPS rolled none

Description Scott Graham 2011-10-05 12:24:11 PDT
This is the full patch for reference, still WiP. There will be separate bugs&patches for landing.
Comment 1 Scott Graham 2011-10-05 12:48:52 PDT
Created attachment 109841 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-05 12:53:07 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 WebKit Review Bot 2011-10-05 13:26:37 PDT
Comment on attachment 109841 [details]
Patch

Attachment 109841 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9964033

New failing tests:
fast/dom/navigator-detached-no-crash.html
Comment 4 Darin Fisher (:fishd, Google) 2011-10-05 13:28:34 PDT
Comment on attachment 109841 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109841&action=review

> Source/WebKit/chromium/public/WebViewClient.h:303
> +    // Gamepad -------------------------------------------------------------

nit: new line after section divider.  please preserve the 2 new lines between sections.

it seems like the concept of a gamepad really belongs more at the platform layer of
webkit.  this seems like something you would get via the WebKitPlatformSupport interface
instead of the WebViewClient.  i'd imagine that you probably need something to be contextual
to the WebView however.  can you explain that part?  maybe we could meet up to review this
in a little more detail?
Comment 5 Alexey Proskuryakov 2011-10-05 22:52:25 PDT
There hasn't been any agreement or support for this feature on webkit-dev mailing list. To the contrary, there has been much disconcert.

One of the ways to move forward with prototyping that we've been discussing was working on a branch. This seems like a great candidate for a branch, as merging to trunk shouldn't be hard.
Comment 6 Darin Fisher (:fishd, Google) 2011-10-05 22:58:24 PDT
(In reply to comment #5)
> There hasn't been any agreement or support for this feature on webkit-dev mailing list. To the contrary, there has been much disconcert.
> 
> One of the ways to move forward with prototyping that we've been discussing was working on a branch. This seems like a great candidate for a branch, as merging to trunk shouldn't be hard.


I don't think this needs to be developed on a branch.  I think this is low cost to develop on the trunk behind a flag.  It is largely orthogonal to existing code, so it should have low impact.

This is being actively developed by Mozilla and Google, and we'd like to proceed to experiment with it in our developer channel (disabled by default behind a command line flag of course).

Can you please share your feedback on the API with the w3c-events working group?
Comment 7 Darin Fisher (:fishd, Google) 2011-10-05 23:02:44 PDT
Alexey, perhaps you missed this one?
https://bugs.webkit.org/show_bug.cgi?id=66859#c5

Notice that there is a spec under development.
Comment 8 Alexey Proskuryakov 2011-10-05 23:34:25 PDT
I don't have any feedback that I'm willing to share with the working group at this time. I see that Simon Fraser objected against adding this to WebKit, as well as against the idea itself. How was that resolved?

Quoting <http://www.webkit.org/projects/goals.html>,

"WebKit is an engineering project not a science project. For new features to be adopted into WebKit, we strongly prefer for the technology or at least the use case for it to be proven."

"WebKit is not the solution to every problem. We focus on web content, not complete solutions to every imaginable technology need."

> Notice that there is a spec under development.

Having a spec under development is certainly an important step.

For what it's worth, the spec doesn't exist where bug 66859 comment 5 points to, even though I can find it in version control history. Also, <http://www.w3.org/2010/webevents/charter/Overview.html> doesn't list gamepad/joystick interfaces in scope or deliverables. I can see informal discussion with some WG members being in support for recharter, but it hasn't happened yet.
Comment 9 Darin Fisher (:fishd, Google) 2011-10-05 23:56:41 PDT
(In reply to comment #8)
> I don't have any feedback that I'm willing to share with the working group at this time.

How should we interpret your silence?


> I see that Simon Fraser objected against adding this to WebKit, as well as against the idea itself. How was that resolved?

It was resolved by creating a W3C spec and working to recharter the W3C-events WG.


> Quoting <http://www.webkit.org/projects/goals.html>,
> 
> "WebKit is an engineering project not a science project. For new features to be adopted into WebKit, we strongly prefer for the technology or at least the use case for it to be proven."
> 
> "WebKit is not the solution to every problem. We focus on web content, not complete solutions to every imaginable technology need."

I don't understand what point you are making.  Mozilla and Google are working to add support for game controllers to the web platform.  It is an engineering project based on a strong use case.  I get over 5 million results when I google for "flash gamepad" and nearly 17 million for "flash gamecontroller."  It seems pretty obvious that HTML5/WebGL-based games would benefit immensely from a gamepad interface.

I honestly believe that there are cases where developing a new API on the trunk could be expensive in terms of its overhead to ports that do not care about the API.  This does not seem to be anywhere close to that.  We're talking about a light touch on the DOM here.


> > Notice that there is a spec under development.
> 
> Having a spec under development is certainly an important step.

Yes, lack of a spec seemed to be Simon's main objection initially.  He then proceeded to wonder about other possible manifestations of such an API.  I think we would very much welcome his input on the API design.  Now is a great time to get involved!


> For what it's worth, the spec doesn't exist where bug 66859 comment 5 points to, even though I can find it in version control history.

Yes, I noticed that too.  Given that it was recently updated, I suspect a snafu that will be easily fixed.


> Also, <http://www.w3.org/2010/webevents/charter/Overview.html> doesn't list gamepad/joystick interfaces in scope or deliverables. I can see informal discussion with some WG members being in support for recharter, but it hasn't happened yet.

Yes, the WG is being rechartered to include this work.
Comment 10 Scott Graham 2011-10-06 07:50:12 PDT
(In reply to comment #8)
> For what it's worth, the spec doesn't exist where bug 66859 comment 5 points to, even though I can find it in version control history.

Apologies, I've updated to the correct the URL above.

> Also, <http://www.w3.org/2010/webevents/charter/Overview.html> doesn't list gamepad/joystick interfaces in scope or deliverables. I can see informal discussion with some WG members being in support for recharter, but it hasn't happened yet.

The Draft from the WG Chair is here if you're interested in commenting on the rechartering process: http://www.w3.org/2010/webevents/charter/2011/Overview.html
Comment 11 Alexey Proskuryakov 2011-10-06 09:13:42 PDT
Darin, these would all be good comments for a webkit-dev discussion. I suggest resuming the thread there.

When suggesting people to actively join development of every spec they have concerns about, please remember that this is a significant time investment. One who cares about immature and/or harmful specs not being in WebKit doesn't necessarily have any interest in making each of them good. Ditto for convincing every person on Internet that they are harmful.

The concern here as I understand it is that providing low level access to every possible controller creates fragmentation, with purportedly "HTML" content that only works on a few devices. There is no clear cut border here - it's been mentioned that even touch events can be seen as rare - and then I advocate that adding more mouse specific events is a bad idea for the same reason.

Finally, I agree that the cost of having this in trunk under an ifdef is not very high. This also means that developing on branch is relatively low cost. Since this is still at prototyping stage, I thought that it could be a small feature to polish prototype-on-branch approach on.
Comment 12 Darin Fisher (:fishd, Google) 2011-10-06 09:56:12 PDT
(In reply to comment #11)
> Darin, these would all be good comments for a webkit-dev discussion. I suggest resuming the thread there.

Yes, we should certainly do that.


> When suggesting people to actively join development of every spec they have concerns about, please remember that this is a significant time investment.

My point is that we are working with others on this feature, and it would be helpful for them to hear your concerns too.   We will struggle to properly articulate/channel your concerns because we do not fully understand them.  You can surely communicate them better than anyone else.

I agree that building new APIs takes time.  We are not looking to ship something rushed or poor in quality.  We are looking to prototype, learn and improve the design.


> Finally, I agree that the cost of having this in trunk under an ifdef is not very high. This also means that developing on branch is relatively low cost. Since this is still at prototyping stage, I thought that it could be a small feature to polish prototype-on-branch approach on.

Doing the development on trunk makes it vastly easier for us to get feedback from developers.  They can switch on a flag, try out the feature, and give us feedback.  A branch just adds cost.  Since there is little burden on the trunk, it seems obvious to me that we should do the work on the trunk.
Comment 13 Scott Graham 2011-10-07 08:44:12 PDT
(In reply to comment #4)

> it seems like the concept of a gamepad really belongs more at the platform layer of
> webkit.  this seems like something you would get via the WebKitPlatformSupport interface
> instead of the WebViewClient.  i'd imagine that you probably need something to be contextual
> to the WebView however.  can you explain that part?  maybe we could meet up to review this
> in a little more detail?

Here's the implementation approach I'm thinking of taking now (hopefully more sensible?):

WebCore/page/Navigator.idl adds ".gamepads" and bindings. WebCore/page adds Gamepad.idl and GamepadList.idl along with bindings to expose to Javascript.

The accessor in Navigator calls into WebCore/platform/chromium/PlatformSupport.h to "pollGamepads()".

That's implemented by WebKit/chromium/src/PlatformSupport.cpp which forwards to something in WebKit/chromium/public/WebKitPlatformSupport.h (something like webPollGamepads() but with a better name). It converts from WebGamepads (a POD shared memory representation) to the binding- & WebCore-friendly representation in GamepadList.idl/Gamepad.idl.

chromium/public/ adds WebGamepads.h, a fixed size buffer representation of gamepad data.

On the Chromium side, the browser uses a background thread to poll the hardware into a WebGamepads structure, and implements "webPollGamepads()" which just returns a handle/pointer to that shared memory.
Comment 14 Darin Fisher (:fishd, Google) 2011-10-07 11:46:48 PDT
(In reply to comment #13)
...
> Here's the implementation approach I'm thinking of taking now (hopefully more sensible?):
> 
> WebCore/page/Navigator.idl adds ".gamepads" and bindings. WebCore/page adds Gamepad.idl and GamepadList.idl along with bindings to expose to Javascript.
> 
> The accessor in Navigator calls into WebCore/platform/chromium/PlatformSupport.h to "pollGamepads()".

We normally define a cross-platform header in WebCore/platform/ that defines the API
cross-platform code should use to talk to the platform.  Then, we provide alternate
implementations of that header based on the needs of specific ports.  For Chromium,
the implementation would use PlatformSupport as you describe.  Other ports might be
implemented by using OS APIs directly.
Comment 15 Scott Graham 2011-10-07 12:04:31 PDT
(In reply to comment #14)
> 
> We normally define a cross-platform header in WebCore/platform/ that defines the API
> cross-platform code should use to talk to the platform.  Then, we provide alternate
> implementations of that header based on the needs of specific ports.

I see, thank you.
Comment 16 Scott Graham 2011-10-12 09:49:26 PDT
Created attachment 110692 [details]
plumb through platform rather than client
Comment 17 Gyuyoung Kim 2011-10-12 09:54:46 PDT
Comment on attachment 110692 [details]
plumb through platform rather than client

Attachment 110692 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10026917
Comment 18 Scott Graham 2011-10-12 10:47:59 PDT
Created attachment 110704 [details]
remove unnecessary include
Comment 19 Alexey Proskuryakov 2011-10-12 10:56:56 PDT
As discussed in webkit-dev, I don't think that we should accept an implementation of a spec that's so immature that it doesn't even have a meaningful name.
Comment 20 Scott Graham 2011-10-12 11:08:19 PDT
(In reply to comment #19)
> As discussed in webkit-dev, I don't think that we should accept an implementation of a spec that's so immature that it doesn't even have a meaningful name.

And naming concerns should still be discussed in the WG.

The spec focuses on gaming use-cases, and as such, the most common implementation of the spec will almost surely be gamepads, hence the name. It was generalized to support other devices as there was no reason not be flexible, but calling it something vague like "inputdevices" or "controllers" isn't helpful.
Comment 21 Alexey Proskuryakov 2011-10-12 12:02:59 PDT
Comment on attachment 110704 [details]
remove unnecessary include

To be clear, my comment is meant to block the patch, marking r- to that effect.

Someone who cares about this feature should work with the WG to make the spec reach some minimal maturity level, like at least having a name that's not actively misleading.
Comment 22 Alexey Proskuryakov 2011-10-12 12:06:35 PDT
> The spec focuses on gaming use-cases, and as such, the most common implementation of the spec will almost surely be gamepads, hence the name. It was generalized to support other devices as there was no reason not be flexible, but calling it something vague like "inputdevices" or "controllers" isn't helpful.

My recollection is that in the webkit-dev discussion, there was consensus that this is a poor name. If you disagree and think that the spec is mature enough to be in WebKit, please post to webkit-dev. That's where we decide what goes in WebKit.
Comment 23 Darin Fisher (:fishd, Google) 2011-10-12 12:46:05 PDT
(In reply to comment #21)
> (From update of attachment 110704 [details])
> To be clear, my comment is meant to block the patch, marking r- to that effect.
> 
> Someone who cares about this feature should work with the WG to make the spec reach some minimal maturity level, like at least having a name that's not actively misleading.

Alexey, I don't think it is appropriate for you to block this patch on the basis you have stated.  You seem to be the one, who doesn't like the name, but you are reluctant to complain about it on the WG.  The folks developing this feature are obviously happy with the name, so it doesn't seem to make sense for them to try to channel you in their discussions on the WG.

From the scope section of the spec:
"Specifically, we choose to only support two categories of input from devices: buttons and axes."

Now, I can actually see why "gamepad" might be an OK name for this feature.  If you think about a gamepad, you immediately imagine those two categories of information.  It just happens to be true that those same two categories of information can be used to express data from other types of input devices.  I don't think that makes the name actively misleading.  Maybe the name could be better.  I always like to ponder better names for things.

Besides, this is all being done behind an ENABLE flag.  No one is trying to make a long-term commitment to this API as it exists today.  We've been very clear that we want to get this out and into the hands of developers so they can give us feedback.  The best way for us to do that is via nightly builds.  The name of the API can and may change.

By the way, would you mind pointing me to the spec for requestFullScreen?
Comment 24 Scott Graham 2011-11-16 16:27:13 PST
Created attachment 115477 [details]
remove custom binding code, add DRT hooks and tests
Comment 25 Scott Graham 2011-11-17 14:09:24 PST
Created attachment 115675 [details]
update Skipped for various ports
Comment 26 Scott Graham 2011-11-21 12:09:37 PST
Created attachment 116114 [details]
remove everything except platform plumbing and DRT/testing
Comment 27 WebKit Review Bot 2011-11-21 12:17:21 PST
Comment on attachment 116114 [details]
remove everything except platform plumbing and DRT/testing

Attachment 116114 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10539023
Comment 28 Darin Fisher (:fishd, Google) 2011-11-21 12:46:19 PST
Comment on attachment 116114 [details]
remove everything except platform plumbing and DRT/testing

It looks like you are going to need to update WebKit/chromium/DEPS to pull in a version of webkit_support.h that provides SetGamepadData.
Comment 29 Scott Graham 2011-11-21 12:58:23 PST
Created attachment 116120 [details]
update WebKit/chromium/DEPS
Comment 30 Scott Graham 2011-11-21 13:11:56 PST
Created attachment 116123 [details]
try a different chromium rev
Comment 31 Scott Graham 2011-11-21 13:32:33 PST
Created attachment 116126 [details]
add apparently newly-needed chromium deps
Comment 32 Adam Barth 2011-11-21 13:42:37 PST
Comment on attachment 116126 [details]
add apparently newly-needed chromium deps

View in context: https://bugs.webkit.org/attachment.cgi?id=116126&action=review

> Source/WebCore/platform/Gamepads.h:35
> +void sampleGamepads(GamepadList* into);

Can we put this in Source/WebCore/platform/gamepad/Gamepads.h ?  I presume there will be more gamepad-related files in platform.

> Source/WebKit/chromium/DEPS:35
> -  'chromium_rev': '109696'
> +  'chromium_rev': '110994'

Please roll Chromium DEPS in a separate patch.

> Source/WebKit/chromium/src/Gamepads.cpp:38
> +namespace WebCore {

Code in the WebCore namespace should be in WebCore.

> Source/WebKit/chromium/src/Gamepads.cpp:48
> +    for (unsigned i = 0; i < WebKit::WebGamepads::itemsLengthCap; ++i) {

Code in the WebCore namespace should not refer to any code in the WebKit namespace.
Comment 33 Scott Graham 2011-11-21 14:22:12 PST
(In reply to comment #32)
> (From update of attachment 116126 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=116126&action=review
> 
> > Source/WebCore/platform/Gamepads.h:35
> > +void sampleGamepads(GamepadList* into);
> 
> Can we put this in Source/WebCore/platform/gamepad/Gamepads.h ?  I presume there will be more gamepad-related files in platform.

I don't think there will be, perhaps a total of 2 files. Would you still prefer it moved?

> 
> > Source/WebKit/chromium/DEPS:35
> > -  'chromium_rev': '109696'
> > +  'chromium_rev': '110994'
> 
> Please roll Chromium DEPS in a separate patch.

OK, https://bugs.webkit.org/show_bug.cgi?id=72911

> 
> > Source/WebKit/chromium/src/Gamepads.cpp:38
> > +namespace WebCore {
> 
> Code in the WebCore namespace should be in WebCore.
> 
> > Source/WebKit/chromium/src/Gamepads.cpp:48
> > +    for (unsigned i = 0; i < WebKit::WebGamepads::itemsLengthCap; ++i) {
> 
> Code in the WebCore namespace should not refer to any code in the WebKit namespace.

Eep, those are due to ignorance about layering (and maybe picking a poor copy/paste source). The goal was to have a prototype in platform for WebCore to available call, and then to have that functionality be implemented by the port. Is that a bad idea?
Comment 34 Adam Barth 2011-11-21 14:27:28 PST
> I don't think there will be, perhaps a total of 2 files. Would you still prefer it moved?

Nope.  That sounds fine.

> > > Source/WebKit/chromium/src/Gamepads.cpp:48
> > > +    for (unsigned i = 0; i < WebKit::WebGamepads::itemsLengthCap; ++i) {
> > 
> > Code in the WebCore namespace should not refer to any code in the WebKit namespace.
> 
> Eep, those are due to ignorance about layering (and maybe picking a poor copy/paste source). The goal was to have a prototype in platform for WebCore to available call, and then to have that functionality be implemented by the port. Is that a bad idea?

The usual approach is for WebCore code in WebCore/platform to call WebKit/chromium code via PlatformSupport.  The PlatformSupport calls then get bounced to the embedder via WebKitPlatformSupport.
Comment 35 Darin Fisher (:fishd, Google) 2011-11-21 14:34:35 PST
(In reply to comment #34)
> > Eep, those are due to ignorance about layering (and maybe picking a poor copy/paste source). The goal was to have a prototype in platform for WebCore to available call, and then to have that functionality be implemented by the port. Is that a bad idea?
> 
> The usual approach is for WebCore code in WebCore/platform to call WebKit/chromium code via PlatformSupport.  The PlatformSupport calls then get bounced to the embedder via WebKitPlatformSupport.

Good catch, Adam!
Comment 36 Scott Graham 2011-11-21 14:52:43 PST
(In reply to comment #34)

> The usual approach is for WebCore code in WebCore/platform to call WebKit/chromium code via PlatformSupport.  The PlatformSupport calls then get bounced to the embedder via WebKitPlatformSupport.

I see, so the usage of WebKitPlatformSupport and translation in Gamepads.cpp should really be in chromium/PlatformSupport.cpp instead, and in fact, Gamepads.h/cpp aren't needed at all.

(It's fine for page/Navigator to call in WebCore/platform/PlatformSupport directly, right?)
Comment 37 Adam Barth 2011-11-21 14:59:28 PST
Usually we have a header and a CPP file in WebCore/platform to that calls into PlatformSupport, but that can be just a call-through if that's all you need.  If you're doing that, you probably want:

WebCore/platform/Gamepads.h
WebCore/platform/chromium/GamespadsChromium.cpp

because calling PlatformSupport is basically Chromium-specific.

I know this looks like a lot of layers without purpose, but the idea is that other ports might implement this functionality in terms of other APIs, such as Mac OS X native APIs.  In that case, they'd have GamepadsMac.mm in WebCore/platform/mac.
Comment 38 Scott Graham 2011-11-21 15:37:55 PST
Created attachment 116151 [details]
fix layering violations
Comment 39 Scott Graham 2011-11-21 15:39:35 PST
Improved? Sorry if I misunderstood still.
Comment 40 Adam Barth 2011-11-21 15:49:10 PST
Comment on attachment 116151 [details]
fix layering violations

The layering stuff looks great.  Thanks!  (I haven't reviewed the details of the gamepad-specific code or the changes to Tools.)
Comment 41 WebKit Review Bot 2011-11-21 16:09:13 PST
Comment on attachment 116151 [details]
fix layering violations

Attachment 116151 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10554019
Comment 42 Scott Graham 2011-11-21 16:19:03 PST
(still (In reply to comment #41)
> (From update of attachment 116151 [details])
> Attachment 116151 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/10554019

(still trying to figure out how to roll chromium/DEPS, things seem kinda broken)
Comment 43 Scott Graham 2011-11-22 15:49:25 PST
Created attachment 116284 [details]
no change, just to EWS again with DEPS rolled
Comment 44 WebKit Review Bot 2011-11-23 04:16:29 PST
Comment on attachment 116284 [details]
no change, just to EWS again with DEPS rolled

Clearing flags on attachment: 116284

Committed r101065: <http://trac.webkit.org/changeset/101065>
Comment 45 WebKit Review Bot 2011-11-23 04:16:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 46 Arun Patole 2013-05-26 22:34:36 PDT
Hi, I know this bug is closed but just want to try posting my question in case anyone is still following:

I would like to know the status of Gamepad API spec and also its implementation status as I see many differences in published version of spec, it's latest editor's draft and its implementation in various browsers.

Some examples,
- Published draft explains navigator.gamepads interface but its replaced by getGamepads() method in editor's draft.
published: http://www.w3.org/TR/gamepad/ latest ED: https://dvcs.w3.org/hg/gamepad/raw-file/default/gamepad.html
-Mozilla wiki talks about GamepadButtonEvent, GamepadAxisEvent, GamepadButtonDown, GamepadButtonDown, GamepadAxisMove. But its not yet there in spec.
https://wiki.mozilla.org/GamepadAPI
-As compared to published draft, editor's draft has some extra attributes in Gamepad interface (attributes: mapping, connected)

Please let me know if the support is still experimental and spec is still being discussed.
Comment 47 Zan Dobersek 2013-06-17 08:00:38 PDT
The specification is still a work in progress.

The WebKit support for any of the specification versions has fell behind. Just for starters, the gamepadconnected and gamepaddisconnected events are not supported in the current implementation.

I believe Blink and Mozilla are most interested in the specification, so their implementations are much more likely to be up to date with the current version of the specification. The two vendors are also most (if not the only) active in standardizing the Gamepad API.