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

Add WebXR support (for Godot 3.2) #42397

Merged
merged 1 commit into from
Jan 4, 2021
Merged

Add WebXR support (for Godot 3.2) #42397

merged 1 commit into from
Jan 4, 2021

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Sep 29, 2020

Implements proposal godotengine/godot-proposals#1581

This is for Godot 3.2, because WebXR depends or WebGL, and there is presently no OpenGL/WebGL support in the master branch. When it's added back, I can port this to master.

This is a work-in-progress! Once it's fully functional, it will get lots of clean-up and better error handling, and I'll squash it or start a clean pull request.

I'm hoping that I can use this PR to get some review and collaborate with other contributors. :-)

@dsnopek dsnopek requested a review from reduz as a code owner September 29, 2020 02:38
@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 29, 2020

Here's a YouTube video demonstrating what's working so far:

https://www.youtube.com/watch?v=UKHjR60i5jI

Unfortunately, at the moment, I'm stuck on figuring out how to render the render targets to the headset (rather than the screen). I explain the problem towards the end of the video, but I'll attempt to explain it here as well:

WebXR passes us a framebuffer object and viewport dimensions that it wants us to render each eye to, in order to send the data to the headset. Ideally, I'd like to bind to that framebuffer, and then blit the render target to the framebuffer in the given viewport for each eye.

Calling VSG::rasterizer->blit_render_target_to_screen(p_render_target, viewport, 0); almost does what I want, but it binds to the screen's framebuffer before blitting, so that doesn't get the data to the headset. In my PR, I copied that method, making a new VSG::rasterizer->blit_render_target_to_current_framebuffer(p_render_target, viewport); where I removed the part where it calls glBindFramebuffer() but that doesn't work for some reason.

If anyone has any advice on how to implement that method, or any other better ways to do this, please let me know!

@BastiaanOlij you're the AR/VR expert - any thoughts?

Thanks in advance! :-)

@dsnopek dsnopek marked this pull request as draft September 29, 2020 02:53
@HeadClot
Copy link

Good work so far @dsnopek. Got a question though :)

Will we be able to use this along side plugins such as Qodot for example?

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 29, 2020

@HeadClot I don't see why not! If you can use Qodot in games exported to HTML5, you should still be able to use it with WebXR enabled :-)

One important note, though: this isn't a plugin, but a change to Godot core. So, until this gets merged into Godot and released (which I hope it does, but that may never happen), in order to use this, you'd have to patch and compile the HTML5 export template yourself (or use the template that someone else already compiled with this patch)

@HeadClot
Copy link

@HeadClot I don't see why not! If you can use Qodot in games exported to HTML5, you should still be able to use it with WebXR enabled :-)

One important note, though: this isn't a plugin, but a change to Godot core. So, until this gets merged into Godot and released (which I hope it does, but that may never happen), in order to use this, you'd have to patch and compile the HTML5 export template yourself (or use the template that someone else already compiled with this patch)

Thank you for the information about Qodot. And totally Understood about the Pull Request. :)

@fire
Copy link
Member

fire commented Sep 29, 2020

Are you able to figure out why the Javascript platform is failing?

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 29, 2020

Are you able to figure out why the Javascript platform is failing?

I'm not sure yet. Here's the actual error from the CI log:

[JSC_UNDEFINED_VARIABLE] variable XRWebGLLayer is undeclared

XRWebGLLayer is a class that should be defined by the browser.

Perhaps its a difference between the Emscripten version that I'm using locally versus the version used by CI?

I will certainly make sure all the tests are passing before taking the PR out of "draft" status. :-)

@Faless
Copy link
Collaborator

Faless commented Sep 29, 2020

@dsnopek probably need to be added as an extern for the closure compiler. I'd say don't worry too much about it, I can help you fix it when you feel the PR is ready.

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 29, 2020

@Faless Ok, great, thanks!

@m4gr3d
Copy link
Contributor

m4gr3d commented Sep 29, 2020

@dsnopek Great work! Leaving it out to @BastiaanOlij to confirm, but it seems that with the exposure of a couple functions to gdnative, this can be turn into an external gdnative plugin similar to the ones in https://github.com/GodotVR.

Do you think there is a strong reason to have it within the engine itself as a module?

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 29, 2020

@m4gr3d Thanks!

[...] it seems that with the exposure of a couple functions to gdnative, this can be turn into an external gdnative plugin similar to the ones in https://github.com/GodotVR. Do you think there is a strong reason to have it within the engine itself as a module?

