immersive-web/webvr-polyfill

An attempt to assign pose to readonly VRFrameData.pose throws exception

Artyom17 opened this issue · 7 comments

frameData.pose = pose;

This line throws an exception when frameData is an instance of actual, non-polyfilled VRFrameData. This scenario is possible when native WebVR is supported AND PROVIDE_MOBILE_VRDISPLAY is set to false. For the PROVIDE_MOBILE_VRDISPLAY itself I will open a separate issue, since the logic around it seems strange.

Thanks for filing @Artyom17! That line can be found here in the cardboard-vr-display repo: https://github.com/immersive-web/cardboard-vr-display/blob/d709396dbbcc8e2b7c5c02cd8c30852667ebd8c7/src/util.js#L442

If WebVR is supported and PROVIDE_MOBILE_VR_DISPLAY is false, the polyfill shouldn't do anything:

if (!this.hasNative || this.config.PROVIDE_MOBILE_VRDISPLAY && isMobile()) {
, how is the polyfill being executed?

Edit: following up on #335

Oh, it actually comes from AFRAME's utils/device.js, here what they are doing:

module.exports.PolyfillControls = function PolyfillControls (object) {
  var frameData;
  var vrDisplay = window.webvrpolyfill.getPolyfillDisplays()[0];
  if (window.VRFrameData) { frameData = new window.VRFrameData(); }
  this.update = function () {
    var pose;
    if (!vrDisplay) { return; }
    vrDisplay.getFrameData(frameData);

(and vrDisplay is already 'polyfilled', therefore it is going to webvr-polyfill.js).
The VRDisplay is polyfilled because of this code:

WebVRPolyfill.prototype.getPolyfillDisplays = function () {
  if (this._polyfillDisplaysPopulated) {
    return this.polyfillDisplays;
  }
  if (isMobile()) {
    var vrDisplay = new CardboardVRDisplay({
      ADDITIONAL_VIEWERS: this.config.ADDITIONAL_VIEWERS,
      DEFAULT_VIEWER: this.config.DEFAULT_VIEWER,
      MOBILE_WAKE_LOCK: this.config.MOBILE_WAKE_LOCK,
      DEBUG: this.config.DEBUG,
      DPDB_URL: this.config.DPDB_URL,
      CARDBOARD_UI_DISABLED: this.config.CARDBOARD_UI_DISABLED,
      K_FILTER: this.config.K_FILTER,
      PREDICTION_TIME_S: this.config.PREDICTION_TIME_S,
      ROTATE_INSTRUCTIONS_DISABLED: this.config.ROTATE_INSTRUCTIONS_DISABLED,
      YAW_ONLY: this.config.YAW_ONLY,
      BUFFER_SCALE: this.config.BUFFER_SCALE,
      DIRTY_SUBMIT_FRAME_BINDINGS: this.config.DIRTY_SUBMIT_FRAME_BINDINGS
    });
    this.polyfillDisplays.push(vrDisplay);
  }
  this._polyfillDisplaysPopulated = true;
  return this.polyfillDisplays;
};

Thus, if it is 'mobile' (unfortunately, OculusBrowser is considered as one) it will always return polyfilled VRDisplay.

So, I am not sure how this supposed to work, tbh and whom to contact about this issue. To me, this getPolyfillDisplays function looks pretty much internal and AFRAME is using it "illegally". Am I correct? Or, should it return 'null' in the case if hasNative set to true (and this should fix the AFRAME too)?

I don't remember all the rational behind this function or if it's intended to be used (definitely at least a power-user feature), but this does sound like an A-Frame issue considering the configuration, but we could also have an exception for the forcing of a polyfilled VRDisplay when viewing in Oculus, but A-Frame would have to update it upstream anyway. What do you think?

That being said, if removing that check in #335 fixes it here, that sounds uncontroversial!

@Artyom17 is this still an issue?

@jsantell I haven't tried the very latest version, but since #335 landed - I think it shouldn't be an issue anymore.