immersive-web / webxr-polyfill

Use the WebXR Device API today, providing fallbacks to native WebVR 1.1 and Cardboard
Apache License 2.0
386 stars 86 forks source link

Adding proposal APIs like hit-testing and anchors to the polyfill #34

Closed TrevorFSmith closed 6 years ago

TrevorFSmith commented 6 years ago

I have Mozilla's WebXR Viewer iOS app using the webxr-polyfill as the basis for WebXR support. It creates a new PolyfilledXRDevice that uses ARKit under the covers to provide tracking. Now I'm looking at adding hit-testing and eventually anchors.

So, I have a couple of questions about how best to work with the polyfill.

Approach 1 would be to to make a second polyfill just for hit-testing and anchors that is loaded after the webxr-polyfill and mucks around with the private data structures and methods of classes like XRSession and PolyfilledXRDevice.

Approach 2 would be to fork the webxr-polyfill and modify the base classes like XRSession to add the proposed APIs and to modify PolyfilledXRDevice to provide hit-testing and anchor methods.

I'm leaning toward approach 2 just because there's less monkey patching and if the proposed APIs are accepted then I can relatively easily submit a PR to the webxr-polyfill with the platform-independent changes.

@jsantell Is this something you're already working on or have a different suggested approach?

jsantell commented 6 years ago

We've done approach 1 for similar problems; now that we're exposing the private tokens when reaching into the source files, we can pull in parts of the polyfill to extend and create a 'root' file that looks a lot like WebXRPolyfill.js:

import XR from 'webxr-polyfill/src/api/XR.js';
import XRDevice from 'webxr-polyfill/src/api/XRDevice.js';
import XRSession, { PRIVATE as XRSessionPrivate } from 'webxr-polyfill/src/api/XRSession.js';
import ARKitPolyfilledXRDevice from './ARKitPolyfilledXRDevice.js';

XRSession.prototype.requestHitTest = (ray, frameOfRef) => {
  // Can access private variables
  console.log(session[XRSessionPrivate].id);
  // @TODO Implement communication to native platform
};

// Injection logic, this is all the stuff in WebXRPolyfill.js to determine what to
// polyfill, etc., I could be wrong on some of the signatures here
const xrDevice = Promise.resolve(new XRDevice(new ARKitPolyfilledXRDevice()));
navigator.xr = new XR(xrDevice);

Approach 1 is nice since there's no modification of the dependency and you still get the guarantee of all the components working together, so if adding only a few functions, this seems preferable. While hit tests are a single function and rather isolated, anchors could be more extensive and require hacking the internals and overwriting functions, where approach 2 may be more suitable, but wonder how difficult it would be to keep up to date with upstream. I don't know how polyfilling some of these AR features would work currently, so not sure how valuable the upstreamability of a patch would be, in addition to the general speed of standards, which would mean upstreaming would be seldom. Not to dissuade from approach 2, just unsure of how much time you'd save when upstreaming changes!

What do you think? Since exposing the private tokens, it's fully customizable now at least, but I'm sure there could be better hooks to make this easier.

TrevorFSmith commented 6 years ago

I'll give approach 1 a try. I agree that adding API like hit testing is pretty straight forward, and we'll see how it goes with anchors.

Thanks!