Skip to content

Refactor: AbstractShadowFilter reduce object allocations #2516

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

Merged
merged 5 commits into from
Jul 12, 2025
Merged

Refactor: AbstractShadowFilter reduce object allocations #2516

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ca191d
Update AbstractShadowFilter.java
capdevon Jun 23, 2025
b4cd0e7
AbstractShadowFilter: update
capdevon Jul 10, 2025
76a446f
AbstractShadowFilter: deprecate getShadowMaterial() method
capdevon Jul 11, 2025
9f65464
AbstractShadowFilter: use TempVars instead of temp variables
capdevon Jul 11, 2025
0ab5def
AbstractShadowFilter: restore temp variables
capdevon Jul 11, 2025
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
48 changes: 18 additions & 30 deletions jme3-core/src/main/java/com/jme3/shadow/AbstractShadowFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@
package com.jme3.shadow;

import com.jme3.asset.AssetManager;
import com.jme3.export.InputCapsule;
import com.jme3.export.JmeExporter;
import com.jme3.export.JmeImporter;
import com.jme3.export.OutputCapsule;
import com.jme3.material.Material;
import com.jme3.material.RenderState;
import com.jme3.math.Matrix4f;
Expand All @@ -45,38 +41,38 @@
import com.jme3.renderer.ViewPort;
import com.jme3.renderer.queue.RenderQueue;
import com.jme3.texture.FrameBuffer;
import com.jme3.util.TempVars;
import com.jme3.util.clone.Cloner;
import com.jme3.util.clone.JmeCloneable;

import java.io.IOException;

/**
* Generic abstract filter that holds common implementations for the different
* shadow filters
*
* @author Rémy Bouquet aka Nehon
*/
public abstract class AbstractShadowFilter<T extends AbstractShadowRenderer> extends Filter implements Cloneable, JmeCloneable {
public abstract class AbstractShadowFilter<T extends AbstractShadowRenderer> extends Filter implements JmeCloneable {
Copy link
Member

Choose a reason for hiding this comment

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

Why remove Cloneable? (Why was it here in the first place?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JmeCloneable interface already extends java.lang.Cloneable. Therefore, the explicit extension is redundant and its removal simplifies the code without altering its behavior. Therefore, what checks have you already performed that led you to raise this question?

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes good point. No objection to removing it then


protected T shadowRenderer;
protected ViewPort viewPort;

private final Vector4f tempVec4 = new Vector4f();
private final Matrix4f tempMat4 = new Matrix4f();

/**
* For serialization only. Do not use.
*/
protected AbstractShadowFilter() {
}

/**
* Abstract class constructor
* Creates an AbstractShadowFilter. Subclasses invoke this constructor.
*
* @param manager the application asset manager
* @param shadowMapSize the size of the rendered shadowmaps (512,1024,2048,
* etc...)
* @param shadowRenderer the shadowRenderer to use for this Filter
* @param assetManager The application's asset manager.
* @param shadowMapSize The size of the rendered shadow maps (e.g., 512, 1024, 2048).
* @param shadowRenderer The shadowRenderer to use for this Filter
*/
@SuppressWarnings("all")
protected AbstractShadowFilter(AssetManager manager, int shadowMapSize, T shadowRenderer) {
protected AbstractShadowFilter(AssetManager assetManager, int shadowMapSize, T shadowRenderer) {
super("Post Shadow");
this.shadowRenderer = shadowRenderer;
// this is legacy setting for shadows with backface shadows
Expand All @@ -93,18 +89,21 @@ protected boolean isRequiresDepthTexture() {
return true;
}

/**
* @deprecated Use {@link #getMaterial()} instead.
* @return The Material used by this filter.
*/
@Deprecated
public Material getShadowMaterial() {
return material;
}

Vector4f tmpv = new Vector4f();

@Override
protected void preFrame(float tpf) {
shadowRenderer.preFrame(tpf);
material.setMatrix4("ViewProjectionMatrixInverse", viewPort.getCamera().getViewProjectionMatrix().invert());
Matrix4f m = viewPort.getCamera().getViewProjectionMatrix();
material.setVector4("ViewProjectionMatrixRow2", tmpv.set(m.m20, m.m21, m.m22, m.m23));
material.setMatrix4("ViewProjectionMatrixInverse", tempMat4.set(m).invertLocal());
material.setVector4("ViewProjectionMatrixRow2", tempVec4.set(m.m20, m.m21, m.m22, m.m23));
}

@Override
Expand Down Expand Up @@ -337,15 +336,4 @@ public void cloneFields(final Cloner cloner, final Object original) {
shadowRenderer.setPostShadowMaterial(material);
}

@Override
public void write(JmeExporter ex) throws IOException {
super.write(ex);
OutputCapsule oc = ex.getCapsule(this);
}

@Override
public void read(JmeImporter im) throws IOException {
super.read(im);
InputCapsule ic = im.getCapsule(this);
}
}