Skip to content

Commit

Permalink
fix(face): Use a relative tolerance to identify colinear vertices
Browse files Browse the repository at this point in the history
This was able to address the issue here:
https://discourse.ladybug.tools/t/lb-generate-point-grid-for-non-rectangular-surfaces/14386/2

... without breaking any of the existing tests. In fact, it helped me realize that one of the existing tests was not correct.
  • Loading branch information
chriswmackey committed May 6, 2021
1 parent 7362304 commit 1326fe6
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 16 deletions.
27 changes: 16 additions & 11 deletions ladybug_geometry/geometry3d/face.py
Original file line number Diff line number Diff line change
Expand Up @@ -1912,15 +1912,22 @@ def _plane_from_vertices(verts):
verts: The vertices to be used to extract the normal.
"""
try:
x1, y1, z1 = Face3D._normal_from_3pts(verts[-2], verts[-1], verts[0])
x2, y2, z2 = Face3D._normal_from_3pts(verts[-1], verts[0], verts[1])
normal = [x1 + x2, y1 + y2, z1 + z2]
# walk around the whole shape to avoid colinear vertices
# walk around the shape and get cross products and magnitudes
cprod1, d1 = Face3D._normal_from_3pts(verts[-2], verts[-1], verts[0])
cprod2, d2 = Face3D._normal_from_3pts(verts[-1], verts[0], verts[1])
cprods, ds = [cprod1, cprod2], [d1, d2]
for i in range(len(verts) - 2):
x, y, z = Face3D._normal_from_3pts(*verts[i:i + 3])
normal[0] += x
normal[1] += y
normal[2] += z
cprodx, dx = Face3D._normal_from_3pts(*verts[i:i + 3])
cprods.append(cprodx)
ds.append(dx)
# sum together the cross products of any vertices that are not colinear
min_d = (sum(ds) / len(ds)) * 1e-3 # rel tolerance for colinear vertices
normal = [0, 0, 0]
for cprodx, dx in zip(cprods, ds):
if dx > min_d:
normal[0] += cprodx[0] / dx
normal[1] += cprodx[1] / dx
normal[2] += cprodx[2] / dx
except Exception as e:
raise ValueError('Incorrect vertices input for Face3D:\n\t{}'.format(e))
return Plane(Vector3D(normal[0], normal[1], normal[2]), verts[0]) \
Expand All @@ -1944,9 +1951,7 @@ def _normal_from_3pts(pt1, pt2, pt3):
v1[0] * v2[1] - v1[1] * v2[0])
# normalize the vector to make it a unit vector
d = math.sqrt(c_prod[0] ** 2 + c_prod[1] ** 2 + c_prod[2] ** 2)
if d < 1e-9: # effectively duplicate vertices in triplet
return (0, 0, 0)
return (c_prod[0] / d, c_prod[1] / d, c_prod[2] / d)
return c_prod, d

@staticmethod
def _corner_pt_verts(corner_pt, verts3d, verts2d):
Expand Down
13 changes: 12 additions & 1 deletion tests/face3d_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,12 +697,23 @@ def test_normal_with_slightly_nonplanar():
geo_dict = json.load(fp)
face_geo = Face3D.from_dict(geo_dict)

assert -0.99 > face_geo.normal.z > -1.01
assert 0.99 < face_geo.normal.z < 1.01
assert face_geo.check_planar(0.000001, False) is False
with pytest.raises(Exception):
face_geo.check_planar(0.000001)


def test_normal_with_colinear_vertices():
"""Test that shapes with colinear vertices have a relatively close normal."""
geo_file = './tests/json/faces_colinear_verts.json'
with open(geo_file, 'r') as fp:
geo_dict = json.load(fp)
face_geos = [Face3D.from_dict(geo) for geo in geo_dict]

assert -0.01 < face_geos[0].normal.z < 0.01
assert -0.01 < face_geos[1].normal.z < 0.01


def test_flip():
"""Test the flip method of Face3D."""
pts_1 = (Point3D(0, 0, 2), Point3D(2, 0, 2), Point3D(2, 2, 2), Point3D(0, 2, 2))
Expand Down
87 changes: 87 additions & 0 deletions tests/json/faces_colinear_verts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
[
{
"type": "Face3D",
"boundary": [
[
729649.966818291,
4372019.5732027004,
0.0
],
[
729649.966818291,
4372019.5732027004,
18.52
],
[
729647.81681538199,
4372019.6330983499,
18.52
],
[
729647.81681538199,
4372019.6330983499,
14.889999999999997
],
[
729645.33681198698,
4372019.7029779796,
14.890000000000001
],
[
729645.33681198698,
4372019.7029779796,
11.26
],
[
729645.33681198698,
4372019.7029779796,
0.0
],
[
729647.81681538199,
4372019.6330983499,
0.0
]
]
},
{
"type": "Face3D",
"boundary": [
[
729662.13598920987,
4372026.3537933398,
0.0
],
[
729662.13598920987,
4372026.3537933398,
11.709999999999997
],
[
729661.96410812088,
4372023.9037849903,
11.709999999999997
],
[
729661.96410812088,
4372023.9037849903,
9.1399999999999988
],
[
729661.64632798498,
4372019.3737695599,
9.1400000000000006
],
[
729661.64632798498,
4372019.3737695599,
0.0
],
[
729661.96410812088,
4372023.9037849903,
0.0
]
]
}
]
9 changes: 5 additions & 4 deletions tests/plane_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@


def test_plane_init():
"""Test the initalization of Plane objects and basic properties."""
"""Test the initialization of Plane objects and basic properties."""
pt = Point3D(2, 0, 2)
vec = Vector3D(0, 2, 0)
plane = Plane(vec, pt)
str(plane) # test the string representation
hash(plane)

assert plane.o == Point3D(2, 0, 2)
assert plane.n == Vector3D(0, 1, 0)
Expand Down Expand Up @@ -57,7 +58,7 @@ def test_equality():


def test_plane_to_from_dict():
"""Test the initalization of Plane objects and basic properties."""
"""Test the initialization of Plane objects and basic properties."""
pt = Point3D(2, 0, 2)
vec = Vector3D(0, 2, 0)
plane = Plane(vec, pt)
Expand All @@ -68,7 +69,7 @@ def test_plane_to_from_dict():


def test_init_from_three_points():
"""Test the initalization of Plane from end points."""
"""Test the initialization of Plane from end points."""
plane = Plane.from_three_points(Point3D(0, 0, 2), Point3D(0, 2, 2),
Point3D(2, 2, 2))
assert plane.o == Point3D(0, 0, 2)
Expand All @@ -87,7 +88,7 @@ def test_init_from_three_points():


def test_init_from_normal_k():
"""Test the initalization of Plane from end points."""
"""Test the initialization of Plane from end points."""
plane = Plane.from_normal_k(Vector3D(0, 0, 1), 2)
assert plane.o == Point3D(0, 0, 2)
assert plane.n == Vector3D(0, 0, 1)
Expand Down

0 comments on commit 1326fe6

Please sign in to comment.