Make tech logic impls responsible for light filtering

jme3-core/src/main/java/com/jme3/material/Material.java

@@ -81,9 +81,9 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
     private AssetKey key;
     private String name;
     private MaterialDef def;
-    private ListMap<String, MatParam> paramValues = new ListMap<String, MatParam>();
+    private ListMap<String, MatParam> paramValues = new ListMap<>();
     private Technique technique;
-    private HashMap<String, Technique> techniques = new HashMap<String, Technique>();
+    private HashMap<String, Technique> techniques = new HashMap<>();
     private RenderState additionalState = null;
     private RenderState mergedRenderState = new RenderState();
     private boolean transparent = false;
@@ -143,10 +143,12 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
         this.name = name;
     }
 
+    @Override
     public void setKey(AssetKey key) {
         this.key = key;
     }
 
+    @Override
     public AssetKey getKey() {
         return key;
     }
@@ -200,9 +202,9 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
                 mat.additionalState = additionalState.clone();
             }
             mat.technique = null;
-            mat.techniques = new HashMap<String, Technique>();
+            mat.techniques = new HashMap<>();
 
-            mat.paramValues = new ListMap<String, MatParam>();
+            mat.paramValues = new ListMap<>();
             for (int i = 0; i < paramValues.size(); i++) {
                 Map.Entry<String, MatParam> entry = paramValues.getEntry(i);
                 mat.paramValues.put(entry.getKey(), entry.getValue().clone());
@@ -933,10 +935,9 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
      * </ul>
      *
      * @param geometry The geometry to render
-     * @param lights Presorted and filtered light list to use for rendering
      * @param renderManager The render manager requesting the rendering
      */
-    public void render(Geometry geometry, LightList lights, RenderManager renderManager) {
+    public void render(Geometry geometry, RenderManager renderManager) {
         if (technique == null) {
             selectTechnique(TechniqueDef.DEFAULT_TECHNIQUE_NAME, renderManager);
         }
@@ -956,7 +957,7 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
         SafeArrayList<MatParamOverride> overrides = geometry.getWorldMatParamOverrides();
 
         // Select shader to use
-        Shader shader = technique.makeCurrent(renderManager, overrides, renderManager.getForcedMatParams(), lights, rendererCaps);
+        Shader shader = technique.makeCurrent(renderManager, geometry, overrides, renderManager.getForcedMatParams(), rendererCaps);
         
         // Begin tracking which uniforms were changed by material.
         clearUniformsSetByCurrent(shader);
@@ -965,29 +966,24 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
         renderManager.updateUniformBindings(shader);
         
         // Set material parameters
-        int unit = updateShaderMaterialParameters(renderer, shader, overrides, renderManager.getForcedMatParams());
+        int nextTextureUnit = updateShaderMaterialParameters(renderer, shader, overrides, renderManager.getForcedMatParams());
 
         // Clear any uniforms not changed by material.
         resetUniformsNotSetByCurrent(shader);
         
         // Delegate rendering to the technique
-        technique.render(renderManager, shader, geometry, lights, unit);
+        technique.render(renderManager, shader, geometry, nextTextureUnit);
     }
 
-    /**
+    @Override
-     * Called by {@link RenderManager} to render the geometry by
+    public String toString() {
-     * using this material.
+        return "Material[name=" + name + 
-     *
+                ", def=" + (def != null ? def.getName() : null) + 
-     * Note that this version of the render method
+                ", tech=" + (technique != null && technique.getDef() != null ? technique.getDef().getName() : null) + 
-     * does not perform light filtering.
+                "]";
-     *
-     * @param geom The geometry to render
-     * @param rm The render manager requesting the rendering
-     */
-    public void render(Geometry geom, RenderManager rm) {
-        render(geom, geom.getWorldLightList(), rm);
     }
     
+    @Override
     public void write(JmeExporter ex) throws IOException {
         OutputCapsule oc = ex.getCapsule(this);
         oc.write(def.getAssetName(), "material_def", null);
@@ -998,13 +994,6 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
     }
     
     @Override
-    public String toString() {
-        return "Material[name=" + name + 
-                ", def=" + (def != null ? def.getName() : null) + 
-                ", tech=" + (technique != null && technique.getDef() != null ? technique.getDef().getName() : null) + 
-                "]";
-    }
-
     public void read(JmeImporter im) throws IOException {
         InputCapsule ic = im.getCapsule(this);
 
@@ -1054,7 +1043,7 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
         }
 
         def = (MaterialDef) im.getAssetManager().loadAsset(new AssetKey(defName));
-        paramValues = new ListMap<String, MatParam>();
+        paramValues = new ListMap<>();
 
         // load the textures and update nextTexUnit
         for (Map.Entry<String, MatParam> entry : params.entrySet()) {

jme3-core/src/main/java/com/jme3/material/Technique.java

@@ -133,9 +133,10 @@ public final class Technique {
      * @param rendererCaps The renderer capabilities which the shader should support.
      * @return A compatible shader.
      */
-    Shader makeCurrent(RenderManager renderManager, SafeArrayList<MatParamOverride> worldOverrides,
+    Shader makeCurrent(RenderManager renderManager, Geometry geometry,
+            SafeArrayList<MatParamOverride> worldOverrides,
             SafeArrayList<MatParamOverride> forcedOverrides,
-            LightList lights, EnumSet<Caps> rendererCaps) {
+            EnumSet<Caps> rendererCaps) {
         TechniqueDefLogic logic = def.getLogic();
         AssetManager assetManager = owner.getMaterialDef().getAssetManager();
 
@@ -149,7 +150,7 @@ public final class Technique {
             applyOverrides(dynamicDefines, forcedOverrides);
         }
 
-        return logic.makeCurrent(assetManager, renderManager, rendererCaps, lights, dynamicDefines);
+        return logic.makeCurrent(assetManager, renderManager, rendererCaps, geometry, dynamicDefines);
     }
     
     /**
@@ -161,9 +162,9 @@ public final class Technique {
      * @param geometry The geometry to render
      * @param lights Lights which influence the geometry.
      */
-    void render(RenderManager renderManager, Shader shader, Geometry geometry, LightList lights, int lastTexUnit) {
+    void render(RenderManager renderManager, Shader shader, Geometry geometry, int lastTextureUnit) {
         TechniqueDefLogic logic = def.getLogic();
-        logic.render(renderManager, shader, geometry, lights, lastTexUnit);
+        logic.render(renderManager, shader, geometry, lastTextureUnit);
     }
     
     /**

jme3-core/src/main/java/com/jme3/material/logic/DefaultTechniqueDefLogic.java

@@ -48,6 +48,7 @@ import java.util.EnumSet;
 public class DefaultTechniqueDefLogic implements TechniqueDefLogic {
 
     protected final TechniqueDef techniqueDef;
+    protected final LightList filteredLightList = new LightList(null);
 
     public DefaultTechniqueDefLogic(TechniqueDef techniqueDef) {
         this.techniqueDef = techniqueDef;
@@ -55,7 +56,7 @@ public class DefaultTechniqueDefLogic implements TechniqueDefLogic {
 
     @Override
     public Shader makeCurrent(AssetManager assetManager, RenderManager renderManager,
-            EnumSet<Caps> rendererCaps, LightList lights, DefineList defines) {
+            EnumSet<Caps> rendererCaps, Geometry geometry, DefineList defines) {
         return techniqueDef.getShader(assetManager, rendererCaps, defines);
     }
 
@@ -71,6 +72,19 @@ public class DefaultTechniqueDefLogic implements TechniqueDefLogic {
         }
     }
     
+    @Override
+    public void render(RenderManager renderManager, Shader shader, Geometry geometry, int lastTexUnit) {
+        Renderer renderer = renderManager.getRenderer();
+        renderer.setShader(shader);
+        renderMeshFromGeometry(renderer, geometry);
+    }
+    
+    protected LightList getFilteredLightList(RenderManager renderManager, Geometry geom) {
+        filteredLightList.clear();
+        renderManager.getLightFilter().filterLights(geom, filteredLightList);
+        return filteredLightList;
+    }
+
     protected static ColorRGBA getAmbientColor(LightList lightList, boolean removeLights, ColorRGBA ambientLightColor) {
         ambientLightColor.set(0, 0, 0, 1);
         for (int j = 0; j < lightList.size(); j++) {
@@ -85,13 +99,4 @@ public class DefaultTechniqueDefLogic implements TechniqueDefLogic {
         ambientLightColor.a = 1.0f;
         return ambientLightColor;
     }
-
-
-
-    @Override
-    public void render(RenderManager renderManager, Shader shader, Geometry geometry, LightList lights, int lastTexUnit) {
-        Renderer renderer = renderManager.getRenderer();
-        renderer.setShader(shader);
-        renderMeshFromGeometry(renderer, geometry);
-    }
 }

jme3-core/src/main/java/com/jme3/material/logic/MultiPassLightingLogic.java

@@ -31,7 +31,6 @@
  */
 package com.jme3.material.logic;
 
-import com.jme3.asset.AssetManager;
 import com.jme3.light.AmbientLight;
 import com.jme3.light.DirectionalLight;
 import com.jme3.light.Light;
@@ -41,20 +40,16 @@ import com.jme3.light.SpotLight;
 import com.jme3.material.RenderState;
 import com.jme3.material.TechniqueDef;
 import com.jme3.math.ColorRGBA;
-import com.jme3.math.FastMath;
 import com.jme3.math.Quaternion;
 import com.jme3.math.Vector3f;
 import com.jme3.math.Vector4f;
-import com.jme3.renderer.Caps;
 import com.jme3.renderer.RenderManager;
 import com.jme3.renderer.Renderer;
 import com.jme3.scene.Geometry;
-import com.jme3.shader.DefineList;
 import com.jme3.shader.Shader;
 import com.jme3.shader.Uniform;
 import com.jme3.shader.VarType;
 import com.jme3.util.TempVars;
-import java.util.EnumSet;
 
 public final class MultiPassLightingLogic extends DefaultTechniqueDefLogic {
 
@@ -73,7 +68,7 @@ public final class MultiPassLightingLogic extends DefaultTechniqueDefLogic {
     }
 
     @Override
-    public void render(RenderManager renderManager, Shader shader, Geometry geometry, LightList lights, int lastTexUnit) {
+    public void render(RenderManager renderManager, Shader shader, Geometry geometry, int lastTexUnit) {
         Renderer r = renderManager.getRenderer();
         Uniform lightDir = shader.getUniform("g_LightDirection");
         Uniform lightColor = shader.getUniform("g_LightColor");
@@ -82,6 +77,7 @@ public final class MultiPassLightingLogic extends DefaultTechniqueDefLogic {
         boolean isFirstLight = true;
         boolean isSecondLight = false;
         
+        LightList lights = getFilteredLightList(renderManager, geometry);
         getAmbientColor(lights, false, ambientLightColor);
 
         for (int i = 0; i < lights.size(); i++) {

jme3-core/src/main/java/com/jme3/material/logic/SinglePassAndImageBasedLightingLogic.java

@@ -72,15 +72,18 @@ public final class SinglePassAndImageBasedLightingLogic extends DefaultTechnique
 
     @Override
     public Shader makeCurrent(AssetManager assetManager, RenderManager renderManager,
-            EnumSet<Caps> rendererCaps, LightList lights, DefineList defines) {
+            EnumSet<Caps> rendererCaps, Geometry geometry, DefineList defines) {
         defines.set(nbLightsDefineId, renderManager.getSinglePassLightBatchSize() * 3);
         defines.set(singlePassLightingDefineId, true);
 
-
+        // TODO: here we have a problem, this is called once before render, 
-        //TODO here we have a problem, this is called once before render, so the define will be set for all passes (in case we have more than NB_LIGHTS lights)
+        // so the define will be set for all passes (in case we have more than NB_LIGHTS lights)
-        //Though the second pass should not render IBL as it is taken care of on first pass like ambient light in phong lighting.
+        // Though the second pass should not render IBL as it is taken care of on 
-        //We cannot change the define between passes and the old technique, and for some reason the code fails on mac (renders nothing).
+        // first pass like ambient light in phong lighting.
-        if(lights != null) {
+        // We cannot change the define between passes and the old technique, and 
+        // for some reason the code fails on mac (renders nothing).
+        LightList lights = getFilteredLightList(renderManager, geometry);
+        if (lights != null) {
             lightProbe = extractIndirectLights(lights, false);
             if (lightProbe == null) {
                 defines.set(indirectLightingDefineId, false);
@@ -89,7 +92,7 @@ public final class SinglePassAndImageBasedLightingLogic extends DefaultTechnique
             }
         }
 
-        return super.makeCurrent(assetManager, renderManager, rendererCaps, lights, defines);
+        return super.makeCurrent(assetManager, renderManager, rendererCaps, geometry, defines);
     }
 
     /**
@@ -224,10 +227,11 @@ public final class SinglePassAndImageBasedLightingLogic extends DefaultTechnique
     }
 
     @Override
-    public void render(RenderManager renderManager, Shader shader, Geometry geometry, LightList lights, int lastTexUnit) {
+    public void render(RenderManager renderManager, Shader shader, Geometry geometry, int lastTexUnit) {
         int nbRenderedLights = 0;
         Renderer renderer = renderManager.getRenderer();
         int batchSize = renderManager.getSinglePassLightBatchSize();
+        LightList lights = getFilteredLightList(renderManager, geometry);
         if (lights.size() == 0) {
             updateLightListUniforms(shader, geometry, lights,batchSize, renderManager, 0, lastTexUnit);
             renderer.setShader(shader);
@@ -239,7 +243,6 @@ public final class SinglePassAndImageBasedLightingLogic extends DefaultTechnique
                 renderMeshFromGeometry(renderer, geometry);
             }
         }
-        return;
     }
 
     protected LightProbe extractIndirectLights(LightList lightList, boolean removeLights) {

jme3-core/src/main/java/com/jme3/material/logic/SinglePassLightingLogic.java

@@ -77,25 +77,31 @@ public final class SinglePassLightingLogic extends DefaultTechniqueDefLogic {
     }
 
     @Override
-    public Shader makeCurrent(AssetManager assetManager, RenderManager renderManager,
+    public Shader makeCurrent(
-            EnumSet<Caps> rendererCaps, LightList lights, DefineList defines) {
+            AssetManager assetManager, 
+            RenderManager renderManager,
+            EnumSet<Caps> rendererCaps, 
+            Geometry geometry,
+            DefineList defines) {
         defines.set(nbLightsDefineId, renderManager.getSinglePassLightBatchSize() * 3);
         defines.set(singlePassLightingDefineId, true);
-        return super.makeCurrent(assetManager, renderManager, rendererCaps, lights, defines);
+        return super.makeCurrent(assetManager, renderManager, rendererCaps, geometry, defines);
     }
 
     /**
-     * Uploads the lights in the light list as two uniform arrays.<br/><br/> *
+     * Uploads the lights in the light list as two uniform arrays.<br><br>
+     *
      * <p>
-     * <code>uniform vec4 g_LightColor[numLights];</code><br/> //
+     * <code>uniform vec4 g_LightColor[numLights];</code><br>
-     * g_LightColor.rgb is the diffuse/specular color of the light.<br/> //
+     * g_LightColor.rgb is the diffuse/specular color of the light.<br>
-     * g_Lightcolor.a is the type of light, 0 = Directional, 1 = Point, <br/> //
+     * g_Lightcolor.a is the type of light, 0 = Directional, 1 = Point, <br>
-     * 2 = Spot. <br/> <br/>
+     * 2 = Spot. <br> <br>
-     * <code>uniform vec4 g_LightPosition[numLights];</code><br/> //
+     * <code>uniform vec4 g_LightPosition[numLights];</code><br>
-     * g_LightPosition.xyz is the position of the light (for point lights)<br/>
+     * g_LightPosition.xyz is the position of the light (for point lights)<br>
-     * // or the direction of the light (for directional lights).<br/> //
+     * // or the direction of the light (for directional lights).<br>
      * g_LightPosition.w is the inverse radius (1/r) of the light (for
-     * attenuation) <br/> </p>
+     * attenuation) <br>
+     * </p>
      */
     protected int updateLightListUniforms(Shader shader, Geometry g, LightList lightList, int numLights, RenderManager rm, int startIndex) {
         if (numLights == 0) { // this shader does not do lighting, ignore.
@@ -201,10 +207,11 @@ public final class SinglePassLightingLogic extends DefaultTechniqueDefLogic {
     }
 
     @Override
-    public void render(RenderManager renderManager, Shader shader, Geometry geometry, LightList lights, int lastTexUnit) {
+    public void render(RenderManager renderManager, Shader shader, Geometry geometry, int lastTexUnit) {
         int nbRenderedLights = 0;
         Renderer renderer = renderManager.getRenderer();
         int batchSize = renderManager.getSinglePassLightBatchSize();
+        LightList lights = getFilteredLightList(renderManager, geometry);
         if (lights.size() == 0) {
             updateLightListUniforms(shader, geometry, lights, batchSize, renderManager, 0);
             renderer.setShader(shader);

jme3-core/src/main/java/com/jme3/material/logic/TechniqueDefLogic.java

@@ -32,7 +32,9 @@
 package com.jme3.material.logic;
 
 import com.jme3.asset.AssetManager;
-import com.jme3.light.LightList;
+import com.jme3.material.MatParam;
+import com.jme3.material.RenderState;
+import com.jme3.material.TechniqueDef;
 import com.jme3.material.TechniqueDef.LightMode;
 import com.jme3.renderer.Caps;
 import com.jme3.renderer.RenderManager;
@@ -63,11 +65,10 @@ public interface TechniqueDefLogic {
      * 
      * @param assetManager The asset manager to use for loading shader source code,
      * shader nodes, and and lookup textures.
+     * @param geometry The geometry being rendered
      * @param renderManager The render manager for which rendering is to be performed.
      * @param rendererCaps Renderer capabilities. The returned shader must
      * support these capabilities.
-     * @param lights The lights with which the geometry shall be rendered. This
-     * list must not include culled lights.
      * @param defines The define list used by the technique, any 
      * {@link TechniqueDef#addShaderUnmappedDefine(java.lang.String) unmapped defines}
      * should be set here to change shader behavior.
@@ -75,7 +76,7 @@ public interface TechniqueDefLogic {
      * @return The shader to use for rendering.
      */
     public Shader makeCurrent(AssetManager assetManager, RenderManager renderManager,
-            EnumSet<Caps> rendererCaps, LightList lights, DefineList defines);
+            EnumSet<Caps> rendererCaps, Geometry geometry, DefineList defines);
     
     /**
      * Requests that the <code>TechniqueDefLogic</code> renders the given geometry.
@@ -88,10 +89,10 @@ public interface TechniqueDefLogic {
      * can still be overriden.
      * 
      * @param renderManager The render manager to perform the rendering against.
-     * * @param shader The shader that was selected by this logic in 
+     * @param shader The shader that was selected by this logic in 
      * {@link #makeCurrent(com.jme3.asset.AssetManager, com.jme3.renderer.RenderManager, java.util.EnumSet, com.jme3.shader.DefineList)}.
      * @param geometry The geometry to render
-     * @param lights Lights which influence the geometry.
+     * @param lastTextureUnit The last unused texture unit
      */
-    public void render(RenderManager renderManager, Shader shader, Geometry geometry, LightList lights, int lastTexUnit);
+    public void render(RenderManager renderManager, Shader shader, Geometry geometry, int lastTextureUnit);
 }

jme3-core/src/main/java/com/jme3/renderer/RenderManager.java

@@ -84,7 +84,6 @@ public class RenderManager {
     private final SafeArrayList<MatParamOverride> forcedOverrides = new SafeArrayList<>(MatParamOverride.class);
     private int viewX, viewY, viewWidth, viewHeight;
     private final Matrix4f orthoMatrix = new Matrix4f();
-    private final LightList filteredLightList = new LightList(null);
     private boolean handleTranlucentBucket = true;
     private AppProfiler prof;
     private LightFilter lightFilter = new DefaultLightFilter();
@@ -563,15 +562,6 @@ public class RenderManager {
             setWorldMatrix(geom.getWorldMatrix());
         }
 
-        // Perform light filtering if we have a light filter.
-        LightList lightList = geom.getWorldLightList();
-        
-        if (lightFilter != null) {
-            filteredLightList.clear();
-            lightFilter.filterLights(geom, filteredLightList);
-            lightList = filteredLightList;
-        }
-
         Material material = geom.getMaterial();
 
         //if forcedTechnique we try to force it for render,
@@ -595,7 +585,7 @@ public class RenderManager {
                     forcedRenderState = geom.getMaterial().getActiveTechnique().getDef().getForcedRenderState();
                 }
                 // use geometry's material
-                material.render(geom, lightList, this);
+                material.render(geom, this);
                 material.selectTechnique(previousTechniqueName, this);
 
                 //restoring forcedRenderState
@@ -605,13 +595,13 @@ public class RenderManager {
                 //If forcedTechnique does not exists, and forcedMaterial is not set, the geom MUST NOT be rendered
             } else if (forcedMaterial != null) {
                 // use forced material
-                forcedMaterial.render(geom, lightList, this);
+                forcedMaterial.render(geom, this);
             }
         } else if (forcedMaterial != null) {
             // use forced material
-            forcedMaterial.render(geom, lightList, this);
+            forcedMaterial.render(geom, this);
         } else {
-            material.render(geom, lightList, this);
+            material.render(geom, this);
         }
     }
 

jme3-core/src/test/java/com/jme3/material/MaterialMatParamTest.java

@@ -430,7 +430,6 @@ public class MaterialMatParamTest {
 
     private final Geometry geometry = new Geometry("Geometry", new Box(1, 1, 1));
     private final Node root = new Node("Root Node");
-    private final LightList lightList = new LightList(geometry);
 
     @Before
     public void setUp() {
@@ -526,7 +525,7 @@ public class MaterialMatParamTest {
     private void evaluateTechniqueDef() {
         Assert.assertFalse(evaluated);
         Material mat = geometry.getMaterial();
-        mat.render(geometry, lightList, renderManager);
+        mat.render(geometry, renderManager);
         Assert.assertTrue(evaluated);
     }