Skip to content

Refactor: AudioListener reduce object allocations #2501

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.

Sign up for GitHub

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

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

Refactor: AudioListener reduce object allocations #2501

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions jme3-core/src/main/java/com/jme3/audio/Listener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,20 @@
*/
public class Listener {

private final Vector3f location;
private final Vector3f velocity;
private final Quaternion rotation;
private float volume = 1;
private final Vector3f location = new Vector3f();
private final Vector3f velocity = new Vector3f();
private final Quaternion rotation = new Quaternion();
private float volume = 1f;
private AudioRenderer renderer;

private final Vector3f left = new Vector3f();
private final Vector3f up = new Vector3f();
private final Vector3f direction = new Vector3f();

/**
* Constructs a new {@code Listener} with default parameters.
*/
public Listener() {
location = new Vector3f();
velocity = new Vector3f();
rotation = new Quaternion();
}

/**
Expand All @@ -62,9 +63,9 @@ public Listener() {
* @param source The {@code Listener} to copy the properties from.
*/
public Listener(Listener source) {
this.location = source.location.clone();
this.velocity = source.velocity.clone();
this.rotation = source.rotation.clone();
this.location.set(source.location);
this.velocity.set(source.velocity);
this.rotation.set(source.rotation);
this.volume = source.volume;
this.renderer = source.renderer; // Note: Renderer is also copied
}
Expand Down Expand Up @@ -130,32 +131,29 @@ public Vector3f getVelocity() {

/**
* Gets the left direction vector of the listener.
* This vector is derived from the listener's rotation.
*
* @return The listener's left direction as a {@link Vector3f}.
*/
public Vector3f getLeft() {
return rotation.getRotationColumn(0);
return rotation.getRotationColumn(0, left);
Copy link
Member

@richardTingle richardTingle Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This changes the behaviour. Previously this returned a fresh Vector3f that you had exclusive ownership of. Now you are returned a vector3f that may randomly change at some future point.

Unless this is giving a major boost in performance (from profiling) in typical flows I don't think we should do this

Copy link
Contributor Author

@capdevon capdevon Jun 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This solution maintains consistency with the existing getLocation(), getRotation(), and getVelocity() methods, which already expose internal variables. This approach is not only consistent but also crucial for performance optimization and efficient reuse of internal variables, preventing new vector allocations every time.

The primary reason for this approach is that Listener is an internally managed class by ALAudioRenderer and AudioListenerState. It's designed as a unique instance created by LegacyApplication and is not intended for external direct use. Therefore, the concerns you've raised regarding its internal exposure are mitigated by its strictly internal scope and established usage patterns.

Edit:
Furthermore, specifically within ALAudioRenderer.applyListenerRotation(), having redundant copies of up and direction variables is unnecessary. The underlying audio buffer only needs to read the x, y, and z coordinates directly from the existing variables. Creating and passing extra copies would just add needless overhead.

}

/**
* Gets the up direction vector of the listener.
* This vector is derived from the listener's rotation.
*
* @return The listener's up direction as a {@link Vector3f}.
*/
public Vector3f getUp() {
return rotation.getRotationColumn(1);
return rotation.getRotationColumn(1, up);
}

/**
* Gets the forward direction vector of the listener.
* This vector is derived from the listener's rotation.
*
* @return The listener's forward direction.
*/
public Vector3f getDirection() {
return rotation.getRotationColumn(2);
return rotation.getRotationColumn(2, direction);
}

/**
Expand Down