mozilla / webxr-polyfill

A polyfill and example code for building augmented reality (AR) and virtual reality (VR) applications using WebXR.
Mozilla Public License 2.0
259 stars 67 forks source link

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

Open speigg opened 6 years ago

speigg commented 6 years ago

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 commented 6 years ago

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.