Re: [w3c/gamepad] Enhance Gamepad interface description for Touch (PR #168)

@nondebug commented on this pull request.



> +              `touchData.touchID`.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/surfaceId}} to be
+              device surfaceId.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/position}}[0] to be
+              |normalizedX:float|.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/position}}[1] to be
+              |normalizedY:float|.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/surfaceDimensions}}[0] to be
+              device max x-axis coordinate.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/surfaceDimensions}}[1] to be
+              device max y-axis coordinate.<p>(<i>note: surfaceDimensions can be set to `null`</i>)</p>

Please rewrite these steps with a branch where surfaceDimensions is set to null. We can leave "why is it null?" up to the browser implementer, but if we don't have steps where it can be set to null then it should never be null.

It should be something like:

- If the touch surface exposes maximum surface dimensions in device units:
  - Set touchEvent.surfaceDimensions[0] to ...
  - Set touchEvent.surfaceDimensions[1] to ...
- Otherwise:
  - Set touchEvent.surfaceDimensions to null

> +              <li>Set |touchEvent|.{{GamepadTouch/surfaceId}} to be
+              device surfaceId.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/position}}[0] to be
+              |normalizedX:float|.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/position}}[1] to be
+              |normalizedY:float|.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/surfaceDimensions}}[0] to be
+              device max x-axis coordinate.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/surfaceDimensions}}[1] to be
+              device max y-axis coordinate.<p>(<i>note: surfaceDimensions can be set to `null`</i>)</p>
+              </li>
+              <li>Add |touchEvent| to |gamepad|.{{Gamepad/[[touchEvents]]}}.

We're adding to the touchEvents internal slot but never remove from it. This algorithm should start by emptying touchEvents since we plan to replace the whole list each time we update.

> +                  coordinate and 1.0 is the bottommost coordinate.
+                </p>
+                <p>
+                <p>Possible implementation:</p>
+                <ul>
+                  <li>`normalizedY = (2.0 * touchData.y / surfaceDimensionY) - 1`</li>
+                </ul>
+                </p>
+              </li>
+              <li>Let |touchEvent:GamepadTouch| be a {{GamepadTouch}}.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/touchId}} to be
+              `touchData.touchID`.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/surfaceId}} to be
+              device surfaceId.

It's unclear where the device surfaceId comes from. I think it would be simpler to specify it as an index into whatever the surface ordering happens to be:

- Let surfaceId be 0
- For each touch surface:
  - For each touch input on that surface:
    - Create and add the touch event (with surfaceId)
  - Increment surfaceId

> +                <p>Let |normalizedY:float| be a value calculated from the current
+                  touch input y coordinate in relationship to the surfaceDimensionY
+                  and normalized to the range [-1.0, 1.0] where -1.0 is the topmost
+                  coordinate and 1.0 is the bottommost coordinate.
+                </p>
+                <p>
+                <p>Possible implementation:</p>
+                <ul>
+                  <li>`normalizedY = (2.0 * touchData.y / surfaceDimensionY) - 1`</li>
+                </ul>
+                </p>
+              </li>
+              <li>Let |touchEvent:GamepadTouch| be a {{GamepadTouch}}.
+              </li>
+              <li>Set |touchEvent|.{{GamepadTouch/touchId}} to be
+              `touchData.touchID`.

We need to specify more of how the touch event's touchID is initialized since it's important for how applications will interact with the API. Let's try to leverage the Touch Events specification which already defines "active touch point":

https://w3c.github.io/touch-events/#dfn-active-touch-point

- If the touch data is part of an existing [=active touch point=] tracked by the user agent:
  - Set touchID to the touchID of the active touch point
  - Otherwise, set touchID to nextTouchID and increment nextTouchID

nextTouchID should be a Gamepad internal slot so it's always unique even when there are multiple touch surfaces on the same gamepad.

>          </dd>
         <dt>
           <dfn>touchEvents</dfn>
         </dt>
         <dd>
           A list of touch events generated from all touch surfaces.
-          <code>null</code> if the device does not support touch events.
+          If the device does not support touch events, MUST be set to `null`.

touchEvents should define a getter algorithm that returns the data from the internal slot.

>          </dd>
       </dl>
+      <section>
+        <h3>
+          Receiving inputs
+        </h3>
+        <p>
+          This section supplements the <a href="https://w3c.github.io/gamepad/#receiving-inputs">
+          Receiving inputs</a> section of the main 
+          <a href="https://w3c.github.io/gamepad/">Gamepad specification</a>.
+        </p>
+        <p>
+          In addtion to the steps defined in the main 
+          <a href="https://w3c.github.io/gamepad/">Gamepad specification</a>.
+          When the system receives new touch input values,

> When the system receives new touch input values,

This seems ambiguous to me, it's unclear whether "new touch input values" includes updates where there are no touches, for example a DS4 input report that doesn't contain any active touch points. We want this method to run for every DS4 input report, even when there are no active touch points.

Perhaps something like:

"When the user agent modifies the list of [=active touch points=] for a touch surface by adding touch points, removing touch points, or updating the values of existing touch points, update touchEvents by running the following steps:"

> +        </h3>
+        <p>
+          This section supplements the <a href="https://w3c.github.io/gamepad/#constructing-a-gamepad">
+            Constructing a `Gamepad`</a> section of the main 
+          <a href="https://w3c.github.io/gamepad/">Gamepad specification</a>.
+        </p>
+        <p>
+          In addtion to the steps defined in the main 
+          <a href="https://w3c.github.io/gamepad/">Gamepad specification</a>.
+          When constructing <dfn>a new `Gamepad`</dfn> representing a connected gamepad device
+          perform the following steps:
+        </p>
+        <ol>
+          <li>Let |gamepad:Gamepad| be a newly created {{Gamepad}} instance:
+            <ol>
+              <li>Initialize |gamepad|'s {{Gamepad/touchEvents}} to the result of

I think here we should initialize the touchEvents internal slot to an empty list if the gamepad has any touch surfaces, otherwise null. The [=updating touchEvents=] algorithm will be called automatically whenever the first touch inputs are received so we don't need to call it here.

The actual touchEvents attribute (not the internal slot) should define a getter algorithm that returns the data from the internal slot. That way the attribute is initialized when we initialize the internal slot. We use the internal slot because the attribute is readonly so technically it has no setter but we still need a way to update it.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/gamepad/pull/168#pullrequestreview-1048470468
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/gamepad/pull/168/review/1048470468@github.com>

Received on Saturday, 23 July 2022 01:11:33 UTC