Skip to content
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

Merged
merged 7 commits into from
Jan 9, 2020

Conversation

klausw
Copy link
Contributor

@klausw klausw commented Nov 20, 2019

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


<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:
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@NellWaliczek NellWaliczek self-requested a review November 21, 2019 18:54
@toji toji added this to the November 2019 milestone Nov 21, 2019
@toji toji added the agenda Request discussion in the next telecon/FTF label Dec 2, 2019
@klausw
Copy link
Contributor Author

klausw commented Dec 13, 2019

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=].
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 0527c57

@AdaRoseCannon AdaRoseCannon removed the agenda Request discussion in the next telecon/FTF label Dec 17, 2019
Copy link
Member

@toji toji left a 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.
@klausw
Copy link
Contributor Author

klausw commented Jan 9, 2020

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.

@toji
Copy link
Member

toji commented Jan 9, 2020

I think we're good to merge given that Manish and I have approved, and Nell is going to be out for a while.

@toji toji merged commit 5ca7b6f into immersive-web:master Jan 9, 2020
kearwood pushed a commit to kearwood/webxr that referenced this pull request Mar 11, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support input sources without primary action?
5 participants