Implement glfw joystick API #5175

satoshinm · 2017-04-29T23:08:54Z

Adds support for glfw3's joystick functions: http://www.glfw.org/docs/latest/input_guide.html#joystick

Includes a test based on the sdl joystick test:

python tests/runner.py browser.test_glfw_joystick

Also manually tested with a real joystick and a more complex real-world application which had native glfw joystick support (satoshinm/NetCraft#80)

Add _free()s on gamepaddisconnect, .id .buttons
Add _free()s on gamepaddisconnect, .axes
Incorporate most review feedback
Browser test: python tests/runner.py browser.test_glfw_joystick
Interactive test: python tests/runner.py interactive.test_glfw_joystick
Test versus emscripten gamepad API, and native glfw API
Poll once per frame, see gamepad_only_once_per_frame #4292

satoshinm · 2017-04-29T23:10:49Z

This is functional but is it ok to refresh the joystick status the calls to glfwGetJoystickAxes and glfwGetJoystickButtons? Wasn't sure if the joystick state should be read on each frame instead. Also do I need to free the memory allocated for the axes and buttons arrays (from allocate, couldn't find out how to free it)

kripken · 2017-05-01T21:33:15Z

I'm not sure about the refreshing, but looks like it might be fast enough. I guess if you don't see it show up in profiling, it's probably fine. Or was the concern something other than speed?

For the allocations, ALLOC_NORMAL uses malloc, so you can do _free(..).

juj · 2017-05-03T13:29:07Z

src/library_glfw.js

@@ -57,6 +56,9 @@ var LibraryGLFW = {
      };
      this.buttons = 0;
      this.keys = new Array();
+      this.joyLast = 0; // GLFW_JOYSTICK_1
+      this.joys = {};
+      this.index2joy = {};


Can you document here what is the purpose of these three variables. The name index2joy is a bit ad hoc, it would be good to have some details here.

juj · 2017-05-03T13:34:46Z

src/library_glfw.js

@@ -382,6 +385,38 @@ var LibraryGLFW = {
 #endif
    },

