commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [commons-geometry] svenrathgeber commented on a change in pull request #31: Geometry 29 2 sven
Date Wed, 03 Apr 2019 17:01:42 GMT
svenrathgeber commented on a change in pull request #31: Geometry 29 2 sven
URL: https://github.com/apache/commons-geometry/pull/31#discussion_r271840951
 
 

 ##########
 File path: commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Plane.java
 ##########
 @@ -16,174 +16,193 @@
  */
 package org.apache.commons.geometry.euclidean.threed;
 
+import java.util.Objects;
+
 import org.apache.commons.geometry.core.exception.IllegalNormException;
 import org.apache.commons.geometry.core.partitioning.Embedding;
 import org.apache.commons.geometry.core.partitioning.Hyperplane;
 import org.apache.commons.geometry.core.precision.DoublePrecisionContext;
-import org.apache.commons.geometry.euclidean.internal.Vectors;
 import org.apache.commons.geometry.euclidean.oned.Vector1D;
 import org.apache.commons.geometry.euclidean.threed.rotation.QuaternionRotation;
 import org.apache.commons.geometry.euclidean.twod.PolygonsSet;
 import org.apache.commons.geometry.euclidean.twod.Vector2D;
 
-/** The class represent planes in a three dimensional space.
+/**
+ * The class represents a plane in a three dimensional space.
  */
