-
-
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
Remove WebVR support and polyfill #5451
Conversation
Not sure if ready to remove WebVR yet because of Firefox. |
firefox reality is now https://www.wolvic.com/en/ and they support webxr |
Meant Firefox desktop |
@dmarcos Firefox desktop has WebVR disabled by default since 98 (link). It can be enabled by setting |
3ef7474
to
2b2d7f3
Compare
This needs rebase. Also cardboard support is still around albeit disabled by default. Heard of some people still using it |
This needs rebase |
2b2d7f3
to
1347df4
Compare
@dmarcos Rebased.
One option for cardboard users could be to use the webxr-polyfill, which uses the same underlying cardboard-vr library as the webvr-polyfill currently does. Should just be a matter of including and initializing it before A-Frame. Instructions for it could be included in the release notes and/or noted somewhere in the documentation. In any case, cardboard doesn't need WebVR controller support, so those changes can be done regardless. |
Let me know if you want me to split off a PR that focusses on removing the WebVR controller logic. |
as reference Babylon removed it with 7.0.0 https://github.com/BabylonJS/Babylon.js/releases/tag/7.0.0 |
Can I bump this PR? I believe I am currently experiencing issues where my Firefox (beta) for Android mobile is breaking because of the PolyFill.
|
@danineuss what A-Frame version? |
Version 1.6.0 does still use the polyfills (or at least that is what the
log says). But I believe I could fix the issue in the meantime. Thank
you so much for your quick reply and continued effort in this wonderful
framework 🙏🏽
On June 13, 2024, radioman85 ***@***.***> wrote:
@danineuss <https://github.com/danineuss> what A-Frame version?
—
Reply to this email directly, view it on GitHub
<#5451 (comment)>,
or unsubscribe <https://github.com/notifications/unsubscribe-
auth/ADUA74YHTHYG7Q3LMROLPRTZHGJ43AVCNFSM6AAAAABC7Z54JGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRVGYYDKNZSG4>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I think I'm comfortable now removing the WebVR code path. Can we rebase this? Thanks |
1347df4
to
4251f4c
Compare
4251f4c
to
d2b55ae
Compare
@dmarcos I've rebased the PR |
Thanks. Always happy with PRs that are more red than green. |
So we can also remove the first three commits related to WebVRManager right? |
Description:
This PR removes WebVR support and WebVR polyfill (#5447), leaving only the WebXR code paths. This allows quite a lot of code to be removed and simplified. Especially the various
device-controls
components. While making the adjustments to the controllers and their unit tests it became painfully clear that there are many inconsistencies in the way controllers are handled. Many of them also exhibit changed behaviour compared to WebVR (e.g. naming of buttons changed), which showed as regressions in the unit tests.Marked as a draft as this PR will require extensive reviewing/testing. Seeing the poor state of the controller components and their associated tests, I'll probably create separate PRs that focus on moving the unit tests over to WebXR, followed by simplifying/fixing the controller components, which will leave this PR purely with the WebVR removal.
Changes proposed:
controls
components to only support/use WebXR