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

Tighten up spec text around native origins #49

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Manishearth
Copy link

@Manishearth Manishearth commented Jun 2, 2020

Fixes #46

This PR:

  • More rigidly specifies how the pose argument works
  • Specifies native origins of anchors tracking native entities to change over time
  • Specifies the coordinate space used for native entities.

💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Jan 15, 2021, 4:35 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

index.bs Outdated
Each {{XRAnchor}} has an associated <dfn for=XRAnchor>native origin</dfn>.
An {{XRAnchor}} may have an associated <dfn for=XRAnchor>tracked entity</dfn>, which is a [=native entity=] that it is [=attached=] to.

An {{XRAnchor}} may have an associated <dfn for=XRAnchor>static origin</dfn>, which is an {{XRRigidTransform}}. An {{XRAnchor}} returned to the user MUST either have a [=XRAnchor/tracked entity=] or an [=XRAnchor/static origin=].
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not necessarily the right way to look at it - even free-floating anchors will have their poses evolve over time. If we want to make it so that they contain XRRigidTransform, we need to also specify a) what is the meaning of this transform (i.e. relative to which space it is expressed), and b) when does it get updated (since it will most certainly be changing).

It seems to me that the core spec will also have to be more precise about when native origins get updated if we want them to resemble something like a transform (I've added a comment on immersive-web/webxr#1071 that tries to pinpoint what the problems are). It may be easier to make native origins into a bit more fuzzy things ("handles that could be used when querying XR systems about poses" worked for me) instead of trying to make them into something concrete - I feel that if we want to make them into something concrete, we'd have to go all the way.

Copy link
Author

Choose a reason for hiding this comment

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

It's in world space. I actually should not define this as an XRRigidTransform, I should define it as a position and orientation in world space, good catch.

"handles that could be used when querying XR systems about poses" is fine as an interpretation! That's not functionally different from the status quo IMO, in the status quo the behavior of the handle is specified on the parent object.

The problem arises when you attempt to manipulate them, e.g. operations like multiplying an offset pose on a native origin to obtain another native origin does not make sense, and needs to be explicitly specced out. That is why I'm making these changes.

Copy link
Author

@Manishearth Manishearth Jun 5, 2020

Choose a reason for hiding this comment

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

For some more context on the XRRigidTransform thing, really we use that type in internal spec text a lot more places than we should because we have no native "rigid transform" type. I would like to eventually fix that (w3c/csswg-drafts#4076)

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the core spec does not yet have the concept of world space (we do introduce something like this for test API though) & that it was by design (world space is not observable by the users of the API, some devices do not have a stable world space) - my main worry is there's a lot of things that we gloss over and it may get tough to close all those holes.

As for manipulating, I think the only thing we have been doing is copying the native origins around (to say that this thing should report same poses as some other thing). The same thing should be possible with your proposed changes so we should be fine here.

Copy link
Author

Choose a reason for hiding this comment

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

@bialpio yes, not having a stable world space is fine, the point is that native origins track real-world objects. The exact relationship with other real-world objects is not stable, and this is why the populate-the-pose has the language it does.

As for manipulating, I think the only thing we have been doing is copying the native origins around (to say that this thing should report same poses as some other thing). The same thing should be possible with your proposed changes so we should be fine here.

Yes, except copying them around is ambiguous (is it a by-value copy or by-ref? hard to say), and furthermore the original text was applying an offset to a native origin, which is weirder.

@Manishearth Manishearth force-pushed the native-origins branch 2 times, most recently from 32057a7 to f0fbbe6 Compare June 5, 2020 23:05
@Manishearth
Copy link
Author

I've rebased and updated things a little bit.

Ultimately, one of my initial approaches to defining native origins was closer to how you describe it. However, if you want to spec native origins as magical objects that have their own semantics, it's significantly more verbose, since you end up needing to define multiple "types" of native origins, including all their references to the objects they track, and we didn't end up going that way.

It is easier to instead define native origin as a field that the parent object can attach semantics to. This means that copying them around won't work, but you can still query what its value was at a given time. It might be worth specifying this clearer in the core spec.

@@ -1,4 +1,4 @@
<pre class="metadata">
{<pre class="metadata">
Shortname: webxr-anchors
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo?

Copy link
Contributor

Choose a reason for hiding this comment

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

(rogue { in line 1 - GH seemed to add my comment at line 2)

Each {{XRAnchor}} has an associated <dfn for=XRAnchor>native origin</dfn>.
An {{XRAnchor}} may have an associated <dfn for=XRAnchor>tracked entity</dfn>, which is a [=native entity=] that it is [=attached=] to.

An {{XRAnchor}} may have an associated <dfn for=XRAnchor>static origin</dfn>, which is a position and orientation in world space. An {{XRAnchor}} returned to the user MUST either have a [=XRAnchor/tracked entity=] or an [=XRAnchor/static origin=].
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'm slightly bothered by the use of "static" in "static origin", but as long as it's understood that this is something that may evolve even for anchors that are free-floating, we should be ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, seems that we are missing a part of the semantics of free-floating anchors (it needs to track the real world) - see my comment below.

1. [=Create new anchor object=] |anchor| using |anchor native origin| and |session|.
1. Let |offset origin| be the [=multiply transforms|multiplication=] of |pose| and the current value of |effective origin|.
1. [=Create new anchor object=] |anchor| using |session|.
1. Set |anchor|'s [=static origin=] to |effective origin|.
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes me to raise my eyebrows - we need to ensure that static origin is something that is supposed to maintain its relationship to the real world (as understood by the underlying XR system). Other words to say the same thing - creating free-floating anchors should behave similarly as creating new "local" reference spaces with a pose initialized to something that the app passes.

@bialpio
Copy link
Contributor

bialpio commented Jun 8, 2020

Thanks for going through this, I've added some comments - LMK if I'm missing something.

(...) it's significantly more verbose, since you end up needing to define multiple "types" of native origins,

I was trying to get away with it by saying that the device is supposed to provide an API that is something like getPose(origin_1, origin_2, time) & it's up to the device to do the right thing and track the native origins appropriately depending on what they are supposed to represent.

In the end, as long as we are able to express what we need in a cross-compat way, it should not matter that much which route we take here. Expect to be added to future PRs related to native origins, I feel that I do not have a good intuition about them yet. :)

@himorin himorin changed the base branch from master to main January 15, 2021 04:34
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.

Create an anchor from hit test result should not use native origins
2 participants