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

VR support #3394

Merged
merged 38 commits into from
Jan 15, 2016
Merged

VR support #3394

merged 38 commits into from
Jan 15, 2016

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Jan 6, 2016

Only adds support for Cardboard and features that will make adding support for WebVR easier.

We should add another frustum type for an offset eye that creates a custom PerspectiveOffCenterFrustum, but we expect a PerspectiveFrustum whenever in 3D or Columbus view.

For a demo following an aircraft based on the interpolation Sandcastle example, go to bagnell.github.io/WebVRDemo.

  • Prevent the display from sleeping on mobile
  • Add Sandcastle example
  • Doc
  • Tests

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 6, 2016

The demo is awesome on my iPhone.

I was holding it in "landscape mode", then rotated it to "Portrait mode" and it looked like one (or one and part of the other) viewport went away when iOS rotated the canvas 90 degrees. Is this expected? Can we disable it? What is typical?

Quaternion) {
"use strict";

function DeviceOrientationCameraController(scene) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be @private for now? Or do you plan to expose it right away?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I plan to keep it @private. This should be expanded to use a PositionSensorVRDevice when we fully support WebVR. Still, there isn't anything configurable there so I don't know if we would every expose it.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 6, 2016

Part of #3001.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 6, 2016

As for the 1x1 video, perhaps let's see what it looks like in a Sandcastle example first.

@mramato
Copy link
Contributor

mramato commented Jan 12, 2016

Another thought would be perhaps VR support should be a CesiumViewer reference app feature and not a Viewer widget feature, but I guess that depends on how cleanly is can be supported in Viewer (in my opinion Viewer has grown way to big for it's own britches and eventually I would like a major refactor to occur).

var vrSubscription;
var vrModeSubscription;
var vrCallback;
if (options.vrButton === true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since vrButton is a boolean, no need for === true here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is every other boolean checked with !== false? Which is what I meant to put to be consistent with the other checks.

@bagnell
Copy link
Contributor Author

bagnell commented Jan 13, 2016

This is ready for another review.

@lilleyse lilleyse mentioned this pull request Jan 13, 2016
24 tasks
@@ -9,6 +9,7 @@ Change Log
* Fixed a bug that prevented WMS feature picking from working with THREDDS XML and msGMLOutput in Internet Explorer 11.
* Added `getExtensionFromUri` helper function.
* Added `getAbsoluteUri` helper function.
* Added `VRButton` which is a simple, single-button widget that toggles VR mode. It is off by default. To enable the button, set the `vrButton` option to `Viewer` to `true`. Only Cardboard for mobile is supported. More VR devices will be supported when the WebVR API is more stable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also mention scene.useWebVR?

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 14, 2016

This is ready once you make the Sandcastle/camera changes discussed offline.

@bagnell
Copy link
Contributor Author

bagnell commented Jan 15, 2016

@pjcozzi This is ready.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 15, 2016

The tests are crashing Firefox. Does that happen for you?

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 15, 2016

However, the Sandcastle demo works in Firefox. Perhaps the crash is an awkward viewport size.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 15, 2016

Nevermind, I updated Firefox and we are good!

pjcozzi added a commit that referenced this pull request Jan 15, 2016
@pjcozzi pjcozzi merged commit b87a4aa into master Jan 15, 2016
@pjcozzi pjcozzi deleted the vr branch January 15, 2016 18:48
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.

4 participants