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

AudioListener: prevent invalid data being passed to linearRampToValueAtTime #27588

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

hybridherbst
Copy link
Contributor

Description

This issue has been bugging us for a while – under certain circumstances, entering an XR session can result in the cameras being non-decomposable for a frame or two (for reasons not entirely clear to me, randomly happens with three.js XR examples as well), and then the render loop breaks when an AudioListener is used, since passing non-finite numbers to the audio filters throws an exception. This PR fixes that and prevents invalid data being passed into AudioListener.

I understand this does not "feel clean", but it fixes a super hard to debug issue that has been bugging us for a while.

This contribution is funded by Needle

@hybridherbst hybridherbst changed the title Fix issue with AudioListener breaking the render loop on non-decomposable matrix AudioListener: prevent invalid data being passed to linearRampToValueAtTime Jan 18, 2024
Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
671.3 kB (166.8 kB) 671.3 kB (166.8 kB) +71 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
452.3 kB (109.8 kB) 452.3 kB (109.8 kB) +0 B

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 19, 2024

under certain circumstances, entering an XR session can result in the cameras being non-decomposable for a frame or two (for reasons not entirely clear to me, randomly happens with three.js XR examples as well)

How does the world matrix look like when it is non-decomposable in these cases?

I wonder if we could detect somehow in WebXRManager these cases and apply a fix there instead of AudioListener. Non-decomposable matrices could cause issues elsewhere as well.

@hybridherbst
Copy link
Contributor Author

I'm honestly not sure under which conditions it exactly happens I'm afraid

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 21, 2024

Some thinking about this:

The glTF spec says transformation matrices must be decomposable to TRS properties. Transformation matrices cannot skew or shear.

gltf-rs/gltf#21 (comment) says a 4x4 matrix is decomposable if (A) the last row is [0 0 0 1] and (B) the columns of its upper-left 3x3 sub-block are non-zero and pairwise orthogonal.

For testing we could write a helper according to this policy and check the camera's world matrix whether it is decomposable into TRS or not. We could then throw an exception and log the matrix so we know how its elements look like. Eventually we could decide whether this is a browser issue or an issue in our implementation.

@hybridherbst
Copy link
Contributor Author

I think only the transformation matrices of invidual nodes must be decomposable, nested hierarchies can easily end up with non-decomposable world matrices.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 23, 2024

nested hierarchies can easily end up with non-decomposable world matrices.

How can that happen if all world matrices in the hierarchy are proper decomposable?

@hybridherbst
Copy link
Contributor Author

  1. they are not "cleanly" decomposable anymore as soon as any parent is non-uniformly scaled and a child rotated
  2. they are completely not decomposable anymore if any parent is scaled to 0

In a current project the latter happened: a user scaled some object that makes sound to zero as part of an animation, and that resulted in errors in three.js for positional audio in either the listener or the source.

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.

None yet

2 participants