WebXRManager: Fix XRCamera Local Space Behavior by Reverting #21964 #22362
Conversation
This PR attempts to move XR Camera calculations back into local space by undoing mrdoob#21964 (which appears to conflate `matrixWorld` and `matrix`). This enables the XR camera to be parented to another object again without odd behavior. If this isn't the correct fix; I'd be happy to test alternatives in the context of my application.
zalo
changed the title
|
@elalish Apologies for the delay, I just re-added the decompose from the prior PR so the However, given that the |
|
This appears to fix the bug I referenced here: #21964 (comment) |
|
Any chance this could get included in the next release? My VR apps have not been working properly for months because of this bug. |
|
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. |
|
Wow! Can you please share its code? |
|
+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) |
1 similar comment
|
+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) |
|
This issue has been resolved for quite some time now |
|
@ashconnell When and where was it fixed? |
|
@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. |
|
@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 :) |
|
Is there an issue with this PR? It keeps getting pushed back but seems like a trivial merge. |
|
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...) |
|
Plenty of things have been broken for nearly a year because of this. |
|
Please go ahead and merge this. It fixes a number of problems that just can't be worked around without it. |
|
@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. |
|
Thanks! |
That looks amazing! 🤩 |
…b#22362) This PR attempts to move XR Camera calculations back into local space by undoing mrdoob#21964 (which appears to conflate `matrixWorld` and `matrix`). This enables the XR camera to be parented to another object again without odd behavior. If this isn't the correct fix; I'd be happy to test alternatives in the context of my application.
…b#22362) This PR attempts to move XR Camera calculations back into local space by undoing mrdoob#21964 (which appears to conflate `matrixWorld` and `matrix`). This enables the XR camera to be parented to another object again without odd behavior. If this isn't the correct fix; I'd be happy to test alternatives in the context of my application.
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
matrixWorldandmatrix)).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