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

0.10.0 #298

Merged
merged 13 commits into from
Dec 18, 2017
Merged

0.10.0 #298

merged 13 commits into from
Dec 18, 2017

Conversation

jsantell
Copy link
Contributor

Have been working on these changes for a bit now, quite a big overhaul! Wanted to put up a PR to get comments and concerns from the WebVR community before landing. Changes and rationale from the release notes draft as follows:

v0.10.0

This is one of the largest updates for the Polyfill in quite some time! Most changes are refactoring, focusing on pulling pieces out to make it easier to scope, test, and work on future changes as the WebXR Device API approaches, so we can be more confident in changes without breaking edge cases. The polyfill was also scope creeping a bit, doing more than just providing a stable API surface for developers to target, and many of these features have been removed that are better off in userland and on a per-project basis.

Feature Changes

  • The polyfill is now enabled by calling the exposed constructor, new window.WebVRPolyfill(options). Previously this was handled by defining options on a global window.WebVRConfig object, and the polyfill would instantiate itself immediately, or require the user to call window.InitializeWebVRPolyfill() if DEFER_INITIALIZATION was set. DEFER_INITIALIZATION has been removed, the polyfill no longer instantiates itself, and options are passed into the constructor rather than relying on a global object. This makes it easier to work with in module environments and other JS build systems.
  • The webvr-polyfill no longer supports adapting 1.0 WebVR APIs to work with 1.1 content. In the event of a 1.0 browser using the polyfill, everything will be polyfilled as if 1.0 support does not exist. InstallWebVRSpecShim() has been removed.
  • navigator.vrEnabled property has been removed. This was used to indicate if displays exist, which can be done by checking the return value of getVRDisplays().
  • MouseKeyboardVRDisplay has been removed. This provided a monoview VRDisplay with some primitive controls for a 3DOF experience. This is better handled with custom controls or OrbitControls depending on the experience. MOUSE_KEYBOARD_CONTROLS_DISABLED option has been removed. This means on desktop, the only displays that can exist are native, so user code should handle when no displays are found.
  • Rules for injecting polyfilled displays have changed and simplified. With the removal of MouseKeyboardVRDisplay, and the WebXR Device API only allowing one display rather than an array of displays, ALWAYS_APPEND_POLYFILL_DISPLAY has been removed. A new option, PROVIDE_MOBILE_VRDISPLAY has been created to provide a CardboardVRDisplay when on mobile and the native API exists but no native displays exists. true by default, this means that the polyfill is always injected on mobile when enabled, since we won't know immediately if any native displays exist. FORCE_ENABLE_VR has been removed. This always provided a CardboardVRDisplay, even on desktop, used mostly for debugging purposes.
  • Added a DEBUG option. Previously this was triggered by a ?debug query parameter, which caused issues with other environments.
  • The main package.json entry now points to the built output rather than src/node-entry. This lets us take advantage of our build system and not require consumers to handle any transformations implemented.
  • Many WebVRPolyfill methods and properties have been removed: isFullScreenAvailable, isCardboardCompatible, isMobile, NativeVRFrameData, getVRDevices, enableDeprecatedPolyfill, isDeprecatedWebVRAvailable, isWebVRAvailable.

Infrastructure Changes

  • The CardboardVRDisplay has been pulled out of webvr-polyfill into it's own repo googlevr/cardboard-vr-display. This is by far the most complex part of the polyfill, and pulling it into its own repo makes it easier to draw an abstraction line between the polyfill and CardboardVRDisplay, consolidate cardboard-specific rendering issues, allow others to consume and use the CardboardVRDisplay alone, and make it easier to contribute.
  • We now have some tests! The tests cover the logic behind when and how things are polyfilled. There aren't many tests yet as it took a bit to get to this point, but looking forward to even more test coverage in the future.
  • Migrated from Webpack to Rollup for building. We've experimented with Rollup for our team's other projects and prefer it for libraries that are consumed by other projects due to its tree-shaking (227kb to 179kb for webvr-polyfill.js) and it's more simple configurations.

Copy link

@lincolnfrog lincolnfrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome change!

test/init.js Outdated
// Logic similar to https://github.com/webmodules/custom-event
// but can't rely on it's feature detection due to timing
// of including the shim and populating window/document
global.CustomEvent = function CustomEvent (type, params) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no space after CustomEvent

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why not use (type, params) => {

Seems clearer...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint fixed, and this is a constructor to simulate the global in cardboard-vr-display (which relies on globals unfortunately) as these run in a node environment

});

describe('hasNative', () => {
it('false when no native support found', () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standard it string would be 'should be false when no native support found'. Same for below (add 'should be')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

…ee.js source code, fix #297 with global.document clobbering, and fix typo in webvr-polyfill.js
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

Successfully merging this pull request may close these issues.

None yet

2 participants