diff --git a/src/bonsai/bonsai/bim/module/qto/calculator.py b/src/bonsai/bonsai/bim/module/qto/calculator.py index 63b55c0166..d30dac1f7c 100644 --- a/src/bonsai/bonsai/bim/module/qto/calculator.py +++ b/src/bonsai/bonsai/bim/module/qto/calculator.py @@ -451,19 +451,24 @@ def get_net_ceiling_area(obj: bpy.types.Object) -> float: return total_net_ceiling_area -def get_space_net_volume(obj: bpy.types.Object) -> float: +def get_space_net_volume(obj: bpy.types.Object) -> Union[float, None]: decompositions = get_obj_decompositions(obj) if not decompositions: return get_gross_volume(obj) total_space_net_volume = get_gross_volume(obj) + if total_space_net_volume is None: + return None for decomposition in decompositions: decomposition_type = decomposition.get_info()["type"] if decomposition_type == "IfcWall" or decomposition_type == "IfcColumn": decomposition_obj = tool.Ifc.get_object(decomposition) assert isinstance(decomposition_obj, bpy.types.Object) - total_space_net_volume -= get_net_volume(decomposition_obj) + decomposition_net_volume = get_net_volume(decomposition_obj) + if decomposition_net_volume is None: + return None + total_space_net_volume -= decomposition_net_volume return total_space_net_volume @@ -578,16 +583,32 @@ def is_polygon_in_vg(polygon: bpy.types.MeshPolygon, vertices_in_vg: list[int]) return True -def get_net_volume(o: bpy.types.Object) -> float: +def is_manifold(bm: bmesh.types.BMesh) -> bool: + """Checks whether a bmesh is a closed, consistently oriented manifold. + + calc_volume assumes a watertight mesh with matching face winding across + every edge. An open mesh or one with inconsistent winding gives a + meaningless result, see https://github.com/IfcOpenShell/IfcOpenShell/issues/6125. + + :param bm: A bmesh instance. + :return: ``True`` if every edge is shared by exactly two faces with matching winding. + """ + return all(edge.is_contiguous for edge in bm.edges) + + +def get_net_volume(o: bpy.types.Object) -> Union[float, None]: assert isinstance(o.data, bpy.types.Mesh) o_mesh = bmesh.new() o_mesh.from_mesh(o.data) + if not is_manifold(o_mesh): + o_mesh.free() + return None volume = o_mesh.calc_volume() o_mesh.free() return volume -def get_gross_volume(o: bpy.types.Object) -> float: +def get_gross_volume(o: bpy.types.Object) -> Union[float, None]: if not has_openings(o): return get_net_volume(o) @@ -596,6 +617,11 @@ def get_gross_volume(o: bpy.types.Object) -> float: mesh = get_gross_element_mesh(element) bm = get_bmesh_from_mesh(mesh) + if not is_manifold(bm): + bm.free() + delete_mesh(mesh) + return None + gross_volume = bm.calc_volume() bm.free() @@ -634,6 +660,8 @@ def get_gross_weight(obj: bpy.types.Object) -> Union[float, None]: return gross_volume = get_gross_volume(obj) + if gross_volume is None: + return None gross_weight = obj_mass_density * gross_volume return gross_weight @@ -656,6 +684,8 @@ def get_net_weight(obj: bpy.types.Object) -> Union[float, None]: return net_volume = get_net_volume(obj) + if net_volume is None: + return None net_weight = obj_mass_density * net_volume return net_weight diff --git a/src/bonsai/test/tool/test_qto.py b/src/bonsai/test/tool/test_qto.py index 9ec7dc4efd..846491faf0 100644 --- a/src/bonsai/test/tool/test_qto.py +++ b/src/bonsai/test/tool/test_qto.py @@ -181,6 +181,79 @@ class TestGetCalculatedObjectQuantities(test.bim.bootstrap.NewFile): assert quantities["NetVolume"] == 282.517 +class TestGetCalculatedObjectQuantitiesNonManifold(test.bim.bootstrap.NewFile): + """Regression test for #6125: a non-manifold mesh must not produce a bogus volume.""" + + def test_run(self): + import bmesh + import ifc5d.qto + + import bonsai.core.root + + self.ifc = ifcopenshell.file() + tool.Ifc.set(self.ifc) + ifcopenshell.api.root.create_entity(self.ifc, ifc_class="IfcProject", name="My Project") + import logging + + import bonsai.bim.import_ifc as import_ifc + + ifc_import_settings = import_ifc.IfcImportSettings.factory( + bpy.context, tool.Ifc.get_path(), logging.getLogger("ImportIFC") + ) + ifc_importer = import_ifc.IfcImporter(ifc_import_settings) + ifc_importer.file = self.ifc + ifc_importer.create_project() + + context = ifcopenshell.api.context.add_context(self.ifc, context_type="Model") + + bpy.ops.mesh.primitive_cube_add(location=(0.0, 0.0, 0.0), size=2) + obj = bpy.context.active_object + element = bonsai.core.root.assign_class( + tool.Ifc, + tool.Collector, + tool.Root, + obj=obj, + ifc_class="IfcWall", + predefined_type="ELEMENTEDWALL", + context=context, + ) + + # Corrupt the mesh into an open (non-watertight) shape after the IFC + # representation is assigned, simulating a badly authored import + # rather than something Bonsai's own authoring tools would produce. + bm = bmesh.new() + bm.from_mesh(obj.data) + bm.faces.ensure_lookup_table() + bmesh.ops.delete(bm, geom=[bm.faces[0]], context="FACES") + bm.to_mesh(obj.data) + bm.free() + + rules = { + "calculators": { + "Blender": { + "IfcWall": { + "Qto_WallBaseQuantities": { + "GrossFootprintArea": "get_gross_footprint_area", + "GrossVolume": "get_gross_volume", + "NetVolume": "get_net_volume", + } + }, + } + } + } + + ifc_file = tool.Ifc.get() + results = ifc5d.qto.quantify(ifc_file, {element}, rules) + quantities = results[element]["Qto_WallBaseQuantities"] + + # Topology-independent quantities are unaffected. + assert quantities["GrossFootprintArea"] == 4 + # Volume is undefined for a non-manifold mesh, so it is skipped rather + # than reporting a wrong number. + assert "GrossVolume" not in quantities + assert "NetVolume" not in quantities + + class TestGetBaseQto(test.bim.bootstrap.NewFile): def test_run(self): ifc = ifcopenshell.file() diff --git a/src/ifcopenshell-python/ifcopenshell/util/shape.py b/src/ifcopenshell-python/ifcopenshell/util/shape.py index 442301c237..753d1b424f 100644 --- a/src/ifcopenshell-python/ifcopenshell/util/shape.py +++ b/src/ifcopenshell-python/ifcopenshell/util/shape.py @@ -68,14 +68,43 @@ def is_x(value: float, x: float, tolerance: Optional[float] = None) -> bool: return abs(x - value) < tolerance +def is_manifold(geometry: W.Triangulation) -> bool: + """Checks whether a triangulated geometry is a closed, consistently oriented manifold + + Two conditions are checked for every edge of every triangle: + + - Unoriented use: as an unordered pair of vertices, an edge must be shared + by exactly two triangles. A count of 1 means an open hole or boundary, + a count above 2 means more than two triangles meet at that edge. + - Oriented use: as an ordered pair of vertices, an edge must be used by at + most one triangle. If two triangles use the same ordered edge, their + windings are inconsistent (e.g. a flipped or duplicated face), which + also invalidates volume calculations that rely on consistent winding. + + :param geometry: Geometry output calculated by IfcOpenShell + :return: ``True`` if the geometry is a closed, consistently oriented manifold + """ + faces = geometry.faces + directed_use: dict[tuple[int, int], int] = {} + undirected_use: dict[tuple[int, int], int] = {} + for i in range(0, len(faces), 3): + tri = (faces[i], faces[i + 1], faces[i + 2]) + for a, b in ((tri[0], tri[1]), (tri[1], tri[2]), (tri[2], tri[0])): + directed_use[(a, b)] = directed_use.get((a, b), 0) + 1 + edge = (a, b) if a < b else (b, a) + undirected_use[edge] = undirected_use.get(edge, 0) + 1 + return all(count == 2 for count in undirected_use.values()) and all(count == 1 for count in directed_use.values()) + + def get_volume(geometry: W.Triangulation) -> float: """Calculates the total internal volume of a geometry The volume is derived from the divergence theorem (summing signed - tetrahedra), which is only meaningful for a closed manifold (watertight) - mesh. For non-manifold or open geometry that value is undefined and can be - wildly over- or under-estimated, so ``float("nan")`` is returned instead of - a bogus number. See https://github.com/IfcOpenShell/IfcOpenShell/issues/6125. + tetrahedra), which is only meaningful for a closed, consistently oriented + manifold (watertight) mesh. For non-manifold or open geometry that value + is undefined and can be wildly over- or under-estimated, so + ``float("nan")`` is returned instead of a bogus number. See + https://github.com/IfcOpenShell/IfcOpenShell/issues/6125. :param geometry: Geometry output calculated by IfcOpenShell :return: The volume in m3, or ``nan`` if the mesh is not a closed manifold @@ -91,22 +120,12 @@ def get_volume(geometry: W.Triangulation) -> float: v123 = p1[0] * p2[1] * p3[2] return (1.0 / 6.0) * (-v321 + v231 + v312 - v132 - v213 + v123) + if not is_manifold(geometry): + return float("nan") + # Can't optimize it using buffers - performance seems to get only worse. verts = geometry.verts faces = geometry.faces - - # A watertight (closed manifold) mesh shares every edge between exactly two - # triangles. If that does not hold the signed-tetrahedra sum below is - # meaningless, so bail out with nan rather than returning a wild value. - edge_face_count: dict[tuple[int, int], int] = {} - for i in range(0, len(faces), 3): - tri = (faces[i], faces[i + 1], faces[i + 2]) - for a, b in ((tri[0], tri[1]), (tri[1], tri[2]), (tri[2], tri[0])): - edge = (a, b) if a < b else (b, a) - edge_face_count[edge] = edge_face_count.get(edge, 0) + 1 - if any(count != 2 for count in edge_face_count.values()): - return float("nan") - grouped_verts = [[verts[i], verts[i + 1], verts[i + 2]] for i in range(0, len(verts), 3)] volumes = [ signed_triangle_volume(grouped_verts[faces[i]], grouped_verts[faces[i + 1]], grouped_verts[faces[i + 2]]) diff --git a/src/ifcopenshell-python/test/util/test_shape.py b/src/ifcopenshell-python/test/util/test_shape.py new file mode 100644 index 0000000000..df80ad5613 --- /dev/null +++ b/src/ifcopenshell-python/test/util/test_shape.py @@ -0,0 +1,104 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2026 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . +# This file was generated with the assistance of an AI coding tool. + +import math + +import ifcopenshell.util.shape as subject + + +class FakeTriangulation: + """A minimal stand-in for W.Triangulation, exposing only what get_volume/is_manifold use.""" + + def __init__(self, verts: list[tuple[float, float, float]], faces: list[tuple[int, int, int]]): + self.verts = [c for v in verts for c in v] + self.faces = [i for tri in faces for i in tri] + + +def cube(size: float = 1.0) -> tuple[list[tuple[float, float, float]], list[tuple[int, int, int]]]: + s = size + verts = [ + (0, 0, 0), + (s, 0, 0), + (s, s, 0), + (0, s, 0), + (0, 0, s), + (s, 0, s), + (s, s, s), + (0, s, s), + ] + # Consistently wound (outward normals) triangulated cube. + faces = [ + (0, 2, 1), + (0, 3, 2), + (4, 5, 6), + (4, 6, 7), + (0, 1, 5), + (0, 5, 4), + (3, 7, 6), + (3, 6, 2), + (0, 4, 7), + (0, 7, 3), + (1, 2, 6), + (1, 6, 5), + ] + return verts, faces + + +class TestIsManifold: + def test_closed_consistently_wound_mesh_is_manifold(self): + verts, faces = cube() + assert subject.is_manifold(FakeTriangulation(verts, faces)) is True + + def test_open_mesh_is_not_manifold(self): + verts, faces = cube() + # Remove one face, leaving an open boundary. + geometry = FakeTriangulation(verts, faces[:-1]) + assert subject.is_manifold(geometry) is False + + def test_inconsistent_winding_is_not_manifold(self): + # A single flipped triangle keeps every edge shared by exactly two + # triangles (an unordered edge-count check alone would miss this), + # but two faces now use the same directed edge. + verts, faces = cube() + faces = list(faces) + i = faces.index((1, 2, 6)) + faces[i] = (1, 6, 2) + geometry = FakeTriangulation(verts, faces) + assert subject.is_manifold(geometry) is False + + +class TestGetVolume: + def test_manifold_cube_volume(self): + verts, faces = cube(size=2) + geometry = FakeTriangulation(verts, faces) + assert math.isclose(subject.get_volume(geometry), 8.0, rel_tol=1e-9) + + def test_open_mesh_returns_nan(self): + verts, faces = cube() + geometry = FakeTriangulation(verts, faces[:-1]) + assert math.isnan(subject.get_volume(geometry)) + + def test_inconsistent_winding_returns_nan_instead_of_wrong_value(self): + verts, faces = cube() + faces = list(faces) + i = faces.index((1, 2, 6)) + faces[i] = (1, 6, 2) + geometry = FakeTriangulation(verts, faces) + # Without the manifold guard this silently returns 0.667 instead of 1.0. + assert math.isnan(subject.get_volume(geometry))