[SOLVED] InstancedNode doesn't work with light

Troubleshooting
#1

Hi!

I use InstancedNode with geometries with unshaded material and I am happy with it. Now I want to use this node for geometries, which use Lighting material, and I have a problem, when I use PointLight.

RenderManager filters out geometries (using LightFilter), which don’t touch light (which is good). Classes inherited from BoundingVolume are used for this. All the geometries, which are instanced, are grouped in the InstancedGeometry. InstancedNode keeps correct up-to-date BoundingVolume, which is based on all instanced geometries, but InstancedGeometry keeps only BoundingVolume for a single geometry and using local coordinate system. Need to note, that InstancedGeometry keeps a list of all the instanced geometries, so it would not be a big deal (or will?) to update InstancedGeometry.worldBound, when a new geometry is added to the InstancedGeometry.

LightFilter works with geometries, but not nodes, so InstancedGeometry.worldBound will be used, and of course its BoundingVolume does not cover all the instanced geometries, so all the instanced geometries will be skipped from the light technique, unless PointLight is close to the World (0, 0, 0), and I see such result in a test. For example, if light’s radius is 10 units, then instanced geometries are lit only when light position is within 10 + geometry’s half extent around World’s (0, 0, 0). If to place the same PointLight at (15, 0, 0), then instanced geometry with world’s coordinates (12, 0 0) will not be lit, even if it is 3 units away from the point light.

So, I want to ask, how to deal with it? If it is JME bug, what is the best way to fix it? Update InstancedGeometry.worldBound, if a new geometry is added to InstancedGeometry by setting some flag like RF_BOUND and calculate it like regular node does, but using InstancedGeometry.geometries? Even if InstancedGeometry is a Geometry, but it behaves like a Node for me.

Thank you!

#2

ha, yeah sounds like a bug.
Do you have a test case by any chance?

#3

Here is a simple test:

package jme3test.scene.instancing;

import com.jme3.app.SimpleApplication;
import com.jme3.light.PointLight;
import com.jme3.material.Material;
import com.jme3.math.ColorRGBA;
import com.jme3.math.Vector3f;
import com.jme3.scene.Geometry;
import com.jme3.scene.instancing.InstancedNode;
import com.jme3.scene.shape.Box;

public class TestInstanceNodeWithLight extends SimpleApplication {
    // Try to test with different offset
    private static final float OFFSET = 13.4f;

    public static void main(String[] args){
        TestInstanceNodeWithLight app = new TestInstanceNodeWithLight();
        app.start();
    }

    @Override
    public void simpleInitApp() {
        InstancedNode instancedNode = new InstancedNode("testInstancedNode");
        rootNode.attachChild(instancedNode);

        Geometry box = new Geometry("Box", new Box(0.5f, 0.5f, 0.5f));
        Material material = new Material(assetManager, "Common/MatDefs/Light/Lighting.j3md");
        material.setBoolean("UseInstancing", true);
        material.setColor("Diffuse", ColorRGBA.Red);
        material.setBoolean("UseMaterialColors", true);
        box.setMaterial(material);

        instancedNode.attachChild(box);
        instancedNode.instance();

        PointLight pointLight = new PointLight();
        pointLight.setColor(ColorRGBA.White);
        pointLight.setRadius(10f);
        rootNode.addLight(pointLight);

        box.setLocalTranslation(new Vector3f(OFFSET, 0, 0));
        pointLight.setPosition(new Vector3f(OFFSET - 3f, 0, 0));

        cam.setLocation(new Vector3f(OFFSET - 5f, 0, 0));
        cam.lookAtDirection(Vector3f.UNIT_X, Vector3f.UNIT_Y);
    }
}

Box has the same distance (3 units) from the point light location no matter of offset. This test should always show red box no matter of offset, but it fails, if you try to use offset > 13.5 or offset < -7.5.

Currently box will be visible, when light position is within 10 + 0.5 = 10.5 units from the center of the World (point light radius + half extent of the box, which is a bounding box using local coordinate system).

Bug is reproduced on current JME 3.2 with both lighting techniques: single and multi pass. Lighting shaders are ok. Issue is because of bounding volume, as described in the original post.

1 Like
#4

One issue I had, that might be germain. I had a PointLight inside a sphere (my sun). The light didn’t hit the inside 'cause the ‘normals’ were on the outside of the sphere. I had to invert them for them to be lit (from the inside).

#5

@Mipada, good example, but there is a difference: in your case sphere comes to the lighting shader code, and in my case it doesn’t, because JME logic thinks that light does not touch the geometry at all.

