Open Manishearth opened 4 years ago
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?
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.
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."
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.
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.
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.
"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.
There are basically two issues falling out of this imprecision:
I'm attempting to pick reasonable defaults for both of these.
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.
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.
The core spec updates are here: https://github.com/immersive-web/webxr/pull/1071
https://immersive-web.github.io/anchors/#create-an-anchor-from-hit-test-result
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:
anchorSpace
is just defined as anXRSpace
with an "anchor" slot set toAnchor
. If you want you can define this asXRAnchorSpace
instead for ease of spec use.XRSpace
s that have "anchor" set to be: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 ifXRAnchorSpace
andXRAnchor
can be merged (filed https://github.com/immersive-web/anchors/issues/47)