-public class Plane implements Hyperplane<Vector3D>, Embedding<Vector3D, Vector2D>
{
-
-    /** Offset of the origin with respect to the plane. */
-    private double originOffset;
+public final class Plane implements Hyperplane<Vector3D>, Embedding<Vector3D, Vector2D>
{
 
-    /** Origin of the plane frame. */
-    private Vector3D origin;
+    /** First normalized vector of the plane frame (in plane). */
+    private final Vector3D u;
 
-    /** First vector of the plane frame (in plane). */
-    private Vector3D u;
+    /** Second normalized vector of the plane frame (in plane). */
+    private final Vector3D v;
 
-    /** Second vector of the plane frame (in plane). */
-    private Vector3D v;
+    /** Normalized plane normal. */
+    private final Vector3D w;
 
-    /** Third vector of the plane frame (plane normal). */
-    private Vector3D w;
+    /** Offset of the origin with respect to the plane. */
+    private final double originOffset;
 
+    /** orthogonal projection of the 3D-space origin in the plane. */
+    private final Vector3D projectedOrigin;
+    
     /** Precision context used to compare floating point numbers. */
     private final DoublePrecisionContext precision;
 
-    /** Build a plane normal to a given direction and containing the origin.
-     * @param normal normal direction to the plane
+    /** 
+     * Constructor to build a new plane with the given values.
+     * Made private to prevent inheritance.
+     * @param u u vector (on plane)
+     * @param v v vector (on plane)
+     * @param w unit normal vector
+     * @param projectedOrigin orthogonal projection of the 3D-space origin in the plane.
      * @param precision precision context used to compare floating point values
-     * @exception IllegalArgumentException if the normal norm is too small
+     * @throws IllegalArgumentException if the provided vectors are coplanar or not normalized
      */
-    public Plane(final Vector3D normal, final DoublePrecisionContext precision)
-        throws IllegalArgumentException {
-        setNormal(normal);
+    private Plane(Vector3D u, Vector3D v, Vector3D w, Vector3D projectedOrigin, double originOffset,
+            DoublePrecisionContext precision) {
+        this.u = u;
+        this.v = v;
+        this.w = w;
+        
+        if (!areVectorsNormalized(u, v, w, precision))
+        {
+            throw new IllegalArgumentException("Provided vectors must be normalized.");
+        }
+        if (Vector3D.areCoplanar(u, v, w, precision))
+        {
+            throw new IllegalArgumentException("Provided vectors must not be coplanar.");
+        }
+        this.projectedOrigin = projectedOrigin;
+        this.originOffset = originOffset;
         this.precision = precision;
-        originOffset = 0;
-        setFrame();
     }
-
-    /** Build a plane from a point and a normal.
-     * @param p point belonging to the plane
-     * @param normal normal direction to the plane
+    
+    /**
+     * Build a plane from a point and two (on plane) vectors.
+     * @param p the provided point (on plane)
+     * @param u u vector (on plane)
+     * @param v v vector (on plane)
      * @param precision precision context used to compare floating point values
-     * @exception IllegalArgumentException if the normal norm is too small
+     * @return a new plane
+     * @throws IllegalNormException if the norm of the given values is zero, NaN, or infinite.
+     * @throws IllegalArgumentException if the provided vectors are collinear 
      */
-    public Plane(final Vector3D p, final Vector3D normal, final DoublePrecisionContext precision)
-        throws IllegalArgumentException {
-        setNormal(normal);
-        this.precision = precision;
-        this.originOffset = -p.dot(w);
-        setFrame();
+    public static Plane fromPointAndPlaneVectors (Vector3D p, final Vector3D u, final Vector3D
v, final DoublePrecisionContext precision)
+    {
+        Vector3D u_norm = u.normalize();
+        Vector3D v_norm = v.normalize();
+        Vector3D w_norm = u_norm.cross(v_norm).normalize();
+        double originOffset = -p.dot(w_norm);
+        Vector3D projectedOrigin = calculateOrigin(w_norm, originOffset);
+        return new Plane(u_norm, v_norm, w_norm, projectedOrigin, originOffset, precision);
     }
-
-    /** Build a plane from three points.
-     * <p>The plane is oriented in the direction of
-     * {@code (p2-p1) ^ (p3-p1)}</p>
-     * @param p1 first point belonging to the plane
-     * @param p2 second point belonging to the plane
-     * @param p3 third point belonging to the plane
+    
+    /**
+     * Build a plane from a normal.
+     * Chooses origin as point on plane. 
+     * @param normal    normal direction to the plane
      * @param precision precision context used to compare floating point values
-     * @exception IllegalArgumentException if the points do not constitute a plane
-     */
-    public Plane(final Vector3D p1, final Vector3D p2, final Vector3D p3, final DoublePrecisionContext
precision)
-        throws IllegalArgumentException {
-        this(p1, p2.subtract(p1).cross(p3.subtract(p1)), precision);
-    }
-
-    /** Copy constructor.
-     * <p>The instance created is completely independent of the original
-     * one. A deep copy is used, none of the underlying object are
-     * shared.</p>
-     * @param plane plane to copy
-     */
-    public Plane(final Plane plane) {
-        originOffset = plane.originOffset;
-        origin       = plane.origin;
-        u            = plane.u;
-        v            = plane.v;
-        w            = plane.w;
-        precision    = plane.precision;
-    }
-
-    /** Copy the instance.
-     * <p>The instance created is completely independant of the original
-     * one. A deep copy is used, none of the underlying objects are
-     * shared (except for immutable objects).</p>
-     * @return a new hyperplane, copy of the instance
+     * @return a new plane
+     * @throws IllegalNormException if the norm of the given values is zero, NaN, or infinite.
      */
-    @Override
-    public Plane copySelf() {
-        return new Plane(this);
+    public static Plane fromNormal(final Vector3D normal, final DoublePrecisionContext precision){
+        return fromPointAndNormal(Vector3D.ZERO, normal, precision);
     }
 
-    /** Reset the instance as if built from a point and a normal.
-     * @param p point belonging to the plane
-     * @param normal normal direction to the plane
-     * @exception IllegalArgumentException if the normal norm is too small
+    /**
+     * Build a plane from a point and a normal.
+     * 
+     * @param p         point belonging to the plane
+     * @param normal    normal direction to the plane
+     * @param precision precision context used to compare floating point values
+     * @return a new plane
+     * @throws IllegalNormException if the norm of the given values is zero, NaN, or infinite.
      */
-    public void reset(final Vector3D p, final Vector3D normal) {
-        setNormal(normal);
-        originOffset = -p.dot(w);
-        setFrame();
+    public static Plane fromPointAndNormal(final Vector3D p, final Vector3D normal, final
DoublePrecisionContext precision) {
+        Vector3D w = normal.normalize();
+        double originOffset = -p.dot(w);
+        Vector3D projectedOrigin = calculateOrigin(w, originOffset);
+        Vector3D u = w.orthogonal();
+        Vector3D v = w.cross(u);
+        return new Plane(u, v, w, projectedOrigin, originOffset, precision);
     }
-
-    /** Reset the instance from another one.
-     * <p>The updated instance is completely independant of the original
-     * one. A deep reset is used none of the underlying object is
-     * shared.</p>
-     * @param original plane to reset from
+    
+    /**
+     * Build a plane from three points.
+     * <p>
+     * The plane is oriented in the direction of {@code (p2-p1) ^ (p3-p1)}
+     * </p>
+     * 
+     * @param p1        first point belonging to the plane
+     * @param p2        second point belonging to the plane
+     * @param p3        third point belonging to the plane
+     * @param precision precision context used to compare floating point values
+     * @return a new plane
+     * @throws IllegalNormException if the points do not constitute a plane
      */
-    public void reset(final Plane original) {
-        originOffset = original.originOffset;
-        origin       = original.origin;
-        u            = original.u;
-        v            = original.v;
-        w            = original.w;
+    public static Plane fromPoints(final Vector3D p1, final Vector3D p2, final Vector3D p3,
+            final DoublePrecisionContext precision) {
+        return Plane.fromPointAndNormal(p1, p2.subtract(p1).cross(p3.subtract(p1)), precision);
 
 Review comment:
   I'm struggeling with this one. Some PolyhedronsSetTest fail, when I make the change ...
I have to investigate this
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message