Repository: adamlwgriffiths/Pyrr

Issue:
Inconsitent Position / Distance to plane
Thank you for this useful Library.
There is some unexpected behavior for planes concerning the sign of the distance:
Take for example the following Plane:
```
plane = pyrr.plane.create_from_position(position=[0, 0, 1], normal=[0, 0, 1])
```
which returns the expected plane: `array([ 0,  0,  1, -1])`. However `pyrr.plane.position(plane)` returns `array([ 0,  0, -1])`, instead of `[0, 0, 1]`
 A related issue can be observed when calculating the distance of a point calculated to lie on the plane, which should therefore be 0:
```pyrr.geometric_tests.point_height_above_plane(pyrr.geometric_tests.point_closest_point_on_plane([0, 0, 1], plane), plane)```
but returns -2






Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9215a38..624dd4e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [0.10.3] - 2019-04-19
+
+- Fix some of the plane.create_(xz,xy,yz) not inverting correctly.
+- Fix incorrect distance in point_height_above_plane.
+
 ## [0.10.2] - 2019-03-13
 
 - Add MANIFEST.in and add /tests and /docs to source dist (#90)
diff --git a/pyrr/plane.py b/pyrr/plane.py
index 97e5b98..12d06e0 100755
--- a/pyrr/plane.py
+++ b/pyrr/plane.py
@@ -5,7 +5,8 @@ Planes are represented using a numpy.array of shape (4,).
 The values represent the plane equation using the values A,B,C,D.
 
 The first three values are the normal vector.
-The fourth value is the distance of the plane from the origin, down the normal.
+The fourth value is the distance of the origin from the plane, down the normal.
+A negative value indicates the origin is behind the plane, relative to the normal.
 
 .. seealso: http://en.wikipedia.org/wiki/Plane_(geometry)
 .. seealso: http://mathworld.wolfram.com/Plane.html
@@ -17,12 +18,15 @@ from .utils import all_parameters_as_numpy_arrays, parameters_as_numpy_arrays
 
 
 def create(normal=None, distance=0.0, dtype=None):
-    """Creates a plane that runs along the X,Y plane.
+    """Creates a plane oriented toward the normal, at distance below the origin.
+    If no normal is provided, the plane will by created at the origin with a normal
+    of [0, 0, 1].
 
-    It crosses the origin with a normal of 0,0,1 (+Z).
+    Negative distance indicates the plane is facing toward the origin.
 
     :rtype: numpy.array
-    :return: A plane that runs along the X,Y plane.
+    :return: A plane with the specified normal at a distance from the origin of
+    -distance.
     """
     if normal is None:
         normal = [0.0, 0.0, 1.0]
@@ -75,37 +79,43 @@ def create_from_position(position, normal, dtype=None):
         normal.
     """
     dtype = dtype or position.dtype
-    # -d = a * px  + b * py + c * pz
+    # -d = a * x  + b * y + c * z
     n = vector.normalize(normal)
     d = -np.sum(n * position)
-    return create(n, d, dtype)
+    return create(n, -d, dtype)
 
 def create_xy(invert=False, distance=0., dtype=None):
     """Create a plane on the XY plane, starting at the origin with +Z being
     the up vector.
 
-    The distance is the distance along the normal (-Z if inverted, otherwise +Z).
+    The plane is distance units along the Z axis. -Z if inverted.
     """
-    invert = -1. if invert else 1.
-    return np.array([0., 0., 1. * invert, distance])
+    pl = np.array([0., 0., 1., distance])
+    if invert:
+        pl = invert_normal(pl)
+    return pl
 
 def create_xz(invert=False, distance=0., dtype=None):
     """Create a plane on the XZ plane, starting at the origin with +Y being
     the up vector.
 
-    The distance is the distance along the normal (-Y if inverted, otherwise +Y).
+    The plane is distance units along the Y axis. -Y if inverted.
     """
-    invert = -1. if invert else 1.
-    return np.array([0., 1. * invert, 0., distance])
+    pl = np.array([0., 1., 0., distance])
+    if invert:
+        pl = invert_normal(pl)
+    return pl
 
 def create_yz(invert=False, distance=0., dtype=None):
     """Create a plane on the YZ plane, starting at the origin with +X being
     the up vector.
 
-    The distance is the distance along the normal (-X if inverted, otherwise +X).
+    The plane is distance units along the X axis. -X if inverted.
     """
-    invert = -1. if invert else 1.
-    return np.array([1. * invert, 0., 0., distance])
+    pl = np.array([1., 0., 0., distance])
+    if invert:
+        pl = invert_normal(pl)
+    return pl
 
 def invert_normal(plane):
     """Flips the normal of the plane.
@@ -127,7 +137,7 @@ def position(plane):
     :rtype: numpy.array
     :return: A valid position that lies on the plane.
     """
-    return plane[:3] * plane[3]
+    return normal(plane) * distance(plane)
 
 def normal(plane):
     """Extracts the normal vector from a plane.
@@ -137,3 +147,10 @@ def normal(plane):
     :return: The normal vector of the plane.
     """
     return plane[:3].copy()
+
+def distance(plane):
+    """Distance the plane is from the origin along its the normal.
+
+    Negative value indicates the plane is facing the origin.
+    """
+    return plane[3]
diff --git a/pyrr/version.py b/pyrr/version.py
index cb94a70..a76b028 100755
--- a/pyrr/version.py
+++ b/pyrr/version.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
 # the version of software
 # this is used by the setup.py script
-__version__ = '0.10.2'
+__version__ = '0.10.3'
+


Test patch:
diff --git a/pyrr/geometric_tests.py b/pyrr/geometric_tests.py
index 2f60c2e..601e8f6 100755
--- a/pyrr/geometric_tests.py
+++ b/pyrr/geometric_tests.py
@@ -5,7 +5,7 @@ various forms data types.
 from __future__ import absolute_import, division, print_function
 import math
 import numpy as np
-from . import rectangle, vector, vector3
+from . import rectangle, vector, vector3, plane
 from .utils import all_parameters_as_numpy_arrays, parameters_as_numpy_arrays, solve_quadratic_equation
 
 """
@@ -75,12 +75,12 @@ def point_intersect_rectangle(point, rect):
         return None
     return point
 
-@parameters_as_numpy_arrays('ray', 'plane')
-def ray_intersect_plane(ray, plane, front_only=False):
+@parameters_as_numpy_arrays('ray', 'pl')
+def ray_intersect_plane(ray, pl, front_only=False):
     """Calculates the intersection point of a ray and a plane.
 
     :param numpy.array ray: The ray to test for intersection.
-    :param numpy.array plane: The ray to test for intersection.
+    :param numpy.array pl: The plane to test for intersection.
     :param boolean front_only: Specifies if the ray should
     only hit the front of the plane.
     Collisions from the rear of the plane will be
@@ -103,8 +103,8 @@ def ray_intersect_plane(ray, plane, front_only=False):
     if rd.n == 0, the ray is parallel to the
     plane.
     """
-    p = plane[:3] * plane[3]
-    n = plane[:3]
+    p = plane.position(pl)
+    n = plane.normal(pl)
     rd_n = vector.dot(ray[1], n)
 
     if rd_n == 0.0:
@@ -306,7 +306,7 @@ def ray_intersect_aabb(ray, aabb):
     return point
 
 @all_parameters_as_numpy_arrays
-def point_height_above_plane(point, plane):
+def point_height_above_plane(point, pl):
     """Calculates how high a point is above a plane.
 
     :param numpy.array point: The point to check.
@@ -316,17 +316,17 @@ def point_height_above_plane(point, plane):
         negative if the point is behind the plane.
     """
     """
-    http://www.vitutor.com/geometry/distance/point_plane.html
-    d(P) = (AX + BY + CZ + D) / sqrt(A^2 + B^2 + C^2)
-
-    Normal is unit length, so it's length is 1.0.
-    Therefore, we can ignore the division all together.
-    Just perform Pn . [XYZ1]
+    Because we store normalised normal, we can simply
+    use: n . (p - p0)
+    where:
+        n is the plane normal
+        p is the plane position
+        p0 is the point
     """
-    return np.dot(plane, [point[0], point[1], point[2], 1.0])
+    return vector.dot(plane.normal(pl), point - plane.position(pl))
 
 @all_parameters_as_numpy_arrays
-def point_closest_point_on_plane(point, plane):
+def point_closest_point_on_plane(point, pl):
     """Calculates the point on a plane that is closest to a point.
 
     :param numpy.array point: The point to check with.
@@ -343,8 +343,8 @@ def point_closest_point_on_plane(point, plane):
     d is the value of normal dot position
     n is the plane normal
     """
-    n = plane[:3]
-    p = n * plane[3]
+    n = plane.normal(pl)
+    p = n * plane.distance(pl)
     d = np.dot(p, n)
     qn = np.dot(point, n)
     return point + (n * (d - qn))
diff --git a/tests/test_geometric_tests.py b/tests/test_geometric_tests.py
index c982b12..0d52e3e 100644
--- a/tests/test_geometric_tests.py
+++ b/tests/test_geometric_tests.py
@@ -204,6 +204,15 @@ class test_geometric_tests(unittest.TestCase):
         self.assertEqual(result, None)
 
     def test_point_height_above_plane(self):
+        pl = plane.create([0., 1., 0.], 1.)
+        p = np.array([0., 1., 0.])
+        result = gt.point_height_above_plane(p, pl)
+        self.assertEqual(result, 0.)
+
+        p = np.array([0., 0., 0.])
+        result = gt.point_height_above_plane(p, pl)
+        self.assertEqual(result, -1.)
+
         v1 = np.array([ 0.0, 0.0, 1.0])
         v2 = np.array([ 1.0, 0.0, 1.0])
         v3 = np.array([ 0.0, 1.0, 1.0])
@@ -215,6 +224,11 @@ class test_geometric_tests(unittest.TestCase):
         result = gt.point_height_above_plane(p, pl)
         self.assertEqual(result, 19.)
 
+        pl = plane.create_xz(distance=5.)
+        p = np.array([0., 5., 0.])
+        h = gt.point_height_above_plane(p, pl)
+        self.assertEqual(h, 0.)
+
     def test_point_closest_point_on_plane(self):
         pl = np.array([ 0.0, 1.0, 0.0, 0.0])
         p = np.array([ 5.0, 20.0, 5.0])
@@ -300,4 +314,3 @@ class test_geometric_tests(unittest.TestCase):
 
 if __name__ == '__main__':
     unittest.main()
-
diff --git a/tests/test_plane.py b/tests/test_plane.py
index 79fa8b8..0cb7ba6 100644
--- a/tests/test_plane.py
+++ b/tests/test_plane.py
@@ -3,7 +3,7 @@ try:
 except:
     import unittest
 import numpy as np
-from pyrr import plane
+from pyrr import plane, vector
 
 class test_plane(unittest.TestCase):
     def test_import(self):
@@ -15,18 +15,26 @@ class test_plane(unittest.TestCase):
         result = plane.create()
         self.assertTrue(np.allclose(result, [0,0,1,0]))
 
-    def test_create_2(self):
         result = plane.create([1.,0.,0.], 5.)
         self.assertTrue(np.allclose(result, [1.,0.,0.,5.]))
 
     def test_create_from_points(self):
-        vecs = np.array([
-            [ 1.0, 0.0, 0.0 ],
-            [ 0.0, 1.0, 0.0 ],
-            [ 1.0, 1.0, 0.0 ]
-        ])
-        result = plane.create_from_points(*vecs)
+        result = plane.create_from_points(
+            [1., 0., 0.],
+            [0., 1., 0.],
+            [1., 1., 0.],
+        )
         self.assertTrue(np.allclose(result, [0.,0.,1.,0.]))
+        self.assertTrue(np.allclose(plane.position(result), [0., 0., 0.]))
+
+        result = plane.create_from_points(
+            [1., 1., 0.],
+            [1., 1., 1.],
+            [0., 1., 1.],
+        )
+        expected = plane.create([0.,1.,0.], 1.)
+        self.assertTrue(np.allclose(result, expected))
+        self.assertTrue(np.allclose(plane.position(result), [0., 1., 0.]))
 
     def test_create_from_position(self):
         position = np.array([1.0, 0.0, 0.0])
@@ -34,29 +42,42 @@ class test_plane(unittest.TestCase):
         result = plane.create_from_position(position, normal)
         self.assertTrue(np.allclose(result, [0., 1., 0., 0.]))
 
+        p0 = position + [1., 0., 0.]
+        p = position
+        n = vector.normalise(normal)
+        coplanar = p - p0
+        self.assertEqual(np.sum(n * coplanar), 0.)
+
     def test_create_xy(self):
         result = plane.create_xy()
         self.assertTrue(np.allclose(result, [0., 0., 1., 0.]))
 
-    def test_create_xy_invert_distance(self):
+        result = plane.create_xy(distance=2.)
+        self.assertTrue(np.allclose(result, [0., 0., 1., 2.]))
+
         result = plane.create_xy(invert=True, distance=2.)
-        self.assertTrue(np.allclose(result, [0., 0., -1., 2.]))
+        self.assertTrue(np.allclose(result, [0., 0., -1., -2.]))
 
     def test_create_xz(self):
         result = plane.create_xz()
         self.assertTrue(np.allclose(result, [0., 1., 0., 0.]))
 
-    def test_create_xz_invert_distance(self):
+        result = plane.create_xz(distance=2.)
+        self.assertTrue(np.allclose(result, [0., 1., 0., 2.]))
+
         result = plane.create_xz(invert=True, distance=2.)
-        self.assertTrue(np.allclose(result, [0., -1., 0., 2.]))
+        self.assertTrue(np.allclose(result, [0., -1., 0., -2.]))
 
     def test_create_yz(self):
         result = plane.create_yz()
         self.assertTrue(np.allclose(result, [1., 0., 0., 0.]))
 
-    def test_create_yz_invert_distance(self):
+        result = plane.create_yz(distance=2.)
+        self.assertTrue(np.allclose(result, [1., 0., 0., 2.]))
+
         result = plane.create_yz(invert=True, distance=2.)
-        self.assertTrue(np.allclose(result, [-1., 0., 0., 2.]))
+        print(result)
+        self.assertTrue(np.allclose(result, [-1., 0., 0., -2.]))
 
     def test_invert_normal(self):
         p = np.array([1.0, 0.0, 0.0, 1.0])
@@ -64,9 +85,12 @@ class test_plane(unittest.TestCase):
         self.assertTrue(np.allclose(result, [-1.0, 0.0, 0.0, -1.0]))
 
     def test_position(self):
-        p = plane.create_xz(distance=5.)
+        p = plane.create_xz(distance=-5.)
         result = plane.position(p)
-        self.assertTrue(np.allclose(result, [0.,5.,0.]))
+        self.assertTrue(np.allclose(result, [0.,-5.,0.]))
+
+        p = plane.create_from_position(position=[0., 0., 1.], normal=[0., 0., 1.])
+        self.assertTrue(np.allclose(plane.position(p), [0., 0., 1.]))
 
     def test_normal(self):
         p = plane.create_xz(distance=5.)
@@ -75,4 +99,3 @@ class test_plane(unittest.TestCase):
 
 if __name__ == '__main__':
     unittest.main()
-