RESOLVED FIXED 161086
GameController.framework backend for gamepad API
https://bugs.webkit.org/show_bug.cgi?id=161086
Summary GameController.framework backend for gamepad API
Brady Eidson
Reported 2016-08-23 09:37:27 PDT
GameController.framework backend for gamepad API This will be necessary for iOS support
Attachments
Patch (30.84 KB, patch)
2016-08-24 17:41 PDT, Brady Eidson
no flags
Patch (53.73 KB, patch)
2016-08-25 10:13 PDT, Brady Eidson
no flags
Patch (53.88 KB, patch)
2016-08-25 11:22 PDT, Brady Eidson
no flags
Patch (53.98 KB, patch)
2016-08-25 12:18 PDT, Brady Eidson
no flags
Patch (54.01 KB, patch)
2016-08-25 12:48 PDT, Brady Eidson
no flags
Patch (55.96 KB, patch)
2016-08-30 14:52 PDT, Brady Eidson
achristensen: review+
Brady Eidson
Comment 1 2016-08-23 09:37:58 PDT
And will include getting the tests working on iOS-sim
Brady Eidson
Comment 2 2016-08-24 17:41:44 PDT
Created attachment 286923 [details] Patch Attaching WIP patch. Still some cleanup to do in WebCore, and the WK2 part is obviously a hack just to let me test it... ...but it works as is!
Brady Eidson
Comment 3 2016-08-25 10:08:28 PDT
(In reply to comment #1) > And will include getting the tests working on iOS-sim Not actually true - Filing a new separate bug for that.
Brady Eidson
Comment 4 2016-08-25 10:09:16 PDT
(In reply to comment #3) > (In reply to comment #1) > > And will include getting the tests working on iOS-sim > > Not actually true - Filing a new separate bug for that. Filed https://bugs.webkit.org/show_bug.cgi?id=161198 for that.
Brady Eidson
Comment 5 2016-08-25 10:13:08 PDT
WebKit Commit Bot
Comment 6 2016-08-25 10:22:37 PDT
Attachment 286979 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:61: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:85: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:88: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:94: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:97: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:100: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:103: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:106: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:110: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:113: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:116: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:119: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:122: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:125: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:136: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:144: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:147: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:150: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:153: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:156: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:159: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:163: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:166: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 24 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brady Eidson
Comment 7 2016-08-25 11:22:14 PDT
WebKit Commit Bot
Comment 8 2016-08-25 11:24:56 PDT
Attachment 286988 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:60: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:84: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:87: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:90: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:93: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:96: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:102: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:105: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:109: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:112: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:115: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:118: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:121: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:124: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:135: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:143: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:146: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:149: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:152: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:155: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:158: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:162: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:165: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 24 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brady Eidson
Comment 9 2016-08-25 12:18:26 PDT
WebKit Commit Bot
Comment 10 2016-08-25 12:22:14 PDT
Attachment 286994 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:60: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:84: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:87: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:90: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:93: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:96: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:102: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:105: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:109: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:112: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:115: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:118: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:121: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:124: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:135: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:143: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:146: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:149: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:152: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:155: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:158: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:162: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:165: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 24 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brady Eidson
Comment 11 2016-08-25 12:48:09 PDT
WebKit Commit Bot
Comment 12 2016-08-25 12:52:51 PDT
Attachment 286996 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:60: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:84: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:87: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:90: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:93: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:96: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:102: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:105: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:109: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:112: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:115: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:118: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:121: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:124: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:135: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:143: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:146: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:149: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:152: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:155: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:158: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:162: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:165: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 24 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Simon Fraser (smfr)
Comment 13 2016-08-25 17:36:23 PDT
Comment on attachment 286996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286996&action=review > Source/WebCore/ChangeLog:16 > + * platform/gamepad/cocoa/GameControllerGamepad.h: Copied from Source/WebKit2/UIProcess/Gamepad/mac/UIGamepadProviderHID.cpp. > + * platform/gamepad/cocoa/GameControllerGamepad.mm: Added. > + (WebCore::GameControllerGamepad::GameControllerGamepad): > + (WebCore::GameControllerGamepad::setupAsExtendedGamepad): > + (WebCore::GameControllerGamepad::setupAsGamepad): Do these actually get used by the UI process? Seems a bit weird for WK2 code to be conjuring up classes from WebCore/platform. > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:28 > +#if ENABLE(GAMEPAD) && defined(__LP64__) Shouldn't ENABLE(GAMEPAD) only be set in 64-bit, via xcconfig files? > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:45 > + GameControllerGamepad(GCController *, unsigned index); Gamepads are identified by index? What happens when they plug and unplug? > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:82 > + m_axisValues.resize(6); > + m_buttonValues.resize(8); > + > + m_buttonValues[0] = m_extendedGamepad.get().buttonA.value; > + m_buttonValues[1] = m_extendedGamepad.get().buttonB.value; > + m_buttonValues[2] = m_extendedGamepad.get().buttonX.value; > + m_buttonValues[3] = m_extendedGamepad.get().buttonY.value; > + m_buttonValues[4] = m_extendedGamepad.get().leftShoulder.value; > + m_buttonValues[5] = m_extendedGamepad.get().rightShoulder.value; > + m_buttonValues[6] = m_extendedGamepad.get().leftTrigger.value; > + m_buttonValues[7] = m_extendedGamepad.get().rightTrigger.value; > + > + m_axisValues[0] = m_extendedGamepad.get().leftThumbstick.xAxis.value; > + m_axisValues[1] = m_extendedGamepad.get().leftThumbstick.yAxis.value; > + m_axisValues[2] = m_extendedGamepad.get().rightThumbstick.xAxis.value; > + m_axisValues[3] = m_extendedGamepad.get().rightThumbstick.yAxis.value; > + m_axisValues[4] = m_extendedGamepad.get().dpad.xAxis.value; > + m_axisValues[5] = m_extendedGamepad.get().dpad.yAxis.value; Cognitive dissonance that you resize m_axisValues first, then fill it second. > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:86 > + m_extendedGamepad.get().buttonA.valueChangedHandler = ^(GCControllerButtonInput *, float value, BOOL) { > + m_buttonValues[0] = value; > + }; Are these callbacks called on the main thread? Is there really one per button? > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:136 > + m_lastUpdateTime = monotonicallyIncreasingTime(); Performance.now() time? > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86 > + Timer m_inputNotificationTimer; I don't think Timers work in the UI process. > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:37 > +static const double InputNotificationDelay = 0.05; What units? > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:51 > +void GameControllerGamepadProvider::controllerDidConnect(GCController *controller, ConnectionVisibility visibility) Can GCController be a reference? > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:64 > + > + Two lines. > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:76 > +void GameControllerGamepadProvider::controllerDidDisconnect(GCController *controller) Ditto. > Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:297 > + WebKit::UIGamepadProvider::setUsesGameControllerFramework(); What is happening here?
Brady Eidson
Comment 14 2016-08-27 22:16:47 PDT
(In reply to comment #13) > Comment on attachment 286996 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286996&action=review > > > Source/WebCore/ChangeLog:16 > > + * platform/gamepad/cocoa/GameControllerGamepad.h: Copied from Source/WebKit2/UIProcess/Gamepad/mac/UIGamepadProviderHID.cpp. > > + * platform/gamepad/cocoa/GameControllerGamepad.mm: Added. > > + (WebCore::GameControllerGamepad::GameControllerGamepad): > > + (WebCore::GameControllerGamepad::setupAsExtendedGamepad): > > + (WebCore::GameControllerGamepad::setupAsGamepad): > > Do these actually get used by the UI process? Seems a bit weird for WK2 code > to be conjuring up classes from WebCore/platform. Yes they do, and that's not weird. > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:28 > > +#if ENABLE(GAMEPAD) && defined(__LP64__) > > Shouldn't ENABLE(GAMEPAD) only be set in 64-bit, via xcconfig files? ENABLE(GAMEPAD) works fine in 32 bit with the IOHID backend. > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:45 > > + GameControllerGamepad(GCController *, unsigned index); > > Gamepads are identified by index? Yes. > What happens when they plug and unplug? When you unplug, that index is freed. e.g. Plug-in two gamepads, indices 0 and 1 are taken. Unplug gamepad 0, index 0 is now null. Plug in a new gamepad, it takes over index 0. This is specced behavior we already implement in IOHID and the other browsers implement. > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:82 > > + m_axisValues.resize(6); > > + m_buttonValues.resize(8); > > + > > + m_buttonValues[0] = m_extendedGamepad.get().buttonA.value; > > + m_buttonValues[1] = m_extendedGamepad.get().buttonB.value; > > + m_buttonValues[2] = m_extendedGamepad.get().buttonX.value; > > + m_buttonValues[3] = m_extendedGamepad.get().buttonY.value; > > + m_buttonValues[4] = m_extendedGamepad.get().leftShoulder.value; > > + m_buttonValues[5] = m_extendedGamepad.get().rightShoulder.value; > > + m_buttonValues[6] = m_extendedGamepad.get().leftTrigger.value; > > + m_buttonValues[7] = m_extendedGamepad.get().rightTrigger.value; > > + > > + m_axisValues[0] = m_extendedGamepad.get().leftThumbstick.xAxis.value; > > + m_axisValues[1] = m_extendedGamepad.get().leftThumbstick.yAxis.value; > > + m_axisValues[2] = m_extendedGamepad.get().rightThumbstick.xAxis.value; > > + m_axisValues[3] = m_extendedGamepad.get().rightThumbstick.yAxis.value; > > + m_axisValues[4] = m_extendedGamepad.get().dpad.xAxis.value; > > + m_axisValues[5] = m_extendedGamepad.get().dpad.yAxis.value; > > Cognitive dissonance that you resize m_axisValues first, then fill it second. Maybe I misunderstand... Are you suggesting reserveInitialCapacity with uncheckedAppend? Because while I might be able to do that in this patch, I would immediately have to revert back to this in a near-future patch, where the number of elements might dynamically change. > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:86 > > + m_extendedGamepad.get().buttonA.valueChangedHandler = ^(GCControllerButtonInput *, float value, BOOL) { > > + m_buttonValues[0] = value; > > + }; > > Are these callbacks called on the main thread? Yes > Is there really one per button? Yup. > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:136 > > + m_lastUpdateTime = monotonicallyIncreasingTime(); > > Performance.now() time? I'm deferring until https://bugs.webkit.org/show_bug.cgi?id=161150, as that will require changing more than just the platform layers. > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86 > > + Timer m_inputNotificationTimer; > > I don't think Timers work in the UI process. Hmmmm... Weird. The HIDGamepadProvider also uses them and works just fine in the UI process. What problem should I be on the lookout for? > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:37 > > +static const double InputNotificationDelay = 0.05; > > What units? Will update with std::chrono. > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:51 > > +void GameControllerGamepadProvider::controllerDidConnect(GCController *controller, ConnectionVisibility visibility) > > Can GCController be a reference? Probably! > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:64 > > + > > + > > Two lines. > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:76 > > +void GameControllerGamepadProvider::controllerDidDisconnect(GCController *controller) > > Ditto. > > > Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:297 > > + WebKit::UIGamepadProvider::setUsesGameControllerFramework(); > > What is happening here? Allows an SPI client (such as MiniBrowser) tell WK2 on Mac to use GameController.framework instead of the default IOHID. Critical for testing.
Brady Eidson
Comment 15 2016-08-27 22:18:18 PDT
(In reply to comment #14) > (In reply to comment #13) > > Comment on attachment 286996 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=286996&action=review > > > > > Source/WebCore/ChangeLog:16 > > > + * platform/gamepad/cocoa/GameControllerGamepad.h: Copied from Source/WebKit2/UIProcess/Gamepad/mac/UIGamepadProviderHID.cpp. > > > + * platform/gamepad/cocoa/GameControllerGamepad.mm: Added. > > > + (WebCore::GameControllerGamepad::GameControllerGamepad): > > > + (WebCore::GameControllerGamepad::setupAsExtendedGamepad): > > > + (WebCore::GameControllerGamepad::setupAsGamepad): > > > > Do these actually get used by the UI process? Seems a bit weird for WK2 code > > to be conjuring up classes from WebCore/platform. > > Yes they do, and that's not weird. I meant to expand on this before saving the comment: It's not weird in that this is certainly not the first use, and there's no real reason I'm aware of that it shouldn't be done.
Tim Horton
Comment 16 2016-08-28 17:36:04 PDT
> > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86 > > > + Timer m_inputNotificationTimer; > > > > I don't think Timers work in the UI process. > > Hmmmm... Weird. The HIDGamepadProvider also uses them and works just fine in > the UI process. > > What problem should I be on the lookout for? It works, but you shouldn't, because of WK1/WK2 coexistence issues (web thread vs main thread confusion IIRC). Use RunLoop timer instead, like you'll notice all of the WK2 UI process code does.
Brady Eidson
Comment 17 2016-08-30 14:33:13 PDT
(In reply to comment #14) > (In reply to comment #13)> > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:51 > > > +void GameControllerGamepadProvider::controllerDidConnect(GCController *controller, ConnectionVisibility visibility) > > > > Can GCController be a reference? > > Probably! Update: GCController is an Objective-C object from a framework - Should not be passed around as a C++ reference.
Brady Eidson
Comment 18 2016-08-30 14:39:12 PDT
(In reply to comment #16) > > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86 > > > > + Timer m_inputNotificationTimer; > > > > > > I don't think Timers work in the UI process. > > > > Hmmmm... Weird. The HIDGamepadProvider also uses them and works just fine in > > the UI process. > > > > What problem should I be on the lookout for? > > It works, but you shouldn't, because of WK1/WK2 coexistence issues (web > thread vs main thread confusion IIRC). Use RunLoop timer instead, like > you'll notice all of the WK2 UI process code does. Changing the WebCore::Timer over in this patch, and filed https://bugs.webkit.org/show_bug.cgi?id=161407 for the IOHID provider.
Brady Eidson
Comment 19 2016-08-30 14:52:09 PDT
Brady Eidson
Comment 20 2016-08-30 14:52:43 PDT
This new patch addresses all of Simon's feedback.
Brady Eidson
Comment 21 2016-08-30 15:26:58 PDT
Note You need to log in before you can comment on or make changes to this bug.