-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split input sources into primary/auxiliary #929
Conversation
|
||
<div class="algorithm" data-algorithm="on-transient-input-start"> | ||
|
||
When a [=transient input source=] |source| for {{XRSession}} |session| begins its [=primary action=] the UA MUST run the following steps: | ||
When a [=transient input source=] |source| for {{XRSession}} |session| begins its [=transient action=] the UA MUST run the following steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text below seems to imply that transient actions can also be primary actions, but this means that select events would be fired twice since this line no longer "replaces" the primary action spec text the way it used to.
Is there ever a case for a non-primary transient action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rephrased it in 9d55efa. I wasn't aware that this was a replacement, that's rather subtle...
An example for a non-primary transient action would be adding additional fingers to a multitouch screen, these would create transient input sources and corresponding inputsourceschange
events, but no select{,start,end}
events, similar to how non-primary pointer events don't generate compatibility mouse events. I had updated the example in the previous paragraph to cover that.
As a side note, one additional reason for introducing this distinction is that I'm currently working on the DOM Overlay draft spec, where it would be useful to have a concept of transient sources that don't necessarily trigger XR select events, i.e. because they were handled as a DOM event. That's beyond the scope of this PR, and we can revisit this issue as part of that discussion, but I thought that the concept of non-primary transient sources would also be useful on its own.
9d55efa
to
bf439b4
Compare
As requested in the discussion, bf439b4 removes the auxiliary input source changes and tracker example, keeping just the transient input source changes. @Manishearth , how does this look? |
index.bs
Outdated
@@ -1611,43 +1611,43 @@ When an [=XR input source=] |source| for {{XRSession}} |session| has its [=prima | |||
Transient input {#transient-input} | |||
--------------- | |||
|
|||
Some [=/XR device=]s may support <dfn>transient input sources</dfn>, where the [=XR input source=] is only meaningful while performing its [=primary action=]. An example would be mouse, touch, or stylus input against an {{XRSessionMode/"inline"}} {{XRSession}}, which MUST produce a transient {{XRInputSource}} with a {{targetRayMode}} set to {{screen}}. [=Transient input sources=] are only present in the session's [=list of active XR input sources=] for the duration of the the {{selectstart}}, {{select}}, and {{selectend}} event sequence. | |||
Some [=/XR device=]s may support <dfn>transient input sources</dfn>, where the [=XR input source=] is only meaningful while performing a <dfn>transient action</dfn>, either the [=primary action=] for a [=primary input source=], or a device-specific action for an [=auxiliary input source=]. An example would be mouse, touch, or stylus input against an {{XRSessionMode/"inline"}} {{XRSession}}, which MUST produce a transient {{XRInputSource}} with a {{targetRayMode}} set to {{screen}}, treated as a [=primary action=] for the [[POINTEREVENTS#dfn-primary-pointer]], and as a non-primary [=transient action=] for a non-primary pointer. [=Transient input sources=] are only present in the session's [=list of active XR input sources=] for the duration of the [=transient action=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've deleted the definition of auxiliary input source but still refer to it here. We should still be defining it somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the mismatch. I've reverted the deletion, and made a new change that basically just updates the example text to remove the tracker reference and links to transient input sources for secondary screen touches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a small change to this section in commit a1723b3, using the new term "auxiliary action" to refer to a device-specific non-primary action. It was too easy to misintepret "non-primary [=transient action=]" as implying that all transient actions are non-primary, but that's not the case - they include both types.
index.bs
Outdated
Each [=XR input source=] MUST define a <dfn>primary action</dfn>. The [=primary action=] is a platform-specific action that, when engaged, produces {{XRSession/selectstart}}, {{XRSession/selectend}}, and {{XRSession/select}} events. Examples of possible [=primary action=]s are pressing a trigger, touchpad, or button, speaking a command, or making a hand gesture. If the platform guidelines define a recommended primary input then it should be used as the [=primary action=], otherwise the user agent is free to select one. | ||
An [=XR input source=] is a <dfn>primary input source</dfn> if it supports a <dfn>primary action</dfn>. The [=primary action=] is a platform-specific action that, when engaged, produces {{XRSession/selectstart}}, {{XRSession/selectend}}, and {{XRSession/select}} events. Examples of possible [=primary action=]s are pressing a trigger, touchpad, or button, speaking a command, or making a hand gesture. If the platform guidelines define a recommended primary input then it should be used as the [=primary action=], otherwise the user agent is free to select one. The device MUST support at least one [=primary input source=]. | ||
|
||
An [=XR input source=] is a <dfn>auxiliary input source</dfn> if it does not support a [=primary action=], for example [=transient input sources=] associated with secondary screen touches on a multitouch device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An [=XR input source=] is a <dfn>auxiliary input source</dfn> if it does not support a [=primary action=], for example [=transient input sources=] associated with secondary screen touches on a multitouch device. | |
An [=XR input source=] is an <dfn>auxiliary input source</dfn> if it does not support a [=primary action=], for example [=transient input sources=] associated with secondary screen touches on a multitouch device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 0527c57
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @klausw and @Manishearth, for working through this. The change looks good, and I think this is an important distinction to add to the spec.
The transient input change accidentally reused the previously defined term "transient action" in a place that was intended to refer to a non-primary action, and that was inconsistent since transient actions include both primary actions and device-specific auxiliary actions such as secondary pointers.
Can this be merged, or are we waiting for additional approvals? Nell is still listed as pending reviewer, but she's said she won't be available for reviews. |
I think we're good to merge given that Manish and I have approved, and Nell is going to be out for a while. |
This introduces new definitions for primary/auxiliary XR input sources, and "transient action" for transient input device to support a transient input device that isn't a primary input source. Fixes immersive-web#920
This introduces new definitions for primary/auxiliary XR input sources, and "transient action" for transient input device to support a transient input device that isn't a primary input source.
Fixes #920