supermedium / moonrider

🌕🏄🏿 Surf the musical road among the stars. Side project built by two people in a few months to demonstrate WebXR.
https://moonrider.xyz
MIT License
569 stars 218 forks source link

Underlying THREE.js version is broken for Chromium #184

Open alcooper91 opened 5 months ago

alcooper91 commented 5 months ago

When launching moonrider.xyz on a chromium based browser (e.g. Chrome on Windows), the first session created never submits any frames. If I end that session and re-enter, everything looks to be fine.

We tracked this down and it looks like there is a race with creating an XrWebGlLayer and not awaiting on gl.makeXrCompatible(), which was fixed in THREE.js here. However, it looks as though that fix is not in the version of THREE.js used by the version of AFrame that Moonrider is using.

The fork linked in this PR, does appear to resolve the issue by adding that fix in; but it may be possible to simply cherry-pick the previously linked fix in if desired to keep this to a more scoped fix.

dmarcos commented 4 months ago

Thanks so much for the heads up and pointing to the solution. I took the PR and deployed it. After a quick test everything seems to work. If it doesn't stick I'll go for the more surgical approach. Let me know if it works you.

alcooper91 commented 4 months ago

Looks like it worked for me as well.

svillar commented 4 months ago

Yeah we hit that also in the Chromium port of Wolvic.

dmarcos commented 4 months ago

I reverted the update to A-Frame 1.3.0 due to regressions from updating THREE. I also patched THREE in the old A-Frame 1.1.0 to capture the gl.makeXrCompatible promise. It's an interim quick hack that hopefully fixes the problem on Chrome and Wolvic

@alcooper91 @svillar Can you give https://moonrider.xyz/ a try and verify everything works as expected? Thanks!

svillar commented 4 months ago

I reverted the update to A-Frame 1.3.0 due to regressions from updating THREE. I also patched THREE in the old A-Frame 1.1.0 to capture the gl.makeXrCompatible promise. It's an interim quick hack that hopefully fixes the problem on Chrome and Wolvic

@alcooper91 @svillar Can you give https://moonrider.xyz/ a try and verify everything works as expected? Thanks!

Wolvic with Gecko works fine after the revert, thanks!

That said you mention the patch for makeXrCompatible. Not sure if you're talking about https://github.com/mrdoob/three.js/issues/21126 but at least for Wolvic Chromium, what you patched is not enough, we still need a downstream patch for Chromium to start the WebXR session. Looks like this is needed

dmarcos commented 4 months ago

I didn't take any of the THREE patches. I just did "gl.makeXRCompatible().then(function () {})" A hack but I think should work around issues for now. My understanding is that capturing the promise is mandatory.

We will bump THREE (A-Frame) properly in the future. There's always a ton of side effects when updating THREE in particular (hence the revert)

alcooper91 commented 4 months ago

I haven’t had a chance to check Chrome yet, but my understanding is that you’ll need to wait for that promise to resolve before attempting to create the XRWebGLLayer, I’m not sure that capturing the promise is enough. The error dialog in the issue @svillar linked looks like exactly the error from not waiting on the promise, so it’s likely this is still occurring/the hack is insufficient. I’ll hopefully be able to check into this more tomorrow.

dmarcos commented 4 months ago

Thanks @alcooper91 @svillar. @Maxmystere found a fix for the keyboard on A-Frame 1.3.0 and now underlying THREE should incorporate the necessary fixes. Can you confirm on your side? Thanks again.

svillar commented 4 months ago

Thanks @alcooper91 @svillar. @Maxmystere found a fix for the keyboard on A-Frame 1.3.0 and now underlying THREE should incorporate the necessary fixes. Can you confirm on your side? Thanks again.

It seems to work indeed, however there is now a visual artifact when hovering a letter. See screenshot Screenshot_20240223_101356

alcooper91 commented 4 months ago

Did not test the keyboard; but confirmed the experience launches for us.

dmarcos commented 4 months ago

Thanks for giving it a try