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

Webvr to webxr #42

Merged
merged 19 commits into from
Apr 3, 2024
Merged

Webvr to webxr #42

merged 19 commits into from
Apr 3, 2024

Conversation

catmajor
Copy link
Contributor

@catmajor catmajor commented Dec 31, 2023

Converts webVR API to webXR for fish tank

Demo at https://catmajor.github.io/WebGLSamples.github.io/aquarium/aquarium.html

@kainino0x
Copy link

@toji?

Copy link
Contributor

@toji toji left a comment

Choose a reason for hiding this comment

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

Thank you for taking on this update! I've wanted to do it myself for a while and never had the time. It's a big undertaking, and there's some subtle and easy-to-trip-over quirks to working in VR in any environment, so please don't take this review as a knock on your code. I'm just trying to focus on the possible issues so that this can be a comfortable, fun experience for those who try it!

I tried this both on an Android phone with Cardboard and on Quest 3, and it's failing differently on both devices.

On Cardboard things initially look OK but there's no stereo depth to the scene? It feels like everything is projected on a dome very far away. Also, looking a little to the left and right feels OK but when turning around 360 degrees the view stops responding, locks up, and eventually clicks back into place again. It indicates something is being done wrong in the view matrix math.

On the Quest 3 I initially only got a blue background with some bubbles rendering when I first tried to enter VR. The second time I saw the fish but the eye offsets were wrong and I couldn't converge the two images, which was quite uncomfortable. The third time I tried I was back to the blue backgrounds with a random bubble effect as the only thing visible.

When doing development I highly recommend testing the full range of head motion on at least an Android device in cardboard mode. Even if you don't have a cardboard viewer available you'll be able to observe areas where the camera motion doesn't match the rotation of the device.

Also, if you don't have a device like a Quest handy I would advise not attempting to include features that only work in their browser, like multiview. That can come as a follow-up if needed.

I've added some comments to specific areas of the code that I think are likely to be misbehaving at the moment. Feel free to ask further questions or toss alternate ideas my way and we can discuss them as you go! Again, this is a complex API with lots of unfortunate sharp edges, so don't get discouraged that things aren't working as expected on the first try.

eyePosition[1] = g.globals.eyeHeight;
eyePosition[2] = g.globals.eyeRadius;

