Terrain collision exception

yaRnMcDonuts · February 22, 2023, 9:04pm

I have frequently gotten this error in my custom scene editor whenever I save large scenes. The scene gets saved on the main thread still so it will typically it will freeze up the application for a few seconds, and then will often crash and give this error right after the secne is saved:

java.lang.IllegalStateException: Scene graph must be updated before checking collision
	at com.jme3.terrain.geomipmap.TerrainPatch.collideWith(TerrainPatch.java:812)
	at com.jme3.scene.Node.collideWith(Node.java:615)
	at com.simsilica.lemur.event.PickEventSession.cursorMoved(PickEventSession.java:567)
	at com.simsilica.lemur.event.MouseAppState.dispatchMotion(MouseAppState.java:94)
	at com.simsilica.lemur.event.BasePickState.update(BasePickState.java:239)
	at com.jme3.app.state.AppStateManager.update(AppStateManager.java:356)
	at com.jme3.app.SimpleApplication.update(SimpleApplication.java:255)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.runLoop(LwjglAbstractDisplay.java:151)
	at com.jme3.system.lwjgl.LwjglDisplay.runLoop(LwjglDisplay.java:197)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:232)
	at java.base/java.lang.Thread.run(Thread.java:834)

This specific error appears to be triggered from Lemur’s picking, but I have gotten the same error on the same line in TerrainPatch from my decal system when doing collisions with terrains.

The issue appears to be this exception that gets manually thrown in the TerrainPatch class:

github.com

jMonkeyEngine/jmonkeyengine/blob/master/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainPatch.java#L802


      
          }

          

          /*public void setLodCalculator(LodCalculatorFactory lodCalculatorFactory) {

              this.lodCalculatorFactory = lodCalculatorFactory;

              setLodCalculator(lodCalculatorFactory.createCalculator(this));

          }*/

          

          @Override

          public int collideWith(Collidable other, CollisionResults results) throws UnsupportedCollisionException {

              if (refreshFlags != 0)

                  throw new IllegalStateException("Scene graph must be updated" +

                                                  " before checking collision");

          

              if (other instanceof BoundingVolume)

                  if (!getWorldBound().intersects((BoundingVolume)other))

                      return 0;

          

              if(other instanceof Ray)

                  return collideWithRay((Ray)other, results);

              else if (other instanceof BoundingVolume)

                  return collideWithBoundingVolume((BoundingVolume)other, results);

I bypassed this error in my own scene editor a long time ago by making my own fork of the terrain library that removed this line of code. I never ran into any issues after I removed this line of code, so it does not seem like there was any benefit to throwing a crash error in this situation.

But now that I just upgraded the terrain library used for my scene editor to the latest jme version, I have started hitting this error again, so I thought it might be worth investigating further and submitting a PR to fix in jme’s terrain library.

Nakano · February 22, 2023, 9:21pm

You could call

updateGeometricState();

on the changed spatial to update it and clear the refreshFlags immediately.
Alternatively, you could wait until the next update() run of the SimpleApplication. This calls this method each time on the root- and guiNode.

yaRnMcDonuts · February 22, 2023, 9:45pm

I’m mostly wondering if there’s any reason to be throwing that error in the first place, since there is only 1 frame of time that the refreshFlags is != 0 since it will always fix itself the next frame, which is why I never ran into any trouble in my own fork where it doesn’t throw any exception.

The collision code for normal Geometries also does not throw any similar collision Exception based on the value of refreshFlags either, and if a terrain’s collisionData is empty it just won’t return any collision values anyways (which only ever happens on the first frame a terrain is attached, which is when this error can sometimes occur) so unless there’s something else I’m missing, it seems like the best solution is to just remove the line of code that’s throwing a useless exception in terrain patch.

Nakano · February 22, 2023, 10:06pm

I am not a JME3 professional, but as far as I see, updateGeometricState() of Node calls updateWorldBound() which update the worldBound. This is required in collideWith() of TerrainQuad, if “other” is a BoundingVolume.

If I understand you right, it is a bad Idea to don’t return a collision value (or any other wrong value), if something failed. It is always better to graceful fail instead doing “anything” in case of an error. That would cause in inconsistency and may dangerous applications.

pspeed · February 22, 2023, 11:08pm

I don’t have an opinion on this. I’ve never liked the terrain library and if we ever throw a terrain-lib-bashing party, I’m right there with my party hat.

…this does seem like a surprising way for the collideWith() method to handle this, though. I’m fine with your fix but I similarly don’t know what the implications are. It’s a relatively fragile library so it might be non-trivial to see all edge cases. But if you have experience using it with your change then it seems fine.

yaRnMcDonuts · February 22, 2023, 11:37pm

I would typically agree, but in this case I think throwing this error is so useless that it doesn’t even cause the app to gracefully fail when collisions actually are broken. It also seems extra concerning that the Geometry class doesn’t do the same thing, and both use refreshFlags in the exact same way since they both extend Spatial, and the only extra code in the terrain class related to refreshFlags just throws this seemingly useless exception.

I have never seen this error thrown when one of my terrains was actually broken with collisions and needed entirely re-created, and I’ve only ever encountered this error as a result of this bug when a terrain is attached during a lag spike, and by the next frame the collisions are working again so it isn’t even catching a real problem in this case, its just throwing an error in cases where anything is trying to collide with a terrain that is taking an extra frame or two to be ready for some strange reason.

Yes since I made the change on my own fork I have never had any trouble with terrain collision. It was also about 4 years ago I originally ran into this error when I first started working on my own terrain editor, so in that time I have never experienced any other problem as a result of removing this exception.

Edit: I opened a PR to replace the Exception with just a warning

Ali_RS · February 23, 2023, 8:12am

For normal Geometry, it does check with assertion. So it will only be checked if the assertion is enabled.

github.com

jMonkeyEngine/jmonkeyengine/blob/375015f14c5f3cbda18302b3aa990e5f023e9da1/jme3-core/src/main/java/com/jme3/scene/Geometry.java#L465


      
              //updateModelBound();
          }
          
          @Override
          public int collideWith(Collidable other, CollisionResults results) {
              // Force bound to update
              checkDoBoundUpdate();
              // Update transform, and compute cached world matrix
              computeWorldMatrix();
          
              assert (refreshFlags & (RF_BOUND | RF_TRANSFORM)) == 0;
          
              if (mesh != null) {
                  // NOTE: BIHTree in mesh already checks collision with the
                  // mesh's bound
                  int prevSize = results.size();
                  int added = mesh.collideWith(other, cachedWorldMat, worldBound, results);
                  int newSize = results.size();
                  for (int i = prevSize; i < newSize; i++) {
                      results.getCollisionDirect(i).setGeometry(this);
                  }