From 260a387069f9fabcc4f35db88b8027b2bfaf6679 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 18 Jun 2026 17:13:03 +0200 Subject: [PATCH 1/8] Fix loading project library without IfcProject Per IFC4+, IfcContext is the abstract supertype of IfcProject and IfcProjectLibrary; library-only files legitimately contain only IfcProjectLibrary as their root context. Bonsai assumed an IfcProject was always present at three crash sites: the parent-library enum (reported in #8183), RefreshLibrary's tree view, and AddProjectLibrary. Introduce tool.Project.get_root_context() that prefers IfcProject and falls back to IfcProjectLibrary, and route the three sites through it. get_parent_library() now returns None for a root IfcProjectLibrary; get_project_hierarchy() and the EditProjectLibrary parent-swap branch handle that. AddProjectLibrary creates the nested sub-library via IfcRelNests when the root is an IfcProjectLibrary, matching the existing convention for library-under-library nesting. For the separate "Open IFC Project" path, abort with a friendly error pointing users to Project Setup -> Project Library -> Select Library File instead of letting set_units() crash deep in the importer. Closes #8183. Partly generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/project/data.py | 4 +- .../bonsai/bim/module/project/operator.py | 24 +++- src/bonsai/bonsai/bim/module/project/prop.py | 3 +- src/bonsai/bonsai/tool/project.py | 30 ++++- .../project/test_project_library_data.py | 121 ++++++++++++++++++ 5 files changed, 170 insertions(+), 12 deletions(-) create mode 100644 src/bonsai/test/bim/module/project/test_project_library_data.py diff --git a/src/bonsai/bonsai/bim/module/project/data.py b/src/bonsai/bonsai/bim/module/project/data.py index 8ecf30eef5..db64041a89 100644 --- a/src/bonsai/bonsai/bim/module/project/data.py +++ b/src/bonsai/bonsai/bim/module/project/data.py @@ -162,8 +162,8 @@ class ProjectLibraryData: library_file = IfcStore.library_file if library_file is None or library_file.schema == "IFC2X3": return results - project = library_file.by_type("IfcProject")[0] - results.append((str(project.id()), f"IfcProject {project.Name or 'Unnamed'}", project.Description or "")) + root = tool.Project.get_root_context(library_file) + results.append((str(root.id()), f"{root.is_a()} {root.Name or 'Unnamed'}", root.Description or "")) for library_id, data in cls.data["project_libraries"].items(): results.append((str(library_id), data["Name"] or "Unnamed", data["Description"] or "")) return results diff --git a/src/bonsai/bonsai/bim/module/project/operator.py b/src/bonsai/bonsai/bim/module/project/operator.py index 31b1248785..dbe799b857 100644 --- a/src/bonsai/bonsai/bim/module/project/operator.py +++ b/src/bonsai/bonsai/bim/module/project/operator.py @@ -281,9 +281,9 @@ class RefreshLibrary(bpy.types.Operator): elements = {e for e in elements if not tool.Project.is_element_assigned_to_project_library(e, rels)} self.props.add_library_project_library("Unassigned", len(elements), 0, False) - ifc_project = library_file.by_type("IfcProject")[0] + root_context = tool.Project.get_root_context(library_file) hierarchy = tool.Project.get_project_hierarchy(library_file) - tool.Project.load_project_libraries_to_ui(ifc_project, hierarchy) + tool.Project.load_project_libraries_to_ui(root_context, hierarchy) return {"FINISHED"} @@ -763,7 +763,10 @@ class EditProjectLibrary(bpy.types.Operator): previous_parent_library = tool.Project.get_parent_library(project_library) new_parent_library = library_file.by_id(int(props.parent_library)) if previous_parent_library != new_parent_library: - if previous_parent_library.is_a("IfcProject"): + if previous_parent_library is None: + # Edited library was a root in a library-only file; nest it under the new parent. + ifcopenshell.api.nest.assign_object(library_file, [project_library], new_parent_library) + elif previous_parent_library.is_a("IfcProject"): # Then new one is IfcProjectLibrary. ifcopenshell.api.nest.assign_object(library_file, [project_library], new_parent_library) else: # Previous is IfcProjectLibrary. @@ -804,9 +807,12 @@ class AddProjectLibrary(bpy.types.Operator): props = tool.Project.get_project_props() library_file = IfcStore.library_file assert library_file - project = library_file.by_type("IfcProject")[0] + root_context = tool.Project.get_root_context(library_file) project_library = ifcopenshell.api.root.create_entity(library_file, "IfcProjectLibrary") - ifcopenshell.api.project.assign_declaration(library_file, [project_library], project) + if root_context.is_a("IfcProject"): + ifcopenshell.api.project.assign_declaration(library_file, [project_library], root_context) + else: + ifcopenshell.api.nest.assign_object(library_file, [project_library], root_context) ProjectLibraryData.load() # Update enum. props.selected_project_library = str(project_library.id()) props.is_editing_project_library = True @@ -1113,6 +1119,14 @@ class LoadProject(bpy.types.Operator, IFCFileSelector, ImportHelper): f"Error loading IFC file from filepath '{filepath}'. See logs above in the system console for the details.", ) return {"CANCELLED"} + if not tool.Ifc.get().by_type("IfcProject"): + self.report( + {"ERROR"}, + "This file contains no IfcProject. It is likely an IFC project library — " + "load it via Project Setup → Project Library → Select Library File instead.", + ) + IfcStore.purge() + return {"CANCELLED"} props = tool.Project.get_project_props() props.is_loading = True props.total_elements = len(tool.Ifc.get().by_type("IfcElement")) diff --git a/src/bonsai/bonsai/bim/module/project/prop.py b/src/bonsai/bonsai/bim/module/project/prop.py index 53b239ee6c..d7a11a5e05 100644 --- a/src/bonsai/bonsai/bim/module/project/prop.py +++ b/src/bonsai/bonsai/bim/module/project/prop.py @@ -98,7 +98,8 @@ def is_editing_project_library_update(self: "BIMProjectProperties", context: bpy project_library = library_file.by_id(int(self.selected_project_library)) self.project_library_attributes.clear() bonsai.bim.helper.import_attributes(project_library, self.project_library_attributes) - self.parent_library = str(tool.Project.get_parent_library(project_library).id()) + if parent_library := tool.Project.get_parent_library(project_library): + self.parent_library = str(parent_library.id()) ProjectLibraryData.load() # Show edit icon in enum. return diff --git a/src/bonsai/bonsai/tool/project.py b/src/bonsai/bonsai/tool/project.py index 1bfb239726..9edfd5821d 100644 --- a/src/bonsai/bonsai/tool/project.py +++ b/src/bonsai/bonsai/tool/project.py @@ -32,6 +32,7 @@ from typing import ( NotRequired, Optional, TypedDict, + Union, ) import bpy @@ -376,12 +377,31 @@ class Project(bonsai.core.tool.Project): ) @classmethod - def get_parent_library(cls, project_library: ifcopenshell.entity_instance) -> ifcopenshell.entity_instance: + def get_parent_library( + cls, project_library: ifcopenshell.entity_instance + ) -> Union[ifcopenshell.entity_instance, None]: + """Return the IfcContext that declares or nests ``project_library``. + + Returns ``None`` when ``project_library`` is itself the root of a + library-only file (no IfcRelNests, no IfcRelDeclares). + """ if nests := project_library.Nests: - # IfcProjectLibrary. return nests[0].RelatingObject - # IfcProject. - return project_library.HasContext[0].RelatingContext + if has_context := project_library.HasContext: + return has_context[0].RelatingContext + return None + + @classmethod + def get_root_context(cls, ifc_file: ifcopenshell.file) -> ifcopenshell.entity_instance: + """Return the file's root IfcContext. + + Prefers IfcProject if present, otherwise falls back to IfcProjectLibrary — + library-only files are valid per IFC4+ and contain no IfcProject. Caller is + responsible for the IFC2X3 guard; IfcContext does not exist in that schema. + """ + if projects := ifc_file.by_type("IfcProject"): + return projects[0] + return ifc_file.by_type("IfcProjectLibrary")[0] @classmethod def get_project_hierarchy(cls, ifc_file: ifcopenshell.file) -> HiearchyDict: @@ -401,6 +421,8 @@ class Project(bonsai.core.tool.Project): return hierarchy for project_library in ifc_file.by_type("IfcProjectLibrary"): parent_library = cls.get_parent_library(project_library) + if parent_library is None: + continue hierarchy[parent_library][project_library] = hierarchy[project_library] return hierarchy diff --git a/src/bonsai/test/bim/module/project/test_project_library_data.py b/src/bonsai/test/bim/module/project/test_project_library_data.py new file mode 100644 index 0000000000..08f5573b00 --- /dev/null +++ b/src/bonsai/test/bim/module/project/test_project_library_data.py @@ -0,0 +1,121 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +import ifcopenshell +import ifcopenshell.api.nest +import ifcopenshell.api.project +import ifcopenshell.api.root +import pytest + +import bonsai.tool as tool +from bonsai.bim.ifc import IfcStore +from bonsai.bim.module.project.data import ProjectLibraryData +from test.bim.bootstrap import NewIfc + +pytestmark = pytest.mark.project + + +def _make_library_only_file(*, with_child: bool = False) -> ifcopenshell.file: + """Build a minimal IFC4 file containing only an IfcProjectLibrary (no IfcProject). + + Per IFC4+, a file must contain at least one IfcContext; IfcProjectLibrary is a + valid root on its own. ``with_child=True`` nests a sub-library under the root via + IfcRelNests, mirroring real authored library files. + """ + library_file = ifcopenshell.api.project.create_file(version="IFC4") + root = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib") + if with_child: + child = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="ChildLib") + ifcopenshell.api.nest.assign_object(library_file, [child], root) + return library_file + + +class TestLibraryOnlyFile(NewIfc): + def test_get_root_context_returns_project_library_when_no_project(self): + library_file = _make_library_only_file() + assert not library_file.by_type("IfcProject") + + root = tool.Project.get_root_context(library_file) + + assert root.is_a("IfcProjectLibrary") + assert root.Name == "RootLib" + + def test_get_parent_library_returns_none_for_root_library(self): + library_file = _make_library_only_file() + root = library_file.by_type("IfcProjectLibrary")[0] + + assert tool.Project.get_parent_library(root) is None + + def test_get_project_hierarchy_skips_root_library(self): + library_file = _make_library_only_file(with_child=True) + root = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "RootLib") + child = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "ChildLib") + + hierarchy = tool.Project.get_project_hierarchy(library_file) + + assert root in hierarchy + assert child in hierarchy[root] + + def test_project_library_data_loads_without_crash(self): + IfcStore.library_file = _make_library_only_file() + try: + ProjectLibraryData.is_loaded = False + ProjectLibraryData.load() + assert ProjectLibraryData.is_loaded + enum = ProjectLibraryData.data["parent_libraries_enum"] + assert len(enum) == 1 + assert enum[0][1].startswith("IfcProjectLibrary ") + finally: + IfcStore.library_file = None + ProjectLibraryData.is_loaded = False + + def test_refresh_library_succeeds_on_library_only_file(self): + import bpy + + IfcStore.library_file = _make_library_only_file(with_child=True) + try: + result = bpy.ops.bim.refresh_library() + assert result == {"FINISHED"} + finally: + IfcStore.library_file = None + ProjectLibraryData.is_loaded = False + + def test_add_project_library_nests_under_root_when_no_project(self): + import bpy + + IfcStore.library_file = _make_library_only_file() + library_file = IfcStore.library_file + try: + root = library_file.by_type("IfcProjectLibrary")[0] + before = set(library_file.by_type("IfcProjectLibrary")) + + result = bpy.ops.bim.add_project_library() + + assert result == {"FINISHED"} + after = set(library_file.by_type("IfcProjectLibrary")) + new_libraries = after - before + assert len(new_libraries) == 1 + new_library = next(iter(new_libraries)) + assert new_library.Nests + assert new_library.Nests[0].RelatingObject == root + assert not new_library.HasContext + finally: + IfcStore.library_file = None + ProjectLibraryData.is_loaded = False From 104eeaf0cdcf1f5e1fd8c3934c6a21e0d38c4ddf Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 18 Jun 2026 17:35:56 +0200 Subject: [PATCH 2/8] Fix clip-box edit-mode picker + rotation margin Edit-mode click-select rejected verts inside the clip volume because clip_bb stayed at the first-arm view; the depsgraph and PRE_VIEW handlers updated clip_planes but skipped the view3d.clip_border call that refreshes clip_bb. Schedule a full re-arm at transform-commit, IFC reload, and view drift. The empty's wireframe was clipped by its own planes when rotated at non-trivial scale because the 1e-6 absolute margin can't absorb float-precision drift that scales with the box's world half-extent. Add a 1e-5 relative expand. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/clip_box.py | 53 +++++++-- .../test/bim/module/clip_box/test_clip_box.py | 108 ++++++++++++++++++ 2 files changed, 151 insertions(+), 10 deletions(-) diff --git a/src/bonsai/bonsai/tool/clip_box.py b/src/bonsai/bonsai/tool/clip_box.py index 1b30328a1d..c477e32ad9 100644 --- a/src/bonsai/bonsai/tool/clip_box.py +++ b/src/bonsai/bonsai/tool/clip_box.py @@ -38,15 +38,16 @@ if TYPE_CHECKING: PlaneTuple = tuple[float, float, float, float] PlaneSet = tuple[PlaneTuple, PlaneTuple, PlaneTuple, PlaneTuple, PlaneTuple, PlaneTuple] -# Outward margin (world units) so the empty's CUBE display edges sit -# safely INSIDE the clip volume. Absolute (not relative-to-scale) -# because a relative multiplier balloons with scale and produces a -# visibly-wrong gap between the wireframe and the clipped geometry. -# Sub-mesh-precision value: visually invisible at any reasonable IFC -# scale yet large enough to keep the empty's own wireframe edges off -# the clip planes when float-precision accumulation pushes a corner -# a fractional epsilon outward. +# Outward margins so the empty's CUBE display edges sit safely INSIDE +# the clip volume. A fixed absolute margin fails under rotation: the +# float error in computing each column's length and in per-vertex dot +# products at GPU rasterisation scales with the axis's world +# half-extent, so once the box is spawned at any non-trivial scale it +# can exceed an absolute floor. The relative term tracks that drift; +# the absolute term catches sub-unit boxes where the relative term +# shrinks below float precision. _CLIP_EXPAND_ABS = 1e-6 +_CLIP_EXPAND_REL = 1e-5 class ClipBox: @@ -63,6 +64,11 @@ class ClipBox: _owned: set[int] = set() _region_by_key: dict[int, tuple[Any, Any]] = {} + # View matrix per region at last clip_border arm. PRE_VIEW only + # updates clip_planes; without a snapshot, the C-side clip_bb stays + # aligned to the prior view and the edit-mode picker rejects verts + # inside the current clip_planes after orbit/pan/zoom. + _view_matrix_at_arm: dict[int, tuple] = {} _refresh_pending: bool = False _last_seen_ifc_id: int = 0 # Tracks the last matrix we persisted to the pset, keyed by Blender @@ -152,7 +158,9 @@ class ClipBox: prevents the cube's own wireframe from being clipped by its own planes. """ - return tool.Cad.obb_clip_planes_from_matrix(obj.matrix_world, expand=_CLIP_EXPAND_ABS) + return tool.Cad.obb_clip_planes_from_matrix( + obj.matrix_world, expand=_CLIP_EXPAND_ABS, expand_rel=_CLIP_EXPAND_REL + ) @classmethod def compute_planes_from_matrix(cls, matrix: Any) -> PlaneSet: @@ -163,7 +171,7 @@ class ClipBox: transform offset, while ``obj.matrix_world`` stays at the pre-transform value until the operator commits on release. """ - return tool.Cad.obb_clip_planes_from_matrix(matrix, expand=_CLIP_EXPAND_ABS) + return tool.Cad.obb_clip_planes_from_matrix(matrix, expand=_CLIP_EXPAND_ABS, expand_rel=_CLIP_EXPAND_REL) @classmethod def apply_clip_planes(cls, planes: PlaneSet) -> None: @@ -183,6 +191,7 @@ class ClipBox: cls._owned.add(key) cls._region_by_key[key] = (area, region) cls._arm_region(area, region, region_3d, planes) + cls._view_matrix_at_arm[key] = tuple(tuple(row) for row in region_3d.view_matrix) @classmethod def _arm_region(cls, area: Any, region: Any, region_3d: Any, planes: PlaneSet) -> None: @@ -271,6 +280,7 @@ class ClipBox: """Drop the ownership table without touching any region. Used on register/reload.""" cls._owned.clear() cls._region_by_key.clear() + cls._view_matrix_at_arm.clear() PSET_NAME = "BBIM_ClipBoxes" COLLECTION_NAME = "BBIM_ClipBoxes" @@ -533,10 +543,16 @@ class ClipBox: cls._last_seen_ifc_id = ifc_id cls._owned.clear() cls._region_by_key.clear() + cls._view_matrix_at_arm.clear() cls._persisted_matrices.clear() cls._last_seen_object_matrices.clear() if ifc_file is not None: cls.load_from_project_pset(scene) + # .blend carries use_clip_planes / clip_bb forward; the + # C-side picker is armed for the prior session's view. + # Re-arm against the current view so click-select matches + # what the user sees. + cls.schedule_refresh() # Orphan-empty adoption is deferred while a transform modal is # dragging so the active-index change on adoption can't disrupt # the move. @@ -557,6 +573,12 @@ class ClipBox: # one save on release, not N saves per frame. if ifc_file is not None and prev_matrix is not None: cls.mark_dirty_for_save(obj.name) + # clip_bb is stale once the box settles elsewhere. The modal + # gate suppresses per-tick re-arms during a live drag and + # fires once on release (or on external sets — Python, undo, + # constraint). + if prev_matrix is not None and not tool.Blender.is_transform_modal_active(bpy.context): + cls.schedule_refresh() cls.flush_pending_saves(scene) try: @@ -594,6 +616,17 @@ class ClipBox: return region_3d.clip_planes = cls.compute_planes_from_matrix(matrix) region_3d.update() + # clip_bb captured by view3d.clip_border is view-aligned, so an + # orbit/pan/zoom leaves the picker testing against the old + # frustum even after clip_planes refresh. Re-arm so the picker + # matches the current view. + region = getattr(bpy.context, "region", None) + if region is not None: + key = region.as_pointer() + prev_view = cls._view_matrix_at_arm.get(key) + current_view = tuple(tuple(row) for row in region_3d.view_matrix) + if prev_view is not None and prev_view != current_view: + cls.schedule_refresh() # ------------------------------------------------------------------ # Cross-section caps diff --git a/src/bonsai/test/bim/module/clip_box/test_clip_box.py b/src/bonsai/test/bim/module/clip_box/test_clip_box.py index 92a51e6390..2bcef8ec8c 100644 --- a/src/bonsai/test/bim/module/clip_box/test_clip_box.py +++ b/src/bonsai/test/bim/module/clip_box/test_clip_box.py @@ -114,6 +114,33 @@ class TestComputePlanes(NewFile): planes = tool.ClipBox.compute_planes(host) assert tool.Cad.point_is_inside_clip_planes(planes, Vector((5, 7, 0))) + def test_margin_grows_with_scale(self): + # The empty's CUBE display lives at local +-1; the GPU dot + # product that tests each wireframe vertex against the clip + # planes has float error that scales with the axis's world + # half-extent. A fixed absolute margin gets eaten by that drift + # once the box is spawned at non-trivial scale, so the half- + # extent the planes encode must include a relative term — the + # margin between the wireframe edge and the plane must grow + # with the scale. + bpy.ops.bim.add_clip_box() + host = tool.ClipBox.get_active_clip_box() + + host.matrix_world = Matrix.Identity(4) + planes_unit = tool.ClipBox.compute_planes(host) + # +X plane: normal (-1, 0, 0), d = half_x. Read half from d. + half_unit = planes_unit[0][3] + margin_unit = half_unit - 1.0 + + host.matrix_world = Matrix.Diagonal((100.0, 100.0, 100.0, 1.0)) + planes_scaled = tool.ClipBox.compute_planes(host) + half_scaled = planes_scaled[0][3] + margin_scaled = half_scaled - 100.0 + + assert margin_scaled > margin_unit * 10, ( + f"margin must scale with extent: unit={margin_unit:g}, " f"scale-100={margin_scaled:g}" + ) + class TestToggleEnabled(NewFile): def test_flips_scene_enabled_flag(self): @@ -690,3 +717,84 @@ class TestCapRebuildDebounce(NewFile): ): tool.ClipBox.on_depsgraph_update_caps(bpy.context.scene, None) mock_handle.assert_not_called() + + +class TestClipBbReArmTriggers(NewFile): + """The C-side clip_bb captured by view3d.clip_border for edit-mode + click-select is view-aligned and tied to the box pose at arm time, + so it goes stale on either a clip-box transform commit, an external + matrix mutation, or an IFC reload that rehydrates from pset. The + depsgraph handler schedules a full re-arm at those events; the + modal gate suppresses per-tick re-arms during a live drag. + """ + + def setup_method(self): + tool.ClipBox._persisted_matrices.clear() + tool.ClipBox._last_seen_ifc_id = 0 + tool.ClipBox._refresh_pending = False + + def teardown_method(self): + tool.ClipBox._persisted_matrices.clear() + tool.ClipBox._last_seen_ifc_id = 0 + tool.ClipBox._refresh_pending = False + + def test_matrix_change_outside_modal_re_arms(self): + bpy.ops.bim.add_clip_box() + host = tool.ClipBox.get_active_clip_box() + # Seed a stale baseline so prev_matrix != current_matrix. + stale = tuple(tuple(row) for row in Matrix.Translation((-99.0, 0.0, 0.0))) + tool.ClipBox._persisted_matrices[host.name] = stale + + with ( + patch.object(tool.Blender, "is_transform_modal_active", return_value=False), + patch.object(tool.ClipBox, "schedule_refresh") as mock_refresh, + ): + tool.ClipBox.on_depsgraph_update(bpy.context.scene, bpy.context.evaluated_depsgraph_get()) + mock_refresh.assert_called_once() + + def test_matrix_change_during_modal_skips_re_arm(self): + bpy.ops.bim.add_clip_box() + host = tool.ClipBox.get_active_clip_box() + stale = tuple(tuple(row) for row in Matrix.Translation((-99.0, 0.0, 0.0))) + tool.ClipBox._persisted_matrices[host.name] = stale + + with ( + patch.object(tool.Blender, "is_transform_modal_active", return_value=True), + patch.object(tool.ClipBox, "schedule_refresh") as mock_refresh, + ): + tool.ClipBox.on_depsgraph_update(bpy.context.scene, bpy.context.evaluated_depsgraph_get()) + mock_refresh.assert_not_called() + + def test_first_matrix_sighting_does_not_re_arm(self): + # No prior persisted-matrix entry: the branch records the + # baseline and exits without re-arming. The add path already + # armed once; a per-tick re-arm on first sight would double-arm. + bpy.ops.bim.add_clip_box() + host = tool.ClipBox.get_active_clip_box() + tool.ClipBox._persisted_matrices.pop(host.name, None) + + with ( + patch.object(tool.Blender, "is_transform_modal_active", return_value=False), + patch.object(tool.ClipBox, "schedule_refresh") as mock_refresh, + ): + tool.ClipBox.on_depsgraph_update(bpy.context.scene, bpy.context.evaluated_depsgraph_get()) + mock_refresh.assert_not_called() + + def test_ifc_reload_re_arms(self): + bpy.ops.bim.create_project() + bpy.ops.bim.add_clip_box() + # Force an ifc-id mismatch so the rehydrate-from-pset branch + # fires. The .blend carries the prior session's clip_bb forward; + # the picker is armed for the OLD view until this re-arms. + tool.ClipBox._last_seen_ifc_id = 0 + + with ( + patch.object(tool.Blender, "is_transform_modal_active", return_value=False), + patch.object(tool.ClipBox, "schedule_refresh") as mock_refresh, + ): + tool.ClipBox.on_depsgraph_update(bpy.context.scene, bpy.context.evaluated_depsgraph_get()) + # IFC-load triggers a re-arm. (Reading the show_caps pset entry + # writes scene_props.show_caps via its update callback, which + # is a separate pre-existing re-arm path; the test pins the + # invariant "ifc-load arms at least once".) + assert mock_refresh.call_count >= 1 From e2a4e5692f883d6742bc51625f361f06cba0d818 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Fri, 19 Jun 2026 08:52:38 +0200 Subject: [PATCH 3/8] Add network path overlay for walls and MEP Adds a viewport overlay that traces the connected element path from the selected wall or MEP element. Walls follow IfcRelConnectsPathElements and draw each connected wall's reference axis with endpoint dots; MEP elements follow IfcRelConnectsPorts and draw each segment's axis plus a port-to-port spider for each fitting. The new BIMModelProperties.show_paths toggle (Element Paths in the Bonsai Decorators group of Blender's viewport overlay popover) drives install / uninstall of both decorators on flip and on file load, mirroring the show_slab_direction wiring. The popover row also surfaces the pre-existing BIMSystemProperties.should_draw_decorations toggle (System Decorations) so both connectivity overlays sit together. Dot colors split free endpoints (decorator_color_special, blue by default) from junction nodes (decorator_color_selected, green by default) so dangling chain tips read apart from interior joins. Walls classify endpoints by IFC topology: rels expose RelatingConnectionType / RelatedConnectionType and ATPATH dots use tool.Wall.path_connection_location_world for the canonical T-meets join. MEP keeps the geometric classifier because port positions coincide exactly across fitting + segment. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/handler.py | 7 + .../bonsai/bim/module/model/__init__.py | 5 + .../bonsai/bim/module/model/decorator.py | 495 ++++++++++++++++++ src/bonsai/bonsai/bim/module/model/prop.py | 29 + src/bonsai/bonsai/bim/ui.py | 4 + .../test_connected_network_path_decorator.py | 262 +++++++++ 6 files changed, 802 insertions(+) create mode 100644 src/bonsai/test/bim/module/model/test_connected_network_path_decorator.py diff --git a/src/bonsai/bonsai/bim/handler.py b/src/bonsai/bonsai/bim/handler.py index dc4af08813..a7fc1dff67 100644 --- a/src/bonsai/bonsai/bim/handler.py +++ b/src/bonsai/bonsai/bim/handler.py @@ -51,9 +51,11 @@ from bonsai.bim.module.model.decorator import ( BoundingBoxDecorator, DoorSwingReadonlyDecorator, MEPSegmentExtendPreviewDecorator, + MEPSystemPathDecorator, SlabDirectionDecorator, WallAxisDecorator, WallFilletPreviewDecorator, + WallSystemPathDecorator, ) from bonsai.bim.module.model.wall import WallGizmoPreviewDecorator from bonsai.bim.module.nest.decorator import NestDecorator @@ -513,6 +515,8 @@ def _install_viewport_overlays() -> None: NestDecorator.uninstall() WallAxisDecorator.uninstall() SlabDirectionDecorator.uninstall() + MEPSystemPathDecorator.uninstall() + WallSystemPathDecorator.uninstall() WallFilletPreviewDecorator.uninstall() BendPreviewDecorator.uninstall() MEPSegmentExtendPreviewDecorator.uninstall() @@ -532,6 +536,9 @@ def _install_viewport_overlays() -> None: WallAxisDecorator.install(bpy.context) if model_props.show_slab_direction: SlabDirectionDecorator.install(bpy.context) + if model_props.show_paths: + MEPSystemPathDecorator.install(bpy.context) + WallSystemPathDecorator.install(bpy.context) if model_props.show_bounding_box: BoundingBoxDecorator.install(bpy.context) # Always-installed: draw() self-polls on Scene.BIMPreviewProperties. diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index 6583144c89..d602c715a3 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -378,6 +378,11 @@ def unregister(): # half-unloaded module state. opening.DecorationsHandler.uninstall() + # Network path overlays attach SpaceView3D draw handlers on toggle; + # uninstall here so addon disable / Blender shutdown doesn't leak them. + decorator.MEPSystemPathDecorator.uninstall() + decorator.WallSystemPathDecorator.uninstall() + if not bpy.app.background: for tool_data in reversed(tools): bpy.utils.unregister_tool(tool_data.tool) diff --git a/src/bonsai/bonsai/bim/module/model/decorator.py b/src/bonsai/bonsai/bim/module/model/decorator.py index 2d4b72c7a0..0a2c2678ac 100644 --- a/src/bonsai/bonsai/bim/module/model/decorator.py +++ b/src/bonsai/bonsai/bim/module/model/decorator.py @@ -21,6 +21,7 @@ from __future__ import annotations import math +from collections.abc import Sequence from math import cos, pi, radians, sin, tan from typing import Any, Literal, NamedTuple @@ -43,6 +44,7 @@ from mathutils import Matrix, Quaternion, Vector import bonsai.core.geometry import bonsai.tool as tool +from bonsai.bim.decorator_cache import TokenCache from bonsai.bim.module.drawing.gizmos import ( ARC_SEGMENTS, DOOR_SWING_ANGLE_MAX, @@ -62,6 +64,41 @@ def highlight_color(color, alpha=0.1): return color +def _stroke_lines_alpha( + context: bpy.types.Context, + segments: list[tuple[tuple[float, float, float], tuple[float, float, float]]], + color_rgb: tuple[float, float, float], + line_width: float, + line_alpha: float, +) -> None: + """Render ``segments`` (a list of (start, end) tuples) as one anti-aliased + LINES batch in world space. Early-returns when ``context.region`` is + unavailable (e.g. when called from a ``_RestrictContext``).""" + if not segments: + return + verts: list[tuple[float, float, float]] = [] + indices: list[tuple[int, int]] = [] + for start, end in segments: + base = len(verts) + verts.append(tuple(start)) + verts.append(tuple(end)) + indices.append((base, base + 1)) + if not tool.Blender.validate_shader_batch_data(verts, indices): + return + region = getattr(context, "region", None) + if region is None: + return + shader = gpu.shader.from_builtin("POLYLINE_UNIFORM_COLOR") + shader.bind() + shader.uniform_float("viewportSize", (region.width, region.height)) + shader.uniform_float("lineWidth", line_width) + shader.uniform_float("color", (*color_rgb, line_alpha)) + batch = batch_for_shader(shader, "LINES", {"pos": verts}, indices=indices) + gpu.state.blend_set("ALPHA") + batch.draw(shader) + gpu.state.blend_set("NONE") + + class ProfileDecorator: installed = None @@ -2529,3 +2566,461 @@ def draw_polyline_segments( _BBOX_HIGHLIGHT_LINE_WIDTH = 1.8 _BBOX_HIGHLIGHT_LINE_ALPHA = 0.8 + + +class _ConnectedNetworkPathDecorator(tool.Blender.ViewportDecorator): + """Shared scaffolding for "BFS-walk a connected IFC network from a selected + seed and overlay its schematic path" viewport decorators. + + Subclasses implement three hooks: + + ``_is_seed_element(element)``: True if ``element`` can seed a walk + ``_walk(start_element)``: list of network elements reachable from the seed + ``_build_geometry(connected)``: ``(lines, free_points, connection_points)`` + for one walk pass; free dots render in the base selected color, + connection dots in the "special" slot so junctions stand out + + Lifecycle each redraw: gate on ``BIMModelProperties.show_paths`` (the + shared toggle for all network-path overlays), find the first selected + seed element, walk the network (cached per seed-GUID per IFC file), and + render lines + connection-node dots. Geometry is memoised through a + ``TokenCache`` keyed on the decorator-cache token, so depsgraph / undo / + redo / load all invalidate the resolved world-space pass without + re-walking. + + Install / uninstall is driven by the central addon-load handler and + by the toggle's ``update`` callback, so flipping the property takes + effect immediately without a Blender restart.""" + + # Network-path lines + junction dots render in ``decorator_color_selected`` + # (Bonsai's palette slot for "what the user is currently inspecting"); free + # endpoints (dangling chain tips) switch to ``decorator_color_special`` so + # the end of the line stands apart from interior junctions at a glance. + LINE_WIDTH = 1.3 + LINE_ALPHA = 0.85 + # Sized larger than LINE_WIDTH so connection nodes read as discrete + # points rather than line thickenings. + DOT_SIZE = 4.0 + # Squared distance under which two emitted dots are treated as the same + # connection node. In Blender units (typically meters), 1e-4 m ≈ 0.1 mm + # — below the precision at which two IFC reference-line endpoints would + # ever be authored as "the same join" but not so tight that float drift + # from coordinate composition misses a real coincidence. + CONNECTION_EPS_SQ = 1e-4 * 1e-4 + + def __init__(self) -> None: + # Two-tier cache. Walk cache keyed on (start_guid, ifc_file): re-walk + # only on selection change or file reload. Compare ``ifc_file`` with + # ``is`` (not id()) so a GC-recycled id() can't produce a false hit. + self._cached_start_guid: str | None = None + self._cached_ifc_file: Any = None + self._cached_walk: list[Any] = [] + # Geometry cache: shared TokenCache so resolved world-space lines + + # dots re-build on every depsgraph / undo / redo / load. + self._geom_cache: TokenCache[ + tuple[ + list[tuple[tuple[float, float, float], tuple[float, float, float]]], + list[tuple[float, float, float]], + list[tuple[float, float, float]], + ] + ] = TokenCache() + # One-shot guards so a corrupted walk or build surfaces in the console + # once per decorator instance instead of every redraw. + self._walk_failure_logged: bool = False + self._build_failure_logged: bool = False + # Short-circuit re-running a known-broken walk or build for the same + # seed every frame; cleared the moment the user picks a different seed. + self._failed_seed_guid: str | None = None + + _ABSTRACT_HOOKS = ("_is_seed_element", "_walk", "_build_geometry") + + def __init_subclass__(cls, **kwargs): + super().__init_subclass__(**kwargs) + # Pin the template-method contract at class-definition time, mirroring + # ViewportDecorator's draw_method check: a subclass that forgets to + # override one of the three hooks would otherwise pass class creation + # and only raise NotImplementedError on the first walk — deferred long + # past the offending declaration. + missing = [ + name for name in cls._ABSTRACT_HOOKS if getattr(cls, name) is getattr(_ConnectedNetworkPathDecorator, name) + ] + if missing: + raise TypeError(f"{cls.__name__}: must override abstract hook(s) {sorted(missing)}") + + def _is_seed_element(self, element: Any) -> bool: + raise NotImplementedError + + def _walk(self, start_element: Any) -> list[Any]: + raise NotImplementedError + + def _build_geometry( + self, + connected: list[Any], + ) -> tuple[ + list[tuple[tuple[float, float, float], tuple[float, float, float]]], + list[tuple[float, float, float]], + list[tuple[float, float, float]], + ]: + """Resolve world-space line segments + dots for one walk pass. Returns + ``(lines, free_points, connection_points)`` — free dots get the base + selected color, connection dots get the special color so junctions + between two consecutive elements pop out. Never raises; skips + degenerate elements.""" + raise NotImplementedError + + @classmethod + def _partition_points_by_coincidence( + cls, + points: list[tuple[float, float, float]], + lines: Sequence[tuple[tuple[float, float, float], tuple[float, float, float]]] = (), + ) -> tuple[list[tuple[float, float, float]], list[tuple[float, float, float]]]: + """Split ``points`` into ``(free, connection)``. A point is "connection" + when (a) at least one other point in the list lies within + ``CONNECTION_EPS_SQ`` (corner / end-to-end joins), or (b) it lies within + ``CONNECTION_EPS_SQ`` of the interior of any segment in ``lines`` + (T-junctions / ATPATH joins, where one wall's end lands on another + wall's axis interior rather than its endpoint). Connection points + dedupe to one representative each so coincident dots don't stack the + same color.""" + eps_sq = cls.CONNECTION_EPS_SQ + n = len(points) + shared = [False] * n + for i in range(n): + xi, yi, zi = points[i] + for j in range(i + 1, n): + xj, yj, zj = points[j] + dx, dy, dz = xi - xj, yi - yj, zi - zj + if dx * dx + dy * dy + dz * dz <= eps_sq: + shared[i] = True + shared[j] = True + for i, point in enumerate(points): + if shared[i]: + continue + if cls._point_touches_any_segment_interior(point, lines, eps_sq): + shared[i] = True + free: list[tuple[float, float, float]] = [] + connection: list[tuple[float, float, float]] = [] + seen_connection: list[tuple[float, float, float]] = [] + for i, point in enumerate(points): + if not shared[i]: + free.append(point) + continue + for existing in seen_connection: + dx, dy, dz = point[0] - existing[0], point[1] - existing[1], point[2] - existing[2] + if dx * dx + dy * dy + dz * dz <= eps_sq: + break + else: + seen_connection.append(point) + connection.append(point) + return free, connection + + @staticmethod + def _point_touches_any_segment_interior( + point: tuple[float, float, float], + lines: Sequence[tuple[tuple[float, float, float], tuple[float, float, float]]], + eps_sq: float, + ) -> bool: + """True iff ``point`` lies within ``sqrt(eps_sq)`` of the interior of + any segment in ``lines``. Endpoints are excluded so a point cannot + match its own owning segment via either of that segment's tips — the + endpoint-coincidence pass already handles those cases. The qualifying + projection must land strictly inside the segment (``0 < t < 1``) AND + sit further than ``eps`` from either tip, catching ATPATH/T-junction + joins without false-flagging walls that share a corner.""" + px, py, pz = point + for (ax, ay, az), (bx, by, bz) in lines: + dxa, dya, dza = px - ax, py - ay, pz - az + if dxa * dxa + dya * dya + dza * dza <= eps_sq: + continue + dxb, dyb, dzb = px - bx, py - by, pz - bz + if dxb * dxb + dyb * dyb + dzb * dzb <= eps_sq: + continue + ex, ey, ez = bx - ax, by - ay, bz - az + seg_len_sq = ex * ex + ey * ey + ez * ez + if seg_len_sq <= eps_sq: + continue + t = (dxa * ex + dya * ey + dza * ez) / seg_len_sq + if t <= 0.0 or t >= 1.0: + continue + qx, qy, qz = ax + t * ex, ay + t * ey, az + t * ez + dx, dy, dz = px - qx, py - qy, pz - qz + if dx * dx + dy * dy + dz * dz <= eps_sq: + return True + return False + + def draw(self, context: bpy.types.Context) -> None: + model_props = tool.Model.get_model_props() + if not getattr(model_props, "show_paths", False): + return + ifc_file = tool.Ifc.get() + if ifc_file is None: + return + + start_element = None + active = context.active_object + if active is not None: + element = tool.Ifc.get_entity(active) + if element is not None and self._is_seed_element(element): + start_element = element + if start_element is None: + for obj in context.selected_objects or []: + if obj is active: + continue + element = tool.Ifc.get_entity(obj) + if element is None or not self._is_seed_element(element): + continue + start_element = element + break + if start_element is None: + self._cached_start_guid = None + self._cached_walk = [] + return + + start_guid = start_element.GlobalId + if start_guid == self._failed_seed_guid: + return + if start_guid == self._cached_start_guid and ifc_file is self._cached_ifc_file and self._cached_walk: + connected = self._cached_walk + else: + try: + connected = self._walk(start_element) + except Exception: + if not self._walk_failure_logged: + import traceback + + traceback.print_exc() + self._walk_failure_logged = True + self._cached_walk = [] + self._failed_seed_guid = start_guid + return + self._cached_start_guid = start_guid + self._cached_ifc_file = ifc_file + self._cached_walk = connected + if not connected: + return + + prefs = tool.Blender.get_addon_preferences() + line_color = tuple(prefs.decorator_color_selected[:3]) + # Junction dots get the "selected" palette slot (green by default) so + # they read as the currently-inspected network's spine; free endpoints + # get the "special" slot (blue by default) so dangling line ends stand + # apart from junctions at a glance. + connection_color = line_color + free_color = tuple(prefs.decorator_color_special[:3]) + + try: + lines, free_points, connection_points = self._geom_cache.get_or_compute( + (start_guid, id(ifc_file)), + lambda: self._build_geometry(connected), + ) + except Exception: + if not self._build_failure_logged: + import traceback + + traceback.print_exc() + self._build_failure_logged = True + self._failed_seed_guid = start_guid + return + + if lines: + _stroke_lines_alpha(context, lines, line_color, self.LINE_WIDTH, self.LINE_ALPHA) + + if free_points or connection_points: + # POINTS via UNIFORM_COLOR; point_size_set only affects the next batch. + point_shader = gpu.shader.from_builtin("UNIFORM_COLOR") + point_shader.bind() + gpu.state.point_size_set(self.DOT_SIZE) + gpu.state.blend_set("ALPHA") + if free_points: + point_shader.uniform_float("color", (*free_color, self.LINE_ALPHA)) + batch = batch_for_shader(point_shader, "POINTS", {"pos": free_points}) + batch.draw(point_shader) + if connection_points: + point_shader.uniform_float("color", (*connection_color, self.LINE_ALPHA)) + batch = batch_for_shader(point_shader, "POINTS", {"pos": connection_points}) + batch.draw(point_shader) + gpu.state.blend_set("NONE") + + +class MEPSystemPathDecorator(_ConnectedNetworkPathDecorator): + """Schematic-path overlay for the selected MEP element's connected + distribution system. + + Walk: BFS through ``IfcRelConnectsPorts`` from the first selected MEP + element. Segments render as one axis line + endpoint dots. Fittings + render as: + + - 2-port (transition, coupler, bend): one line port-to-port, keeping + the schematic continuous through the fitting. The "spider from + origin" pattern produces V-shaped flares when the fitting's local + origin is offset from its ports. + - 3+-port (tee, cross, branching): spider from origin to each port. + Drawing all N*(N-1)/2 port pairs would clutter the view at high N + (N=4 → 6 lines); the spider gives one line per port. + - 0-port / 1-port: degenerate, no lines (dots still emit).""" + + def _is_seed_element(self, element: Any) -> bool: + return tool.System.is_mep_element(element) + + def _walk(self, start_element: Any) -> list[Any]: + return tool.System.walk_connected_mep_elements(start_element) + + def _build_geometry( + self, + connected: list[Any], + ) -> tuple[ + list[tuple[tuple[float, float, float], tuple[float, float, float]]], + list[tuple[float, float, float]], + list[tuple[float, float, float]], + ]: + lines: list[tuple[tuple[float, float, float], tuple[float, float, float]]] = [] + port_positions: list[tuple[float, float, float]] = [] + for element in connected: + if element.is_a("IfcFlowSegment"): + if not tool.Geometry.has_axis_representation(element): + continue + obj = tool.Ifc.get_object(element) + if obj is None: + continue + start_world, end_world = tool.Model.get_flow_segment_axis(obj) + lines.append((tuple(start_world), tuple(end_world))) + # Segment ports sit at the two axis endpoints — emit dots so + # the connection node is visible whether the neighbour is a + # fitting (also emits) or another segment (doesn't). + port_positions.append(tuple(start_world)) + port_positions.append(tuple(end_world)) + elif element.is_a("IfcFlowFitting"): + obj = tool.Ifc.get_object(element) + if obj is None: + continue + ports = tool.System.get_ports(element) + port_world_positions = [tool.System.get_port_world_position(p) for p in ports] + if len(port_world_positions) == 2: + lines.append((tuple(port_world_positions[0]), tuple(port_world_positions[1]))) + elif len(port_world_positions) >= 3: + origin = obj.matrix_world.translation + for port_pos in port_world_positions: + lines.append((tuple(origin), tuple(port_pos))) + for port_pos in port_world_positions: + port_positions.append(tuple(port_pos)) + free_points, connection_points = self._partition_points_by_coincidence(port_positions) + return lines, free_points, connection_points + + +class WallSystemPathDecorator(_ConnectedNetworkPathDecorator): + """Schematic-path overlay for the selected wall's connected wall network. + + Walk: BFS through ``IfcRelConnectsPathElements`` from the first selected + wall. Each wall renders as one reference-line segment + a dot at each + axis endpoint. Endpoints are classified by IFC topology — every wall in + the walked set inspects its ``IfcRelConnectsPathElements`` rels filtered + to walls in the same set, and uses ``Relating*``/``Related*ConnectionType`` + (ATSTART / ATEND / ATPATH) to decide which endpoint participates. ATPATH + rels also emit a connection dot at the canonical join location (a T-meets + point sits on the through-wall's interior, not at any endpoint). The + framework's geometric classifier is bypassed for walls because authoring + tolerance and post-edit float drift commonly exceed the 0.1 mm coincidence + threshold, so T-junctions otherwise fell into the free bucket.""" + + def _is_seed_element(self, element: Any) -> bool: + return element.is_a("IfcWall") and tool.Geometry.has_axis_representation(element) + + def _walk(self, start_element: Any) -> list[Any]: + return tool.Wall.walk_connected_walls(start_element) + + def _build_geometry( + self, + connected: list[Any], + ) -> tuple[ + list[tuple[tuple[float, float, float], tuple[float, float, float]]], + list[tuple[float, float, float]], + list[tuple[float, float, float]], + ]: + lines: list[tuple[tuple[float, float, float], tuple[float, float, float]]] = [] + refs: dict[int, tuple[tuple[float, float, float], tuple[float, float, float]]] = {} + for element in connected: + obj = tool.Ifc.get_object(element) + if obj is None: + continue + ref = tool.Wall.get_world_reference_line(obj) + if ref is None: + continue + p1, p2 = tuple(ref[0]), tuple(ref[1]) + refs[element.id()] = (p1, p2) + lines.append((p1, p2)) + + free_points, connection_points = self._classify_endpoints_from_rels(connected, refs) + connection_points = self._dedupe_close_points(connection_points, self.CONNECTION_EPS_SQ) + return lines, free_points, connection_points + + @staticmethod + def _classify_endpoints_from_rels( + connected: Sequence[Any], + refs: dict[int, tuple[tuple[float, float, float], tuple[float, float, float]]], + ) -> tuple[list[tuple[float, float, float]], list[tuple[float, float, float]]]: + """For each wall in ``connected`` with a reference line in ``refs``, + classify its endpoints by walking its ``IfcRelConnectsPathElements`` + rels filtered to walls also in ``refs``. ATSTART side present → + reference-line start is a connection; ATEND side present → reference- + line end is a connection; otherwise free. ATPATH side present → emit + an extra connection dot at the canonical join via + ``tool.Wall.path_connection_location_world``. Returns + ``(free, connection)`` un-deduped.""" + free_points: list[tuple[float, float, float]] = [] + connection_points: list[tuple[float, float, float]] = [] + for element in connected: + self_seg = refs.get(element.id()) + if self_seg is None: + continue + sides: set[str] = set() + atpath_dots: list[tuple[float, float, float]] = [] + for rel in getattr(element, "ConnectedTo", []) or (): + if not rel.is_a("IfcRelConnectsPathElements"): + continue + other = rel.RelatedElement + other_seg = refs.get(other.id()) if other is not None else None + if other_seg is None: + continue + self_type = rel.RelatingConnectionType + other_type = rel.RelatedConnectionType + sides.add(self_type) + if self_type == "ATPATH": + join = tool.Wall.path_connection_location_world(self_seg, self_type, other_seg, other_type) + atpath_dots.append(tuple(join)) + for rel in getattr(element, "ConnectedFrom", []) or (): + if not rel.is_a("IfcRelConnectsPathElements"): + continue + other = rel.RelatingElement + other_seg = refs.get(other.id()) if other is not None else None + if other_seg is None: + continue + self_type = rel.RelatedConnectionType + other_type = rel.RelatingConnectionType + sides.add(self_type) + if self_type == "ATPATH": + join = tool.Wall.path_connection_location_world(self_seg, self_type, other_seg, other_type) + atpath_dots.append(tuple(join)) + p1, p2 = self_seg + (connection_points if "ATSTART" in sides else free_points).append(p1) + (connection_points if "ATEND" in sides else free_points).append(p2) + connection_points.extend(atpath_dots) + return free_points, connection_points + + @staticmethod + def _dedupe_close_points( + points: Sequence[tuple[float, float, float]], + eps_sq: float, + ) -> list[tuple[float, float, float]]: + """Drop later occurrences of points within ``sqrt(eps_sq)`` of an + earlier one. Used to collapse overlapping connection dots so an ATPATH + join computed at the same point as a neighbour's wall endpoint + renders once.""" + result: list[tuple[float, float, float]] = [] + for point in points: + for existing in result: + dx, dy, dz = point[0] - existing[0], point[1] - existing[1], point[2] - existing[2] + if dx * dx + dy * dy + dz * dz <= eps_sq: + break + else: + result.append(point) + return result diff --git a/src/bonsai/bonsai/bim/module/model/prop.py b/src/bonsai/bonsai/bim/module/model/prop.py index e2f8a2a9a8..e0b4952708 100644 --- a/src/bonsai/bonsai/bim/module/model/prop.py +++ b/src/bonsai/bonsai/bim/module/model/prop.py @@ -33,8 +33,10 @@ from bonsai.bim.module.drawing.decoration import CutDecorator from bonsai.bim.module.model.data import AuthoringData from bonsai.bim.module.model.decorator import ( BoundingBoxDecorator, + MEPSystemPathDecorator, SlabDirectionDecorator, WallAxisDecorator, + WallSystemPathDecorator, ) from bonsai.bim.module.model.door import update_door_modifier_bmesh from bonsai.bim.module.model.window import update_window_modifier_bmesh @@ -132,6 +134,19 @@ def update_slab_direction_decorator(self: "BIMModelProperties", context: bpy.typ SlabDirectionDecorator.uninstall() +def update_paths_decorator(self: "BIMModelProperties", context: bpy.types.Context) -> None: + """Unified toggle for connected-element path overlays. Drives both the + MEP and wall path decorators — each decorator's ``draw`` short-circuits + when its kind of element isn't selected, so leaving both installed is + cheap and lets one toggle cover any connected-element family.""" + if self.show_paths: + MEPSystemPathDecorator.install(bpy.context) + WallSystemPathDecorator.install(bpy.context) + else: + MEPSystemPathDecorator.uninstall() + WallSystemPathDecorator.uninstall() + + def update_measure_xyz(self: "BIMModelProperties", context: bpy.types.Context) -> None: if self.show_bounding_box: BoundingBoxDecorator.install(context) @@ -354,6 +369,19 @@ class BIMModelProperties(PropertyGroup): default=False, update=update_slab_direction_decorator, ) + show_paths: bpy.props.BoolProperty( + name="Show Paths", + default=False, + update=update_paths_decorator, + description=( + "Trace the connected element path from the selected element. For " + "walls, follows IfcRelConnectsPathElements and draws each " + "connected wall's reference axis with endpoint dots. For MEP " + "elements, follows IfcRelConnectsPorts and draws each segment's " + "axis plus a port-to-port spider for each fitting. Toggle off to " + "skip the BFS traversal entirely." + ), + ) prev_transform_orientation_slot_type: bpy.props.StringProperty(name="Previous Gizmo Orientation Type") prev_show_gizmo_object_translate: bpy.props.BoolProperty(name="Previous Gizmo Translate") @@ -401,6 +429,7 @@ class BIMModelProperties(PropertyGroup): offset: float show_wall_axis: bool show_slab_direction: bool + show_paths: bool prev_transform_orientation_slot_type: str prev_show_gizmo_object_translate: bool diff --git a/src/bonsai/bonsai/bim/ui.py b/src/bonsai/bonsai/bim/ui.py index ca1d176fda..8af70e54c9 100644 --- a/src/bonsai/bonsai/bim/ui.py +++ b/src/bonsai/bonsai/bim/ui.py @@ -1925,6 +1925,7 @@ class BIM_PT_decorators_overlay(Panel): aggregate_props = tool.Aggregate.get_aggregate_props() nest_props = tool.Nest.get_nest_props() model_props = tool.Model.get_model_props() + system_props = tool.System.get_system_props() display_all = overlay.show_overlays col = layout.column() @@ -1942,6 +1943,9 @@ class BIM_PT_decorators_overlay(Panel): row = col.row(align=True) row.prop(model_props, "show_slab_direction", text="Slab Direction") row = col.row(align=True) + row.prop(model_props, "show_paths", text="Element Paths") + row.prop(system_props, "should_draw_decorations", text="System Decorations") + row = col.row(align=True) row.prop(model_props, "show_bounding_box", text="Bounding Box Dimensions") row = col.row(align=True) row.prop(model_props, "show_cut_decorator", text="Cut Decorator") diff --git a/src/bonsai/test/bim/module/model/test_connected_network_path_decorator.py b/src/bonsai/test/bim/module/model/test_connected_network_path_decorator.py new file mode 100644 index 0000000000..a956a8ecbe --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_connected_network_path_decorator.py @@ -0,0 +1,262 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Pin the template-method contract for the connected-network-path decorator +base. The base class defines three abstract hooks (`_is_seed_element`, +`_walk`, `_build_geometry`) and an `__init_subclass__` that rejects any +subclass which leaves a hook un-overridden. Without this guard, a forgotten +override would only surface as `NotImplementedError` on the first redraw +that hit the missing hook — long after the class declaration.""" + +import pytest + +pytestmark = pytest.mark.model + + +_GOOD_HOOKS = { + "_is_seed_element": lambda self, element: False, + "_walk": lambda self, start_element: [], + "_build_geometry": lambda self, connected: ([], [], []), +} + + +def _build_subclass(name, omit=()): + from bonsai.bim.module.model.decorator import _ConnectedNetworkPathDecorator + + namespace = {name: fn for name, fn in _GOOD_HOOKS.items() if name not in omit} + return type(name, (_ConnectedNetworkPathDecorator,), namespace) + + +@pytest.mark.parametrize("missing_hook", sorted(_GOOD_HOOKS)) +def test_subclass_missing_any_single_hook_raises(missing_hook): + with pytest.raises(TypeError, match="must override abstract hook"): + _build_subclass(f"DecoratorMissing_{missing_hook}", omit=(missing_hook,)) + + +def test_subclass_missing_all_hooks_raises_naming_each(): + with pytest.raises(TypeError) as excinfo: + _build_subclass("DecoratorMissingEverything", omit=tuple(_GOOD_HOOKS)) + message = str(excinfo.value) + for hook in _GOOD_HOOKS: + assert hook in message, f"missing-hook error must name {hook!r}" + + +def test_fully_overridden_subclass_is_accepted(): + cls = _build_subclass("DecoratorWithAllHooks") + assert cls.__name__ == "DecoratorWithAllHooks" + + +# --------------------------------------------------------------------------- +# Pure-geometry classifier contract. +# +# Pins the free/connection split that drives the dot colors. The classifier +# is plain Python (no bpy / no ifcopenshell), so it runs unconditionally — +# the autouse Blender skip in conftest still applies but doesn't bite here. + +_EPS = 1e-5 # well under CONNECTION_EPS_SQ's sqrt (1e-4) + + +def _cls(): + from bonsai.bim.module.model.decorator import _ConnectedNetworkPathDecorator + + return _ConnectedNetworkPathDecorator + + +def test_classifier_empty_input_returns_two_empty_lists(): + free, conn = _cls()._partition_points_by_coincidence([]) + assert free == [] + assert conn == [] + + +def test_classifier_single_point_is_free(): + p = (1.0, 2.0, 3.0) + free, conn = _cls()._partition_points_by_coincidence([p]) + assert free == [p] + assert conn == [] + + +def test_classifier_coincident_pair_dedupes_to_one_connection(): + p = (1.0, 2.0, 3.0) + near = (1.0 + _EPS, 2.0, 3.0) + free, conn = _cls()._partition_points_by_coincidence([p, near]) + assert free == [] + assert len(conn) == 1 + + +def test_classifier_far_points_stay_free(): + p1 = (0.0, 0.0, 0.0) + p2 = (10.0, 0.0, 0.0) + free, conn = _cls()._partition_points_by_coincidence([p1, p2]) + assert sorted(free) == sorted([p1, p2]) + assert conn == [] + + +def test_classifier_t_junction_point_on_segment_interior_is_connection(): + a1, a2 = (0.0, 0.0, 0.0), (5.0, 0.0, 0.0) # wall A endpoints (own segment) + b1, b2 = (2.5, -2.0, 0.0), (2.5, 0.0, 0.0) # wall B: T-meets A's midpoint + points = [a1, a2, b1, b2] + lines = [(a1, a2), (b1, b2)] + free, conn = _cls()._partition_points_by_coincidence(points, lines) + assert b2 in conn, "T-junction interior touch must be flagged as a connection" + assert a1 in free and a2 in free, "wall A free endpoints must stay free" + assert b1 in free, "wall B's far endpoint must stay free" + + +def test_classifier_endpoint_of_own_segment_is_not_a_t_junction(): + """A free endpoint sits exactly on its own segment's tip; the interior + check must exclude segment endpoints, not just the line interior.""" + a1, a2 = (0.0, 0.0, 0.0), (5.0, 0.0, 0.0) + free, conn = _cls()._partition_points_by_coincidence([a1, a2], [(a1, a2)]) + assert conn == [], "own-segment endpoints must not self-classify as connection" + assert sorted(free) == sorted([a1, a2]) + + +def test_classifier_zero_length_segment_does_not_match(): + """A segment whose two endpoints coincide has no interior; the interior + check must skip it rather than divide by a near-zero seg_len_sq.""" + a = (0.0, 0.0, 0.0) + p_far = (1.0, 1.0, 1.0) + free, conn = _cls()._partition_points_by_coincidence([p_far], [(a, a)]) + assert free == [p_far] + assert conn == [] + + +# --------------------------------------------------------------------------- +# Wall topology classifier — IFC-rel-driven endpoint classification. +# +# Pins the rule "an endpoint is a connection iff an IfcRelConnectsPathElements +# rel says so", independent of geometric coincidence. Replaces the geometric +# classifier on the wall path because authoring tolerance routinely exceeds +# the 0.1 mm epsilon, leaving T-junction dots mis-coloured. + +from unittest.mock import Mock, patch + + +def _stub_wall(wid, connected_to=(), connected_from=()): + e = Mock() + e.id.return_value = wid + e.is_a = lambda kind: kind == "IfcWall" + e.ConnectedTo = list(connected_to) + e.ConnectedFrom = list(connected_from) + return e + + +def _stub_rel(relating, related, relating_type, related_type): + r = Mock() + r.is_a = lambda kind: kind == "IfcRelConnectsPathElements" + r.RelatingElement = relating + r.RelatedElement = related + r.RelatingConnectionType = relating_type + r.RelatedConnectionType = related_type + return r + + +def _wall_cls(): + from bonsai.bim.module.model.decorator import WallSystemPathDecorator + + return WallSystemPathDecorator + + +def test_wall_topology_single_wall_no_rels_both_endpoints_free(): + a = _stub_wall(1) + refs = {1: ((0.0, 0.0, 0.0), (5.0, 0.0, 0.0))} + free, conn = _wall_cls()._classify_endpoints_from_rels([a], refs) + assert sorted(free) == sorted([(0.0, 0.0, 0.0), (5.0, 0.0, 0.0)]) + assert conn == [] + + +def test_wall_topology_l_corner_atend_to_atstart_flags_both_endpoints(): + """Two walls meeting at a corner: A's ATEND joins B's ATSTART. Each wall's + join-side endpoint flips to connection; the far endpoints stay free.""" + a = _stub_wall(1) + b = _stub_wall(2) + rel = _stub_rel(relating=a, related=b, relating_type="ATEND", related_type="ATSTART") + a.ConnectedTo = [rel] + b.ConnectedFrom = [rel] + refs = { + 1: ((0.0, 0.0, 0.0), (5.0, 0.0, 0.0)), + 2: ((5.0, 0.0, 0.0), (5.0, 5.0, 0.0)), + } + free, conn = _wall_cls()._classify_endpoints_from_rels([a, b], refs) + assert (5.0, 0.0, 0.0) in conn, "A's ATEND endpoint at the corner must be connection" + assert (5.0, 0.0, 0.0) in conn, "B's ATSTART endpoint at the corner must be connection" + assert (0.0, 0.0, 0.0) in free, "A's far end must stay free" + assert (5.0, 5.0, 0.0) in free, "B's far end must stay free" + + +def test_wall_topology_t_junction_atpath_emits_canonical_join_dot(): + """B's ATEND meets A's interior (ATPATH). A's two endpoints stay free, + B's ATSTART stays free, B's ATEND is connection, and an extra connection + dot is emitted at the T-meets point computed by + ``tool.Wall.path_connection_location_world``.""" + a = _stub_wall(1) + b = _stub_wall(2) + rel = _stub_rel(relating=b, related=a, relating_type="ATEND", related_type="ATPATH") + a.ConnectedFrom = [rel] + b.ConnectedTo = [rel] + refs = { + 1: ((0.0, 0.0, 0.0), (5.0, 0.0, 0.0)), + 2: ((2.5, -2.0, 0.0), (2.5, 0.0, 0.0)), + } + t_meets = (2.5, 0.0, 0.0) + with patch("bonsai.tool.Wall.path_connection_location_world", return_value=t_meets): + free, conn = _wall_cls()._classify_endpoints_from_rels([a, b], refs) + assert t_meets in conn, "T-meets canonical join must be a connection dot" + assert (2.5, 0.0, 0.0) in conn, "B's ATEND at the junction must also be a connection" + assert (0.0, 0.0, 0.0) in free and (5.0, 0.0, 0.0) in free, "A's endpoints stay free" + assert (2.5, -2.0, 0.0) in free, "B's ATSTART (far end) stays free" + + +def test_wall_topology_rel_to_wall_outside_walked_set_is_ignored(): + """A rel pointing at a wall whose id is not in ``refs`` must not classify + the participating endpoint as connection — only intra-set joins count.""" + a = _stub_wall(1) + outside = _stub_wall(99) + rel = _stub_rel(relating=a, related=outside, relating_type="ATEND", related_type="ATSTART") + a.ConnectedTo = [rel] + refs = {1: ((0.0, 0.0, 0.0), (5.0, 0.0, 0.0))} + free, conn = _wall_cls()._classify_endpoints_from_rels([a], refs) + assert sorted(free) == sorted([(0.0, 0.0, 0.0), (5.0, 0.0, 0.0)]) + assert conn == [] + + +def test_wall_topology_non_path_rels_are_ignored(): + """``ConnectedTo`` can carry ``IfcRelConnectsElements`` (slab clip rels); + only ``IfcRelConnectsPathElements`` contribute to wall endpoint topology.""" + a = _stub_wall(1) + non_path_rel = Mock() + non_path_rel.is_a = lambda kind: kind == "IfcRelConnectsElements" + a.ConnectedTo = [non_path_rel] + refs = {1: ((0.0, 0.0, 0.0), (5.0, 0.0, 0.0))} + free, conn = _wall_cls()._classify_endpoints_from_rels([a], refs) + assert sorted(free) == sorted([(0.0, 0.0, 0.0), (5.0, 0.0, 0.0)]) + assert conn == [] + + +def test_wall_topology_dedupe_collapses_overlapping_connection_dots(): + """Two connection dots at the same world point (within eps) collapse to + one — used by ``_build_geometry`` to keep ATPATH joins from stacking on + neighbour-wall endpoints.""" + p = (1.0, 2.0, 3.0) + near = (1.0 + 1e-6, 2.0, 3.0) + far = (10.0, 0.0, 0.0) + result = _wall_cls()._dedupe_close_points([p, near, far], 1e-4 * 1e-4) + assert len(result) == 2 + assert p in result and far in result From 9396340b13a2bb616fc6c688a0c53bb108f9eacd Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 19 Jun 2026 20:38:37 +0200 Subject: [PATCH 4/8] parse examples: Include -definitions.h header as well so that preprocessor switches works --- src/examples/IfcParseExamples.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/examples/IfcParseExamples.cpp b/src/examples/IfcParseExamples.cpp index 2951285a60..1cf202343e 100644 --- a/src/examples/IfcParseExamples.cpp +++ b/src/examples/IfcParseExamples.cpp @@ -65,6 +65,30 @@ static_assert(false, "A boost preprocessor sequence of schema identifiers is nee #include INCLUDE_SCHEMA_N(14) #include INCLUDE_SCHEMA_N(15) +#undef INCLUDE_SCHEMA_N + +#define INCLUDE_SCHEMA_N(n) \ + BOOST_PP_IIF(BOOST_PP_GREATER(BOOST_PP_SEQ_SIZE(SCHEMA_SEQ), n), \ + BOOST_PP_STRINGIZE(ifcparse/BOOST_PP_CAT(Ifc, BOOST_PP_SEQ_ELEM(BOOST_PP_MIN(n, BOOST_PP_SEQ_SIZE(BOOST_PP_SEQ_POP_BACK(SCHEMA_SEQ))), SCHEMA_SEQ))-definitions.h), \ + "ifcgeom/empty.h") + +#include INCLUDE_SCHEMA_N(0) +#include INCLUDE_SCHEMA_N(1) +#include INCLUDE_SCHEMA_N(2) +#include INCLUDE_SCHEMA_N(3) +#include INCLUDE_SCHEMA_N(4) +#include INCLUDE_SCHEMA_N(5) +#include INCLUDE_SCHEMA_N(6) +#include INCLUDE_SCHEMA_N(7) +#include INCLUDE_SCHEMA_N(8) +#include INCLUDE_SCHEMA_N(9) +#include INCLUDE_SCHEMA_N(10) +#include INCLUDE_SCHEMA_N(11) +#include INCLUDE_SCHEMA_N(12) +#include INCLUDE_SCHEMA_N(13) +#include INCLUDE_SCHEMA_N(14) +#include INCLUDE_SCHEMA_N(15) + #include #if USE_VLD From c0f2fb1860de14d140a7143a86346c1d1a3bd85c Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 19 Jun 2026 20:50:40 +0200 Subject: [PATCH 5/8] Store actual subentities in CgalShape instead of the strange degeneracies convention --- .../kernels/cgal/CgalConversionResult.cpp | 482 +++++++++++++----- .../kernels/cgal/CgalConversionResult.h | 29 +- 2 files changed, 370 insertions(+), 141 deletions(-) diff --git a/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp b/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp index fc3e642b6c..749fd1435e 100644 --- a/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp +++ b/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp @@ -30,6 +30,140 @@ typedef Polyhedron::Facet_const_handle Facet_const_handle; typedef Polyhedron::Halfedge_around_facet_const_circulator Halfedge_around_facet_circulator; namespace { + cgal_placement_t make_transform(const ifcopenshell::geometry::taxonomy::matrix4& place) { + const auto& m = place.ccomponents(); + return cgal_placement_t( + m(0, 0), m(0, 1), m(0, 2), m(0, 3), + m(1, 0), m(1, 1), m(1, 2), m(1, 3), + m(2, 0), m(2, 1), m(2, 2), m(2, 3)); + } + + OpaqueCoordinate<3> opaque_point(const cgal_point_t& p) { + return OpaqueCoordinate<3>( + new NumberType(p.cartesian(0)), + new NumberType(p.cartesian(1)), + new NumberType(p.cartesian(2)) + ); + } + + typename Kernel_::FT max_abs3(const typename Kernel_::FT& a, const typename Kernel_::FT& b, const typename Kernel_::FT& c) { + std::array abc{ a, b, c }; + auto minel = std::min_element(abc.begin(), abc.end()); + auto maxel = std::max_element(abc.begin(), abc.end()); + return ((-*minel) > *maxel) ? (-*minel) : *maxel; + } + + OpaqueCoordinate<3> opaque_axis(const cgal_vector_t& v) { + auto maxval = max_abs3(v.x(), v.y(), v.z()); + if (maxval == 0) { + throw std::runtime_error("Invalid shape type"); + } + return OpaqueCoordinate<3>( + new NumberType(v.x() / maxval), + new NumberType(v.y() / maxval), + new NumberType(v.z() / maxval) + ); + } + + OpaqueCoordinate<4> opaque_plane(const cgal_plane_t& p) { + auto maxval = max_abs3(p.a(), p.b(), p.c()); + if (maxval == 0) { + throw std::runtime_error("Invalid shape type"); + } + return OpaqueCoordinate<4>( + new NumberType(p.a() / maxval), + new NumberType(p.b() / maxval), + new NumberType(p.c() / maxval), + new NumberType(p.d() / maxval) + ); + } + + cgal_vector_t wire_normal(const cgal_wire_t& wire) { + typename Kernel_::FT a(0), b(0), c(0); + if (wire.size() < 3) { + return cgal_vector_t(a, b, c); + } + for (std::size_t i = 0; i < wire.size(); ++i) { + const auto& curr = wire[i]; + const auto& next = wire[(i + 1) % wire.size()]; + a += (curr.y() - next.y()) * (curr.z() + next.z()); + b += (curr.z() - next.z()) * (curr.x() + next.x()); + c += (curr.x() - next.x()) * (curr.y() + next.y()); + } + return cgal_vector_t(a, b, c); + } + + cgal_point_t wire_centroid(const cgal_wire_t& wire) { + if (wire.empty()) { + throw std::runtime_error("Invalid shape type"); + } + std::array p{ Kernel_::FT(0), Kernel_::FT(0), Kernel_::FT(0) }; + for (const auto& point : wire) { + for (int i = 0; i < 3; ++i) { + p[i] += point.cartesian(i); + } + } + Kernel_::FT n(wire.size()); + return cgal_point_t(p[0] / n, p[1] / n, p[2] / n); + } + + Kernel_::FT wire_length(const cgal_wire_t& wire) { + Kernel_::FT len(0); + if (wire.size() < 2) { + return len; + } + for (std::size_t i = 1; i < wire.size(); ++i) { + len += CGAL::approximate_sqrt(CGAL::Segment_3(wire[i - 1], wire[i]).squared_length()); + } + if (wire.size() > 2) { + len += CGAL::approximate_sqrt(CGAL::Segment_3(wire.back(), wire.front()).squared_length()); + } + return len; + } + + Kernel_::FT wire_area(const cgal_wire_t& wire) { + Kernel_::FT area(0); + if (wire.size() < 3) { + return area; + } + const auto& origin = wire.front(); + for (std::size_t i = 1; i + 1 < wire.size(); ++i) { + auto v1 = wire[i] - origin; + auto v2 = wire[i + 1] - origin; + area += CGAL::approximate_sqrt(CGAL::cross_product(v1, v2).squared_length()) / Kernel_::FT(2); + } + return area; + } + + cgal_wire_t moved_wire(const cgal_wire_t& wire, const cgal_placement_t& trsf) { + cgal_wire_t result; + result.reserve(wire.size()); + for (const auto& point : wire) { + result.push_back(point.transform(trsf)); + } + return result; + } + + void write_off_point(std::stringstream& sstream, const cgal_point_t& point) { + sstream << "OFF\n1 0 0\n"; + sstream << point.x() << " " << point.y() << " " << point.z() << "\n"; + } + + void write_off_wire(std::stringstream& sstream, const cgal_wire_t& wire) { + const bool face = wire.size() >= 3; + sstream << "OFF\n" << wire.size() << " " << (face ? 1 : 0) << " 0\n"; + for (const auto& point : wire) { + sstream << point.x() << " " << point.y() << " " << point.z() << "\n"; + } + if (face) { + sstream << wire.size(); + for (std::size_t i = 0; i < wire.size(); ++i) { + sstream << " " << i; + } + sstream << "\n"; + } + } + template CGAL::Direction_3 newell(Facet& face) { typename Kernel_::FT a(0), b(0), c(0); @@ -102,10 +236,11 @@ namespace { ifcopenshell::geometry::CgalShape::CgalShape(const cgal_shape_t& shape, bool convex, Logger& logger) { shape_ = shape; convex_tag_ = convex; + auto& poly = std::get(*shape_); std::set faces_to_remove; - for (const auto& face : CGAL::faces(*shape_)) { + for (const auto& face : CGAL::faces(poly)) { auto V = newell(*face).to_vector(); CGAL::Plane_3 plane(CGAL::Point_3(), V); auto b1 = plane.base1(); @@ -127,7 +262,7 @@ ifcopenshell::geometry::CgalShape::CgalShape(const cgal_shape_t& shape, bool con std::vector> ps; - for (auto& he1 : CGAL::halfedges_around_face(face->halfedge(), *shape_)) { + for (auto& he1 : CGAL::halfedges_around_face(face->halfedge(), poly)) { const auto& source = he1->vertex()->point(); ps.push_back(transform_point(source)); } @@ -140,33 +275,41 @@ ifcopenshell::geometry::CgalShape::CgalShape(const cgal_shape_t& shape, bool con { for (auto& face : faces_to_remove) { - CGAL::Euler::remove_face(face->halfedge(), *shape_); + CGAL::Euler::remove_face(face->halfedge(), poly); } } +} - if (shape.size_of_facets() != 1) { - // the size_of_facets() == 1 check is for handling the specical case of - // storing a single point in a polyhedron as a degenerate triangle - // - // @todo come up with a proper variant for storing lower dimensional entities - - // @todo we don't have access to settings here so we don't know whether we should triangulate - // remove_degenerate_faces() is also called in the triangulate() call below though... - // CGAL::Polygon_mesh_processing::triangulate_faces(*shape_); - // CGAL::Polygon_mesh_processing::remove_degenerate_faces(*shape_); +ifcopenshell::geometry::CgalShape::CgalShape(const cgal_point_t& point, bool convex) { + shape_ = point; + convex_tag_ = convex; +} + +ifcopenshell::geometry::CgalShape::CgalShape(const cgal_wire_t& wire, bool convex) { + shape_ = wire; + convex_tag_ = convex; +} + +const cgal_shape_t& ifcopenshell::geometry::CgalShape::poly() const { +#ifndef IFOPSH_SIMPLE_KERNEL + to_poly(); +#endif + if (!shape_ || !std::holds_alternative(*shape_)) { + throw std::runtime_error("Invalid shape type"); } + return std::get(*shape_); } #ifndef IFOPSH_SIMPLE_KERNEL void ifcopenshell::geometry::CgalShape::to_poly() const { if (!shape_) { - shape_.emplace(); - - convert_to_polyhedron(*nef_, *shape_); - if (shape_->size_of_vertices() > 0) { + cgal_shape_t poly; + convert_to_polyhedron(*nef_, poly); + if (poly.size_of_vertices() > 0) { // @todo why is this necessary? we have the mark of the volumes? - CGAL::Polygon_mesh_processing::orient_to_bound_a_volume(*shape_); + CGAL::Polygon_mesh_processing::orient_to_bound_a_volume(poly); } + shape_ = poly; // nef_->convert_to_polyhedron(*shape_); } @@ -174,18 +317,23 @@ void ifcopenshell::geometry::CgalShape::to_poly() const { void ifcopenshell::geometry::CgalShape::to_nef() const { if (!nef_) { + auto shp = poly(); if (!convex_tag_) { - if (CGAL::Polygon_mesh_processing::does_self_intersect(*shape_)) { + if (CGAL::Polygon_mesh_processing::does_self_intersect(shp)) { throw std::runtime_error("Self-intersections detected, unable to proceed"); } } - nef_ = utils::create_nef_polyhedron(*shape_); + nef_ = utils::create_nef_polyhedron(shp); } } #endif void ifcopenshell::geometry::CgalShape::Triangulate(ifcopenshell::geometry::Settings settings, const ifcopenshell::geometry::taxonomy::matrix4& place, IfcGeom::Representation::Triangulation* t, int item_id, int surface_style_id, Logger& logger) const { - const bool all_triangles = std::all_of(shape_->facets_begin(), shape_->facets_end(), [](auto f) { return f.is_triangle(); }); + if (is_point() || is_wire()) { + return; + } + const auto& base_shape = poly(); + const bool all_triangles = std::all_of(base_shape.facets_begin(), base_shape.facets_end(), [](auto f) { return f.is_triangle(); }); const bool has_iden_transform = place.is_identity(); std::unique_ptr shape_copy_holder; @@ -193,10 +341,10 @@ void ifcopenshell::geometry::CgalShape::Triangulate(ifcopenshell::geometry::Sett if (!all_triangles || !has_iden_transform) { // A copy is made when triangulate_faces() is required or when vertex positions need be transformed - shape_copy_holder.reset(new cgal_shape_t(*this)); + shape_copy_holder.reset(new cgal_shape_t(base_shape)); shape_to_use = shape_copy_holder.get(); } else { - shape_to_use = &*shape_; + shape_to_use = const_cast(&base_shape); } const bool setting_use_original_edges = settings.get().get(); @@ -403,25 +551,33 @@ void ifcopenshell::geometry::CgalShape::Triangulate(ifcopenshell::geometry::Sett } void ifcopenshell::geometry::CgalShape::Serialize(const ifcopenshell::geometry::taxonomy::matrix4& place, std::string& r) const { - cgal_shape_t s = *this; - - if (!place.is_identity()) { - const auto& m = place.ccomponents(); - - // @todo check - const cgal_placement_t trsf( - m(0, 0), m(0, 1), m(0, 2), m(0, 3), - m(1, 0), m(1, 1), m(1, 2), m(1, 3), - m(2, 0), m(2, 1), m(2, 2), m(2, 3)); - - // Apply transformation - for (auto &vertex : s.vertex_handles()) { - vertex->point() = vertex->point().transform(trsf); - } - } - std::stringstream sstream; - sstream << s; + if (is_point()) { + auto p = point(); + if (!place.is_identity()) { + p = p.transform(make_transform(place)); + } + write_off_point(sstream, p); + } else if (is_wire()) { + auto w = wire(); + if (!place.is_identity()) { + w = moved_wire(w, make_transform(place)); + } + write_off_wire(sstream, w); + } else { + cgal_shape_t s = poly(); + + if (!place.is_identity()) { + const auto trsf = make_transform(place); + + // Apply transformation + for (auto &vertex : s.vertex_handles()) { + vertex->point() = vertex->point().transform(trsf); + } + } + + sstream << s; + } r = sstream.str(); } @@ -432,12 +588,26 @@ double ifcopenshell::geometry::CgalShape::bounding_box(void *& b) const { b = new CGAL::Bbox_3; } auto& bb = (*((CGAL::Bbox_3*)b)); - bb += CGAL::Polygon_mesh_processing::bbox(static_cast(*this)); + if (is_point()) { + bb += point().bbox(); + } else if (is_wire()) { + for (const auto& point : wire()) { + bb += point.bbox(); + } + } else { + bb += CGAL::Polygon_mesh_processing::bbox(poly()); + } return (bb.xmax() - bb.xmin()) * (bb.ymax() - bb.ymin()) * (bb.zmax() - bb.zmin()); } int ifcopenshell::geometry::CgalShape::num_vertices() const { - return (int) static_cast(*this).size_of_vertices(); + if (is_point()) { + return 1; + } + if (is_wire()) { + return (int) wire().size(); + } + return (int) poly().size_of_vertices(); } void ifcopenshell::geometry::CgalShape::set_box(void * b) { @@ -448,10 +618,13 @@ void ifcopenshell::geometry::CgalShape::set_box(void * b) { } int ifcopenshell::geometry::CgalShape::surface_genus() const { - to_poly(); - auto nv = shape_->size_of_vertices(); - auto ne = shape_->size_of_halfedges() / 2; - auto nf = shape_->size_of_facets(); + if (is_point() || is_wire()) { + return 0; + } + const auto& shp = poly(); + auto nv = shp.size_of_vertices(); + auto ne = shp.size_of_halfedges() / 2; + auto nf = shp.size_of_facets(); auto euler = nv - ne + nf; auto genus = (2 - euler) / 2; @@ -461,14 +634,22 @@ int ifcopenshell::geometry::CgalShape::surface_genus() const { bool ifcopenshell::geometry::CgalShape::is_manifold() const { // @todo ? - to_poly(); - return shape_->is_valid(); + return (is_point() || is_wire()) ? true : poly().is_valid(); } int ifcopenshell::geometry::CgalShape::num_edges() const { - to_poly(); - return (int) shape_->size_of_halfedges() / 2; + if (is_point()) { + return 0; + } + if (is_wire()) { + const auto n = wire().size(); + if (n < 2) { + return 0; + } + return (int)(n == 2 ? 1 : n); + } + return (int) poly().size_of_halfedges() / 2; } int ifcopenshell::geometry::CgalShape::num_faces() const @@ -479,7 +660,13 @@ int ifcopenshell::geometry::CgalShape::num_faces() const } else #endif if (shape_) { - return (int) shape_->size_of_facets(); + if (is_poly()) { + return (int) poly().size_of_facets(); + } + if (is_wire() && wire().size() >= 3) { + return 1; + } + return 0; } else { return 0; } @@ -487,46 +674,63 @@ int ifcopenshell::geometry::CgalShape::num_faces() const OpaqueNumber* ifcopenshell::geometry::CgalShape::CgalShape::length() { - to_poly(); Kernel_::FT len = 0; - for (auto it = shape_->edges_begin(); it != shape_->edges_end(); ++it) { - len += CGAL::approximate_sqrt(CGAL::Segment_3( - it->vertex()->point(), - it->next()->vertex()->point() - ).squared_length()); + if (is_wire()) { + len = wire_length(wire()); + } else if (!is_point()) { + const auto& shp = poly(); + for (auto it = shp.edges_begin(); it != shp.edges_end(); ++it) { + len += CGAL::approximate_sqrt(CGAL::Segment_3( + it->vertex()->point(), + it->opposite()->vertex()->point() + ).squared_length()); + } } return new NumberType(len); } OpaqueNumber* ifcopenshell::geometry::CgalShape::area() { - to_poly(); - auto s = *shape_; + if (is_wire()) { + return new NumberType(wire_area(wire())); + } + if (is_point()) { + return new NumberType(Kernel_::FT(0)); + } + auto s = poly(); CGAL::Polygon_mesh_processing::triangulate_faces(s); return new NumberType(CGAL::Polygon_mesh_processing::area(s)); } OpaqueNumber* ifcopenshell::geometry::CgalShape::volume() { - to_poly(); - auto s = *shape_; + if (is_point() || is_wire()) { + return new NumberType(Kernel_::FT(0)); + } + auto s = poly(); CGAL::Polygon_mesh_processing::triangulate_faces(s); return new NumberType(CGAL::Polygon_mesh_processing::volume(s)); } OpaqueCoordinate<3> ifcopenshell::geometry::CgalShape::position() { - to_poly(); - if (shape_->size_of_facets() == 1) { + if (is_point()) { + return opaque_point(point()); + } + if (is_wire()) { + return opaque_point(wire_centroid(wire())); + } + const auto& shp = poly(); + if (shp.size_of_facets() == 1) { // return centroid; // CGAL::Vector_3 p; - std::array p; - for (auto it = shape_->points_begin(); it != shape_->points_end(); ++it) { + std::array p{ Kernel_::FT(0), Kernel_::FT(0), Kernel_::FT(0) }; + for (auto it = shp.points_begin(); it != shp.points_end(); ++it) { for (int i = 0; i < 3; ++i) { p[i] += it->cartesian(i); } } - Kernel_::FT N(std::distance(shape_->points_begin(), shape_->points_end())); + Kernel_::FT N(std::distance(shp.points_begin(), shp.points_end())); for (int i = 0; i < 3; ++i) { p[i] /= N; } @@ -542,19 +746,19 @@ OpaqueCoordinate<3> ifcopenshell::geometry::CgalShape::position() OpaqueCoordinate<3> ifcopenshell::geometry::CgalShape::axis() { - to_poly(); - if (shape_->size_of_facets() == 1) { - auto pl = Plane_equation()(*shape_->facets_begin()); - std::array abc{ pl.a(), pl.b(), pl.c() }; - auto minel = std::min_element(abc.begin(), abc.end()); - auto maxel = std::max_element(abc.begin(), abc.end()); - auto maxval = ((-*minel) > *maxel) ? (-*minel) : *maxel; - - return OpaqueCoordinate<3>( - new NumberType(pl.a() / maxval), - new NumberType(pl.b() / maxval), - new NumberType(pl.c() / maxval) - ); + if (is_wire()) { + if (wire().size() == 2) { + return opaque_axis(wire()[1] - wire()[0]); + } + if (wire().size() >= 3) { + return opaque_axis(wire_normal(wire())); + } + throw std::runtime_error("Invalid shape type"); + } + auto shp = poly(); + if (shp.size_of_facets() == 1) { + auto pl = Plane_equation()(*shp.facets_begin()); + return opaque_axis(cgal_vector_t(pl.a(), pl.b(), pl.c())); } else { throw std::runtime_error("Invalid shape type"); } @@ -562,6 +766,14 @@ OpaqueCoordinate<3> ifcopenshell::geometry::CgalShape::axis() OpaqueCoordinate<4> ifcopenshell::geometry::CgalShape::plane_equation() { + if (is_wire() && wire().size() >= 3) { + auto normal = wire_normal(wire()); + return opaque_plane(cgal_plane_t(wire().front(), CGAL::Direction_3(normal))); + } + auto shp = poly(); + if (shp.size_of_facets() == 1) { + return opaque_plane(Plane_equation()(*shp.facets_begin())); + } throw std::runtime_error("Invalid shape type"); } @@ -612,75 +824,71 @@ ConversionResultShape * ifcopenshell::geometry::CgalShape::box() ConversionResultShape* ifcopenshell::geometry::CgalShape::wrap_in_compound() { - return new CgalShape(poly(), convex_tag_); + return clone(); } std::vector ifcopenshell::geometry::CgalShape::vertices() { - // @todo this is ridiculous - to_poly(); std::vector result; - for (auto& p : shape_->points()) { - std::vector ps = { - p, p, p - }; - - std::vector> ids(1); - ids.front().push_back(0); - ids.front().push_back(1); - ids.front().push_back(2); - - cgal_shape_t poly; - CGAL::Polygon_mesh_processing::polygon_soup_to_polygon_mesh(ps, ids, poly); - - result.push_back(new CgalShape(poly)); + if (is_point()) { + result.push_back(new CgalShape(point())); + return result; + } + if (is_wire()) { + for (const auto& p : wire()) { + result.push_back(new CgalShape(p)); + } + return result; + } + for (const auto& p : poly().points()) { + result.push_back(new CgalShape(p)); } return result; } std::vector ifcopenshell::geometry::CgalShape::edges() { - // @todo this is ridiculous - to_poly(); std::vector result; - for (auto& ed : shape_->edges()) { - std::vector ps = { - ed.vertex()->point(), - ed.vertex()->point(), - ed.next()->vertex()->point() - }; - - std::vector> ids(1); - ids.front().push_back(0); - ids.front().push_back(1); - ids.front().push_back(2); - - cgal_shape_t poly; - CGAL::Polygon_mesh_processing::polygon_soup_to_polygon_mesh(ps, ids, poly); - - result.push_back(new CgalShape(poly)); + if (is_point()) { + return result; + } + if (is_wire()) { + const auto& w = wire(); + for (std::size_t i = 1; i < w.size(); ++i) { + result.push_back(new CgalShape(cgal_wire_t{ w[i - 1], w[i] })); + } + if (w.size() > 2) { + result.push_back(new CgalShape(cgal_wire_t{ w.back(), w.front() })); + } + return result; + } + for (auto ed : poly().edges()) { + result.push_back(new CgalShape(cgal_wire_t{ ed.vertex()->point(), ed.opposite()->vertex()->point() })); } return result; } std::vector ifcopenshell::geometry::CgalShape::facets() { - to_poly(); std::vector result; - for (auto &face : faces(*shape_)) { + if (is_point()) { + return result; + } + if (is_wire()) { + if (wire().size() >= 3) { + result.push_back(new CgalShape(wire())); + } + return result; + } + for (auto face : faces(poly())) { std::vector ps; - std::vector> ids(1); auto it = face->facet_begin(); do { ps.push_back(it->vertex()->point()); - ids.front().push_back(ids.front().size()); } while (++it != face->facet_begin()); - cgal_shape_t poly; - CGAL::Polygon_mesh_processing::polygon_soup_to_polygon_mesh(ps, ids, poly); - - result.push_back(new CgalShape(poly)); + result.push_back(new CgalShape(ps)); } return result; } @@ -756,21 +964,21 @@ std::pair, OpaqueCoordinate<3>> ifcopenshell::geometry::Cgal ConversionResultShape* ifcopenshell::geometry::CgalShape::moved(ifcopenshell::geometry::taxonomy::matrix4::ptr place) const { - cgal_shape_t s = *this; + if (place->is_identity()) { + return clone(); + } - if (!place->is_identity()) { - const auto& m = place->ccomponents(); + const auto trsf = make_transform(*place); + if (is_point()) { + return new CgalShape(point().transform(trsf), convex_tag_); + } + if (is_wire()) { + return new CgalShape(moved_wire(wire(), trsf), convex_tag_); + } - // @todo check - const cgal_placement_t trsf( - m(0, 0), m(0, 1), m(0, 2), m(0, 3), - m(1, 0), m(1, 1), m(1, 2), m(1, 3), - m(2, 0), m(2, 1), m(2, 2), m(2, 3)); - - // Apply transformation - for (auto &vertex : s.vertex_handles()) { - vertex->point() = vertex->point().transform(trsf); - } + cgal_shape_t s = poly(); + for (auto &vertex : s.vertex_handles()) { + vertex->point() = vertex->point().transform(trsf); } return new CgalShape(s, convex_tag_); @@ -1012,4 +1220,4 @@ ConversionResultShape* ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition:: throw std::runtime_error("Not implemented"); } -#endif \ No newline at end of file +#endif diff --git a/src/ifcgeom/kernels/cgal/CgalConversionResult.h b/src/ifcgeom/kernels/cgal/CgalConversionResult.h index b361e13332..b441776d6e 100644 --- a/src/ifcgeom/kernels/cgal/CgalConversionResult.h +++ b/src/ifcgeom/kernels/cgal/CgalConversionResult.h @@ -39,6 +39,8 @@ #include #include +#include + #ifdef IFOPSH_SIMPLE_KERNEL #include @@ -179,13 +181,17 @@ namespace ifcopenshell { namespace geometry { class IFC_GEOMLIBRARY_API CgalShape : public IfcGeom::ConversionResultShape { private: + typedef std::variant cgal_shape_storage_t; + bool convex_tag_ = false; - mutable boost::optional shape_; + mutable boost::optional shape_; #ifndef IFOPSH_SIMPLE_KERNEL mutable boost::optional> nef_; #endif public: CgalShape(const cgal_shape_t& shape, bool convex = false, Logger& logger = Logger::Root()); + CgalShape(const cgal_point_t& point, bool convex = false); + CgalShape(const cgal_wire_t& wire, bool convex = false); #ifndef IFOPSH_SIMPLE_KERNEL CgalShape(const CGAL::Nef_polyhedron_3& shape, bool convex = false) { @@ -206,14 +212,29 @@ namespace ifcopenshell { namespace geometry { void to_poly() const {} #endif - operator const cgal_shape_t& () const { to_poly(); return *shape_; } - const cgal_shape_t& poly() const { to_poly(); return *shape_; } + operator const cgal_shape_t& () const { return poly(); } + const cgal_shape_t& poly() const; + bool is_poly() const { return shape_ && std::holds_alternative(*shape_); } + bool is_point() const { return shape_ && std::holds_alternative(*shape_); } + bool is_wire() const { return shape_ && std::holds_alternative(*shape_); } + const cgal_point_t& point() const { return std::get(*shape_); } + const cgal_wire_t& wire() const { return std::get(*shape_); } virtual void Triangulate(ifcopenshell::geometry::Settings settings, const ifcopenshell::geometry::taxonomy::matrix4& place, IfcGeom::Representation::Triangulation* t, int item_id, int surface_style_id, Logger& logger = Logger::Root()) const; virtual void Serialize(const ifcopenshell::geometry::taxonomy::matrix4& place, std::string&) const; virtual IfcGeom::ConversionResultShape* clone() const { - return new CgalShape(*shape_); + if (shape_) { + return std::visit([this](const auto& value) -> IfcGeom::ConversionResultShape* { + return new CgalShape(value, convex_tag_); + }, *shape_); + } +#ifndef IFOPSH_SIMPLE_KERNEL + if (nef_) { + return new CgalShape(*nef_, convex_tag_); + } +#endif + return nullptr; } virtual bool is_manifold() const; From 669e04664d08896ddce41102a72f02a768533e8b Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sat, 20 Jun 2026 13:30:17 +0200 Subject: [PATCH 6/8] Consistent policy on normalization in halfspace eq map() --- src/ifcgeom/ConversionResult.h | 5 +- .../kernels/cgal/CgalConversionResult.cpp | 71 ++++++++++--------- .../kernels/cgal/CgalConversionResult.h | 8 +-- .../kernels/cgal/nef_to_halfspace_tree.h | 45 +++++++----- .../OpenCascadeConversionResult.cpp | 4 +- .../opencascade/OpenCascadeConversionResult.h | 6 +- 6 files changed, 78 insertions(+), 61 deletions(-) diff --git a/src/ifcgeom/ConversionResult.h b/src/ifcgeom/ConversionResult.h index 058fe8f867..1b5946f796 100644 --- a/src/ifcgeom/ConversionResult.h +++ b/src/ifcgeom/ConversionResult.h @@ -24,6 +24,7 @@ #include "../ifcgeom/ConversionSettings.h" #include "../ifcgeom/taxonomy.h" +#include #include #include #include @@ -293,8 +294,8 @@ namespace IfcGeom { virtual ConversionResultShape* intersect(ConversionResultShape*) = 0; virtual ConversionResultShape* concat(ConversionResultShape*) = 0; - virtual void map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to) = 0; - virtual void map(const std::vector>& from, const std::vector>& to) = 0; + virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to) = 0; + virtual std::size_t map(const std::vector>& from, const std::vector>& to) = 0; virtual ConversionResultShape* moved(ifcopenshell::geometry::taxonomy::matrix4::ptr) const = 0; virtual bool surface_area_along_direction(double tol, const ifcopenshell::geometry::taxonomy::matrix4::ptr&, double& along_x, double& along_y, double& along_z) const = 0; diff --git a/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp b/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp index 749fd1435e..f8fa868946 100644 --- a/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp +++ b/src/ifcgeom/kernels/cgal/CgalConversionResult.cpp @@ -78,6 +78,31 @@ namespace { ); } + cgal_plane_t plane_from_opaque(const OpaqueCoordinate<4>& p) { +#ifdef IFOPSH_SIMPLE_KERNEL + return cgal_plane_t( + p.get(0)->to_double(), + p.get(1)->to_double(), + p.get(2)->to_double(), + p.get(3)->to_double() + ); +#else + return cgal_plane_t( + static_cast(p.get(0))->value(), + static_cast(p.get(1))->value(), + static_cast(p.get(2))->value(), + static_cast(p.get(3))->value() + ); +#endif + } + + void insert_normalized_plane_map(plane_map& mp, const OpaqueCoordinate<4>& from, const OpaqueCoordinate<4>& to) { + mp.insert({ + normalized_plane_for_map(plane_from_opaque(from)), + normalized_plane_for_map(plane_from_opaque(to)) + }); + } + cgal_vector_t wire_normal(const cgal_wire_t& wire) { typename Kernel_::FT a(0), b(0), c(0); if (wire.size() < 3) { @@ -984,11 +1009,11 @@ ConversionResultShape* ifcopenshell::geometry::CgalShape::moved(ifcopenshell::ge return new CgalShape(s, convex_tag_); } -void ifcopenshell::geometry::CgalShape::map(OpaqueCoordinate<4>&, OpaqueCoordinate<4>&) { +std::size_t ifcopenshell::geometry::CgalShape::map(OpaqueCoordinate<4>&, OpaqueCoordinate<4>&) { throw std::runtime_error("Not implemented"); } -void ifcopenshell::geometry::CgalShape::map(const std::vector>&, const std::vector>&) { +std::size_t ifcopenshell::geometry::CgalShape::map(const std::vector>&, const std::vector>&) { throw std::runtime_error("Not implemented"); } @@ -1165,27 +1190,16 @@ ConversionResultShape* ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition:: throw std::runtime_error("Not implemented"); } -void ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition::map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to) { +std::size_t ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition::map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to) { plane_map mp; - mp.insert({ - CGAL::Plane_3( - static_cast(from.get(0))->value(), - static_cast(from.get(1))->value(), - static_cast(from.get(2))->value(), - static_cast(from.get(3))->value() - ), - CGAL::Plane_3( - static_cast(to.get(0))->value(), - static_cast(to.get(1))->value(), - static_cast(to.get(2))->value(), - static_cast(to.get(3))->value() - ) - }); - auto nw = shape_->map(mp); + insert_normalized_plane_map(mp, from, to); + std::size_t mutated = 0; + auto nw = shape_->map(mp, mutated); shape_ = std::move(nw); + return mutated; } -void ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition::map(const std::vector>& froms, const std::vector>& tos) { +std::size_t ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition::map(const std::vector>& froms, const std::vector>& tos) { plane_map mp; if (froms.size() != tos.size()) { throw std::runtime_error("Expected equal size"); @@ -1195,23 +1209,12 @@ void ifcopenshell::geometry::CgalShapeHalfSpaceDecomposition::map(const std::vec for (; it < froms.end(); ++it, ++jt) { auto& from = *it; auto& to = *jt; - mp.insert({ - CGAL::Plane_3( - static_cast(from.get(0))->value(), - static_cast(from.get(1))->value(), - static_cast(from.get(2))->value(), - static_cast(from.get(3))->value() - ), - CGAL::Plane_3( - static_cast(to.get(0))->value(), - static_cast(to.get(1))->value(), - static_cast(to.get(2))->value(), - static_cast(to.get(3))->value() - ) - }); + insert_normalized_plane_map(mp, from, to); } - auto nw = shape_->map(mp); + std::size_t mutated = 0; + auto nw = shape_->map(mp, mutated); shape_ = std::move(nw); + return mutated; } diff --git a/src/ifcgeom/kernels/cgal/CgalConversionResult.h b/src/ifcgeom/kernels/cgal/CgalConversionResult.h index b441776d6e..d169f5495e 100644 --- a/src/ifcgeom/kernels/cgal/CgalConversionResult.h +++ b/src/ifcgeom/kernels/cgal/CgalConversionResult.h @@ -276,8 +276,8 @@ namespace ifcopenshell { namespace geometry { virtual ConversionResultShape* intersect(ConversionResultShape*); virtual ConversionResultShape* concat(ConversionResultShape*); - virtual void map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); - virtual void map(const std::vector>& from, const std::vector>& to); + virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); + virtual std::size_t map(const std::vector>& from, const std::vector>& to); virtual ConversionResultShape* moved(ifcopenshell::geometry::taxonomy::matrix4::ptr) const; virtual bool surface_area_along_direction(double tol, const ifcopenshell::geometry::taxonomy::matrix4::ptr&, double& along_x, double& along_y, double& along_z) const; @@ -347,8 +347,8 @@ namespace ifcopenshell { namespace geometry { return nullptr; } - virtual void map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); - virtual void map(const std::vector>& from, const std::vector>& to); + virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); + virtual std::size_t map(const std::vector>& from, const std::vector>& to); virtual ConversionResultShape* moved(ifcopenshell::geometry::taxonomy::matrix4::ptr) const; virtual bool surface_area_along_direction(double tol, const ifcopenshell::geometry::taxonomy::matrix4::ptr&, double& along_x, double& along_y, double& along_z) const { diff --git a/src/ifcgeom/kernels/cgal/nef_to_halfspace_tree.h b/src/ifcgeom/kernels/cgal/nef_to_halfspace_tree.h index 368e49a2d2..df90aa2b47 100644 --- a/src/ifcgeom/kernels/cgal/nef_to_halfspace_tree.h +++ b/src/ifcgeom/kernels/cgal/nef_to_halfspace_tree.h @@ -46,6 +46,7 @@ #include #include +#include #include #include #include @@ -116,6 +117,23 @@ template using plane_map = std::map>; // using plane_map = std::unordered_map>; +template +typename Kernel::Plane_3 normalized_plane_for_map(const typename Kernel::Plane_3& plane) { + std::array abc{ plane.a(), plane.b(), plane.c() }; + auto minel = std::min_element(abc.begin(), abc.end()); + auto maxel = std::max_element(abc.begin(), abc.end()); + auto maxval = ((-*minel) > *maxel) ? (-*minel) : *maxel; + if (maxval == 0) { + return plane; + } + return typename Kernel::Plane_3( + plane.a() / maxval, + plane.b() / maxval, + plane.c() / maxval, + plane.d() / maxval + ); +} + // Lexicographic comparator for CGAL Point_d (operator< is deleted in CGAL 6.x) struct Point_d_4d_Less { using Point_d = CGAL::Epick_d>::Point_d; @@ -264,7 +282,11 @@ class halfspace_tree { public: virtual CGAL::Nef_polyhedron_3 evaluate() const = 0; virtual void accumulate(std::list&) const = 0; - virtual std::unique_ptr map(const plane_map&) const = 0; + std::unique_ptr map(const plane_map& m) const { + std::size_t ignored = 0; + return map(m, ignored); + } + virtual std::unique_ptr map(const plane_map&, std::size_t& mutated) const = 0; virtual std::string dump(int level = 0) const = 0; virtual tree_type kind() const = 0; virtual void merge(CGAL::Nef_polyhedron_3&) const = 0; @@ -366,10 +388,10 @@ public: op->accumulate(points); } } - virtual std::unique_ptr> map(const plane_map& m) const { + virtual std::unique_ptr> map(const plane_map& m, std::size_t& mutated) const { decltype(operands_) mapped; for (auto& op : operands_) { - mapped.emplace_back(op->map(m)); + mapped.emplace_back(op->map(m, mutated)); } return std::unique_ptr>(new halfspace_tree_nary_branch(operation_, std::move(mapped))); } @@ -485,21 +507,12 @@ public: virtual void accumulate(std::list& points) const { points.push_back(plane_); } - virtual std::unique_ptr> map(const plane_map& m) const { - - std::array abc{ plane_.a(), plane_.b(), plane_.c() }; - auto minel = std::min_element(abc.begin(), abc.end()); - auto maxel = std::max_element(abc.begin(), abc.end()); - auto maxval = ((-*minel) > *maxel) ? (-*minel) : *maxel; - CGAL::Plane_3 pp( - plane_.a() / maxval, - plane_.b() / maxval, - plane_.c() / maxval, - plane_.d() / maxval - ); + virtual std::unique_ptr> map(const plane_map& m, std::size_t& mutated) const { + CGAL::Plane_3 pp = normalized_plane_for_map(plane_); auto it = m.find(pp); if (it != m.end()) { + ++mutated; return std::unique_ptr>(new halfspace_tree_plane(it->second)); } else { return std::unique_ptr>(new halfspace_tree_plane(plane_)); @@ -1358,4 +1371,4 @@ bool write_to_obj(const CGAL::Nef_polyhedron_3& a, std::ostream& ofs, si return volume_index == std::numeric_limits::max(); } -#endif \ No newline at end of file +#endif diff --git a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp b/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp index 38d7b12330..36e423730e 100644 --- a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp +++ b/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.cpp @@ -696,10 +696,10 @@ bool ifcopenshell::geometry::OpenCascadeShape::surface_area_along_direction(doub return true; } -void ifcopenshell::geometry::OpenCascadeShape::map(OpaqueCoordinate<4>&, OpaqueCoordinate<4>&) { +std::size_t ifcopenshell::geometry::OpenCascadeShape::map(OpaqueCoordinate<4>&, OpaqueCoordinate<4>&) { throw std::runtime_error("Not implemented"); } -void ifcopenshell::geometry::OpenCascadeShape::map(const std::vector>&, const std::vector>&) { +std::size_t ifcopenshell::geometry::OpenCascadeShape::map(const std::vector>&, const std::vector>&) { throw std::runtime_error("Not implemented"); } diff --git a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h b/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h index 927c9f3dac..643c797905 100644 --- a/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h +++ b/src/ifcgeom/kernels/opencascade/OpenCascadeConversionResult.h @@ -101,8 +101,8 @@ namespace ifcopenshell { virtual ConversionResultShape* intersect(ConversionResultShape*); virtual ConversionResultShape* concat(ConversionResultShape*); - virtual void map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); - virtual void map(const std::vector>& from, const std::vector>& to); + virtual std::size_t map(OpaqueCoordinate<4>& from, OpaqueCoordinate<4>& to); + virtual std::size_t map(const std::vector>& from, const std::vector>& to); virtual ConversionResultShape* moved(ifcopenshell::geometry::taxonomy::matrix4::ptr) const; virtual bool surface_area_along_direction(double tol, const ifcopenshell::geometry::taxonomy::matrix4::ptr&, double& along_x, double& along_y, double& along_z) const; @@ -113,4 +113,4 @@ namespace ifcopenshell { } } -#endif \ No newline at end of file +#endif From 7c9df9f98027f7c8baf633592c1ed51e11a12bf4 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sat, 20 Jun 2026 21:18:23 +0200 Subject: [PATCH 7/8] Don't erronously terminate on [SYN004] Non-entity type messages --- src/ifcparse/IfcFile.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ifcparse/IfcFile.cpp b/src/ifcparse/IfcFile.cpp index 1648842bcf..9f938ad41d 100644 --- a/src/ifcparse/IfcFile.cpp +++ b/src/ifcparse/IfcFile.cpp @@ -725,7 +725,8 @@ std::optionalas_entity() == nullptr) { - logger_.get().Message(Logger::LOG_ERROR, "SYN", 4, "Non entity type " + entity_type->name() + " at offset " + std::to_string(token_stream_[2].startPos)); + logger_.get().Message(Logger::LOG_ERROR, "SYN", 4, "Non-entity type " + entity_type->name() + " at offset " + std::to_string(token_stream_[2].startPos)); + current_id = 0; goto advance; } From 312be203c9cbdae0b541e6db445232a4e98fa064 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sat, 20 Jun 2026 21:30:31 +0200 Subject: [PATCH 8/8] SYN004 test case --- src/ifcopenshell-python/test/test_parse.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/ifcopenshell-python/test/test_parse.py diff --git a/src/ifcopenshell-python/test/test_parse.py b/src/ifcopenshell-python/test/test_parse.py new file mode 100644 index 0000000000..1a0a379e93 --- /dev/null +++ b/src/ifcopenshell-python/test/test_parse.py @@ -0,0 +1,19 @@ +import ifcopenshell + +def test_skip_over_non_entity_instance(): + data = """ +ISO-10303-21; +HEADER; +FILE_DESCRIPTION((''),'2;1'); +FILE_NAME('','',(''),(''),'','',''); +FILE_SCHEMA(('IFC2X3')); +ENDSEC; +DATA; +#1=IFCLENGTHMEASURE(0.1); +#5=IFCCARTESIANPOINT((0.,0.)); +ENDSEC; +END-ISO-10303-21; +""" + f = ifcopenshell.file.from_string(data) + print(ifcopenshell.get_log()) + f.by_id(5)