WebXRManager: Fix XRCamera Local Space Behavior by Reverting #21964

[zalo]

Fixed #23597. Fixed #22884.

This PR fixes a bug introduced in r130 by reverting how XR Camera calculations handle local space (by partially undoing #21964 (which appears to conflate matrixWorld and matrix)).

This enables the XR camera to be parented to another moving object without odd behavior once again.

If this isn't the correct fix; I'd be happy to test alternatives in the context of my application.

Related issues: #21964 , https://github.com/google/model-viewer/pull/2464/files#r647866462

[mrdoob]

Apologies, I should have written the reasoning in #21964.

It has been just 2 months and I've already forgotten... @elalish do you remember what it was?

[elalish]

https://github.com/mrdoob/three.js/pull/21964#issuecomment-902074459

[zalo]

@elalish Apologies for the delay, I just re-added the decompose from the prior PR so the cameraVR's position/rotation/scale should be synced with its own matrix.

However, given that the cameraVR operates outside of the scene hierarchy, does it make sense to sync the cameraVR and scene camera's worldMatrix data directly?

[DePasqualeOrg]

This appears to fix the bug I referenced here: https://github.com/mrdoob/three.js/pull/21964#issuecomment-955605614

[DePasqualeOrg]

Any chance this could get included in the next release? My VR apps have not been working properly for months because of this bug.

[DePasqualeOrg]

Since this has already been approved, could we pull it into the next release? VR apps with a parented camera haven't been working properly for about half a year now.

[Mugen87]

@mrdoob I think this PR should be merged.

[marcofugaro]

Bump

[zalo]

I've got a cool three.js OpenXR demo waiting in the wings that could use this PR...

Note that scene locomotion works by parenting the camera to a parent group :-)

[LeviPesin]

Wow! Can you please share its code?

[kalegd]

+1, I think 9 months is a good amount of time for this pull request to have gestated (I say this with only humorous intentions without animosity)

[kalegd]

+1, I think 9 months is a good amount of time for this pull request to have gestated (I say this with only humorous intentions without animosity)

[ashconnell]

This issue has been resolved for quite some time now

[LeviPesin]

@ashconnell When and where was it fixed?

[msub2]

@LeviPesin I think he means to say there's been a consensus on this for months that it's good to go and yet it hasn't been merged in yet. Scrolling up through issues mentioning this you can see that this fix does indeed fix it for people.

[ashconnell]

@LeviPesin Apologies. I meant this should be merged as there are no objections.

I can also verify this PR does indeed fix the crackling PositionalAudio issues in WebXR, and would be delighted to decomission my fork once this is merged :)

[ashconnell]

Is there an issue with this PR? It keeps getting pushed back but seems like a trivial merge.

[zalo]

I suspect it's waiting for an example or test that would break (in the case that this functionality gets broken again at some point in the future...)

[DePasqualeOrg]

Plenty of things have been broken for nearly a year because of this.

[cktben]

Please go ahead and merge this. It fixes a number of problems that just can't be worked around without it.

[Mugen87]

@elalish Would it be a problem for model-viewer if this PR gets merged?

[elalish]

@elalish Would it be a problem for model-viewer if this PR gets merged?

I don't think so. Our usage of XR is pretty vanilla, so I'd expect what's good for others is good for us too.

[mrdoob]

Thanks!

[mrdoob]

@zalo

I've got a cool three.js OpenXR demo waiting in the wings that could use this PR...

That looks amazing! 🤩