BoundingBox.merge stores the updates in the current object instead of the store argument. This hasn't caused any problems yet because people must have been always using mergeLocal.
Index: BoundingBox.java
===================================================================
— BoundingBox.java (revision 4131)
+++ BoundingBox.java (working copy)
@@ -478,11 +478,11 @@
if (_compVect2.z < boxCenter.z + boxZ)
_compVect2.z = boxCenter.z + boxZ;
- center.set(_compVect2).addLocal(_compVect1).multLocal(0.5f);
+ rVal.center.set(_compVect2).addLocal(_compVect1).multLocal(0.5f);
- xExtent = _compVect2.x - center.x;
- yExtent = _compVect2.y - center.y;
- zExtent = _compVect2.z - center.z;
+ rVal.xExtent = _compVect2.x - center.x;
+ rVal.yExtent = _compVect2.y - center.y;
+ rVal.zExtent = _compVect2.z - center.z;
return rVal;
}
Thats funny, the rVal parameter isn't used at all just returned …
The JUnit Test com.jme.bounding.BoundsTest uses the merge Method. It doesn't check the return value though.
Could you also improve the test to verify the change?
I'll take a look at the test in a minute…
Also, found something similar and subtle in BoundingVolume.merge, where the center of the original was modified in the process of setting the output center:
Index: BoundingSphere.java
===================================================================
— BoundingSphere.java (revision 4131)
+++ BoundingSphere.java (working copy)
@@ -606,7 +606,7 @@
}
if (length > radiusEpsilon) {
float coeff = (length + radiusDiff) / (2.0f * length);
- rCenter.set(center.addLocal(diff.multLocal(coeff)));
- rCenter.set(center).addLocal(diff.multLocal(coeff));
} else {
rCenter.set(center);
}