let vrPose = [pose.transform.orientation.x, pose.transform.orientation.y, pose.transform.orientation.z, 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

This discards the w value from the original orientation quaternion, which will yield inaccurate results and is likely part of the cause for the issue with head rotation not responding correctly all the way around. Please use the full quaternion reported by the pose.

Suggested change
let vrPose = [pose.transform.orientation.x, pose.transform.orientation.y, pose.transform.orientation.z, 1]
let vrPose = [pose.transform.orientation.x, pose.transform.orientation.y, pose.transform.orientation.z, pose.transform.orientation.w]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried it on Oculus Browser and Wolvic on Quest 2, and it does not work. Going to investigate it.
Tested on cardboard on Galaxy A34 on chrome versions 120.0.6099.145 and .193 .
Previously, I had problems with older versions of Chrome crashing on entering XR, but this does not happen anymore.

I made this change but later removed this code because it was unnecessary since all matrices were coming from xrPose.

// user could see around but couldn't move around.
eyePosition[0] = g.globals.eyeRadius;
eyePosition[1] = g.globals.eyeHeight;
eyePosition[2] = g.globals.eyeRadius;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear to me what using the eye radius here is intended to do, but I can pretty confidently say the position from the appropriate pose view's transform should be used here instead. Something like:

const viewPos = pose.views[n].transform.position;
eyePosition = [viewPos.x, viewPos.y, viewPos.z];

The comment makes note to "User could see around but couldn't move around" but this code ignores all the positional data provided by the headset if it's capable of positional tracking. (If it's not, like a simple cardboard viewer, the WebXR implementation will provide its own neck model. You don't need to emulate it in your code.)

If you were previously using the view positions from WebXR and still not seeing any apparent movement, that may be because the scale of the aquarium is not in the same units as WebXR reports (I think this is what the comment it saying?) If so, the appropriate way to rectify that is not to either scale the environment and the objects in it to match WebXR's scale (as the comment notes: 1 unit == 1 meter) or apply a scale to the view matrices computed from the WebXR data (position included) to match the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this change but removed later because it was unnecessary since all matrices were coming from xrPose.

eyePosition[2] = g.globals.eyeRadius;

let vrPose = [pose.transform.orientation.x, pose.transform.orientation.y, pose.transform.orientation.z, 1]
calculateViewMatrix(viewInverseTemp, vrPose, eyePosition);
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no stereo effect in VR or when using the "stereo mode" on desktop, which at least in part would be because the same view matrix is used for both views. This will cause each "eye" to render from the same location. To get accurate results the view matrix should be calculated per-view.

Also, WebXR provides its own convenience method for computing the view matrices if you want to use it, instead of building these out manually.

viewInverseTemp = pose.views[n].transform.inverse.matrix;

Once this is fixed if there still appears to be a lack of stereo depth it could possibly be related to the scale mismatch talked about in an above comment. Objects that are very large and far away will lack visible stereo depth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto

xrViewport = glLayer.getViewport(pose.views[1]);
gl.viewport(xrViewport.x, xrViewport.y, xrViewport.width, xrViewport.height);
gl.scissor(xrViewport.x, xrViewport.y, xrViewport.width, xrViewport.height);
render(pose.views[1].projectionMatrix, viewInverseTemp, false, pose);
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that you're using the right viewports and projection matrices here is great, but as noted above you should be recalculating the view matrix as well for each eye. Also, you can make this more robust to a variety of XR devices (and make the code easier on yourself) by looping through all of the given views rather than assuming there are only ever 2. Something like the following:

for (const view of pose.views) {
  let xrViewport = glLayer.getViewport(pose.views[0]);
  gl.viewport(xrViewport.x, xrViewport.y, xrViewport.width, xrViewport.height);
  gl.scissor(xrViewport.x, xrViewport.y, xrViewport.width, xrViewport.height);
  render(view.projectionMatrix, view.transform.inverse.matrix, false, view); // Not clear on what the final arg is used for here? Glancing at the code it looks like it should probably be view instead of pose.
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Also removed the obsolete calculateViewMatrix block.

@catmajor
Copy link
Contributor Author

catmajor commented Jan 4, 2024

@toji
Thank you Brandon for your review. I will address those issues over the weekend.

@toji
Copy link
Contributor

toji commented Jan 10, 2024

Has the link in the first comment been updated with the latest changes? It seems different at least.

Assuming it has been, I now see with a Cardboard viewer that the view orientation is inverted. (As in, if you look up the virtual view looks down, if you look left the virtual view looks right.)

Looking into the code, I can see in the render() function implementation that the viewInverse variable is, ironically enough, being inverted and then multiplied with the projection matrix to make the final projection+view matrix used by the shaders. Since the "view" matrix in most rendering is, effectively, the inverse of the camera transform, this feels like the naming is backwards to me. Regardless, it suggests that you should maybe be using view.transform.matrix instead of view.transform.inverse.matrix.

Sorry for the possibly misleading initial suggestion, I blame the unclear variable names. 🤷

fast.matrix4.inverse(view, viewInverse);

@catmajor
Copy link
Contributor Author

Hi Brandon,
Sorry for the delay with the reply. I was puzzled as to why it wasn't working on Oculus.
However, after a recent Oculus update, it looks like it works in Oculus browser version 122.0.6261.64 .
Wolvic browser still doesn't work.
Thanks,
Nikita

Copy link
Contributor

@toji toji left a comment

Choose a reason for hiding this comment

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

This is looking much better! Tested with Cardboard and a Pixel 8 just now and the head tracking is working as expected. Thanks for pushing it this far! I think that there's some additional work to do in terms of presenting the content well, but that can probably come as a separate PR.

Specifically, while the head tracking is working correctly it looks like the fish tank itself is gigantic relative to the viewer. The viewport is positioned very low to the floor and the fish seem very large and far away, meaning that any stereo effect is minimal. I haven't tested with my Quest headsets yet, but I suspect that walking around would yield very little movement relative to the tank floor because of the apparent scale involved. I suspect that if the tank was scaled down or the viewer's movements scaled up (I'm not yet sure which would be easier) then the effect would be much more striking. Ideally we probably want the user's head to end up somewhere near the center of the aquarium by default, with the virtual floor lining up with their physical one. That would likely still make the fish somewhat large relative to the user, but they'd be at a scale and distance that makes them much easier to observe and feel like you're in the same space with. But like I said, that can come later. The fact that this works at all means it's a big improvement over the old, deprecated WebVR code.

I will say that I still don't quite understand the motive behind the "Stereo mode". The two viewports it presents appear to be identical, so it doesn't provide a stereo effect even if you cross your eyes or have some other way of combining the images. And when you do the page UX still overlaps only one side of the canvas, which would interfere with any stereo effect that was present. I'd prefer to remove that option unless there's a known environment in which it's beneficial (custom kiosk, maybe?) and if that's the case it can probably be only shown with a specific URL argument.

I'm not sure what would be wrong with Wolvic, but I suspect their implementation is still considered a WIP. I'll see if I can test it later and dig up what the problematic part is.

In any case, lets either remove or situationally hide the Stereo View button and address a couple of other comments and I think we can land this!

aquarium/aquarium.js Outdated Show resolved Hide resolved
aquarium/aquarium.js Outdated Show resolved Hide resolved
aquarium/aquarium.js Show resolved Hide resolved
If set to true will render the stereo demo button
Also changed stereo demo button comment to reflect this
translationScale adjusts vr mode movement to be close to real world.
floorAdjust lifts eye position to the fish level.
@catmajor
Copy link
Contributor Author

catmajor commented Apr 2, 2024

Hi Brandon,
I adjusted the position scale to be closer to the real world and eye position to be on the fishs' level.
Also scaled the fishes down a bit.
I changed the stereo mode button to only appear when stereo-enabled=true in the URL.

I have a question about VR UI. It resides now in aquarium-vr.
I'd like to move it to aquarium folder and try to make it work in another PR.
What do you think?

Thanks,
Nikita

Copy link
Contributor

@toji toji left a comment

Choose a reason for hiding this comment

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

This looks great now! The scale changes you made do help quite a bit, as the fish and the elements in the tank clearly feel like they have depth now. Testing with a Quest 3 I can move around and the tank and fish have a nice scale and position to them.

I know this was a pain to work through, but I really appreciate you sticking with it and working out the bugs. It's really cool to be able to experience this classic WebGL demo in a new way! Thank you!

@toji
Copy link
Contributor

toji commented Apr 2, 2024

I'm not entirely sure what you mean by the VR UI in aquarium-vr? I've never tried it. I'd generally say that unless there's a large amount of code differences between the two, however, it seems like a reasonable idea to pull any remaining changes over into this one and have just the single "aquarium" folder.

Also, every WebXR-capable device can support a local-floor reference space, but you have to request an additional feature that I forgot to mention. In any case, I tested it out locally and it does help with the user's position quite a bit! (Puts your head approximately in the center of the tank, which is nice.) The floor doesn't naturally line up, though, which feels a little disconcerting. I think once this lands I'll take a pass at adjusting the reference space and getting the floor to line up so that people with 6DoF headsets feel more grounded. Regardless, it's a fun demo even without those adjustments so I think this should be landed as-is!

@catmajor
Copy link
Contributor Author

catmajor commented Apr 3, 2024

Thanks Brandon,
I'll play more with local floor.
There's some sort of VR UI in the aquarium-vr folder (vr_assets/ui). It looks like VR code was taken from aquarium-vr. It refers to VR UI but fails because the folder does not exist. I'd like to move it and make it functional.

@toji
Copy link
Contributor

toji commented Apr 3, 2024

Got it. I'd be interested to see what that entails. In the meantime I still think this can land as-is with the other tasks you're talking about being left to follow-up PRs.

@kainino0x, do you have write access to this repo? If not, who should we ping about merging?

@kenrussell
Copy link
Member

Thanks @toji for reviewing. I invited you to be a co-owner of this repo, but merging this myself. Thanks @catmajor for your contribution.

@kenrussell kenrussell merged commit 06f825a into WebGLSamples:master Apr 3, 2024
@GKraats
Copy link

GKraats commented Jun 8, 2024

Since a short time aquarium totally fails if webgl2 not available.
I am running an old Dell laptop with linux Debian Testing 
and firefox, which does not support webgl2.
It only shows a page with misleading errormessage:
"It does not appear your computer supports WebGL"
and loops with varying fps-count.

Problem is caused by this Merge-Request.
At aquarium/aquarium-config.js it sets enableVR: true, 
causing webgl2 to become the preferred driver.
The first check for driver webgl2 fails, causing the 
display of the error-page.
The next check for driver webgl succeeds, causing the 
program to continue, but probably because of the active error-page,
it fails displaying the aquarium.

Problem can be solved at tdl/webgl.js at
tdl.webgl.setupWebGL.
Function handleCreationError should only
generate the message but not show it.
The message should be only shown at the end of
tdl.webgl.setupWebGL, if no context is available.

I added pull-request #44

phemavax pushed a commit to phemavax/WebGLSamples.github.io that referenced this pull request Jul 5, 2024
Convert all WebVR usage to WebXR. Extensive revisions based on feedback from @toji.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants