-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
redo #2991: extend PR #2985 (moved canvas init) to also solve issue #2967 (vr2vr traversal in Oculus Browser) #2996
Conversation
rebase needed to remove the commit that has already landed |
c0324f5
to
0c1b945
Compare
rebase done - are you in a position to sanity check? |
I tried quickly but it does not seem to work for me. I did not have the controller though (I hacked the link traversal demo with a gaze based cursor). Tomorrow I'll try with the proper controller again |
Works for me with this test page as example https://s.codepen.io/machenmusik/debug/XaVMWJ/mVAbGERyxabk Once you enter VR, it reloads in 5 seconds. So if you end up out of VR, you know it is not working. |
I'm trying the link traversal example with your branch and the controller and I get dropped of vr. Does |
for me, it is working for simpler scenes, but not for ones with big assets to download -- I suspect it runs into the timeout issue in those cases |
7346c30
to
4df1a9a
Compare
looks like the setTimeout from prior PR is still needed for the VREffect to be available in the heavier cases. |
4df1a9a
to
14ff5a6
Compare
@dmarcos can you confirm latest PR version works for you? |
// This event may also fire in other circumstances (e.g. putting on the Rift headset). | ||
window.addEventListener('vrdisplayactivate', function (evt) { | ||
var s = window.AFRAME.scenes[0]; | ||
if (s.effect) { s.enterVR(); } else { setTimeout(s.enterVRBound); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is guaranteed that in the next tick the effect is going to be defined. I'm trying to find a more robust way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also is not what I would expect the event to behave following the spec. The user action policy should not apply to the next tick logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you find a more robust way, great. I tried several things and did not find another way for gesture permission to survive. See WICG/interventions#12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(More specifically, WICG/interventions#12 (comment))
@dmarcos any progress on a more robust solution? (Where this.effect will definitely exist while preserving user gesture?) |
@machenmusik out of curiosity, which example(s) are you testing with this? just https://aframe.io/aframe/examples/showcase/link-traversal/, yeah? |
This will be handled in un upcoming PR implementing a loading screen |
@cvan perhaps irrelevant now, but link-traversal example, this codepen (https://s.codepen.io/machenmusik/debug/XaVMWJ/mVAbGERyxabk) and a non-public app |
@dmarcos ok, as we discussed at OC4 |
@machenmusik thanks for the conversation at OC4 |
@machenmusik that CodePen URL gave me a 403. just posting this in case anyone else's reading: https://codepen.io/machenmusik/pen/XaVMWJ/ |
Thanks and sorry @cvan! |
no sweat! I was just testing WebVR flows in Oculus Browser. and I know you were working on this, so wanted to check. thanks for working on this. before I do a deeper dive, are there any known issues you know of for A-Frame + Gear VR? I ask because the new Oculus headsets are coming soon, and I want to make sure A-Frame and three.js are prepared. thanks again for all your PRs and keeping your finger on the pulse 👍 |
per @dmarcos:
I merged by mistake and reverted. Sorry for that. Can you please reopen? #2985 has landed so we need to rebase
(Rebase forthcoming.)