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

Get WebXR fully working in Godot 4! #68870

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Nov 19, 2022

This PR gets WebXR fully working in Godot 4, pretty much on par with how it works in Godot 3!

It's working in my testing with:

  • The WebXR emulator on desktop Chrome (only supports VR)
  • The Quest 2 (in both VR and AR mode!)
  • My smartphone in AR mode

There's a lot of good stuff in here:

  • It ports the changes from [3.x] Fix touch events when using smartphone AR with WebXR #56819 and Add WebXRInterface.xr_standard_mapping flag to attempt to convert button/axis ids to match other AR/VR interfaces #59994 which were merged into Godot 3, but not Godot 4 (because there was no way to test them at the time)
  • It switches to using the WebXR Layers API which allows us to render directly to textures provided by WebXR, skipping a copy that is costly on mobile. In Godot 3, we're stuck with this copy, but in Godot 4 we're finally free!! :-) The WebXR Layers API isn't supported on all browsers, but there is a polyfill that I'm planning to recommend in a future tutorial which will fill in support if it's missing. The only platform I have that has builtin support is the Quest, but the polyfill is working great for me on desktop and my smartphone!
  • It adds the necessary externs for the WebXR Layers API (for the closure compiler), and it's fully working when built with use_closure_compiler=yes
  • There are two small changes to the OpenGL renderer:
    • I didn't get the clean-up right when switching a render target to/from overridden textures in Add support for OpenGL to OpenXR #67775, but it didn't cause any problems until I tested with smartphone AR (which this PR finally gets working). I think I've got it doing the right thing now in this PR
    • We were already inverting Y when rendering to a render target with overridden textures in 3D - this does the same for 2D, which is needed to correctly render UI on smartphone AR
  • There is one small change to the Web platform:
  • Loads of internal refactoring and minor bug fixes!
    • One of the changes I'm happiest with is that transferring data from the JavaScript side to the C++ side should be more efficient in many cases (in particular, the data we have to transfer every frame)
    • A super minor thing: in Godot 3, the index on touch events was unstable. If you touch the screen with three fingers, and lift the first finger, then all the indexes for the other fingers would shift down by one. Now, they are stable: if a particular finger was assigned touch index 2 (for example) and you lift the first finger, it'll still keep index 2 for that finger. Probably no one was doing sophisticate enough things with multitouch to ever notice this bug, but I'm happy it's finally fixed :-)
    • ... lots more little things

The only way this is inferior to Godot 3, is that XR (not just WebXR) in Godot 4 requires multiview and there are devices which don't support multiview that work with Godot 3, but won't work now. For example, with Godot 3, I could use Google Cardboard with my smartphone, but with Godot 4, I can't because it doesn't support multiview. It would certainly be possible to refactor XR to work both with or without multiview, but that's a problem for another day

In every other way, this implementation of WebXR is superior to what we have in Godot 3! :-)

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Approved for the rendering changes only. Everything in drivers/GLES3 and renderer_scene_cull.cpp look good to me!

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.

The Web parts look good 👍 awesome work 🏆 , see my comment on the (unrelated) sys_env["JS_PRE"], but it's not a blocker, it can be addressed in its own PR.

platform/web/SCsub Show resolved Hide resolved
@dsnopek
Copy link
Contributor Author

dsnopek commented Nov 23, 2022

Thanks so much @Faless and @clayjohn!

(Note: I only asked @Faless to review the web platform changes, so the main changes in this PR in WebXRInterfaceJS still have haven't been reviewed, as far as I know.)

@BastiaanOlij
Copy link
Contributor

sorry @dsnopek , totally forgot to look at this, saw your message on #XR and then totally lost track of it...

Generally speaking everything looks good but I do think we need a few changes around controllers. There are a few hardcoded names that we assume to make things portable, though the tough part here is that OpenXR has a whole binding system that result in us not knowing what the raw inputs are, and giving complete control over the naming of actions to the developer. That does make it pretty hard to allow projects to move between WebXR and OpenXR but worth discussing further.

I'm also missing the head tracker but I may just be overlooking it. I added a head tracker for updating the XRCamera3D, this allows us to do proper predictive positioning for the frame being rendered while the rendering itself can use fresher data.

ping me on Discord and lets have some time to look at things together and nail the final bits.

@dsnopek
Copy link
Contributor Author

dsnopek commented Nov 28, 2022

Thanks, @BastiaanOlij!

Generally speaking everything looks good but I do think we need a few changes around controllers. There are a few hardcoded names that we assume to make things portable

Sure, if there's better names for the buttons, axes, and pressure on standard controllers, I can change them!

(There can be "non-standard input" as well, which there isn't much we can do about, all we get is IDs with no semantic meaning.)

though the tough part here is that OpenXR has a whole binding system that result in us not knowing what the raw inputs are, and giving complete control over the naming of actions to the developer. That does make it pretty hard to allow projects to move between WebXR and OpenXR but worth discussing further.

It'd be really cool if the action map system could work for both OpenXR and WebXR! But I haven't actually had a chance to really use action maps (aside from just poking around), so I'm not sure what the challenges would be there.

I'm also missing the head tracker but I may just be overlooking it.

The head tracker isn't in this PR, that was done earlier (I think by you, actually :-)). Here's the relevant code:

ping me on Discord and lets have some time to look at things together and nail the final bits.

Will do!

@dsnopek
Copy link
Contributor Author

dsnopek commented Dec 2, 2022

I chatted with @BastiaanOlij and he requested some changes to how some of the inputs were named, and that it also provide Vector2's for the thumbstick and touchpad (in addition to the floats for the individual axes). I just pushed those changes!

We also discussed how the OpenXR action map system could be generalized into the "XR action map system" so it could also be used by WebXR, but that's material for Godot 4.1 :-)

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'm very happy with this implementation. @dsnopek and myself spend some time doing the best to get some parity between OpenXR and WebXR so there is a path for developers to create a game that can be deployed on both systems.

We've also mapped out a future to see if we can bring WebXR into the action map system for those who wish to customise the actions, but that will be a 4.1 thing.

If @clayjohn is happy with the changes in the GLES3 driver, I'd say lets get this merged :)

@dsnopek
Copy link
Contributor Author

dsnopek commented Dec 2, 2022

Thanks, @BastiaanOlij!

If clayjohn is happy with the changes in the GLES3 driver, I'd say lets get this merged :)

He's already reviewed and approved those changes :-)

@akien-mga akien-mga merged commit 7ef9947 into godotengine:master Dec 2, 2022
@akien-mga
Copy link
Member

Thanks!

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.

5 participants