@nehon, here is a draft version of the fix, which works in my test:

 package com.jme3.scene.instancing;
 
+import com.jme3.bounding.BoundingVolume;
 import com.jme3.export.InputCapsule;
 import com.jme3.export.JmeExporter;
 import com.jme3.export.JmeImporter;
@@ -309,6 +310,8 @@
         } else {
             // Deleting element in the middle
         }
+
+        setBoundRefresh();
     }
 
     public void addInstance(Geometry geometry) {
@@ -327,6 +330,7 @@
 
         geometries[freeIndex] = geometry;
         InstancedNode.setGeometryStartIndex2(geometry, freeIndex);
+        setBoundRefresh();
     }
 
     public Geometry[] getGeometries() {
@@ -344,6 +348,30 @@
         return allData.toArray(new VertexBuffer[allData.size()]);
     }
 
+    @Override
+    protected void updateWorldBound() {
+        refreshFlagAnd(~RF_BOUND);
+
+        BoundingVolume resultBound = null;
+
+        for (int i = 0; i < firstUnusedIndex; i++) {
+            Geometry geom = geometries[i];
+            assert (geom.getRefreshFlags() & RF_BOUND) == 0;
+
+            if (resultBound != null) {
+                // merge current world bound with child world bound
+                resultBound.mergeLocal(geom.getWorldBound());
+            } else {
+                // set world bound to first non-null child world bound
+                if (geom.getWorldBound() != null) {
+                    resultBound = geom.getWorldBound().clone(this.worldBound);
+                }
+            }
+        }
+
+        this.worldBound = resultBound;
+    }
+
     /**
      *  Called internally by com.jme3.util.clone.Cloner.  Do not call directly.
      */

I have 3 concerns:

  1. Either all the instanced geometries under InstancedGeometry come to the lighting shaders or nothing comes, because sphere to volume bound collision test happens with InstancedGeometry, but not with individual geometries. I think it is not a problem.

  2. InstancedGeometry performance which uses geometries with unshaded materials will decrease, because worldBound will be recalculated no matter of material. I don’t think that it is problem either.

  3. InstancedGeometry.updateWorldBound assumes, that all the instanced geometries InstancedGeometry.geometries have their worldBound already updated. In case of regular Node and Geometry there is no problem, as logic will update all the children before updating the parent. But in our case, InstancedNode is a parent of InstancedGeometry and regular Geometry at the same time. When InstancedNode asks its children to update their worldBound, we don’t know exactly in what order InstancedGeometry and regular Geometry will be updated under this InstancedNode. I didn’t notice assert messages, when I tested with multiple geometries, but I think we need to care about this situation. I propose to add InstancedNode.updateGeometricState, where we update bounds (through child.updateGeometricState()) for regular geometries and then InstancedGeometry geometries, and only then update bound for InstancedNode. What do you think?

Update.I see geom.getWorldBound() updates world bound, if flag is set up, so actually it doesn’t matter, if individual geometries bounds are not updated before updating the instanced geometry. I would remove this line from the above patch:
assert (geom.getRefreshFlags() & RF_BOUND) == 0;

Please, let me know, if I can create a pull request.

Update2. Here is an updated version of InstancedGeometry.updateWorldBound without assert and with geom null check (because geometries array is not recreated on geometry removal because of performance reasons, so null items are possible):

@Override
    protected void updateWorldBound() {
        refreshFlagAnd(~RF_BOUND);

        BoundingVolume resultBound = null;

        for (int i = 0; i < firstUnusedIndex; i++) {
            Geometry geom = geometries[i];

            if (geom != null) {
                if (resultBound != null) {
                    // merge current world bound with child world bound
                    resultBound.mergeLocal(geom.getWorldBound());
                } else {
                    // set world bound to first non-null child world bound
                    if (geom.getWorldBound() != null) {
                        resultBound = geom.getWorldBound().clone(this.worldBound);
                    }
                }
            }
        }

        this.worldBound = resultBound;
    }
2 Likes
#6

So back to this. Sorry for the delay.
I tested it and your patch fixes the issue indeed.
I also made a bench test with the TestInstanceNode (using around 3500 unshaded geoms) and there is no noticeable performance hit. So that’s perfect.

I pushed the change on 3.1 and master.

Thanks a lot, nice work.

4 Likes
#7

Thank you Rémy for the update and testing! And it is perfectly fine to apply the fix after delay, as there are many things to care :slight_smile: I will change the topic to solved.

1 Like