Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Updating WebVR polyfill #110

Closed
wants to merge 1 commit into from
Closed

Updating WebVR polyfill #110

wants to merge 1 commit into from

Conversation

delapuente
Copy link
Contributor

Fix #54

@delapuente delapuente requested review from caseyyee and cvan February 7, 2018 01:40
@delapuente
Copy link
Contributor Author

delapuente commented Feb 7, 2018

@caseyyee I need help testing with all devices since I just have the VIVE

See testing devices suggested by @cvan: #54 (comment)

I've said in the bug there were errors in Android for Chrome and Firefox with both the old and new polyfills so what I would really do is to check if there are regressions. How?

  1. Pass @cvan test suite to master right now and annotate the results per device
  2. Apply the patch.
  3. Pass @cvan test suite again and annotate the new results

Consider the test suite passed if there are the same or less errors.

Copy link
Contributor

@caseyyee caseyyee left a comment

Choose a reason for hiding this comment

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

Tried it out.

Few issues:

  1. Controllers don't work. (Oculus with FF)
  2. User starts at Y-axis at 0 height (Regular browser, no VR, w/ polyfill)
  3. Still get the source map issue error.

@cvan
Copy link
Contributor

cvan commented Feb 7, 2018

@caseyyee: when you pulled this branch, did you generate a new Build? in this PR, the Build/ files have not been updated. so that could explain it, if that is the case.

I looked at the source of https://raw.githubusercontent.com/delapuente/unity-webvr-export/2842060e69ff8d70a80c90c04b97a92e5408a1fe/Assets/WebVR/Plugins/webvr-polyfill.min.jspre, and it looks like the correct latest version of the webvr-Polyfill, and the external source-map comment at the end of the file is correctly removed.

let me know. I’ll test on my devices too.

@caseyyee
Copy link
Contributor

caseyyee commented Feb 7, 2018

Yup, I did do a new build. The polyfill doesn't appear to be loading, so I moved it into the template where the other libraries are and instantiated it as per the docs.

Still doesn't appear to be working -- still troubleshooting.

@caseyyee
Copy link
Contributor

caseyyee commented Feb 7, 2018

OK, So I looked into this further.

It looks like there has been some functionality removed from the latest polyfill. In previous versions, it returned a VR display on desktop and allowed for some basic mouse interactions. With the new version, this been removed and is expected to be managed by the app dev now.

What I propose is if there is no major issue, leave the existing polyfill for the community release, and follow up with an update that implements the new polyfill while also having the responsive features for desktop.

@caseyyee
Copy link
Contributor

caseyyee commented Feb 7, 2018

So we decided that we won't block on this for now. Will address as issue #54

🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants