[commited] com.jme.math.Line documentation

I have reworked Line documentation following forums thread http://www.jmonkeyengine.com/forum/index.php?topic=12532. No changes to code except for some @Override annotations.



I haven’t documented the ortoghonalLineFit method as I don’t understand it.



I also think that both random() methods are not what they suggest they are. It would be great if someone could have a look at how Line.random() is implemented.



Index: src/com/jme/math/Line.java
===================================================================
--- src/com/jme/math/Line.java   (revision 4715)
+++ src/com/jme/math/Line.java   (working copy)
@@ -44,29 +44,46 @@
 import com.jme.util.geom.BufferUtils;
 
 /**
- * <code>Line</code> defines a line. Where a line is defined as infinite along
- * two points. The two points of the line are defined as the origin and direction.
+ * <p>
+ * <code>Line</code> defines an infinite line.
+ * </p>
+ * <p>
+ * Lines are defined by their origin and direction.
+ * <p>
+ * Direction needs to be normalized at any time (otherwise, the distance()
+ * method is known to fail and other problems may exist).
+ * </p>
  *
  * @author Mark Powell
  * @author Joshua Slack
  */
 public class Line implements Serializable, Savable, Cloneable {
-    //todo: merge with Ray?
+
+    // TODO: merge with Ray?
+
     private static final long serialVersionUID = 1L;
 
+    /**
+     * Line origin (may be any point in the line).
+     */
     private Vector3f origin;
+
+    /**
+     * Line direction.
+     */
     private Vector3f direction;
-    
+
     private static final Vector3f compVec1 = new Vector3f();
-   private static final Vector3f compVec2 = new Vector3f();
-   private static final Matrix3f compMat1 = new Matrix3f();
-   private static final Eigen3f compEigen1 = new Eigen3f();
-   
+    private static final Vector3f compVec2 = new Vector3f();
+    private static final Matrix3f compMat1 = new Matrix3f();
+    private static final Eigen3f compEigen1 = new Eigen3f();
 
     /**
      * Constructor instantiates a new <code>Line</code> object. The origin and
-     * direction are set to defaults (0,0,0).
-     *
+     * direction are set to defaults (0,0,0). Note this is an invalid state, as
+     * direction needs to be normalized.
+     *
+     * @see Line
      */
     public Line() {
         origin = new Vector3f();
@@ -74,10 +91,13 @@
     }
 
     /**
-     * Constructor instantiates a new <code>Line</code> object. The origin
-     * and direction are set via the parameters.
-     * @param origin the origin of the line.
-     * @param direction the direction of the line.
+     * Constructor instantiates a new <code>Line</code> object. The origin and
+     * direction are set via the parameters. Direction needs to be normalized.
+     *
+     * @param origin
+     *            the origin of the line.
+     * @param direction
+     *            the direction of the line.
      */
     public Line(Vector3f origin, Vector3f direction) {
         this.origin = origin;
@@ -85,7 +105,6 @@
     }
 
     /**
-     *
      * <code>getOrigin</code> returns the origin of the line.
      * @return the origin of the line.
      */
@@ -94,17 +113,20 @@
     }
 
     /**
-     *
+     * Sets the origin of the line.
      * <code>setOrigin</code> sets the origin of the line.
-     * @param origin the origin of the line.
+     *
+     * @param origin
+     *            the origin of the line.
      */
     public void setOrigin(Vector3f origin) {
         this.origin = origin;
     }
 
     /**
-     *
+     * Gets the direction of the line.
      * <code>getDirection</code> returns the direction of the line.
+     *
      * @return the direction of the line.
      */
     public Vector3f getDirection() {
@@ -112,78 +134,114 @@
     }
 
     /**
-     *
      * <code>setDirection</code> sets the direction of the line.
-     * @param direction the direction of the line.
+     *
+     * @param direction
+     *            the direction of the line.
      */
     public void setDirection(Vector3f direction) {
         this.direction = direction;
     }
-    
+
+    /**
+     * <p>
+     * Returns the square of the distance from the given point to the closest
+     * point of this line.
+     * </p>
+     * <p>
+     * This method is faster than {@link Line#distance(Vector3f)}, so it's a
+     * good alternative when the absolute distance is not needed.
+     * </p>
+     *
+     * @param point
+     *            the point to calculate the distance to this line
+     * @return the distance squared from the given point to this line
+     */
     public float distanceSquared(Vector3f point) {
-       point.subtract(origin, compVec1);
-       float lineParameter = direction.dot(compVec1);
-       origin.add(direction.mult(lineParameter, compVec2), compVec2);
-       compVec2.subtract(point, compVec1);
-       return compVec1.lengthSquared();
+        point.subtract(origin, compVec1);
+        float lineParameter = direction.dot(compVec1);
+        origin.add(direction.mult(lineParameter, compVec2), compVec2);
+        compVec2.subtract(point, compVec1);
+        return compVec1.lengthSquared();
     }
-    
+
+    /**
+     * <p>
+     * Returns the distance from the given point to the closest point of this
+     * line.
+     * </p>
+     * <p>
+     * When the exact distance is not needed the faster method
+     * {@link Line#distanceSquared(Vector3f)} can be used instead (for example
+     * if comparing or sorting several distances, where using the squared
+     * value is equivalent).
+     * </p>
+     *
+     * @param point
+     *            the point to calculate the distance to this line
+     * @return the distance from the given point to this line
+     */
     public float distance(Vector3f point) {
-       return FastMath.sqrt(distanceSquared(point));
+        return FastMath.sqrt(distanceSquared(point));
     }
+
    
+    /**
+     * @param points
+     */
     public void orthogonalLineFit(FloatBuffer points) {
-      if (points == null) {
-         return;
-      }
+        if (points == null) {
+            return;
+        }
 
-      points.rewind();
+        points.rewind();
 
-      // compute average of points
-      int length = points.remaining() / 3;
-      
-      BufferUtils.populateFromBuffer(origin, points, 0);
-      for (int i = 1; i < length; i++) {
-         BufferUtils.populateFromBuffer(compVec1, points, i);
-         origin.addLocal(compVec1);
-      }
+        // compute average of points
+        int length = points.remaining() / 3;
 
-      origin.multLocal(1f / (float) length);
-      
-      // compute sums of products
-      float sumXX = 0.0f, sumXY = 0.0f, sumXZ = 0.0f;
-      float sumYY = 0.0f, sumYZ = 0.0f, sumZZ = 0.0f;
-      
-      points.rewind();
-      for (int i = 0; i < length; i++) {
-         BufferUtils.populateFromBuffer(compVec1, points, i);
-         compVec1.subtract(origin, compVec2);
-         sumXX += compVec2.x * compVec2.x;
-         sumXY += compVec2.x * compVec2.y;
-         sumXZ += compVec2.x * compVec2.z;
-         sumYY += compVec2.y * compVec2.y;
-         sumYZ += compVec2.y * compVec2.z;
-         sumZZ += compVec2.z * compVec2.z;
-      }
+        BufferUtils.populateFromBuffer(origin, points, 0);
+        for (int i = 1; i < length; i++) {
+            BufferUtils.populateFromBuffer(compVec1, points, i);
+            origin.addLocal(compVec1);
+        }
 
-      //find the smallest eigen vector for the direction vector
-      compMat1.m00 = sumYY + sumZZ;
-      compMat1.m01 = -sumXY;
-      compMat1.m02 = -sumXZ;
-      compMat1.m10 = -sumXY;
-      compMat1.m11 = sumXX + sumZZ;
-      compMat1.m12 = -sumYZ;
-      compMat1.m20 = -sumXZ;
-      compMat1.m21 = -sumYZ;
-      compMat1.m22 = sumXX + sumYY;
-      
-      compEigen1.calculateEigen(compMat1);
-      direction = compEigen1.getEigenVector(0);
-   }
+        origin.multLocal(1f / (float) length);
+
+        // compute sums of products
+        float sumXX = 0.0f, sumXY = 0.0f, sumXZ = 0.0f;
+        float sumYY = 0.0f, sumYZ = 0.0f, sumZZ = 0.0f;
+
+        points.rewind();
+        for (int i = 0; i < length; i++) {
+            BufferUtils.populateFromBuffer(compVec1, points, i);
+            compVec1.subtract(origin, compVec2);
+            sumXX += compVec2.x * compVec2.x;
+            sumXY += compVec2.x * compVec2.y;
+            sumXZ += compVec2.x * compVec2.z;
+            sumYY += compVec2.y * compVec2.y;
+            sumYZ += compVec2.y * compVec2.z;
+            sumZZ += compVec2.z * compVec2.z;
+        }
+
+        // find the smallest eigen vector for the direction vector
+        compMat1.m00 = sumYY + sumZZ;
+        compMat1.m01 = -sumXY;
+        compMat1.m02 = -sumXZ;
+        compMat1.m10 = -sumXY;
+        compMat1.m11 = sumXX + sumZZ;
+        compMat1.m12 = -sumYZ;
+        compMat1.m20 = -sumXZ;
+        compMat1.m21 = -sumYZ;
+        compMat1.m22 = sumXX + sumYY;
+
+        compEigen1.calculateEigen(compMat1);
+        direction = compEigen1.getEigenVector(0);
+    }
 
     /**
-     *
-     * <code>random</code> determines a random point along the line.
+     * <p><code>random</code> determines a random point along the line.</p>
+     * <p>Note: this implementation may be broken. Please check source code
+     * before using.</p>
      * @return a random point on the line.
      */
     public Vector3f random() {
@@ -191,9 +249,12 @@
     }
 
     /**
-     * <code>random</code> determines a random point along the line.
-     *
-     * @param result Vector to store result in
+     * <p><code>random</code> determines a random point along the line.</p>
+     * <p>Note: this implementation may be broken. Please check source code
+     * before using.</p>
+     * <p>If the passed vector is null, a vector will be created to store
+     * the result of the operation.</p>
+     * @param result Vector to store result in. If null, a new vector will be created and returned.
      * @return a random point on the line.
      */
     public Vector3f random(Vector3f result) {
@@ -209,22 +270,44 @@
         return result;
     }
 
+    /**
+     * Support for JME exporting.
+     * @param e The JME exporter to write to.
+     * @see com.jme.util.export.Savable#write(com.jme.util.export.JMEExporter)
+     */
+    @Override
     public void write(JMEExporter e) throws IOException {
         OutputCapsule capsule = e.getCapsule(this);
         capsule.write(origin, "origin", Vector3f.ZERO);
         capsule.write(direction, "direction", Vector3f.ZERO);
     }
 
+    /**
+     * Support for JME importing.
+     * @param e The JME importer to read from.
+     * @see com.jme.util.export.Savable#read(com.jme.util.export.JMEImporter)
+     */
+    @Override
     public void read(JMEImporter e) throws IOException {
         InputCapsule capsule = e.getCapsule(this);
-        origin = (Vector3f)capsule.readSavable("origin", Vector3f.ZERO.clone());
-        direction = (Vector3f)capsule.readSavable("direction", Vector3f.ZERO.clone());
+        origin = (Vector3f) capsule.readSavable("origin", Vector3f.ZERO.clone());
+        direction = (Vector3f) capsule.readSavable("direction", Vector3f.ZERO.clone());
     }
-    
+
+    /**
+     * Returns class type (for exporting/importing purposes).
+     * @return Class tag.
+     */
+    @Override
     public Class<? extends Line> getClassTag() {
         return this.getClass();
     }
 
+    /**
+     * Provides deep-copy object clonation.
+     * @return A cloned line, with its own copy of direction and origin vectors.
+     * @see java.lang.Object#clone()
+     */
     @Override
     public Line clone() {
         try {
@@ -236,4 +319,5 @@
             throw new AssertionError();
         }
     }
+    
 }

This looks like a big improvement. Thank you for taking the time to work on this.



My only quibble is with "a line is defined as infinite along two points". I think it should just say:





<code>Line</code> defines a line. The line is defined as infinite based on an origin and a direction.





Like I said, a quibble. I think the phrase "infinite along two points" implies that you provide to points and the line is drawn threw them…  Give that the constructor arguments are named "origin" and "direction" I think most people will figure it out.





Thanks again for adding the documentation. This will really help newbies like myself with the library.

Applied perry2of5 suggestion and commited.



Revision 4765: Added documentation for com.jme.math.Line class.

jjmontes said:

... No changes to code except for some @Override annotations.....


@Override on Interface Methods is a Java 1.6 feature.
They should be removed to maintain 1.5 compatibility.


Index: src/com/jme/math/Line.java
===================================================================
--- src/com/jme/math/Line.java   (revision 4766)
+++ src/com/jme/math/Line.java   (working copy)
@@ -274,7 +274,6 @@
      * @param e The JME exporter to write to.
      * @see com.jme.util.export.Savable#write(com.jme.util.export.JMEExporter)
      */
-    @Override
     public void write(JMEExporter e) throws IOException {
         OutputCapsule capsule = e.getCapsule(this);
         capsule.write(origin, "origin", Vector3f.ZERO);
@@ -286,7 +285,6 @@
      * @param e The JME importer to read from.
      * @see com.jme.util.export.Savable#read(com.jme.util.export.JMEImporter)
      */
-    @Override
     public void read(JMEImporter e) throws IOException {
         InputCapsule capsule = e.getCapsule(this);
         origin = (Vector3f) capsule.readSavable("origin", Vector3f.ZERO.clone());
@@ -297,7 +295,6 @@
      * Returns class type (for exporting/importing purposes).
      * @return Class tag.
      */
-    @Override
     public Class<? extends Line> getClassTag() {
         return this.getClass();
     }

[quote author=Core-Dump link=topic=12564.msg93642#msg93642 date=1259480295]
[quote author=jjmontes link=topic=12564.msg93023#msg93023 date=1258308348]
... No changes to code except for some @Override annotations.....
[/quote]

@Override on Interface Methods is a Java 1.6 feature.
They should be removed to maintain 1.5 compatibility.


[/quote]

Oh! Damn! I should have noticed. My absolute apologies for this :( I'll fix this as soon as possible.

Thanks Core!

I see it has already been corrected. Thanks ttrocha and CoreDump.