From f13edd43afad268bff86d269ed881a6808fbc496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Thu, 5 May 2016 21:29:42 +0300 Subject: [PATCH 1/2] Only poll gamepad api once per application frame in order to be safe if subsequent polls might change the data under the hood, which might lead to missing button events if they are buffered. See https://github.com/w3c/gamepad/issues/22. Also reduces the amount of garbage that is generated, and the number of times we jump to a call inside the browser if application iterates through multiple gamepads per frame. --- src/library_html5.js | 58 ++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/library_html5.js b/src/library_html5.js index 8e2e5c59d8696..9b18f38fbbbf8 100644 --- a/src/library_html5.js +++ b/src/library_html5.js @@ -13,6 +13,13 @@ var LibraryJSEvents = { visibilityChangeEvent: 0, touchEvent: 0, + // In order to ensure most coherent Gamepad API state as possible (https://github.com/w3c/gamepad/issues/22) and + // to minimize the amount of garbage created, we sample the gamepad state at most once per frame, and not e.g. once per + // each controller or similar. To implement that, the following variables retain a cache of the most recent polled gamepad + // state. + lastGamepadState: null, + lastGamepadStateFrame: null, // The integer value of Browser.mainLoop.currentFrameNumber of when the last gamepad state was produced. + // When we transition from fullscreen to windowed mode, we remember here the element that was just in fullscreen mode // so that we can report information about that element in the event message. previousFullscreenElement: null, @@ -1650,37 +1657,36 @@ var LibraryJSEvents = { return {{{ cDefine('EMSCRIPTEN_RESULT_SUCCESS') }}}; }, - emscripten_get_num_gamepads: function() { - if (!navigator.getGamepads && !navigator.webkitGetGamepads) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}}; - if (navigator.getGamepads) { - return navigator.getGamepads().length; - } else if (navigator.webkitGetGamepads) { - return navigator.webkitGetGamepads().length; + _emscripten_sample_gamepad_data: function() { + // Produce a new Gamepad API sample if we are ticking a new game frame, or if not using emscripten_set_main_loop() at all to drive animation. + if (Browser.mainLoop.currentFrameNumber !== JSEvents.lastGamepadStateFrame || !Browser.mainLoop.currentFrameNumber) { + JSEvents.lastGamepadState = navigator.getGamepads ? navigator.getGamepads() : (navigator.webkitGetGamepads ? navigator.webkitGetGamepads : null); + JSEvents.lastGamepadStateFrame = Browser.mainLoop.currentFrameNumber; } }, + + emscripten_get_num_gamepads__deps: ['_emscripten_sample_gamepad_data'], + emscripten_get_num_gamepads: function() { + __emscripten_sample_gamepad_data(); + if (!JSEvents.lastGamepadState) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}}; + return JSEvents.lastGamepadState.length; + }, + emscripten_get_gamepad_status__deps: ['_emscripten_sample_gamepad_data'], emscripten_get_gamepad_status: function(index, gamepadState) { - if (!navigator.getGamepads && !navigator.webkitGetGamepads) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}}; - var gamepads; - if (navigator.getGamepads) { - gamepads = navigator.getGamepads(); - } else if (navigator.webkitGetGamepads) { - gamepads = navigator.webkitGetGamepads(); - } - if (index < 0 || index >= gamepads.length) { - return {{{ cDefine('EMSCRIPTEN_RESULT_INVALID_PARAM') }}}; - } - // For previously disconnected gamepads there should be a null at the index. + __emscripten_sample_gamepad_data(); + if (!JSEvents.lastGamepadState) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}}; + + // INVALID_PARAM is returned on a Gamepad index that never was there. + if (index < 0 || index >= JSEvents.lastGamepadState.length) return {{{ cDefine('EMSCRIPTEN_RESULT_INVALID_PARAM') }}}; + + // NO_DATA is returned on a Gamepad index that was removed. + // For previously disconnected gamepads there should be an empty slot (null/undefined/false) at the index. // This is because gamepads must keep their original position in the array. - // For example, removing the first of two gamepads produces [null, gamepad]. - // Older implementations of the Gamepad API used undefined instead of null. - // The following check works because null and undefined evaluate to false. - if (!gamepads[index]) { - // There is a "false" but no gamepad at index because it was disconnected. - return {{{ cDefine('EMSCRIPTEN_RESULT_NO_DATA') }}}; - } - // There should be a gamepad at index which can be queried. - JSEvents.fillGamepadEventData(gamepadState, gamepads[index]); + // For example, removing the first of two gamepads produces [null/undefined/false, gamepad]. + if (!JSEvents.lastGamepadState[index]) return {{{ cDefine('EMSCRIPTEN_RESULT_NO_DATA') }}}; + + JSEvents.fillGamepadEventData(gamepadState, JSEvents.lastGamepadState[index]); return {{{ cDefine('EMSCRIPTEN_RESULT_SUCCESS') }}}; }, From cb492d83f490c7bcf502bcf4448bc02480cbcf60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Thu, 5 May 2016 21:37:52 +0300 Subject: [PATCH 2/2] Test against EMSCRIPTEN_RESULT_SUCCESS in test_html5.c for gamepad state query to be extra precise. --- tests/test_html5.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_html5.c b/tests/test_html5.c index ece7dd8f09ecc..ad8fba431b410 100644 --- a/tests/test_html5.c +++ b/tests/test_html5.c @@ -264,8 +264,8 @@ void mainloop() for(int i = 0; i < numGamepads && i < 32; ++i) { EmscriptenGamepadEvent ge; - int failed = emscripten_get_gamepad_status(i, &ge); - if (!failed) + int ret = emscripten_get_gamepad_status(i, &ge); + if (ret == EMSCRIPTEN_RESULT_SUCCESS) { int g = ge.index; for(int j = 0; j < ge.numAxes; ++j)