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

Create an anchor from hit test result should not use native origins #46

Open
Manishearth opened this issue May 30, 2020 · 11 comments · May be fixed by #49
Open

Create an anchor from hit test result should not use native origins #46

Manishearth opened this issue May 30, 2020 · 11 comments · May be fixed by #49

Comments

@Manishearth
Copy link

Manishearth commented May 30, 2020

https://immersive-web.github.io/anchors/#create-an-anchor-from-hit-test-result

Let anchor native origin be a new native origin returned from the device’s call to create a new anchor using pose, interpreted as if expressed relative to hitTestResult’s native origin and attached to nativeEntity, at the frame’s time.

Create new anchor object anchor using anchor native origin and session.

Native origins are static transforms, this code is essentially freezing the anchor at the hit test result. I imagine the intent is to track a native entity.

What should be done here is that:

  • An anchor should be able to hold either a native origin or a native entity
  • anchorSpace is just defined as an XRSpace with an "anchor" slot set to Anchor. If you want you can define this as XRAnchorSpace instead for ease of spec use.
  • Define the native origin of XRSpaces that have "anchor" set to be:
    • the native origin, if set
    • the native origin of the native entity otherwise

This is mostly spec pedantry and will not affect how the API is used.

Though if you're going the route of adding XRAnchorSpace, it's worth considering if XRAnchorSpace and XRAnchor can be merged (filed #47)

@bialpio
Copy link
Contributor

bialpio commented Jun 1, 2020

Native origins are static transforms, this code is essentially freezing the anchor at the hit test result. I imagine the intent is to track a native entity.

Native origins in the core spec are already not static (input sources, viewer reference space) - I believe the current approach should work. Using core spec's terminology, the hit test's native origin should be tracking the native entity (if the XR system has the capability), or it will be static if the XR system does not support it. Then, when the anchor gets created, the newly created native origin will also be tracking the entity's native origin (and at least initially, the transform between native entity's origin and anchor's origin will be equivalent to the pose the app passed in). Am I missing something here?

@Manishearth
Copy link
Author

Native origins in the spec are intended to just be transforms, but you can define them to track something so the transform changes per-frame. This is essentially shortform for "every frame, update the native origin to X"

The way the anchors spec is currently written is basically "copying" the native origin. You want to instead define it as tracking the native entity.

It's also worth noting that hitTestResult's native origin isn't defined outside of the frame anyway!

Basically, this spec needs to refer to the native entity somewhere here.

@bialpio
Copy link
Contributor

bialpio commented Jun 1, 2020

I think what I do not understand here is the difference between "an anchor space holds a native origin that tracks a native entity" vs "an anchor space holds a native entity". Using native entities allows us to reuse core spec's algorithm to populate the poses - if the end result is equivalent (i.e. we return the correct poses for anchors), I'd prefer to use existing spec concepts (I believe it should be possible to do so here).

The way the anchors spec is currently written is basically "copying" the native origin. You want to instead define it as tracking the native entity.

This is definitely not the intent - I've tried to express the fact that it's supposed to be a new native origin that gets affected by changes to native entity by introducing the concept of a native origin that is attached to a native entity (this could be changed to just say "tracks").

To break this down a bit:
"Let anchor native origin be a new native origin returned from the device’s call to create a new anchor using pose, interpreted as if expressed relative to hitTestResult’s native origin and attached to nativeEntity, at the frame’s time."

  1. "a new native origin" - we create a brand new native origin, section about native device concepts should list what is needed for that.
  2. "pose, interpreted as if expressed relative to hitTestResult’s native origin" - for anchor creation, we need a pose, the one passed in by the app is to be interpreted relative to hit test result's native origin. This part may be better if we're explicit (in hit test spec) that if the underlying system supports it, the hit test result's native origin tracks the native entity.
  3. "attached to nativeEntity" - this is to say that newly created native origin should be tracking the native entity.
  4. "at frame's time" - this is to say that the interpretation of the app's pose relative to hit test result's should be time-indexed by the frame (pt. 2).

Basically, this spec needs to refer to the native entity somewhere here.

I thought it did. :( See point 3, if that's not clear then we should definitely wordsmith it better.

@Manishearth
Copy link
Author

I think what I do not understand here is the difference between "an anchor space holds a native origin that tracks a native entity" vs "an anchor space holds a native entity"

You can't define it as "holds a native origin that tracks a native entity", you define it as "the anchor space's native origin tracks a native entity". Native origins themselves are just transforms, they have no ability to track.

Native origins aren't copyable objects that retain their tracking. They are defined to track something, which means a particular field will change over time.

This is a reference vs copy issue. You're trying to take a reference to a native origin that is only defined at a certain point in time.

This is definitely not the intent - I've tried to express the fact that it's supposed to be a new native origin that gets affected by changes to native entity by introducing the concept of a native origin that is attached to a native entity (this could be changed to just say "tracks").

Yeah, the issue is that native origins themselves are just transformed, their semantics get defined in their parent object. I added native origins because the spec used to do it the other way and it led to some confusion.


I'll make a PR with my concrete proposal, it's not a major change, and I think it will make things clearer.

@bialpio
Copy link
Contributor

bialpio commented Jun 1, 2020

Native origins themselves are just transforms, they have no ability to track.

If that's the case, we need to revisit the core spec to make it explicit, there are some fragments that seem to be contradicting this interpretation:
"Each XRSpace has a native origin that is tracked by the XR device's underlying tracking system, and an effective origin, which is the basis of the XRSpace's coordinate system." - no mention about what the native origin specifically is, just that the device knows about it, and that a coordinate system is defined by it.
"(...) represents a tracking space with a native origin which tracks the position and orientation of the viewer." - in "viewer" reference space, native origin is said to be tracking the viewer.
"3. Set offsetSpace’s native origin to base’s native origin." - creating offset spaces based on XRReferenceSpaces - to me we take a copy of a native origin to store it in a new space - if the copy does not retain the properties of the original object, our offset spaces are broken.
Target ray space and grip space both have native origins that are said to track something.

My mental model of a native origin was that it is a handle that I can use to ask the XR system about transforms (indexed by some particular point in time), they are not transforms themselves. We never update the native origins of a space (as we would have if they were just point-in-time transforms), we query the underlying system about poses between some particular spaces (and I assumed we can use native origins when talking to the device to achieve this) at some specified point in time.

I'll make a PR with my concrete proposal, it's not a major change, and I think it will make things clearer.

Thanks, this would be helpful, I'm not sure if I understand the root of the issue here.

@Manishearth
Copy link
Author

"Each XRSpace has a native origin that is tracked by the XR device's underlying tracking system, and an effective origin, which is the basis of the XRSpace's coordinate system." - no mention about what the native origin specifically is, just that the device knows about it, and that a coordinate system is defined by it.

Yeah, so here note that we're defining the behavior of the native origin on a space. This is true of all of the cases except the origin offset one, we're defining all of these as having a particular set of semantics for how their native origins are tracked.

The copying in the offset case is a mistake.

The base spec is indeed not 100% clear about this, but it matters less there: it matters here because the following:

Let anchor native origin be a new native origin returned from the device’s call to create a new anchor using pose, interpreted as if expressed relative to hitTestResult’s native origin and attached to nativeEntity, at the frame’s time.

is very imprecise. There is no such thing as "create a new anchor using |pose|". The native origin of the hit test result is only defined for a single point in time. Overall this needs a lot of improvement.

@Manishearth
Copy link
Author

There are basically two issues falling out of this imprecision:

  • It's unclear what the precise coordinate space of a tracked native origin is
  • It's unclear how the pose argument works

I'm attempting to pick reasonable defaults for both of these.

@bialpio
Copy link
Contributor

bialpio commented Jun 2, 2020

The native origin of the hit test result is only defined for a single point in time.

That's not really what the text says - we do not mention for how long the hit test result's native origin is defined. We just say that there is only a single point in time when the app can observe its pose. For hit test results that were based off of an entity, we should be able to relate the hit test's native origin's behavior to the native entity's behavior and get the expected outcome (similarly to defining the native origin's behavior on a space that is tracking something dynamic).

The copying in the offset case is a mistake.

The base spec is indeed not 100% clear about this, but it matters less there: (...)

I'd say it matters a lot more in the core spec, it's hard to create something precise out of imprecise primitives. I think we should make sure that the core spec defines what a native origin is and how it is supposed to be used (to me, given the core spec, my mental model holds and the copy in an offset case is not a mistake since it copies a handle), otherwise it'll be hard to talk about things.

@Manishearth
Copy link
Author

Yes, and I intend to fix it in the core spec, but ultimately the core spec's impreciseness doesn't cause observable ambiguities in behavior. It does here.

@Manishearth Manishearth linked a pull request Jun 2, 2020 that will close this issue
@Manishearth
Copy link
Author

#49

@Manishearth
Copy link
Author

The core spec updates are here: immersive-web/webxr#1071

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 a pull request may close this issue.

2 participants