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

Simplify application logic by exposing XRAnchor-related methods in Reality #72

Open
speigg opened this issue Jan 4, 2018 · 1 comment

Comments

@speigg
Copy link

speigg commented Jan 4, 2018

There seems to be a lot of complexity created by having the XRAnchor creation/finding methods inside XRPresentationFrame.

For example, in the example code, we have to keep track of whether or not we already requested the floor anchor, since we can only get the floor anchor via the frame instance, and thus inside the frame callback:

// If we haven't already, request the floor anchor offset
if(this.requestedFloor === false){
  this.requestedFloor = true
  frame.findFloorAnchor('first-floor-anchor').then(anchorOffset => {
    if(anchorOffset === null){
      console.error('could not find the floor anchor')
      return
  }
  this.addAnchoredNode(anchorOffset, this.floorGroup)
  }).catch(err => {
    console.error('error finding the floor anchor', err)
  })
}

More so, this logic is incorrect, since it does not handle the case when the reality is changed, which would cause all the XRAnchors to become invalid (right?).

Can we just expose the various Anchor-related properties and methods (anchors, getAnchor, findAnchor, findFloorAnchor, etc.) in the Reality class so that these things don't have to be called within a frame callback that is executed repeatedly, thus eliminating the need for things like the requestedFloor flag above? For example, the anchor can easily be requested every time the Reality changes, which is the proper way to do it (assuming all anchors are invalid after a reality changes):

session.addEventListener('realitychanged', () => {
  session.reality.findFloorAnchor('first-floor-anchor').then(anchorOffset => {
    if(anchorOffset === null){
      console.error('could not find the floor anchor')
      return
    }
    this.addAnchoredNode(anchorOffset, this.floorGroup)
  }).catch(err => {
    console.error('error finding the floor anchor', err)
  })
})

More so, we see this pattern of delaying the creation of anchors in the anchor sample:

addAnchoredModel(sceneGraphNode, x, y, z){
  // Save this info for use during the next render frame
  this.anchorsToAdd.push({
    node: sceneGraphNode,
      x: x, y: y, z: z
  })
}

// Called once per frame
updateScene(frame){
  const headCoordinateSystem = frame.getCoordinateSystem(XRCoordinateSystem.HEAD_MODEL)
  // Create anchors and start tracking them
  for(let anchorToAdd of this.anchorsToAdd){
    // Create the anchor and tell the base class to update the node with its position
    const anchorUID = frame.addAnchor(headCoordinateSystem, [anchorToAdd.x, anchorToAdd.y, anchorToAdd.z])
      this.addAnchoredNode(new XRAnchorOffset(anchorUID), anchorToAdd.node)
    }
  this.anchorsToAdd = []
}

This can be simplified to:

addAnchoredModel(sceneGraphNode, x, y, z){
   const anchorUID = session.reality.addAnchor(headCoordinateSystem, [anchorToAdd.x, anchorToAdd.y, anchorToAdd.z])
   this.addAnchoredNode(new XRAnchorOffset(anchorUID), anchorToAdd.node)
}
@blairmacintyre
Copy link
Contributor

Can you take a look at the evolving proposal for the w3c WebXR (https://github.com/immersive-web/webxr) and see what they are doing there? (Sorry, I don't have time right now, so it'll be faster for you to look).

Note: we aren't making any dramatic changes to the polyfill right now, as the WebVR community is moving the WebVR2.0 proposal to be WebXR. After the initial transition stabilizes, we'll want to start following what the standard is doing.

@speigg speigg changed the title Simplify application logic by exposing Anchor methods in Reality Simplify application logic by exposing XRAnchor-related methods in Reality Jan 4, 2018
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

No branches or pull requests

2 participants