Update polyfill to accept standard-compliant code #71
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
wow! lovely, i'll take a look 😄 |
jsantell
reviewed
Aug 6, 2019
| this.pose = null; | ||
| }; | ||
| } | ||
| // If no VR devices are present, return a Cardboard device even |
There was a problem hiding this comment.
Creating a cardboard device on desktop is a downgrade IMO -- devices' gyroscopes are non-existent or useless and it appears to logic outside of the polyfill as if an XR experience is supported, preventing applications from responding in their own way to handle non-XR experiences. e.g. I'd rather AWSD controls for inline content than my laptop's gyro. Is there a reason for the removal of the mobile/allowCardboardOnDesktop option?
|
Also for mergability, removing the |
|
Took a bit, but I should have all the feedback addressed now. Thanks Jordan! And yes, this will absolutely be a squashed commit when all is said and done. 😄 |
jsantell
approved these changes
Aug 8, 2019
|
|
||
| options.compositionDisabled = (session._session_mode === "inline"); | ||
|
|
||
| return new originalXRLayer(session, gl, options); |
There was a problem hiding this comment.
Ahh, in that case, would it be possible for the new global.XRWebGLLayer to be a constructor, proxying methods to originalXRLayer? This would preserve the connection between the exposed constructor and the returned instance. Granted that's less important than all these other changes, maybe something to file an issue for in the future
This change squashes 47 changes from @jacobcdewitt (and a couple from myself) into one for easier history review. The comments for each change have been preserved below. The cumulative changes enable the polyfill to accept code that was written against the "VR complete" version of the WebXR spec, even if support is not fully spec compliant at this time. Squashed commit comments ------------------------ update packages npm install serve no longer checkin build output files Don't checkin webxr samples or media folders. requestFrameOfReference => requestReferenceSpace update reference space types Implement XRSession.updateRenderState Add views array to XRViewerPose. This also requires implementing part of XRRigidTransform and adding a transform to XRView. As of this commit, I have the xr-presentation.html sample from https://storage.googleapis.com/chromium-webxr-test/r672724/xr-presentation.html working with this polyfill when hosting the samples locally. The only change I had to make to the html file was how the polyfill gets injected. I tested in the following browsers on Windows using an Oculus headeset: - Chrome 75 with WebVR flag enabled, but WebXR flag disabled - Firefox 67.0.4 Update reference space types. - remove head-model - eye-level => local - stage => bounded-floor XRCoordinateSystem => XRSpace, XRFrameOfReference => XRReferenceSpace start working on XRReferenceSpace.getOffsetReferenceSpace, which requires updating XRRigidTransform to compose the matrix based on position and orientation values comment out work in progress changes session.inputSources attribute more work for XRFrame.getPose more work for getPose. Use gl-matrix to compose/decompose matrix XRRay from external polyfill more updates to getPose so that input-tracking.html finally works (but it does place the user too low and controllers sometimes go through the floor and a dot is shown for the gaze). handle floor-level reference spaces in a more general way remove log statement more work for getOffsetReferenceSpace fix issue in getOffsetReferenceSpace negate quaternions in XRRigidTransform constructor to fix rotation issue try some more things for getPose to make teleportation work, but there are still issues always return a Cardboard device when no VR displays are connected so that inline sessions are always supported remove console.log statements Expose input poses and gamepad from WebVR Made sure that the data exposed to gamepads via the WebVR API is properly forwarded to the WebXR API surface. Includes both the poses and repeating the original gamepad object on the XRInputSource.gamepad attribute. No remapping of the gamepad API data is being done at this point. Removed the XRInputPose, because it's no longer part of the API. Allow mapping of WebVR gamepad values Provide a mapping table that allows WebVR-style gamepads to be remapped so that they can conform to the WebXR conventions and the `xr-standard` mapping. Also allows for static transforms to be applied to either the grip or the targetRay poses so that the single value that WebVR supplied can be used for both spaces accurately. Add Oculus Go mappings and fix some bugs Fix some offset reference spaces and inlineVerticalFieldOfView Add gamepad mapping for WMR controller. Fix several mappings for Chrome 76 Cleanup Add/update method annotations. Remove commented-out code. Split up long lines. remove XRLayer XRLayer is no longer in the spec. XRWebGLLayer no longer inherits from XRLayer. getViewport is now on XRWebGLLayer Remove XRPresentationContext. It's no longer in the WebXR spec. Teleportation seems to work now. Had to remove the quaternion inversion in XRRigidTransform cleanup Make XRViewerPose inherit from XRPose. Now the positional-audio sample works. remove stale comment Remove XRRay Fix error when loading with polyfill when native WebXR is enabled on mobile devices. Don't try to patch in a cardboard display in this situation because it's a rare corner case. Throw exception when trying to create bounded reference space since the polyfill does not support them. Cleanup XRReferenceSpace + XRViewerPose code. Remove unused function. Only expose views array via XRViewerPose. Update getViewport comments. Added a small shim to allow spec-compliant code to run on Chrome 76 Addressing feedback from Jordan Co-Authored-By: jacobcdewitt <jacobcdewitt@gmail.com>
This was referenced Aug 12, 2019
Closed
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #50, #52, #53, #54
Alright, I've tried 20 different ways to re-package this into cleaner, easier-to-review chunks, but each time it ended up a mess and at this point it's preventing the code from being made more visible, so I'm just going to post this as one big PR for the moment. Apologies to anyone that was hoping to do a detailed review.
In terms of what this change does: It pulls in the work that @jacobcdewitt has done to make the polyfill compatible with the version of WebXR that will be in Chrome 76/77. This is not the same as the "VR Complete" version of the spec, but it's not too far off. It also includes a small shim to avoid the need for applications to specify a chrome-specific WebGL layer option that can be inferred from the session mode. This gets us to a point where any code written against the VR complete standard should actually be accepted by the polyfilled API, but some values are being ignored.
To get us to VR complete on top of this set of changes, we'll need the at least the following:
requiredFeaturesandoptionalFeaturesin theXRSessionInit. (You can pass them now, but they're ignored.)xr-standardlayout and attribute values (There's already comments in the code to indicate the needed changes here.)visibilityStatesession attribute andvisibilityChangeeventOh, and we want to get the tests working again.
I'm sure there's some more minor changes in there that I've neglected to call out, but those are definitely the big ones.