+    onGamepadConnected: function(event) {
+      if (!GLFW.active.joystickFunc) return;


I wonder if GLFW should be tracking gamepads already before the user code has registered a joystick callback function. For example, it can take a while until the user code runs, and a gamepad connected event may have fired already before. Or perhaps the tightest way would be to enumerate all already known joysticks when a callback is registered or GLW initialized to initially populate the already known joysticks. I'm thinking about the scenario where user is clicking on gamepad buttons way before the page has fully loaded up.

juj · 2017-05-03T13:35:44Z

src/library_glfw.js

+
+      var joy = GLFW.active.index2joy[event.gamepad.index];
+
+      Module['dynCall_vii'](GLFW.active.joystickFunc, joy, 0x00040002); // GLFW_DISCONNECTED


What if joy is undefined here? I suppose the callback should not be called then if such an odd situation arises?

juj · 2017-05-03T13:37:39Z

src/library_glfw.js

+
+      Module['dynCall_vii'](GLFW.active.joystickFunc, joy, 0x00040002); // GLFW_DISCONNECTED
+
+      // TODO: free memory .axes, .buttons


This can do _free(GLFW.active.joys[joy].id); and same for buttons and axes. (ALLOC_NORMAL is equivalent to a _malloc())

juj · 2017-05-03T13:38:24Z

src/library_glfw.js

+    },
+
+    onGamepadDisconnected: function(event) {
+      if (!GLFW.active.joystickFunc) return;


Here also if user code has already detached a user callback function, the code below should still run to deinitialize the data for that joystick?

juj · 2017-05-03T13:50:17Z

src/library_glfw.js

+      var j = GLFW.active.joys[joy];
+      if (!j) return;
+
+      var gamepad = navigator.getGamepads()[j.index];


Reading the documentation at http://www.glfw.org/docs/latest/group__input.html#gace54cd930dcd502e118fe4021384ce1b and http://www.glfw.org/docs/latest/input_guide.html#joystick_button, it looks like a suitable usage from user code might be to do a

for(int i = 0; i < numJoysticks; ++i) { int count; const unsigned char* axes = glfwGetJoystickButtons(GLFW_JOYSTICK_1+i, &count); for(int j = 0; j < count; ++j) { // process axes[j]; } }

In that case, there is a possible subtle bug due to an unspecced feature of the Gamepad API regarding navigator.getGamepads(), see w3c/gamepad#22 and w3c/gamepad#8. The issue is that navigator.getGamepads() polls new state for all gamepads, but the above line only reads data from one of them. So if there are e.g. 3 joysticks, the above loop would poll the state of each of the three joysticks three times, and process only one of them at each loop iteration. This might potentially miss out button presses, but whether that happens in practice is somewhat theoretical. Still good to keep in mind though.

See #4292 for a similar change that was done for the HTML5 API counterpart, and also #4921 which might be of interest.

juj · 2017-05-03T13:57:23Z

src/library_glfw.js

+  },
+
+  glfwGetJoystickName: function(joy) {
+    return GLFW.active.joys[joy].id;


If joystick is disconnected right before this function is called, GLFW.active.joys[joy] could be deleted above and be undefined, making this function throw an exception on undefined.id access; so check for if (GLFW.active.joys[joy]) here before proceeding?

juj · 2017-05-03T13:59:05Z

tests/test_browser.py

@@ -1076,6 +1076,53 @@ def test_sdl_joystick_2(self):
    Popen([PYTHON, EMCC, os.path.join(self.get_dir(), 'sdl_joystick.c'), '-O2', '--minify', '0', '-o', 'page.html', '--pre-js', 'pre.js', '-lSDL', '-lGL']).communicate()
    self.run_browser('page.html', '', '/report_result?2')

+  def test_glfw_joystick(self):


Great to have a mock input injecting test! It would be nice to add the same test to the interactive suite as well, but without the API mock implementation part, so that could be tested with a live gamepad.

Added a simple interactive test (run with python tests/runner.py interactive.test_glfw_joystick), no graphics it only logs text of the gamepad state changes but useful for testing (already found & fixed the axes memory allocation error thanks to it)

As opposed to the glfw joysticks, which of course are bridged to the same HTML5 Gamepad API on the web build, but, there may be issues so adding USE_EM_GAMEPAD to switch back (off by default) for testing. This was previously removed in #80 Use glfw joystick everywhere, remove emscripten gamepad due to including an early version of emscripten-core/emscripten#5175 [WIP] Implement glfw joystick API

This reverts commit afc7831.

satoshinm · 2017-05-06T03:54:22Z

Incorporated most (but not all) of the review feedback. Next major change needed is to poll the gamepad state once per frame like in #4292. This should also solve the issues I'm seeing on Chrome where gamepadconnected is fired on disconnect. Wondering if it is worth preserving this API nuance of glfw:

      // HTML5 Gamepad API reuses indexes
      // glfw joystick API spreads them out
      // http://www.glfw.org/docs/latest/input_guide.html#joystick
      // "Once a joystick is detected, it keeps its assigned index until it is disconnected,
      // so as joysticks are connected and disconnected, they will become spread out."

Also I added _free() but not to _free(GLFW.active.joys[joy].axes);, crashes in abort() on corrupted memory, need to investigate more what went wrong there...

GLFW_JOYSTICK_1 is 0, so HTML5 Gamepad index 0 maps to GLFW_JOYSTICK_1. Note: The API subtlety of spreading out glfw joystick IDs is lost.

…-coreGH-4292 Joystick connected/disconnected callbacks are also now called in refreshJoysticks(). The gamepadconnected/disconnected events are only used as a hint to refresh the joysticks, whose getGamepads() state is compared to the previous state to determine the new state.

satoshinm · 2017-05-06T07:33:42Z

Changed to sample once per frame, tracking joystick connect/disconnect on refresh, and fixed the axes memory freeing. I think that's everything, anything else?

satoshinm · 2017-05-16T04:59:33Z

@kripken @juj can this be merged or anything I need to change first?

kripken · 2017-05-16T16:50:20Z

This looks good to me, but let's wait a bit more for @juj.

kripken · 2017-05-18T16:37:37Z

Looks like @juj has no immediate concerns here, so let's merge this, any further stuff can be done in followup work.

Implement glfw joystick API

c0b34ed

satoshinm mentioned this pull request Apr 29, 2017

glfw joystick satoshinm/emscripten#1

Closed

Guard against refreshing non-existent joysticks

54c9c31

satoshinm changed the title ~~Implement glfw joystick API~~ [WIP] Implement glfw joystick API May 3, 2017

juj reviewed May 3, 2017

View reviewed changes

satoshinm added 7 commits May 5, 2017 01:16

Free allocated memory when gamepads are disconnected

afc7831

Clarify state variables purposes, rename to gamepad2joy

a4e3627

Track joysticks even when glfw callback not registered

c638c04

Revert "Free allocated memory when gamepads are disconnected"

52241ed

This reverts commit afc7831.

Fix allocation fo buttons (i8) and axes (float)

a7bd8b1

Free id and buttons, check buttons/axes count vs allocated

45f66b3

glfwGetJoystickName() on invalid joysticks, return null

2d0032e

satoshinm added 7 commits May 5, 2017 20:59

Remove glfw joy <-> HTML5 Gamepad API index translation, use HTML5

ed23211

GLFW_JOYSTICK_1 is 0, so HTML5 Gamepad index 0 maps to GLFW_JOYSTICK_1. Note: The API subtlety of spreading out glfw joystick IDs is lost.

glfwJoystickPresent() refreshes joysticks

8300dfc

test_glfw_joystick: remove mainloop, allow always refreshing joysticks

b1ee33d

Joystick state is a property of GLFW, not per-window

8d7dd9a

Fix gamepad id string allocation

49e61eb

Add interactive glfw joystick test: interactive.test_glfw_joystick

7a1617e

satoshinm added 2 commits May 5, 2017 23:19

Fix axes float array allocation size and _free

af1e5b8

Setting glfw joystick callback refreshes joysticks

bc89073

satoshinm changed the title ~~[WIP] Implement glfw joystick API~~ Implement glfw joystick API May 6, 2017

satoshinm mentioned this pull request May 8, 2017

Analog joystick buttons glfw/glfw#1008

Closed

kripken merged commit 4f42f6b into emscripten-core:incoming May 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement glfw joystick API #5175

Implement glfw joystick API #5175

satoshinm commented Apr 29, 2017 •

edited

Loading

satoshinm commented Apr 29, 2017

kripken commented May 1, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

juj May 3, 2017

satoshinm May 6, 2017

satoshinm commented May 6, 2017

satoshinm commented May 6, 2017

satoshinm commented May 16, 2017

kripken commented May 16, 2017

kripken commented May 18, 2017


		var joy = GLFW.active.index2joy[event.gamepad.index];

		Module['dynCall_vii'](GLFW.active.joystickFunc, joy, 0x00040002); // GLFW_DISCONNECTED


		Module['dynCall_vii'](GLFW.active.joystickFunc, joy, 0x00040002); // GLFW_DISCONNECTED

		// TODO: free memory .axes, .buttons

Implement glfw joystick API #5175

Implement glfw joystick API #5175

Conversation

satoshinm commented Apr 29, 2017 • edited Loading

satoshinm commented Apr 29, 2017

kripken commented May 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satoshinm commented May 6, 2017

satoshinm commented May 6, 2017

satoshinm commented May 16, 2017

kripken commented May 16, 2017

kripken commented May 18, 2017

satoshinm commented Apr 29, 2017 •

edited

Loading