I mentioned this in the proposal (godotengine/godot-proposals#1581): WebXR is for HTML5 only, and, unfortunately, gdnative doesn't work in the HTML5. This may also require some minor changes to the HTML5 export, so at least some part of this needs to be done in Godot core.

@m4gr3d
Copy link
Contributor

m4gr3d commented Sep 29, 2020

@m4gr3d Thanks!

[...] it seems that with the exposure of a couple functions to gdnative, this can be turn into an external gdnative plugin similar to the ones in https://github.com/GodotVR. Do you think there is a strong reason to have it within the engine itself as a module?

I mentioned this in the proposal (godotengine/godot-proposals#1581): WebXR is for HTML5 only, and, unfortunately, gdnative doesn't work in the HTML5. This may also require some minor changes to the HTML5 export, so at least some part of this needs to be done in Godot core.

@dsnopek Sounds good, thanks for the clarification!

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 30, 2020

My latest commit gets this actually displaying to the headset on an Oculus Quest using GLES2! However, something is wrong with the eye transform or projection, because it doesn't give the correct binocular effect. And GLES3 doesn't work for some reason either.

But it's pretty exciting to finally get it displaying something on a real headset! Once it's looking a little better I'll record a video from the headset :-)

@BastiaanOlij
Copy link
Contributor

I have to dive into WebXR a little more but like I said in the comment on the Youtube video, there has to be a way for WebXR to simply provide the texture buffer they want you to render into.
That would do away with the extra copy you are doing right now and possibly solve some of the other issues you are having.

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 30, 2020

@BastiaanOlij Thanks for the code review! I'll try to address both of those things later today.

I have to dive into WebXR a little more but like I said in the comment on the Youtube video, there has to be a way for WebXR to simply provide the texture buffer they want you to render into.

I spent quite a bit of time yesterday trying to see if there was a way to get a texture, I couldn't find anything.

The XRWebGLLayer' object is what connects connects WebXR to WebGL:

https://developer.mozilla.org/en-US/docs/Web/API/XRWebGLLayer

It only gives us access to a WebGLFramebuffer, not a texture, and I couldn't find anything in the WebGL API that would allow us to get the texture object from the framebuffer. That said, I'm really not that great at OpenGL/WebGL, so perhaps there is some way that a more experienced OpenGL developer would know?

However, if we only have a framebuffer, I wonder if we could instead add a new method to Godot's RasterizerStorage like render_target_set_external_framebuffer() to allow having a render target that has a framebuffer without an associated texture?

Then we'd also need to modify the ARVRInterface class to have an new optional virtual method like get_external_framebuffer_for_eye()' that would allow the WebXRInterface` class to return a framebuffer that could be assigned to the render target and allow Godot to render directly to it?

Seems like some big changes to Godot's internals, though, just for WebXR...

@dsnopek
Copy link
Contributor Author

dsnopek commented Oct 3, 2020

The latest changes get a lot closer to correctly rendering!

I ended up realizing that the main problem was that the code for blitting the textures to the headset's framebuffer was stretching the texture in a weird way. So, I ended up doing the thing I really didn't want to do: writing some custom WebGL code to render the texture on a quad with a little custom shader and everything. But that solved it, got it working on both GLES2 and GLES3, and allowed me to remove the changes I made to the rasterizer.

However, it's still not quite right... I think the get_transform_for_eye() is off somehow. It feels like it's rotating around the wrong point or something? Like, rotating my head seems to move me in space in an unnatural way. Testing while it has these problems has been making me feel quite sick, so probably time to stop for the night. :-)

@BastiaanOlij
Copy link
Contributor

BastiaanOlij commented Oct 3, 2020 via email

@dsnopek
Copy link
Contributor Author

dsnopek commented Oct 3, 2020

@BastiaanOlij You are right! Of course, I didn't see your comment until just now, after I'd already found the same solution. :-)

I actually tried swapping column-major vs row-major more than once earlier on, but I think some other bug(s) that I've since solved, must have prevented that from working (or at least showing a noticeable difference) at the time.

So, with the commit I just pushed, it's actually rendering fully correctly as far as I can tell on my Oculus Quest!

There's still lots to do, of course: controllers, some clean-up, some API additions so the developer can initialize WebXR with the desired parameters, etc. And I'd really like to at least try experimenting with AR (since VR is only half of WebXR), but I have no previous experience with AR, so that might take me a bit to figure out.

@dsnopek
Copy link
Contributor Author

dsnopek commented Oct 6, 2020

I just recorded a new video which shows this PR running on a real Oculus Quest:

https://www.youtube.com/watch?v=a5IHo_MLC5c

@HeadClot
Copy link

HeadClot commented Oct 6, 2020

I just recorded a new video which shows this PR running on a real Oculus Quest:

https://www.youtube.com/watch?v=a5IHo_MLC5c

Oh wow! Did not expect this to come this far so soon. Good work!
Cannot wait for controllers to be working :)

If you do not mind me asking @dsnopek what parameters will you be exposing to devs?

@dsnopek dsnopek force-pushed the webxr branch 2 times, most recently from d7f7d9c to bff33ca Compare December 6, 2020 04:59
@dsnopek dsnopek marked this pull request as ready for review December 6, 2020 05:09
@dsnopek
Copy link
Contributor Author

dsnopek commented Dec 6, 2020

Alright!

I've added the last couple things I wanted to add before taking this out of "Draft" status, including adding support for all the events (including select* and squeeze*) such that almost all VR related features are supported, and adding some class reference documentation. There's still a couple small things missing, a couple minor bugs, and, of course, all of AR to implement, but those can be done in later PRs.

So, I've rebased and squashed this whole PR and removed "Draft" status!

@Faless @BastiaanOlij I'd really appreciate your review when you have time! Thanks in advance :-)

EDIT: Oh, and I know I need to make a PR against master as well - I'll work on that tomorrow.

EDIT: This branch contains a backup of all the original history on this PR, in case that helps with review or anyone trying to follow along with the comments: https://github.com/dsnopek/godot/commits/webxr-backup1

@dsnopek dsnopek changed the title [WIP] Add WebXR support (for Godot 3.2) Add WebXR support (for Godot 3.2) Dec 6, 2020
@aaronfranke aaronfranke added this to the 3.2 milestone Dec 6, 2020
@dsnopek
Copy link
Contributor Author

dsnopek commented Dec 7, 2020

PR #44154 is the port to the 'master' branch!

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

I wish I had more time to take this for a proper spin and not just a take a quick glance at the code.

I love this, absolutely love love love this. Been following dsnopek's progress with his video logs and very impressed by him getting this to work,.

I'm all for merging this in, I'm happy with this being part of the core especially seeing it's all implemented as a module and if people really want to save the few bytes it may spare they can turn the module off.

As webXR is an open standard I don't see anything holding us back here.

@dsnopek
Copy link
Contributor Author

dsnopek commented Dec 15, 2020

@BastiaanOlij Thanks so much!!! :-)

And, here is my next progress report video:

https://www.youtube.com/watch?v=qRW9pNTWcVQ

@dsnopek
Copy link
Contributor Author

dsnopek commented Jan 4, 2021

Per code review from @Faless on IRC, I just added a new commit which completely un-monkey-patches window.requestAnimationFrame() when WebXR is uninitialized. I tested it in both Chrome and Firefox with the WebXR emulator extension, and in the Oculus browser on a Quest. All seems to be working!

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Well, with the new year I finally had the time to try out the PR with the browser extension.
Once more, this is really impressive work 👍 and I think it's ready to merge.

If you can update the 4.0 PR I'll approve that one too.

@dsnopek
Copy link
Contributor Author

dsnopek commented Jan 4, 2021

@Faless Thanks so much! The 4.0 PR is updated too #44154

@akien-mga
Copy link
Member

Should the two commits be squashed?

@dsnopek
Copy link
Contributor Author

dsnopek commented Jan 4, 2021

@akien-mga They certainly could be squashed! I didn't squash them in my branch so that it was easy for @Faless to review. You can squash them on merge, or would you like me to squash them?

@akien-mga
Copy link
Member

We don't use squash on merge in the code repo as we want the merged history to match that of PRs (the commit hashes shown here would match what's in 3.2 after merge - if we squash on merge they're dangling commits).

@dsnopek
Copy link
Contributor Author

dsnopek commented Jan 4, 2021

@akien-mga Ah, ok, thanks. In that case, I've squashed the two commits here in the PR (and on the 4.0 one too)!

@akien-mga akien-mga merged commit 3d88ea8 into godotengine:3.2 Jan 4, 2021
@akien-mga
Copy link
Member

Thanks and congrats for this great contribution!

@akien-mga akien-mga modified the milestones: 3.2, 3.3 Apr 20, 2021
@dsnopek dsnopek deleted the webxr branch July 22, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants