From d85f64c2f85219d077068c60c72ed3c187759d47 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Tue, 19 Mar 2019 20:21:43 +0100 Subject: [PATCH 1/8] Fix TerrainPicking by not adding "ghost" collisions to the CollisionResults (which weren't removed). Improve TerrainPicking by allowing to do more ray tracing (previously, it stopped on the first hit, now it will stop on the first hit within range). --- .../jme3/terrain/geomipmap/TerrainQuad.java | 15 +--- .../picking/BresenhamTerrainPicker.java | 70 +++++++++++++------ 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java index 7f41bd3727..88e3b9637a 100644 --- a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java +++ b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java @@ -274,20 +274,11 @@ protected void cacheTerrainTransforms() { } private int collideWithRay(Ray ray, CollisionResults results) { - if (picker == null) + if (picker == null) { picker = new BresenhamTerrainPicker(this); + } - Vector3f intersection = picker.getTerrainIntersection(ray, results); - if (intersection != null) { - if (ray.getLimit() < Float.POSITIVE_INFINITY) { - if (results.getClosestCollision().getDistance() <= ray.getLimit()) - return 1; // in range - else - return 0; // out of range - } else - return 1; - } else - return 0; + return (picker.getTerrainIntersection(ray, results) != null) ? 1 : 0; } /** diff --git a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java index cf7efc085e..aae8c83492 100644 --- a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java +++ b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java @@ -75,23 +75,23 @@ public BresenhamTerrainPicker(TerrainQuad root) { public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) { worldPickRay.set(worldPick); - List pickData = new ArrayList(); + List pickData = new ArrayList<>(); root.findPick(worldPick.clone(), pickData); Collections.sort(pickData); - if (pickData.isEmpty()) + if (pickData.isEmpty()) { return null; + } workRay.set(worldPick); for (TerrainPickData pd : pickData) { TerrainPatch patch = pd.targetPatch; - tracer.getGridSpacing().set(patch.getWorldScale()); tracer.setGridOrigin(patch.getWorldTranslation()); - workRay.getOrigin().set(worldPick.getDirection()).multLocal(pd.cr.getDistance()-.1f).addLocal(worldPick.getOrigin()); + workRay.getOrigin().set(worldPick.getDirection()).multLocal(pd.cr.getDistance() - .1f).addLocal(worldPick.getOrigin()); tracer.startWalk(workRay); @@ -102,14 +102,17 @@ public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) Triangle hit = new Triangle(); checkTriangles(loc.x, loc.y, workRay, intersection, patch, hit); float distance = worldPickRay.origin.distance(intersection); - CollisionResult cr = new CollisionResult(intersection, distance); - cr.setGeometry(patch); - cr.setContactNormal(hit.getNormal()); - results.addCollision(cr); - return intersection; + + if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { + if (distance <= worldPick.getLimit()) { + addCollision(results, patch, intersection, hit.getNormal(), distance); + return intersection; + } // else return null; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + addCollision(results, patch, intersection, hit.getNormal(), distance); + return intersection; + } } - - while (loc.x >= -1 && loc.x <= patch.getSize() && loc.y >= -1 && loc.y <= patch.getSize()) { @@ -120,11 +123,16 @@ public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) if (checkTriangles(loc.x, loc.y, workRay, intersection, patch, hit)) { // we found an intersection, so return that! float distance = worldPickRay.origin.distance(intersection); - CollisionResult cr = new CollisionResult(intersection, distance); - cr.setGeometry(patch); - results.addCollision(cr); - cr.setContactNormal(hit.getNormal()); - return intersection; + + if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { + if (distance <= worldPick.getLimit()) { + addCollision(results, patch, intersection, hit.getNormal(), distance); + return intersection; + } // else return null; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + addCollision(results, patch, intersection, hit.getNormal(), distance); + return intersection; + } } // because of how we get our height coords, we will @@ -148,11 +156,16 @@ public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) if (checkTriangles(loc.x + dx, loc.y + dz, workRay, intersection, patch, hit)) { // we found an intersection, so return that! float distance = worldPickRay.origin.distance(intersection); - CollisionResult cr = new CollisionResult(intersection, distance); - results.addCollision(cr); - cr.setGeometry(patch); - cr.setContactNormal(hit.getNormal()); - return intersection; + + if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { + if (distance <= worldPick.getLimit()) { + addCollision(results, patch, intersection, hit.getNormal(), distance); + return intersection; + } // else return null; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + addCollision(results, patch, intersection, hit.getNormal(), distance); + return intersection; + } } tracer.next(); @@ -162,6 +175,21 @@ public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) return null; } + /** + * This method adds the found Collision to an existing collisionResult. + * @param results The results to add this collision to + * @param patch The TerrainPatch which collided + * @param intersection The actual intersection position + * @param normal The intersection normal + * @param distance The distance at which the hit occurred + */ + private void addCollision(CollisionResults results, TerrainPatch patch, Vector3f intersection, Vector3f normal, float distance) { + CollisionResult cr = new CollisionResult(intersection, distance); + cr.setGeometry(patch); + cr.setContactNormal(normal); + results.addCollision(cr); + } + protected boolean checkTriangles(float gridX, float gridY, Ray pick, Vector3f intersection, TerrainPatch patch, Triangle store) { if (!getTriangles(gridX, gridY, patch)) return false; From 98d0eb9edb1bcdee737f6f8f80999b6994d0e60e Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Fri, 29 Mar 2019 11:57:50 +0100 Subject: [PATCH 2/8] Upgraded TerrainTestCollision to support multiple collisions and print the collision results detailed. MultiCollision can easily be turned on/off in simpleInitApp(). During testing, I noticed a bug where in very rare cases the first collision isn't what is expected but the back side of the clicked mountain. It has to be validated if this is due to the following changes or was already present. --- .../terrain/TerrainTestCollision.java | 67 ++++++++++++++----- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/jme3-examples/src/main/java/jme3test/terrain/TerrainTestCollision.java b/jme3-examples/src/main/java/jme3test/terrain/TerrainTestCollision.java index 6822251ba7..faa67fd137 100644 --- a/jme3-examples/src/main/java/jme3test/terrain/TerrainTestCollision.java +++ b/jme3-examples/src/main/java/jme3test/terrain/TerrainTestCollision.java @@ -64,6 +64,9 @@ import com.jme3.texture.Texture; import com.jme3.texture.Texture.WrapMode; +import java.util.ArrayList; +import java.util.List; + /** * Creates a terrain object and a collision node to go with it. Then * drops several balls from the sky that collide with the terrain @@ -83,7 +86,7 @@ public class TerrainTestCollision extends SimpleApplication { protected BitmapText hintText; PointLight pl; Geometry lightMdl; - Geometry collisionMarker; + List collisionMarkers; private BulletAppState bulletAppState; Geometry collisionSphere; Geometry collisionBox; @@ -103,6 +106,7 @@ public void initialize() { @Override public void simpleInitApp() { + collisionMarkers = new ArrayList<>(); bulletAppState = new BulletAppState(); bulletAppState.setThreadingType(BulletAppState.ThreadingType.PARALLEL); stateManager.attach(bulletAppState); @@ -142,6 +146,8 @@ public void simpleInitApp() { terrain.setLocked(false); // unlock it so we can edit the height rootNode.attachChild(terrain); + // if set to false, only the first collision is returned and collision is slightly faster. + terrain.setSupportMultipleCollisions(true); /** * Create PhysicsRigidBodyControl for collision @@ -227,15 +233,19 @@ public void update() { super.update(); } - private void createCollisionMarker() { - Sphere s = new Sphere(6, 6, 1); - collisionMarker = new Geometry("collisionMarker"); - collisionMarker.setMesh(s); - Material mat = new Material(assetManager, "Common/MatDefs/Misc/Unshaded.j3md"); - mat.setColor("Color", ColorRGBA.Orange); - collisionMarker.setMaterial(mat); - rootNode.attachChild(collisionMarker); + private void createCollisionMarkers(int num) { + for (int i = 0; i < num; i++) { + Sphere s = new Sphere(6, 6, 1); + Geometry collisionMarker = new Geometry("collisionMarker"); + collisionMarker.setMesh(s); + Material mat = new Material(assetManager, "Common/MatDefs/Misc/Unshaded.j3md"); + mat.setColor("Color", i == 0 ? ColorRGBA.Orange : ColorRGBA.Blue); + collisionMarker.setMaterial(mat); + rootNode.attachChild(collisionMarker); + collisionMarkers.add(collisionMarker); + } } + private ActionListener actionListener = new ActionListener() { public void onAction(String binding, boolean keyPressed, float tpf) { @@ -247,24 +257,35 @@ public void onAction(String binding, boolean keyPressed, float tpf) { terrain.setMaterial(matRock); } } else if (binding.equals("shoot") && !keyPressed) { - Vector3f origin = cam.getWorldCoordinates(new Vector2f(settings.getWidth() / 2, settings.getHeight() / 2), 0.0f); Vector3f direction = cam.getWorldCoordinates(new Vector2f(settings.getWidth() / 2, settings.getHeight() / 2), 0.3f); direction.subtractLocal(origin).normalizeLocal(); - Ray ray = new Ray(origin, direction); CollisionResults results = new CollisionResults(); - int numCollisions = terrain.collideWith(ray, results); - if (numCollisions > 0) { - CollisionResult hit = results.getClosestCollision(); - if (collisionMarker == null) { - createCollisionMarker(); + + if (terrain.collideWith(ray, results) > 0) { + CollisionResult hit = results.getClosestCollision(); // sorts the collection before printing + printCollisions(results); + + // Remove old markers. + for (Geometry g: collisionMarkers) { + g.removeFromParent(); } + collisionMarkers.clear(); + + createCollisionMarkers(results.size()); + + // Position Closest Collision Vector2f loc = new Vector2f(hit.getContactPoint().x, hit.getContactPoint().z); float height = terrain.getHeight(loc); - System.out.println("collide " + hit.getContactPoint() + ", height: " + height + ", distance: " + hit.getDistance()); - collisionMarker.setLocalTranslation(new Vector3f(hit.getContactPoint().x, height, hit.getContactPoint().z)); + System.out.println("Closest Collision: " + hit.getContactPoint() + ", height: " + height + ", distance: " + hit.getDistance()); + collisionMarkers.get(0).setLocalTranslation(new Vector3f(hit.getContactPoint().x, height, hit.getContactPoint().z)); + + // Position Rest: When getClosestCollision has been called, the results are sorted, and thus 0 is closest. + for (int i = 1; i < results.size(); i++) { + collisionMarkers.get(i).setLocalTranslation(results.getCollision(i).getContactPoint()); + } } } else if (binding.equals("cameraDown") && !keyPressed) { getCamera().lookAtDirection(new Vector3f(0, -1, 0), Vector3f.UNIT_Y); @@ -302,4 +323,14 @@ private void testCollision(Vector3f oldLoc) { selectedCollisionObject.setLocalTranslation(oldLoc); } } + + private void printCollisions(CollisionResults cr) { + System.out.println("================ Collision Results ================"); + for (int i = 0; i < cr.size(); i++) { + CollisionResult res = cr.getCollision(i); + System.out.println("Result " + i); + System.out.println("\t\t" + res.toString()); + } + System.out.println("================ END Collision Results ================"); + } } From a238b55568e6eb92cff31113703e5ba6eb3c0955 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Fri, 29 Mar 2019 11:58:17 +0100 Subject: [PATCH 3/8] Added Basic Unit Tests for Collision --- jme3-terrain/build.gradle | 4 + .../jme3/terrain/collision/BaseAWTTest.java | 38 ++++++++++ .../com/jme3/terrain/collision/BaseTest.java | 28 +++++++ .../collision/TerrainCollisionTest.java | 76 +++++++++++++++++++ 4 files changed, 146 insertions(+) create mode 100644 jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseAWTTest.java create mode 100644 jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseTest.java create mode 100644 jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java diff --git a/jme3-terrain/build.gradle b/jme3-terrain/build.gradle index 0a77d7d720..030f88d14b 100644 --- a/jme3-terrain/build.gradle +++ b/jme3-terrain/build.gradle @@ -4,4 +4,8 @@ if (!hasProperty('mainClass')) { dependencies { compile project(':jme3-core') + testCompile project(':jme3-core') + testRuntime project(':jme3-desktop') + testCompile project(':jme3-core').sourceSets.test.output + testRuntime project(':jme3-testdata') } diff --git a/jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseAWTTest.java b/jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseAWTTest.java new file mode 100644 index 0000000000..4ff66b4bff --- /dev/null +++ b/jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseAWTTest.java @@ -0,0 +1,38 @@ +package com.jme3.terrain.collision; + +import com.jme3.asset.AssetManager; +import com.jme3.system.JmeDesktopSystem; +import com.jme3.system.JmeSystem; + +/** + * This class provides some utility functions to properly test the jMonkeyEngine.
+ * Thus it contains simple methods to get and create a headless assetManager amongst other things.
+ * In comparison to {@link BaseTest} it provides a DesktopAssetManager capable of loading image formats using AWT, which + * however makes those tests unsuitable for headless ci testing. This requires jme3-desktop to be a testRuntime dependency. + * + * @author MeFisto94 + */ +public abstract class BaseAWTTest { + private AssetManager assetManager; + + static { + //JmeSystem.setSystemDelegate(new JmeDesktopSystem()); + } + + public AssetManager getAssetManager() { + if (assetManager == null) { + assetManager = createAssetManager(); + } + + return assetManager; + } + + private AssetManager createAssetManager() { + /* Desktop.cfg supports the following additional file formats at the time of writing: + LOADER com.jme3.texture.plugins.AWTLoader : jpg, bmp, gif, png, jpeg + LOADER com.jme3.audio.plugins.OGGLoader : ogg + */ + return JmeSystem.newAssetManager(BaseTest.class.getResource("/com/jme3/asset/Desktop.cfg")); + } + +} diff --git a/jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseTest.java b/jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseTest.java new file mode 100644 index 0000000000..15f7518558 --- /dev/null +++ b/jme3-terrain/src/test/java/com/jme3/terrain/collision/BaseTest.java @@ -0,0 +1,28 @@ +package com.jme3.terrain.collision; + +import com.jme3.asset.AssetManager; +import com.jme3.system.TestUtil; + +/** + * This class provides some utility functions to properly test the jMonkeyEngine.
+ * Thus it contains simple methods to get and create a headless assetManager amongst other things.
+ * If you need support for image/texture formats (png, tga, jpg, ...) see {@link BaseAWTTest} + * + * @author MeFisto94 + */ +public abstract class BaseTest { + private AssetManager assetManager; + + public AssetManager getAssetManager() { + if (assetManager == null) { + assetManager = createAssetManager(); + } + + return assetManager; + } + + private AssetManager createAssetManager() { + return TestUtil.createAssetManager(); + } + +} diff --git a/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java b/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java new file mode 100644 index 0000000000..68e4b4923f --- /dev/null +++ b/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java @@ -0,0 +1,76 @@ +package com.jme3.terrain.collision; + +import com.jme3.collision.CollisionResult; +import com.jme3.collision.CollisionResults; +import com.jme3.math.FastMath; +import com.jme3.math.Ray; +import com.jme3.math.Vector3f; +import com.jme3.terrain.geomipmap.TerrainQuad; +import com.jme3.terrain.heightmap.AbstractHeightMap; +import com.jme3.terrain.heightmap.ImageBasedHeightMap; +import com.jme3.texture.Texture; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class TerrainCollisionTest extends BaseAWTTest { + TerrainQuad quad; + + @Before + public void initQuad() { + Texture heightMapImage = getAssetManager().loadTexture("Textures/Terrain/splat/mountains512.png"); + AbstractHeightMap map = new ImageBasedHeightMap(heightMapImage.getImage(), 0.25f); + map.load(); + quad = new TerrainQuad("terrain", 65, 513, map.getHeightMap()); + } + + /** + * Due to a previous bug, when no collision should happen, the CollisionResults struct was still populated, leading + * to an incoherency of data and ghost collisions when passing a non-empty CR. + */ + @Test + public void testNoCollision() { + Ray r = new Ray(new Vector3f(0f, 40f, 0f), Vector3f.UNIT_Y.negate()); + r.setLimit(0.1f); + CollisionResults cr = new CollisionResults(); + long l = System.nanoTime(); + int cw = quad.collideWith(r, cr); + System.out.println((System.nanoTime() - l) + " ns"); + + Assert.assertEquals(0, cw); + Assert.assertEquals(0, cr.size()); + Assert.assertEquals(null, cr.getClosestCollision()); + Assert.assertEquals(null, cr.getFarthestCollision()); + } + + @Test + public void testPerpendicularCollision() { + Ray r = new Ray(new Vector3f(0f, 40f, 0f), Vector3f.UNIT_Y.negate()); + CollisionResults cr = new CollisionResults(); + int cw = quad.collideWith(r, cr); + + Assert.assertEquals(1, cw); + Assert.assertEquals(1, cr.size()); + Assert.assertEquals(new Vector3f(0f, 28f, 0f), cr.getClosestCollision().getContactPoint()); + Assert.assertEquals(new Vector3f(-0.5144958f, 0.6859944f, 0.5144958f), cr.getClosestCollision().getContactNormal()); + Assert.assertEquals(12, cr.getClosestCollision().getDistance(), 0.01d); + Assert.assertEquals(0, cr.getClosestCollision().getTriangleIndex()); + } + + @Test + public void testMultiCollision() { + // Ray parameters obtained by using TerrainTestCollision (manual inspection of a feasible ray and commenting out setLocalScale(2) + Ray r = new Ray(new Vector3f(-38.689114f, 35.622643f, -40.222355f), new Vector3f(0.68958646f, 0.0980845f, 0.7175304f)); + + CollisionResults cr = new CollisionResults(); + long l = System.nanoTime(); + int cw = quad.collideWith(r, cr); + System.out.println((System.nanoTime() - l) + " ns"); + System.out.println(cr.size()); + System.out.println(cr); + Assert.assertEquals(6, cw); + Assert.assertEquals(6, cr.size()); + + } + +} From e2656940fb89889a164853101fa892074d61e1d2 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Fri, 29 Mar 2019 12:00:02 +0100 Subject: [PATCH 4/8] TerrainPicker: Change API to return int to conform with collideWith --- .../com/jme3/terrain/geomipmap/picking/TerrainPicker.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/TerrainPicker.java b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/TerrainPicker.java index 08308849f5..bd45f3a084 100644 --- a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/TerrainPicker.java +++ b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/TerrainPicker.java @@ -47,9 +47,8 @@ public interface TerrainPicker { * * @param worldPick * our pick ray, in world space. - * @return null if no pick is found. Otherwise it returns a Vector3f populated with the pick - * coordinates. + * @return The number of collisions found */ - public Vector3f getTerrainIntersection(final Ray worldPick, CollisionResults results); + public int getTerrainIntersection(final Ray worldPick, CollisionResults results); } From cdf83c1708891f2f94c4d12b53b516ac800605e8 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Fri, 29 Mar 2019 12:00:34 +0100 Subject: [PATCH 5/8] TerrainQuad: Conform with Picker now returning the number of collisions and allow to set multipleCollisions true or false. --- .../jme3/terrain/geomipmap/TerrainQuad.java | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java index 88e3b9637a..f89198f251 100644 --- a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java +++ b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/TerrainQuad.java @@ -123,7 +123,7 @@ public class TerrainQuad extends Node implements Terrain { private int maxLod = -1; private BoundingBox affectedAreaBBox; // only set in the root quad - private TerrainPicker picker; + private TerrainPicker picker = new BresenhamTerrainPicker(this); private Vector3f lastScale = Vector3f.UNIT_XYZ; protected NeighbourFinder neighbourFinder; @@ -274,11 +274,7 @@ protected void cacheTerrainTransforms() { } private int collideWithRay(Ray ray, CollisionResults results) { - if (picker == null) { - picker = new BresenhamTerrainPicker(this); - } - - return (picker.getTerrainIntersection(ray, results) != null) ? 1 : 0; + return picker.getTerrainIntersection(ray, results); } /** @@ -1906,5 +1902,23 @@ else if(((TerrainPatch) s).getQuadrant() == 4) return hm; } + + /** + * When colliding with this terrain, is a report of all collisions wanted or only the closest collision?
+ * If only the closest collision is required, the collision calculation will be faster.
+ * Note: If no collision happens, it takes as long as a collision with multipleCollisions on would take. + * + * @param set Whether to support multiple collisions or not + */ + public void setSupportMultipleCollisions(boolean set) { + if (picker == null) { + // This is so that it doesn't fail at the IllegalStateException because null !instanceof Anything + throw new NullPointerException("TerrainPicker is null"); + } else if (picker instanceof BresenhamTerrainPicker) { + ((BresenhamTerrainPicker)picker).setSupportMultipleCollisions(set); + } else { + throw new IllegalStateException("The underlying picking implementation does not support multiple collisions"); + } + } } From 3d3683cae1d3849c819f701186df6f8388f82f48 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Fri, 29 Mar 2019 12:04:22 +0100 Subject: [PATCH 6/8] TerrainPicking: Fixed a bug where the perpendicular collision always returned true, no matter the result of checkTriangles. Also add support for multiple collisions (which is toggleable to the old behavior, because the picker can early out then). --- .../picking/BresenhamTerrainPicker.java | 175 +++++++++++------- 1 file changed, 110 insertions(+), 65 deletions(-) diff --git a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java index aae8c83492..b95ecde5d3 100644 --- a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java +++ b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java @@ -67,20 +67,30 @@ public class BresenhamTerrainPicker implements TerrainPicker { private final TerrainQuad root; private final BresenhamYUpGridTracer tracer = new BresenhamYUpGridTracer(); + private boolean multipleCollisions = true; + public BresenhamTerrainPicker(TerrainQuad root) { this.root = root; } - public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) { + public void setSupportMultipleCollisions(boolean multipleCollisions) { + this.multipleCollisions = multipleCollisions; + } + + public boolean isSupportingMultipleCollisions() { + return multipleCollisions; + } + public int getTerrainIntersection(Ray worldPick, CollisionResults results) { + int numCollisions = 0; worldPickRay.set(worldPick); List pickData = new ArrayList<>(); root.findPick(worldPick.clone(), pickData); Collections.sort(pickData); if (pickData.isEmpty()) { - return null; + return 0; } workRay.set(worldPick); @@ -99,80 +109,114 @@ public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) final Vector2f loc = tracer.getGridLocation(); if (tracer.isRayPerpendicularToGrid()) { - Triangle hit = new Triangle(); - checkTriangles(loc.x, loc.y, workRay, intersection, patch, hit); - float distance = worldPickRay.origin.distance(intersection); - - if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { - if (distance <= worldPick.getLimit()) { - addCollision(results, patch, intersection, hit.getNormal(), distance); - return intersection; - } // else return null; // < this is the old behavior, since the code checked for the range afterwards. - } else { // unlimited range - addCollision(results, patch, intersection, hit.getNormal(), distance); - return intersection; - } - } - - while (loc.x >= -1 && loc.x <= patch.getSize() && - loc.y >= -1 && loc.y <= patch.getSize()) { - - //System.out.print(loc.x+","+loc.y+" : "); - // check the triangles of main square for intersection. Triangle hit = new Triangle(); if (checkTriangles(loc.x, loc.y, workRay, intersection, patch, hit)) { - // we found an intersection, so return that! float distance = worldPickRay.origin.distance(intersection); + //@TODO: Verify if it's even possible to have a ray hit multiple "PickData"s when being perpendicular at all. + // because otherwise, we could always return 1 here. if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { if (distance <= worldPick.getLimit()) { - addCollision(results, patch, intersection, hit.getNormal(), distance); - return intersection; - } // else return null; // < this is the old behavior, since the code checked for the range afterwards. + addCollision(results, patch, intersection, hit, distance); + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } // else return 0; // < this is the old behavior, since the code checked for the range afterwards. } else { // unlimited range - addCollision(results, patch, intersection, hit.getNormal(), distance); - return intersection; + addCollision(results, patch, intersection, hit, distance); + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } + } // else no collision + } else { // If the ray is perpendicular, tracer.next() would never advance loc, leading to an infinite loop. + while (loc.x >= -1 && loc.x <= patch.getSize() && + loc.y >= -1 && loc.y <= patch.getSize()) { + + //System.out.print(loc.x + "," + loc.y + " : "); + // check the triangles of main square for intersection. + Triangle hit = new Triangle(); + if (checkTriangles(loc.x, loc.y, workRay, intersection, patch, hit)) { + // we found an intersection, so return that! + float distance = worldPickRay.origin.distance(intersection); + + if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { + if (distance <= worldPick.getLimit()) { + addCollision(results, patch, intersection, hit, distance); + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + }// else return 0; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + addCollision(results, patch, intersection, hit, distance); + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } } - } - - // because of how we get our height coords, we will - // sometimes be off by a grid spot, so we check the next - // grid space up. - int dx = 0, dz = 0; - Direction d = tracer.getLastStepDirection(); - switch (d) { - case PositiveX: - case NegativeX: - dx = 0; - dz = 1; - break; - case PositiveZ: - case NegativeZ: - dx = 1; - dz = 0; - break; - } - if (checkTriangles(loc.x + dx, loc.y + dz, workRay, intersection, patch, hit)) { - // we found an intersection, so return that! - float distance = worldPickRay.origin.distance(intersection); + /** + * Commented out by MeFisto94 during refactoring because due to multiCollision we step through the + * terrain and will reach this offset spot anyway. Leaving this method in just gives duplicates. + * It has to be tested what happens at borders, where there would be no next iteration. I suspect + * there the code would fail at checkTriangles anyway, because they would be out of bounds. + */ + /* + // because of how we get our height coords, we will + // sometimes be off by a grid spot, so we check the next + // grid space up. + int dx = 0, dz = 0; + Direction d = tracer.getLastStepDirection(); + switch (d) { + case PositiveX: + case NegativeX: + dx = 0; + dz = 1; + break; + case PositiveZ: + case NegativeZ: + dx = 1; + dz = 0; + break; + } - if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { - if (distance <= worldPick.getLimit()) { + if (checkTriangles(loc.x + dx, loc.y + dz, workRay, intersection, patch, hit)) { + // we found an intersection, so return that! + float distance = worldPickRay.origin.distance(intersection); + + if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { + if (distance <= worldPick.getLimit()) { + addCollision(results, patch, intersection, hit.getNormal(), distance); + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } // else return null; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range addCollision(results, patch, intersection, hit.getNormal(), distance); - return intersection; - } // else return null; // < this is the old behavior, since the code checked for the range afterwards. - } else { // unlimited range - addCollision(results, patch, intersection, hit.getNormal(), distance); - return intersection; - } + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } + }*/ + + tracer.next(); } - - tracer.next(); } } - return null; + return numCollisions; } /** @@ -180,13 +224,14 @@ public Vector3f getTerrainIntersection(Ray worldPick, CollisionResults results) * @param results The results to add this collision to * @param patch The TerrainPatch which collided * @param intersection The actual intersection position - * @param normal The intersection normal + * @param hit The hit triangle * @param distance The distance at which the hit occurred */ - private void addCollision(CollisionResults results, TerrainPatch patch, Vector3f intersection, Vector3f normal, float distance) { - CollisionResult cr = new CollisionResult(intersection, distance); + private void addCollision(CollisionResults results, TerrainPatch patch, Vector3f intersection, Triangle hit, float distance) { + CollisionResult cr = new CollisionResult(intersection.clone(), distance); cr.setGeometry(patch); - cr.setContactNormal(normal); + cr.setContactNormal(hit.getNormal()); + cr.setTriangleIndex(hit.getIndex()); // this will probably always be 0 results.addCollision(cr); } From 59c4bc6d85920349b94e54c689b5e503b9627186 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Sat, 30 Mar 2019 12:26:12 +0100 Subject: [PATCH 7/8] Try to fix Travis Build --- jme3-terrain/build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jme3-terrain/build.gradle b/jme3-terrain/build.gradle index 030f88d14b..a782045c6a 100644 --- a/jme3-terrain/build.gradle +++ b/jme3-terrain/build.gradle @@ -5,7 +5,7 @@ if (!hasProperty('mainClass')) { dependencies { compile project(':jme3-core') testCompile project(':jme3-core') - testRuntime project(':jme3-desktop') + testCompile project(':jme3-desktop') testCompile project(':jme3-core').sourceSets.test.output - testRuntime project(':jme3-testdata') + testCompile project(':jme3-testdata') } From e135dac8933ab00f4725b6b4019b643de3accbe2 Mon Sep 17 00:00:00 2001 From: MeFisto94 Date: Sun, 31 Mar 2019 15:33:36 +0200 Subject: [PATCH 8/8] Fixed a Regression which occurred due to Multi Collision Handling: The method used to provide duplicate results, which is why I commented it out. This lead to corner-cases not colliding at all anymore, thus I added a unit-test and removed the commented code and instead made addCollision de-duplicate entries. --- .../picking/BresenhamTerrainPicker.java | 79 +++++++++++-------- .../collision/TerrainCollisionTest.java | 19 ++++- 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java index b95ecde5d3..5adf4f2bd6 100644 --- a/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java +++ b/jme3-terrain/src/main/java/com/jme3/terrain/geomipmap/picking/BresenhamTerrainPicker.java @@ -117,19 +117,21 @@ public int getTerrainIntersection(Ray worldPick, CollisionResults results) { // because otherwise, we could always return 1 here. if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { if (distance <= worldPick.getLimit()) { - addCollision(results, patch, intersection, hit, distance); + if (addCollision(results, patch, intersection, hit, distance)) { + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } + } // else return 0; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + if (addCollision(results, patch, intersection, hit, distance)) { if (!multipleCollisions) { return 1; } else { numCollisions++; } - } // else return 0; // < this is the old behavior, since the code checked for the range afterwards. - } else { // unlimited range - addCollision(results, patch, intersection, hit, distance); - if (!multipleCollisions) { - return 1; - } else { - numCollisions++; } } } // else no collision @@ -146,30 +148,25 @@ public int getTerrainIntersection(Ray worldPick, CollisionResults results) { if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { if (distance <= worldPick.getLimit()) { - addCollision(results, patch, intersection, hit, distance); + if (addCollision(results, patch, intersection, hit, distance)) { + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } + }// else return 0; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + if (addCollision(results, patch, intersection, hit, distance)) { if (!multipleCollisions) { return 1; } else { numCollisions++; } - }// else return 0; // < this is the old behavior, since the code checked for the range afterwards. - } else { // unlimited range - addCollision(results, patch, intersection, hit, distance); - if (!multipleCollisions) { - return 1; - } else { - numCollisions++; } } } - - /** - * Commented out by MeFisto94 during refactoring because due to multiCollision we step through the - * terrain and will reach this offset spot anyway. Leaving this method in just gives duplicates. - * It has to be tested what happens at borders, where there would be no next iteration. I suspect - * there the code would fail at checkTriangles anyway, because they would be out of bounds. - */ - /* + // because of how we get our height coords, we will // sometimes be off by a grid spot, so we check the next // grid space up. @@ -194,22 +191,24 @@ public int getTerrainIntersection(Ray worldPick, CollisionResults results) { if (worldPick.getLimit() < Float.POSITIVE_INFINITY) { if (distance <= worldPick.getLimit()) { - addCollision(results, patch, intersection, hit.getNormal(), distance); + if (addCollision(results, patch, intersection, hit, distance)) { + if (!multipleCollisions) { + return 1; + } else { + numCollisions++; + } + } + } // else return null; // < this is the old behavior, since the code checked for the range afterwards. + } else { // unlimited range + if (addCollision(results, patch, intersection, hit, distance)) { if (!multipleCollisions) { return 1; } else { numCollisions++; } - } // else return null; // < this is the old behavior, since the code checked for the range afterwards. - } else { // unlimited range - addCollision(results, patch, intersection, hit.getNormal(), distance); - if (!multipleCollisions) { - return 1; - } else { - numCollisions++; } } - }*/ + } tracer.next(); } @@ -226,13 +225,25 @@ public int getTerrainIntersection(Ray worldPick, CollisionResults results) { * @param intersection The actual intersection position * @param hit The hit triangle * @param distance The distance at which the hit occurred + * @return Whether the collision was accepted to the list or whether it has been deduplicated */ - private void addCollision(CollisionResults results, TerrainPatch patch, Vector3f intersection, Triangle hit, float distance) { + private boolean addCollision(CollisionResults results, TerrainPatch patch, Vector3f intersection, Triangle hit, float distance) { CollisionResult cr = new CollisionResult(intersection.clone(), distance); cr.setGeometry(patch); cr.setContactNormal(hit.getNormal()); cr.setTriangleIndex(hit.getIndex()); // this will probably always be 0 + + for (int i = 0; i < results.size(); i++) { + CollisionResult compare = results.getCollision(i); + if (compare.getDistance() == cr.getDistance() && compare.getGeometry() == cr.getGeometry() && + compare.getContactPoint().equals(cr.getContactPoint()) && + compare.getContactNormal().equals(cr.getContactNormal())) { + return false; // Collision already available, deduplicate. + } + } + results.addCollision(cr); + return true; } protected boolean checkTriangles(float gridX, float gridY, Ray pick, Vector3f intersection, TerrainPatch patch, Triangle store) { diff --git a/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java b/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java index 68e4b4923f..32768f4060 100644 --- a/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java +++ b/jme3-terrain/src/test/java/com/jme3/terrain/collision/TerrainCollisionTest.java @@ -1,8 +1,6 @@ package com.jme3.terrain.collision; -import com.jme3.collision.CollisionResult; import com.jme3.collision.CollisionResults; -import com.jme3.math.FastMath; import com.jme3.math.Ray; import com.jme3.math.Vector3f; import com.jme3.terrain.geomipmap.TerrainQuad; @@ -66,11 +64,24 @@ public void testMultiCollision() { long l = System.nanoTime(); int cw = quad.collideWith(r, cr); System.out.println((System.nanoTime() - l) + " ns"); - System.out.println(cr.size()); - System.out.println(cr); Assert.assertEquals(6, cw); Assert.assertEquals(6, cr.size()); } + @Test + public void testPreventRegression() { + // This test is as the multi collision changes lead to a regression where sometimes a collision was ignored + // Ray parameters obtained by using TerrainTestCollision (manual inspection of a feasible ray and commenting out setLocalScale(2)) + Ray r = new Ray(new Vector3f(101.61858f, 78.35965f, 17.645157f), new Vector3f(-0.4188528f, -0.56462675f, 0.71116734f)); + + CollisionResults cr = new CollisionResults(); + quad.collideWith(r, cr); + + Assert.assertEquals(3, cr.size()); + Assert.assertEquals(68.1499f, cr.getClosestCollision().getDistance(), 0.01f); + Assert.assertEquals(new Vector3f(73.07381f, 39.88039f, 66.11114f), cr.getClosestCollision().getContactPoint()); + Assert.assertEquals(new Vector3f(0.9103665f, 0.33104235f, -0.24828176f), cr.getClosestCollision().getContactNormal()); + } + }