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

First draft for adding a property to XRInputSource to say it's visible elsewhere #1352

Merged
merged 6 commits into from
Apr 16, 2024

Conversation

AdaRoseCannon
Copy link
Member

@AdaRoseCannon AdaRoseCannon commented Oct 31, 2023

/agenda As brought up during TPAC, this is a proposal to start a conversation about exposing whether a controller should be drawn.

I think the property should be falsey if the controller is allowed to be drawn so that on implementations that don't have this it remains compatible with libraries that are looking for this attribute to determine whether they should draw an input.

Discussion topics:

  • Should this value be allowed to change during the lifetime of the XRinputSource?
  • Should it be a boolean to just indicate whether or not you should draw the input or a string to give more description about why you shouldn't draw it?

The attribute name definitely needs some bikeshedding some alternative names:

  • .renderedElsewhere (if seen with real eyes it's not rendered)
  • .alreadyVisible
  • .otherwiseVisible
  • .dontDraw/.noDraw (I don't like having negatives in attributes)

Preview | Diff

@probot-label probot-label bot added the agenda Request discussion in the next telecon/FTF label Oct 31, 2023
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.

Thanks for putting this together!

In terms of name bikeshedding, I don't think alreadyVisible is bad, but something about the tense of already feels slightly strange to me (though I can't really put my finger on why). I do agree that having it be falsey if the controller should be drawn by the app is ideal for the forwards compat reasons you mentioned.

Maybe systemDisplayed? Has a similar concern to renderedElsewhere in that the system isn't actively "displaying" anything if you've got, say, transparent optics. But "displayed" feels more flexible than "rendered" in that regard.

As for whether it should be allowed to change or not during the session, I would lean towards no? We already have immutable properties on the inputs (handedness, target ray mode, input profiles) and the established mechanism for updating them is to remove the input and re-add it with the updated properties. I think that would work here as well (especially since the scenarios/devices where this would be dynamic seems rare?)

Regarding the bool vs. string/enum topic, I'm trying to imagine what kind of more nuanced information we'd expose here that would be meaningful to developers. Thinking through all of the variants of input display I can think of I guess the only property that seems potentially meaningful is "Can I occlude the input with my own rendering or not?" Is that a thing that devs care about, to the point where they would alter their application's behavior to account for it?

index.bs Outdated Show resolved Hide resolved
Co-authored-by: Brandon Jones <[email protected]>
@AdaRoseCannon
Copy link
Member Author

Thanks for looking through it. I wasn't sure what a string could be used for either but I was more trying to avoid painting ourselves into a corner if it is something that would be useful to know. I'm not too passionate either way if after discussion the group feels confident a boolean is sufficient I am happy to go with that.

@toji
Copy link
Member

toji commented Nov 9, 2023

The only thing that I would really worry about with a string is that it would be more awkward if we wanted to keep the same falsey property for it.

@Yonet Yonet removed the agenda Request discussion in the next telecon/FTF label Nov 27, 2023
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.

Needs to add the attrib to the IDL, otherwise looks good

index.bs Outdated
@@ -1859,6 +1860,8 @@ The <dfn attribute for="XRInputSource">profiles</dfn> attribute is a [=/list=] o

An <dfn for="XRInputSource">input profile name</dfn> is an ASCII lowercase {{DOMString}} containing no spaces, with separate words concatenated with a hyphen (`-`) character. A descriptive name should be chosen, using the prefered verbiage of the device vendor when possible. If the platform provides an appropriate identifier, such as a USB vendor and product ID, it MAY be used. Values that uniquely identify a single device, such as serial numbers, MUST NOT be used. The [=input profile name=] MUST NOT contain an indication of device handedness. If multiple user agents expose the same device, they SHOULD make an effort to report the same [=input profile name=]. The <a href="https://github.com/immersive-web/webxr-input-profiles/tree/master/packages/registry">WebXR Input Profiles Registry</a> is the recommended location for managing [=input profile name=]s.

The <dfn attribute for="XRInputSource">alreadyVisible</dfn> attribute indicates that this input is visible and does not need to be rendered by the Website. Examples include the input source being between the user and the display or the input being shown by the operating system.
Copy link
Contributor

Choose a reason for hiding this comment

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

also hand input in AR?

Worth covering all common cases here

Copy link
Member Author

Choose a reason for hiding this comment

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

In this particular case the developer could cover the physical input with pixels if they were drawing a large environment around the user in-spite of being in AR but I guess in general you wouldn't want to show the hands in AR and it is just hint to the developer which they can ignore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this is just a hint. Though the discussion does make me wonder if this should be an enum between "not-visible", "visible-real", and "visible-drawn". Probably overkill.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe:
The skipRendering attribute indicates that this input is visible and MAY NOT need to be rendered by the current session. If {{XRInputSource/skipRendering}} is true, the user agent MUST ensure that the [=XR input source=] is always visible.

Note: Examples of this are:

  • the input source is between the user and the display
  • the input source is always rendered on top by the operating system.

Copy link
Member

Choose a reason for hiding this comment

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

If {{XRInputSource/skipRendering}} is true, the user agent MUST ensure that the [=XR input source=] is always visible.

Hm... I don't know about that. I feel like there are examples of cases where the input source simply shouldn't be rendered at all where this would apply. For example: It feels natural to set this to true on gaze input sources? (You don't want to render anything in the user's head, after all.) We already tell devs to look at the targetRayMode for this, but it would be nice to have this as a more universal signal.

Though that also brings up the question of rendering related graphics. For example: If I'm told to skip rendering a controller on Quest, should I still render any applicable pick rays? I assume the answer is yes, but that may be ambiguous given the proposed text. We should probably specify that this property is a hint about rendering the physical input source only. Maybe include a non-normative note to say that things like pick rays and cursors should still be rendered if appropriate for the app and targetRayMode.

Copy link
Member

Choose a reason for hiding this comment

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

If {{XRInputSource/skipRendering}} is true, the user agent MUST ensure that the [=XR input source=] is always visible.

Hm... I don't know about that. I feel like there are examples of cases where the input source simply shouldn't be rendered at all where this would apply. For example: It feels natural to set this to true on gaze input sources? (You don't want to render anything in the user's head, after all.) We already tell devs to look at the targetRayMode for this, but it would be nice to have this as a more universal signal.

Maybe:
The {{XRInputSource/skipRendering}} attribute indicates that this input is visible and MAY NOT need to be rendered by the current session. If {{XRInputSource/skipRendering}} is true and the targetRayMode is "tracked-pointer", the user agent MUST ensure that a representation of the [=XR input source=] is always shown to the user.

Though that also brings up the question of rendering related graphics. For example: If I'm told to skip rendering a controller on Quest, should I still render any applicable pick rays? I assume the answer is yes, but that may be ambiguous given the proposed text. We should probably specify that this property is a hint about rendering the physical input source only. Maybe include a non-normative note to say that things like pick rays and cursors should still be rendered if appropriate for the app and targetRayMode.

NOTE: skipRendering is a hint about not rendering input sources such as controllers. Pick rays and cursor should still be rendered.

Copy link
Member

Choose a reason for hiding this comment

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

Both of those work for me! Maybe a minor tweak to the second one:

NOTE: skipRendering is a hint that input sources such as controllers do not need to be rendered. Associated application-specific input visualizations such as pick rays and cursors should still be rendered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks everyone for the feedback I will update it now.

@AdaRoseCannon
Copy link
Member Author

@cabanier suggested .skipRendering / .maySkipRendering thoughts?

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.

Thanks for adding the IDL. From conversation today I'd be fine with either .skipRendering or .maySkipRendering. Those satisfy a lot of the issues I had with the other proposed names.

index.bs Outdated Show resolved Hide resolved
@cabanier cabanier merged commit 3dec73d into immersive-web:main Apr 16, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Apr 16, 2024
SHA: 3dec73d
Reason: push, by cabanier

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

5 participants