jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.78k stars 1.12k forks source link

Instanced objects are culled when using WaterFilter #2007

Closed foxcc2021 closed 1 year ago

foxcc2021 commented 1 year ago

It seems there is a bug in culling checks of instanced objects.

20230510202939 The tree model should not be culled.

20230510203009 The correct scene expected.

After some debugging, I found that, it seems the instanceCullingFunction used the wrong camera to cull checks in the InstancedGeometry。 Because the WaterFilter has a reflectionCam, the default camera will be replaced when waterFilter rendering. 20230510202701

20230510210756 I've solved my problem using this way,pass the render camera from the instanced node to the instanced geometry for culling checks. Hope this can helps.

JME version:3.6.0-stable System: win10

foxcc2021 commented 1 year ago

20230510212110 The code in the InstancedGeometry I have modified in my project.

Ali-RS commented 1 year ago

I can not reproduce this issue, can you provide a simple test case demonstrating the issue?

Edit: Btw, InstancedGeometry gets the camera passed in the checkCulling method:

https://github.com/jMonkeyEngine/jmonkeyengine/blob/25a3407c9e403dbc0250430a304031c169636a03/jme3-core/src/main/java/com/jme3/scene/instancing/InstancedGeometry.java#L420-L423

it is called from the RenderManager that also uses the viewport camera

https://github.com/jMonkeyEngine/jmonkeyengine/blob/25a3407c9e403dbc0250430a304031c169636a03/jme3-core/src/main/java/com/jme3/renderer/RenderManager.java#L791-L794

foxcc2021 commented 1 year ago

I made a video to show the issue.

https://youtu.be/fbHqHQu6js8

The simple test case is:

public class TestWaterAndInstanced extends SimpleApplication {

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

    private Box mesh;
    private Material mat;
    private InstancedNode instanceNode;

    @Override
    public void simpleInitApp() {
        flyCam.setMoveSpeed(10);

        DirectionalLight light = new DirectionalLight();
        light.setDirection(new Vector3f(-1, -1, -1));
        rootNode.addLight(light);

        mat = new Material(assetManager, "Common/MatDefs/Light/Lighting.j3md");
        mat.setBoolean("UseInstancing", true);

        mesh = new Box(0.5f, 0.5f, 0.5f);

        FilterPostProcessor fpp = new FilterPostProcessor(assetManager);
        WaterFilter waterFilter = new WaterFilter();
        waterFilter.setReflectionScene(rootNode);
        fpp.addFilter(waterFilter);
        viewPort.addProcessor(fpp);

        instanceNode = new InstancedNode("TestInstancedNode");
        rootNode.attachChild(instanceNode);

        for (int i = 0; i < 1000; i++) {
            Geometry obj = new Geometry("TestBox" + i, mesh);
            obj.setLocalTranslation(0, 10, 0);
            obj.setLocalScale(1, 1, 1);
            obj.setMaterial(mat);
            obj.setLocalTranslation(i, i, 0);
            instanceNode.attachChild(obj);
        }
        instanceNode.instance();
    }
}
foxcc2021 commented 1 year ago

public boolean checkCulling(Camera cam) { this.cam = cam; return super.checkCulling(cam); }

I think maybe the problem is with checkCulling method, cam will be replaced by reflectionCam of WaterFilter. InstancedNode renders the instance object in its render method, but the cam has been replaced. But I just found out, that there is something wrong with my solution, causing the whole InstancedNode to disappear. It may need more research.

Ali-RS commented 1 year ago

Thanks for the test and video, I am going to try it soon.

By the way, you can also disable instance culling by setting InstancedGeometry.setInstanceCullingFunction(null); (note that this is a static method) and it should resolve the issue.

foxcc2021 commented 1 year ago

Thank you! Culling check of the instancing object is very useful for large scenes. Hope it can be fixed.

Ali-RS commented 1 year ago

I can reproduce the issue with the test case you provided. Passing the camera from InstancedNodeControl as you suggested seems to resolve the issue.

Would you like to submit a PR?

foxcc2021 commented 1 year ago

I did some more testing and "pass camera from InstancedNodeControl" seems to fix the issue.

But I just found out, that there is something wrong with my solution, causing the whole InstancedNode to disappear. It may need more research.

Another problem seems to be related to my project only. I just copied InstancedNode and InstancedGeometry into my project and renamed them to "MyInstancedNode" and "MyInstancedGeometry" for testing, causing InstancedNode to disappear and I don't know why yet.

foxcc2021 commented 1 year ago

Would you like to submit a PR?

I'm sorry, I don't know how to make the PR.