Bug 212933 - GameController.framework gamepads should support Home buttons
Summary: GameController.framework gamepads should support Home buttons
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: Safari 13
Hardware: iPhone / iPad iOS 13
: P2 Major
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-08 16:02 PDT by goehdavi
Modified: 2020-07-10 17:09 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.05 KB, patch)
2020-07-10 09:06 PDT, Brady Eidson
thorton: review+
Details | Formatted Diff | Diff
PFL (6.12 KB, patch)
2020-07-10 15:31 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description goehdavi 2020-06-08 16:02:23 PDT
Testing on iOS and iPadOS 13.4.1 has revealed that Xbox and PS4 controllers are detected and input events are received over Bluetooth, but some buttons are missing and D-Pad buttons are received as axes input. Looking at the WebKit GameControllerGamepad (https://github.com/WebKit/webkit/blob/89c28d471fae35f1788a0f857067896a10af8974/Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm) implementation on GitHub it appears that the WebKit Game Controller Framework (https://developer.apple.com/documentation/gamecontroller) integration doesn’t include the new controller buttons added to GCF last year (https://developer.apple.com/videos/play/wwdc2019/616/). For instance, buttonMenu (https://developer.apple.com/documentation/gamecontroller/gcextendedgamepad/3237238-buttonmenu) appears to be missing. Can WebKit fix the the DPad and add the additional buttons?
Comment 1 Alexey Proskuryakov 2020-06-08 17:13:52 PDT
rdar://problem/63500696
Comment 2 Justin Uberti 2020-06-20 19:03:13 PDT
This looks like a pretty straightforward change. Is any work planned here? If not, happy to contribute a patch.
Comment 3 goehdavi 2020-06-30 11:39:33 PDT
Looks like the changes have been made: https://github.com/WebKit/webkit/blob/master/Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm#L100. However, it still doesn't appear to work on Safari on iOS 13.5.1 (testing with https://html5gamepad.com/). When will this be deployed to iOS and iPadOS?
Comment 4 Justin Uberti 2020-06-30 12:10:19 PDT
Agreed, also doesn't work in Safari Mac 14.0, or in Safari iPadOS 14 beta 1.
Comment 5 Brady Eidson 2020-07-01 09:02:10 PDT
(In reply to goehdavi from comment #3)
> When will this be deployed to iOS and iPadOS?

That's a question beyond the WebKit open source project - Apple releases those products, and generally doesn't comment on future releases, etc.

(In reply to Justin Uberti from comment #4)
> Agreed, also doesn't work in Safari Mac 14.0, or in Safari iPadOS 14 beta 1.

These are two different issues.

1st is that macOS WebKit doesn't use GC framework.

2nd is that there's a known issues in another system component preventing all Gamepads from work on iOS 14 beta 1.
Comment 6 Justin Uberti 2020-07-01 12:40:22 PDT
Thanks, I understand that there are a number of moving parts and organizational complexities here, and appreciate the clarification regarding the root cause of the iOS 14b1 issues.

Anything you can share regarding GC provider on macOS?
Comment 7 Brady Eidson 2020-07-10 08:57:22 PDT
This bug got way overloaded with lots of various Gamepad issues.

I'm going back to the original report:

"Gamepad support on iOS doesn't support the new GameControllerFramework buttons like Menu and Options"

That support was added in https://trac.webkit.org/changeset/256215/webkit, and I believe is working in iOS 14 b2 (which fixed the general "Gamepads aren't working" bug)

However, there's yet-another-button that would be great to support and that's the Home button.

So let's repurpose this bug for that.

Whoever feels like other things discussed here still deserve discussion, please make separate bugzillas for separate issues.
Comment 8 Brady Eidson 2020-07-10 09:06:43 PDT
Created attachment 403971 [details]
Patch
Comment 9 Tim Horton 2020-07-10 11:57:38 PDT
Comment on attachment 403971 [details]
Patch

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

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:57
> +static GCControllerButtonInput *homeButtonFromExtendedGamepad(GCExtendedGamepad *gamepad)

I don't love the `valueForKey`s. Is there an SPI property you can use? Is the string available anywhere?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:78
> +    m_buttonValues.resize(homeButton ? 17 : 16);

These magic numbers are pretty weird!

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:114
> +        bindButton(homeButton, 16);

Do the numbers match something in another project, or are they ours? If they're ours, can we structure this in a less bizarre way?
Comment 10 Brady Eidson 2020-07-10 14:46:56 PDT
(In reply to Tim Horton from comment #9)
> Comment on attachment 403971 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403971&action=review
> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:57
> > +static GCControllerButtonInput *homeButtonFromExtendedGamepad(GCExtendedGamepad *gamepad)
> 
> I don't love the `valueForKey`s. Is there an SPI property you can use? Is
> the string available anywhere?

For the underscored version, this is it.
For the non-underscored I can do something different.

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:78
> > +    m_buttonValues.resize(homeButton ? 17 : 16);
> 
> These magic numbers are pretty weird!

They are from the "standard layout" of the Gamepad API
> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:114
> > +        bindButton(homeButton, 16);
> 
> Do the numbers match something in another project, or are they ours?

They're ours (Gamepad API spec)

> If they're ours, can we structure this in a less bizarre way?

The whole thing could be structured more better. I'll fix it up in a separate patch.
Comment 11 Brady Eidson 2020-07-10 15:31:49 PDT
Created attachment 404010 [details]
PFL
Comment 12 EWS 2020-07-10 16:34:49 PDT
Committed r264246: <https://trac.webkit.org/changeset/264246>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404010 [details].
Comment 13 Brady Eidson 2020-07-10 17:09:32 PDT
(In reply to Brady Eidson from comment #10)
> (In reply to Tim Horton from comment #9)
> > Do the numbers match something in another project, or are they ours?
> 
> They're ours (Gamepad API spec)
> 
> > If they're ours, can we structure this in a less bizarre way?
> 
> The whole thing could be structured more better. I'll fix it up in a
> separate patch.

Fixing over in https://bugs.webkit.org/show_bug.cgi?id=214210