Add extended buttons to support trackpad (#191) #196

xingri · 2024-03-05T17:20:59Z

Closes #191

The following tasks have been completed:

Modified Web platform tests (link to pull request)

Implementation commitment:

WebKit (https://bugs.webkit.org/show_bug.cgi?id=266293)
Chromium (https://issues.chromium.org/issues/339841686)
Gecko (https://bugzilla.mozilla.org/show_bug.cgi?id=)

Preview | Diff

nondebug · 2024-03-06T02:03:50Z

extensions.html

@@ -649,6 +649,168 @@ <h2>
        </p>
      </dd>
    </section>
+    <section>


Let's define extended buttons in the main spec document (index.html). The extensions document is not officially part of the spec and we want to eventually remove it.

The spec PR should extend GamepadButton to add a new attribute (perhaps called GamepadButton.name) and an algorithm for the getter. The type of the new attribute will be a new enum (perhaps called GamepadButtonName). The enum type should be defined in the spec along with all its values.

Add steps to initialize the new attribute when initializing buttons. The steps should define what to do if a gamepad is missing a button defined in the Standard Gamepad. I think it makes sense to use a special value for these buttons so applications can identify which buttons are not actually mapped. (Ideally we'd set the button to null, but this would break existing apps which don't check for null.)

Also, initialization steps should define what to do if a gamepad has a button that doesn't have a predefined Standard Gamepad index or enum value. Do we enumerate it? If so, what should the new attribute's value be? Perhaps we can add a special "unrecognized" value for this.

nondebug · 2024-03-06T02:11:12Z

extensions.html

+      </h2>
+      <p>
+        This section defines extended gamepad buttons mapping beyonds the Standard Gamepads layout,
+        but are commonly found on some gamepad models .The <code>_ext</code> suffix is used to indicate


The _ext suffix comes from OpenXR's input subpaths specification:

https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#_adding_input_sources_via_extensions

We should decide whether this attribute uses OpenXR input subpaths or something new that we want to define for ourselves. If we're using OpenXR subpaths then we should explicitly link to the OpenXR spec which explains how to extend it. If we're only using it for inspiration then I think we should drop the _ext suffix.

nondebug · 2024-03-06T02:16:12Z

extensions.html

+            <td>Standard: Center button in center cluster</td>
+          </tr>
+          <tr>
+            <td>buttons[17]</td>


This suggests the "trackpad" button can only appear at buttons[17]. My thinking is that we should not continue the current pattern of assigning meaning to button indices, otherwise the buttons array will be very sparse since most gamepads do not share the same extra buttons. Instead of using button indices to identify buttons, apps can check the new enum attribute.

For this non-normative section, I think it will be clearer if we provide a separate table for each gamepad instead of trying to combine them all into a single table.

@nondebug Thanks for the review. Will update the PR based on the feedback shortly.

@nondebug I have updated the PR based on your feedback. I have added the enum for identifying the gamepad buttons. Please review this changes and share your opinions.

nondebug · 2024-03-14T00:23:34Z

index.html

+          </pre>
+          <dl>
+            <dt>
+              <dfn>`"extended"`</dfn>


Introducing a new value for GamepadMappingType will break applications that currently check for "standard". I don't think it's necessary to add a new enum value for this, applications can discover whether the gamepad supports extra buttons by inspecting the buttons array.

nondebug · 2024-03-14T00:26:12Z

index.html

+          </p>
+          <pre class="idl">
+            partial interface GamepadButton {
+              readonly attribute GamepadButtonType type;


Please move the definition to the main interface GamepadButton IDL section.

nondebug · 2024-03-14T00:31:53Z

index.html

+            </tr>
+            <tr>
+              <td>
+                <dfn data-dfn-for="GamepadButton">[[\type]]</dfn>


I don't think we need an internal slot.

For other attributes we use a slot because the attribute is read-only but the implementation needs to be able to change the value. In this case the value should never change after initialization.

nondebug · 2024-03-14T00:33:15Z

index.html

+	  </table>
+          <dl data-dfn-for="GamepadButton">
+          <dt>
+            <dfn>type</dfn>


<dfn>type</dfn> attribute

nondebug · 2024-03-14T00:56:31Z

index.html

+            <dfn>GamepadButtonType</dfn> Enum
+          </h3>
+          <p>
+            This enum defines the set of possible botton types.


botton -> button

nondebug · 2024-03-14T01:01:28Z

index.html

+              "",  /* unknown, or not applicable */
+              "trackpad",
+              "share",
+              "..."


Omit "..." since it is not a valid enum value.

nondebug · 2024-03-14T01:06:26Z

sony_dualshock4_gamepad.svg

The diagrams are difficult to read. For clarity I think we should provide this information as a table instead of a diagram.

nondebug · 2024-03-14T01:15:51Z

index.html

+            <dfn>type</dfn>
+          </dt>
+          <dd>
+            An enumeration, {{GamepadButtonType}}, that indicates which button types the controller


The description for the type attribute needs to be updated

@nondebug Again thank you for the review. I have no objections and will update this PR based on your feedbacks.

marcoscaceres · 2024-03-25T22:07:51Z

index.html

+            `undefined`
+          </td>
+          <td>
+            Indicates the button type the controller is held in.


Suggested change

Indicates the button type the controller is held in.

The button type of the controller.

marcoscaceres · 2024-03-25T22:09:52Z

index.html

+          </p>
+          <pre class="idl">
+            enum GamepadButtonType {
+              "",  /* unknown, or not applicable */


Better to make it explicit.

Suggested change

"", /* unknown, or not applicable */

"unknown",

And if not applicable, maybe we should make . type nullable?

index.html

mrmcpowned · 2024-03-28T05:01:05Z

index.html

+        <dd>
+          An enumeration, {{GamepadButtonType}}, that indicates which button types the controller
+          is being held in.
+        </dd>


Looks like we're still missing getter definitions, no?

Yeah, though it's implied... but yeah, if the others have the getter steps (what it was initialized to) then so should this.

mrmcpowned · 2024-03-28T05:16:19Z

index.html

+        </dt>
+        <dd>
+          An enumeration, {{GamepadButtonType}}, that indicates which button types the controller
+          is being held in.


Is this phrasing in line with other specs? There's an open spec issue regarding consistency in enum phrasing which makes me think this would be better phrased as:

An enumerated {{GamepadButtonType}} attribute that classifies the current button's type in relation to an extended mapping

mrmcpowned · 2024-03-28T05:21:06Z

index.html

+              <th scope="col">Gamepad Models</th>
+            </tr>


It feels like there's some disconnect with this table and the available GamepadButtonType enums. Are we missing a possible third column which maps each of these button to a respective GamepadButtonType?

index.html

mrmcpowned · 2024-03-28T05:24:15Z

index.html

+            </tr>
+            <tr>
+              <td>home</td>
+              <td>Nvidia Shield Gamepad</td>


The Xbox and PlayStation controllers have their own respective home buttons. Are we intending to add those to this list or is there some special reason why we're classifying this separately for the Shield gamepad?

Thanks. updated.

mrmcpowned · 2024-03-28T05:27:04Z

index.html

+              <dfn>unknown</dfn>
+            </dt>
+            <dd>
+              This indicates button type is unknown or not applicable.


Is there a tangible benefit for User Agents to populate all the GamepadButtons with unknown for any buttons that can't be classified as one of the other types? Would it be better to simply have the attribute as undefined when that's the case?

@mrmcpowned Thanks for the review. I agree with your feedbacks. Will update this changes shortly.

Agree with your feedback @mrmcpowned, updated.

marcoscaceres · 2024-03-29T04:43:04Z

index.html

+          <tbody>
+            <tr>
+              <td>trackpad/touchpad</td>
+              <td>Sony DualSense, DualShock 4</td>


As I imagine these products will change a lot over time, so these should probably be in Wiki page (or if we want to formalize it, and actual registry). We can point the spec to the wiki page.

@marcoscaceres Thanks for the feedback. I have updated the rest of the comments.
I agree with you that it will keep changing and it would be great to have the link inside of the gamepad working group wiki. If this is not a hard requirement, I would like to follow up creating the wiki page on the different button mappings of the game controllers mentioned in this table as a separate PR.

sgtm, let's remove the table from this PR and follow up with the wiki page.

marcoscaceres · 2024-03-29T04:45:30Z

index.html

+              <dfn>trackpad</dfn>
+            </dt>
+            <dd>
+              This button is being assigned to trackpad button.


Probably just ok to say:

Suggested change

This button is being assigned to trackpad button.

A trackpad button.

Same for the rest.

marcoscaceres · 2024-03-29T04:46:01Z

index.html

+          This section introduces an extended gamepad button mapping beyond the Standard Gamepad layout.
+          These additional buttons are commonly found on certain gamepad models.
+          The following table defines the extended buttons used by some gamepad models in the following diagrams.
+          It’s important to note that this list is not exhaustive, and user agents may utilize different


Suggested change

It’s important to note that this list is not exhaustive, and user agents may utilize different

It's important to note that this list is not exhaustive, and user agents may utilize different

marcoscaceres · 2024-03-29T04:46:23Z

index.html

+          The following table defines the extended buttons used by some gamepad models in the following diagrams.
+          It’s important to note that this list is not exhaustive, and user agents may utilize different
+          or additional buttons for these or other gamepad models.
+          Consequently, the number of buttons on the Gamepad is not limited to the standard mapping of 17 buttons.


Suggested change

Consequently, the number of buttons on the Gamepad is not limited to the standard mapping of 17 buttons.

Consequently, the number of buttons on the {{Gamepad}} is not limited to the standard mapping of 17 buttons.

marcoscaceres · 2024-03-29T04:46:39Z

index.html

+        </p>
+       <p>
+          To accommodate extended gamepads, we have incorporated an "extended" mapping enumeration
+          into the GamepadMappingType. Additionally, we have defined an enumeration for the various


Suggested change

into the GamepadMappingType. Additionally, we have defined an enumeration for the various

into the {{GamepadMappingType}}. Additionally, we have defined an enumeration for the various

marcoscaceres · 2024-03-29T04:47:25Z

index.html

+       <p>
+          To accommodate extended gamepads, we have incorporated an "extended" mapping enumeration
+          into the GamepadMappingType. Additionally, we have defined an enumeration for the various
+          button types in the table, termed "GamepadButtonType", and have expanded the "GamepadButton"


Suggested change

button types in the table, termed "GamepadButtonType", and have expanded the "GamepadButton"

button types in the table, termed {{GamepadButtonType}}, and have expanded the {{GamepadButton}}

marcoscaceres · 2024-03-29T04:48:28Z

index.html

+          </p>
+          <pre class="idl">
+            enum GamepadButtonType {
+              "trackpad",


Nit: Please alpha sort these

nondebug · 2024-04-11T20:56:37Z

index.html

@@ -952,6 +953,17 @@ <h2>
            .. 1.0]
          </td>
        </tr>
+        <tr>
+          <td>
+            <dfn data-dfn-for="GamepadButton">[[\type]]</dfn>


I think the internal slot [[type]] isn't needed and can be removed.

Normally we use internal slots for read-only attributes that the user agent needs to modify. We expect a GamepadButton created with a button type will always have that type, so we don't need a way to modify it.

nondebug · 2024-04-11T21:27:19Z

index.html

+          <tbody>
+            <tr>
+              <td>trackpad/touchpad</td>
+              <td>Sony DualSense, DualShock 4</td>


sgtm, let's remove the table from this PR and follow up with the wiki page.

nondebug · 2024-04-11T22:47:33Z

index.html

Please add steps to the "initialize buttons" steps to initialize the button type.

https://w3c.github.io/gamepad/#dfn-initializing-buttons

Currently we initialize buttons in two phases. First, determine how many buttons are needed. Then, create all the buttons.

We'll need to remember the button type from the first phase so we can initialize the GamepadButton in the second phase. We also need to handle some special cases:

A GamepadButton may be created without a corresponding button input on the gamepad. (For instance, a gamepad may not have all the Standard Gamepad buttons; the button array entry for a missing button will still have a GamepadButton object but its value should never change.) We can initialize GamepadButton.type to null to represent this case.

A gamepad may have buttons that don't have a canonical button type. We should create a special GamepadButtonType enum value to signal that the button exists but doesn't have a canonical name yet.

@nondebug Thanks for the feedback. Added steps for considering the button types.

marcoscaceres · 2024-04-30T05:49:10Z

index.html

+        <p>
+          This section introduces an extended gamepad button mapping beyond the Standard Gamepad layout.
+          These additional buttons are commonly found on certain gamepad models.
+          The following table defines the extended buttons used by some gamepad models.


Suggested change

The following table defines the extended buttons used by some gamepad models.

The following definition list defines the <dfn>extended gamepad buttons</dfn> used by some gamepad models.

marcoscaceres · 2024-04-30T06:04:23Z

index.html

+              <li>Determine the type of the button at index |rawInputIndex|
+              [=represents a Standard Gamepad button=] or Extended button
+              types as described in the {{ExtendedMapping}} using the provided
+              buttonTypes array. If the button does not have a canonical type,
+              use GamePadButtonType.non_canonical.


This should be specified as:

Suggested change

<li>Determine the type of the button at index |rawInputIndex|

[=represents a Standard Gamepad button=] or Extended button

types as described in the {{ExtendedMapping}} using the provided

buttonTypes array. If the button does not have a canonical type,

use GamePadButtonType.non_canonical.

<li>Let |button| be the button at index |rawInputIndex|.</li>

<li>Let |type| be the result of determining if the |button|'s type represents a [=Standard Gamepad button|represents a Standard Gamepad button=] or an [=Extended Gamepad button|represents a Extended Gamepad button=]. If it's neither, use {{GamePadButtonType/"non_canonical"}}.

</li>

And <dfn>represents a Standard Gamepad button</dfn> should be defined underneath where "represents a Standard Gamepad button" is defined/

marcoscaceres · 2024-04-30T06:16:14Z

index.html

+              <dfn>trackpad</dfn>
+            </dt>
+            <dd>
+              Represent a trackpad input type which beyonds the scope of the standard mapping.


Suggested change

Represent a trackpad input type which beyonds the scope of the standard mapping.

Represent a trackpad input type.

marcoscaceres · 2024-04-30T06:16:44Z

index.html

+            </dt>
+            <dd>
+              Represent a trackpad input type which beyonds the scope of the standard mapping.
+	      <p>More enum types to be defined by the separate W3C wiki page.


On reflection, we should do this in the spec...

Suggested change

<p>More enum types to be defined by the separate W3C wiki page.

marcoscaceres

Some suggested changes.

xingri · 2024-04-30T16:37:54Z

Some suggested changes.

@marcoscaceres thank you so much for the detailed feedbacks.
Will prepare the revised version based on your feedback soon.

xingri · 2024-05-09T22:53:27Z

Some suggested changes.

@marcoscaceres Thanks for the feedback. Applied most of the change requests.

Regarding the extended mapping, based on the working group discussion on last March with @nondebug while you were away,
we discussed that we will not have the additional mapping type of "extended" for those gamepads that have a mapping that is "standard" plus extended bits. So, we'd keep "standard" but just have more controls.

Let's discuss a little bit during the call. Also I will fix the syntax error soon.

xingri · 2024-05-10T22:58:15Z

@marcoscaceres, @nondebug Please review the updated sentences.
Also linked the webkit & chromium issues.

nondebug · 2024-05-10T23:23:15Z

index.html

+              <li>Let |button| be the button at index |rawInputIndex|.
+              </li>
+              <li>Let |type| be the result of determining if the |button| [=represents
+              a Standard Gamepad button=] or represents [=Additional gamepad buttons=]


Let's use singular in both cases and add a period at the end of the sentence:

a Standard Gamepad button=] or represents an [=additional gamepad button=].

I'm not sure if respec will link the definition correctly if we don't use "Additional gamepad buttons"; if not, we can add a data-lt attribute to the tag below:

<dfn data-lt="additional gamepad button">Additional gamepad buttons</dfn>

nondebug · 2024-05-10T23:28:15Z

index.html

+          |buttonsSize| − 1:
+            <ol>
+              <li>Determine the |type| of the button from the provided {{GamepadButtonType}}
+              array, defaulting to null if not specified.


nondebug · 2024-05-10T23:44:14Z

index.html

+              <li>Determine the |type| of the button from the provided {{GamepadButtonType}}
+              array, defaulting to null if not specified.
+              </li>
+              <li>[=list/Append=] a [=new=] {{GamepadButton}} to |buttons|


This should reference the |type| variable we defined above:

<li>Let |button:GamepadButton| be a [=new=] {{GamepadButton}} with its {{GamepadButton/type}} attribute initialized to |type|. </li> <li>[=list/Append=] |button| to |buttons|. </li>

nondebug · 2024-05-10T23:48:19Z

index.html

-          |buttons|.
+          |buttonsSize| − 1:
+            <ol>
+              <li>Determine the |type| of the button from the provided {{GamepadButtonType}}


Determine the |type| of the button from the provided {{GamepadButtonType}} array

This should be phrased "Let |type| be..." so it's clear we're defining a variable. Also, it's unclear where the GamepadButtonType array came from or how it's being used. If it's provided then it should be passed into this algorithm and given a name, for instance: "To initialize buttons for a gamepad with an array |types| of GamepadButtonType values"

@nondebug thanks for the review and detailed guide. Updated this PR to reflect the feedbacks.

xingri · 2024-05-17T17:25:21Z

@marcoscaceres could you please review the changes and let me know if you need anything further.

index.html

xingri · 2024-07-11T23:38:42Z

@marcoscaceres , @nondebug As we discussed today, I have removed the table in the section 12.1.
Please review this PR and let me know your feedback.

xingri requested review from marcoscaceres and nondebug March 5, 2024 23:57

nondebug reviewed Mar 6, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch 6 times, most recently from 5e9abce to ab39929 Compare March 13, 2024 22:07

nondebug reviewed Mar 14, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch from ab39929 to 22010dc Compare March 14, 2024 22:55

marcoscaceres reviewed Mar 26, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch from 22010dc to e8dd177 Compare March 27, 2024 07:14

mrmcpowned reviewed Mar 28, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch 3 times, most recently from 94f73da to 64b7a26 Compare March 28, 2024 23:49

marcoscaceres reviewed Mar 29, 2024

View reviewed changes

index.html

</p>

<pre class="idl">

enum GamepadButtonType {

"trackpad",

Copy link

Member

marcoscaceres Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Please alpha sort these

xingri force-pushed the trackpad-button-support branch from 64b7a26 to 43650d6 Compare March 29, 2024 07:44

xingri force-pushed the trackpad-button-support branch from 2de195a to eb9c037 Compare April 8, 2024 19:48

nondebug reviewed Apr 11, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch from eb9c037 to 2a9ecc2 Compare April 22, 2024 18:37

marcoscaceres reviewed Apr 30, 2024

View reviewed changes

marcoscaceres requested changes Apr 30, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch 3 times, most recently from 8966102 to 7bcd588 Compare May 9, 2024 22:44

xingri force-pushed the trackpad-button-support branch 8 times, most recently from 05f7d5d to 23e192c Compare May 10, 2024 22:48

xingri requested a review from marcoscaceres May 10, 2024 22:58

nondebug reviewed May 10, 2024

View reviewed changes

xingri force-pushed the trackpad-button-support branch from 23e192c to 36d2213 Compare May 11, 2024 22:08

nondebug mentioned this pull request May 15, 2024

Add vendorId, productId, name attributes #202

Open

4 tasks

nondebug reviewed Jun 13, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

xingri force-pushed the trackpad-button-support branch from 36d2213 to 28c67f8 Compare July 8, 2024 17:40

nondebug reviewed Jul 11, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

xingri force-pushed the trackpad-button-support branch from 28c67f8 to af1c086 Compare July 11, 2024 21:17

Add extended buttons to support trackpad (w3c#191)

a925844

xingri force-pushed the trackpad-button-support branch from af1c086 to a925844 Compare July 11, 2024 23:36

Merge branch 'gh-pages' into trackpad-button-support

c07f911

	Indicates the button type the controller is held in.
	The button type of the controller.

	This button is being assigned to trackpad button.
	A trackpad button.

	It’s important to note that this list is not exhaustive, and user agents may utilize different
	It's important to note that this list is not exhaustive, and user agents may utilize different

	Consequently, the number of buttons on the Gamepad is not limited to the standard mapping of 17 buttons.
	Consequently, the number of buttons on the {{Gamepad}} is not limited to the standard mapping of 17 buttons.

	into the GamepadMappingType. Additionally, we have defined an enumeration for the various
	into the {{GamepadMappingType}}. Additionally, we have defined an enumeration for the various

	button types in the table, termed "GamepadButtonType", and have expanded the "GamepadButton"
	button types in the table, termed {{GamepadButtonType}}, and have expanded the {{GamepadButton}}

	The following table defines the extended buttons used by some gamepad models.
	The following definition list defines the <dfn>extended gamepad buttons</dfn> used by some gamepad models.

	Represent a trackpad input type which beyonds the scope of the standard mapping.
	Represent a trackpad input type.

Add extended buttons to support trackpad (#191) #196

Are you sure you want to change the base?

Add extended buttons to support trackpad (#191) #196

Conversation

xingri commented Mar 5, 2024 • edited by pr-preview bot Loading

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoscaceres left a comment

Choose a reason for hiding this comment

xingri commented Apr 30, 2024

xingri commented May 9, 2024

xingri commented May 10, 2024

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

xingri commented May 17, 2024

xingri commented Jul 11, 2024

xingri commented Mar 5, 2024 •

edited by pr-preview bot

Loading