From 43bc9791144822cf506e8e2b7a40a8e719777e30 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sat, 29 Jun 2024 17:01:58 +1000 Subject: [PATCH 1/5] The MergeProject recipe now accepts a file object, not always a filepath --- src/ifcpatch/ifcpatch/recipes/MergeProject.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ifcpatch/ifcpatch/recipes/MergeProject.py b/src/ifcpatch/ifcpatch/recipes/MergeProject.py index d5a5fe191c..6bbc9bc9de 100644 --- a/src/ifcpatch/ifcpatch/recipes/MergeProject.py +++ b/src/ifcpatch/ifcpatch/recipes/MergeProject.py @@ -24,7 +24,7 @@ from logging import Logger class Patcher: - def __init__(self, src: str, file: ifcopenshell.file, logger: Logger, filepath: str): + def __init__(self, src: str, file: ifcopenshell.file, logger: Logger, filepath: Union[str, ifcopenshell.file]): """Merge two IFC models into one Note that other than combining the two IfcProject elements into one, no @@ -37,7 +37,6 @@ class Patcher: :param filepath: The filepath of the second IFC model to merge into the first. The first model is already specified as the input to IfcPatch. - :type filepath: str Example: @@ -51,8 +50,11 @@ class Patcher: self.filepath = filepath def patch(self): - source = ifcopenshell.open(self.filepath) - # make sure models units will match + if isinstance(self.filepath, ifcopenshell.file): + source = self.filepath + else: + source = ifcopenshell.open(self.filepath) + if (main_unit := self.get_unit_name(self.file)) != self.get_unit_name(source): source = ifcopenshell.util.unit.convert_file_length_units(source, main_unit) From 2129350ab725af37dfc2876eeb5130a74642ca29 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sat, 29 Jun 2024 17:03:26 +1000 Subject: [PATCH 2/5] Fix bug where MergeProject didn't clean up duplicate CRS and coordinate operations --- src/ifcpatch/ifcpatch/recipes/MergeProject.py | 13 +++++++++--- src/ifcpatch/test/test_MergeProject.py | 21 ++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/ifcpatch/ifcpatch/recipes/MergeProject.py b/src/ifcpatch/ifcpatch/recipes/MergeProject.py index 6bbc9bc9de..a65cbeeaac 100644 --- a/src/ifcpatch/ifcpatch/recipes/MergeProject.py +++ b/src/ifcpatch/ifcpatch/recipes/MergeProject.py @@ -90,11 +90,18 @@ class Patcher: equivalent_existing_context = self.get_equivalent_existing_context(added_context) if equivalent_existing_context: for inverse in self.file.get_inverse(added_context): + if self.file.schema != "IFC2X3": + if inverse.is_a("IfcCoordinateOperation"): + to_delete.add(inverse.id()) + continue ifcopenshell.util.element.replace_attribute(inverse, added_context, equivalent_existing_context) - to_delete.add(added_context) + to_delete.add(added_context.id()) - for added_context in to_delete: - ifcopenshell.util.element.remove_deep2(self.file, added_context) + for element_id in to_delete: + try: + ifcopenshell.util.element.remove_deep2(self.file, self.file.by_id(element_id)) + except: + pass def get_equivalent_existing_context( self, added_context: ifcopenshell.entity_instance diff --git a/src/ifcpatch/test/test_MergeProject.py b/src/ifcpatch/test/test_MergeProject.py index 93e9b9bbb5..685f4326a5 100644 --- a/src/ifcpatch/test/test_MergeProject.py +++ b/src/ifcpatch/test/test_MergeProject.py @@ -33,9 +33,11 @@ class TestMergeProject(test.bootstrap.IFC4): if ifc_file is None: ifc_file = ifcopenshell.file(schema=self.file.schema) - project = ifcopenshell.api.run("root.create_entity", ifc_file, ifc_class="IfcProject") + ifcopenshell.api.run("root.create_entity", ifc_file, ifc_class="IfcProject") unit = ifcopenshell.api.run("unit.add_si_unit", ifc_file, unit_type="LENGTHUNIT", prefix=prefix) ifcopenshell.api.run("unit.assign_unit", ifc_file, units=[unit]) + model = ifcopenshell.api.context.add_context(ifc_file, "Model") + ifcopenshell.api.context.add_context(ifc_file, "Model", "Body", "MODEL_VIEW", parent=model) matrix = np.eye(4) matrix[:, 3] = (1, 2, 3, 1) @@ -50,6 +52,7 @@ class TestMergeProject(test.bootstrap.IFC4): second_file.write(temp_path) output = ifcpatch.execute({"file": self.file, "recipe": "MergeProject", "arguments": [str(temp_path)]}) + assert self.file == output assert len(output.by_type("IfcWall")) == 2 wall1, wall2 = output.by_type("IfcWall") @@ -61,6 +64,22 @@ class TestMergeProject(test.bootstrap.IFC4): matrix[:, 3] = (1, 2, 3, 1) assert to_tuple(placement1) == to_tuple(placement2) == to_tuple(matrix) + def test_reusing_geometric_contexts(self): + self.file = self.setup_project(self.file) + second_file = self.setup_project() + output = ifcpatch.execute({"file": self.file, "recipe": "MergeProject", "arguments": [second_file]}) + assert len(output.by_type("IfcGeometricRepresentationContext")) == 2 + + def test_using_the_georeferencing_of_the_original_project(self): + if self.file.schema == "IFC2X3": + return + self.file = self.setup_project(self.file) + second_file = self.setup_project() + ifcopenshell.api.georeference.add_georeferencing(self.file) + ifcopenshell.api.georeference.add_georeferencing(second_file) + output = ifcpatch.execute({"file": self.file, "recipe": "MergeProject", "arguments": [second_file]}) + assert len(output.by_type("IfcProjectedCRS")) == 1 + assert len(output.by_type("IfcMapConversion")) == 1 class TestMergeProjectIFC2X3(test.bootstrap.IFC2X3, TestMergeProject): pass From 0d401c9272e0834e62e04f90d75eb4c8adecae32 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sat, 29 Jun 2024 17:04:27 +1000 Subject: [PATCH 3/5] Fix bug where migrating length units didn't change properties. Unit utility for checking attribute types now considers select types too Changing properties is very critical for IFC2X3 georeferencing which is stored in a pset. --- .../ifcopenshell/util/unit.py | 24 +++++++++++++++++-- .../test/util/test_unit.py | 10 ++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/util/unit.py b/src/ifcopenshell-python/ifcopenshell/util/unit.py index 475aca0f71..45905ca595 100644 --- a/src/ifcopenshell-python/ifcopenshell/util/unit.py +++ b/src/ifcopenshell-python/ifcopenshell/util/unit.py @@ -749,10 +749,23 @@ def format_length( def is_attr_type( content_type: ifcopenshell_wrapper.parameter_type, ifc_unit_type_name: str, + include_select_types: bool = True ) -> Union[ifcopenshell_wrapper.type_declaration, None]: cur_decl = content_type + + if include_select_types: + if hasattr(cur_decl, "select_list"): + for select_item in cur_decl.select_list(): + if is_attr_type(select_item, ifc_unit_type_name): + return select_item + while hasattr(cur_decl, "declared_type") is True: cur_decl = cur_decl.declared_type() + if include_select_types: + if hasattr(cur_decl, "select_list"): + for select_item in cur_decl.select_list(): + if is_attr_type(select_item, ifc_unit_type_name): + return select_item if hasattr(cur_decl, "name") is False: continue if cur_decl.name() == ifc_unit_type_name: @@ -800,6 +813,9 @@ def iter_element_and_attributes_per_type( if val is None: continue + if isinstance(val, ifcopenshell.entity_instance) and not val.is_a(attr_type_name): + continue + yield element, attr, val @@ -833,8 +849,12 @@ def convert_file_length_units(ifc_file: ifcopenshell.file, target_units: str = " # Traverse all elements and their nested attributes in the file and convert them for element, attr, val in iter_element_and_attributes_per_type(file_patched, "IfcLengthMeasure"): - new_value = convert_value(val) - setattr(element, attr.name(), new_value) + if isinstance(val, ifcopenshell.entity_instance): + new_value = convert_value(val.wrappedValue) + getattr(element, attr.name()).wrappedValue = new_value + else: + new_value = convert_value(val) + setattr(element, attr.name(), new_value) file_patched.remove(old_length) unit_assignment.Units = tuple([new_length, *unit_assignment.Units]) diff --git a/src/ifcopenshell-python/test/util/test_unit.py b/src/ifcopenshell-python/test/util/test_unit.py index 59b3c3371b..29a09fea5c 100644 --- a/src/ifcopenshell-python/test/util/test_unit.py +++ b/src/ifcopenshell-python/test/util/test_unit.py @@ -82,3 +82,13 @@ class TestFormatLength(test.bootstrap.IFC4): assert ( subject.format_length(25.23, 4, unit_system="imperial", input_unit="inch", output_unit="inch") == '25 1/4"' ) + + +class TestIsAttrType(test.bootstrap.IFC4): + def test_run(self): + schema = ifcopenshell.schema_by_name("IFC4") + declaration = schema.declaration_by_name("IfcPropertySingleValue") + nominal_value = declaration.attribute_by_index(2).type_of_attribute() + assert subject.is_attr_type(nominal_value, "IfcValue") + assert subject.is_attr_type(nominal_value, "IfcLengthMeasure") + assert not subject.is_attr_type(nominal_value, "IfcLengthMeasure", include_select_types=False) From 9a871f145918f053fc6ba90b3606db43a1db748c Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sat, 29 Jun 2024 17:05:06 +1000 Subject: [PATCH 4/5] Merging projects can now handle projects with different false origins Warning: I haven't yet looked at rotations. Yikes. No CRS reprojection either. --- src/ifcpatch/ifcpatch/recipes/MergeProject.py | 32 +++++++++---- src/ifcpatch/test/test_MergeProject.py | 45 ++++++++++++++++++- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/ifcpatch/ifcpatch/recipes/MergeProject.py b/src/ifcpatch/ifcpatch/recipes/MergeProject.py index a65cbeeaac..412a45b256 100644 --- a/src/ifcpatch/ifcpatch/recipes/MergeProject.py +++ b/src/ifcpatch/ifcpatch/recipes/MergeProject.py @@ -16,9 +16,12 @@ # You should have received a copy of the GNU Lesser General Public License # along with IfcPatch. If not, see . +import numpy as np import ifcopenshell -import ifcopenshell.util.element import ifcopenshell.util.unit +import ifcopenshell.util.element +import ifcopenshell.util.geolocation +from ifcpatch.recipes.SetFalseOrigin import Patcher as SetFalseOrigin from typing import Union from logging import Logger @@ -51,12 +54,25 @@ class Patcher: def patch(self): if isinstance(self.filepath, ifcopenshell.file): - source = self.filepath + other = self.filepath else: - source = ifcopenshell.open(self.filepath) + other = ifcopenshell.open(self.filepath) - if (main_unit := self.get_unit_name(self.file)) != self.get_unit_name(source): - source = ifcopenshell.util.unit.convert_file_length_units(source, main_unit) + if (main_unit := self.get_unit_name(self.file)) != self.get_unit_name(other): + other = ifcopenshell.util.unit.convert_file_length_units(other, main_unit) + + existing_enh = np.array( + ifcopenshell.util.geolocation.auto_xyz2enh(self.file, 0, 0, 0, should_return_in_map_units=False) + ) + other_enh = np.array( + ifcopenshell.util.geolocation.auto_xyz2enh(other, 0, 0, 0, should_return_in_map_units=False) + ) + + if not np.allclose(existing_enh, other_enh): + x, y, z = ifcopenshell.util.geolocation.auto_enh2xyz(other, *existing_enh, is_specified_in_map_units=False) + e, n, h = existing_enh + # For now don't handle rotation because my brain is going to explode + SetFalseOrigin("", other, self.logger, name="", x=x, y=y, z=z, e=e, n=n, h=h).patch() self.existing_contexts: list[ifcopenshell.entity_instance] = self.file.by_type( "IfcGeometricRepresentationContext" @@ -64,13 +80,13 @@ class Patcher: self.added_contexts: set[ifcopenshell.entity_instance] = set() original_project = self.file.by_type("IfcProject")[0] - merged_project = self.file.add(source.by_type("IfcProject")[0]) + merged_project = self.file.add(other.by_type("IfcProject")[0]) - for element in source.by_type("IfcGeometricRepresentationContext"): + for element in other.by_type("IfcGeometricRepresentationContext"): new = self.file.add(element) self.added_contexts.add(new) - for element in source: + for element in other: self.file.add(element) for inverse in self.file.get_inverse(merged_project): diff --git a/src/ifcpatch/test/test_MergeProject.py b/src/ifcpatch/test/test_MergeProject.py index 685f4326a5..6be9305e82 100644 --- a/src/ifcpatch/test/test_MergeProject.py +++ b/src/ifcpatch/test/test_MergeProject.py @@ -18,7 +18,7 @@ import ifcpatch import ifcopenshell -import ifcopenshell.api +import ifcopenshell.api.georeference import ifcopenshell.util.placement import test.bootstrap import tempfile @@ -81,5 +81,48 @@ class TestMergeProject(test.bootstrap.IFC4): assert len(output.by_type("IfcProjectedCRS")) == 1 assert len(output.by_type("IfcMapConversion")) == 1 + def test_shifting_the_source_project_to_match_the_original_project_origin(self): + self.file = self.setup_project(self.file) + second_file = self.setup_project() + ifcopenshell.api.georeference.add_georeferencing(self.file) + ifcopenshell.api.georeference.edit_georeferencing( + self.file, coordinate_operation={"Eastings": 10, "Northings": 20}, projected_crs={"Name": "EPSG:1234"} + ) + ifcopenshell.api.georeference.add_georeferencing(second_file) + ifcopenshell.api.georeference.edit_georeferencing( + second_file, coordinate_operation={"Eastings": 30000, "Northings": 40000}, projected_crs={"Name": "EPSG:0"} + ) + + # Original file is in meters + wall1 = self.file.by_type("IfcWall")[0] + m1 = ifcopenshell.util.placement.get_local_placement(wall1.ObjectPlacement) + assert np.allclose(m1[:, 3], (1, 2, 3, 1)) + global_m1 = ifcopenshell.util.geolocation.auto_local2global(self.file, m1, should_return_in_map_units=False) + assert np.allclose(global_m1[:, 3], (11, 22, 3, 1)) + + # Second file is in millimeters with a different false origin + wall1 = second_file.by_type("IfcWall")[0] + m1 = ifcopenshell.util.placement.get_local_placement(wall1.ObjectPlacement) + assert np.allclose(m1[:, 3], (1000, 2000, 3000, 1)) + global_m1 = ifcopenshell.util.geolocation.auto_local2global(second_file, m1, should_return_in_map_units=False) + assert np.allclose(global_m1[:, 3], (31000, 42000, 3000, 1)) + + output = ifcpatch.execute({"file": self.file, "recipe": "MergeProject", "arguments": [second_file]}) + + # In the future we may use proj to support reprojection from different CRSes. For now... nope! + if self.file.schema != "IFC2X3": + assert output.by_type("IfcProjectedCRS")[0].Name == "EPSG:1234" + + # The results should be in meters with the false origin of the original file + params = ifcopenshell.util.geolocation.get_helmert_transformation_parameters(output) + assert params.e == 10 + assert params.n == 20 + wall1, wall2 = output.by_type("IfcWall") + m1 = ifcopenshell.util.placement.get_local_placement(wall1.ObjectPlacement) + m2 = ifcopenshell.util.placement.get_local_placement(wall2.ObjectPlacement) + assert np.allclose(m1[:, 3], (1, 2, 3, 1)) + assert np.allclose(m2[:, 3], (21, 22, 3, 1)) + + class TestMergeProjectIFC2X3(test.bootstrap.IFC2X3, TestMergeProject): pass From 0544bbc8869d56f5496bd44431f5670d7c96d1c3 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sat, 29 Jun 2024 21:39:38 +1000 Subject: [PATCH 5/5] Temporarily disable material listeners as they need to be rewritten anyway (ideally without listeners) with the new style system At least now we can create types for testing --- src/blenderbim/blenderbim/bim/module/model/product.py | 2 ++ src/blenderbim/blenderbim/bim/module/project/ui.py | 1 - src/blenderbim/blenderbim/bim/module/type/operator.py | 11 ++--------- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/blenderbim/blenderbim/bim/module/model/product.py b/src/blenderbim/blenderbim/bim/module/model/product.py index b7dfa0e510..a371cbadcd 100644 --- a/src/blenderbim/blenderbim/bim/module/model/product.py +++ b/src/blenderbim/blenderbim/bim/module/model/product.py @@ -514,6 +514,7 @@ def regenerate_profile_usage(usecase_path, ifc_file, settings): def ensure_material_assigned(usecase_path: str, ifc_file: ifcopenshell.file, settings: dict[str, Any]) -> None: + return # TODO ensure this now works with the new approach of styles elements = settings["products"] material = settings.get("material") if material: @@ -544,6 +545,7 @@ def ensure_material_assigned(usecase_path: str, ifc_file: ifcopenshell.file, set def ensure_material_unassigned(usecase_path: str, ifc_file: ifcopenshell.file, settings: dict[str, Any]) -> None: + return # TODO ensure this now works with the new approach of styles elements = settings["products"] # unassign_material could be called when product is about to get removed diff --git a/src/blenderbim/blenderbim/bim/module/project/ui.py b/src/blenderbim/blenderbim/bim/module/project/ui.py index c6fabef9e1..62a5b38123 100644 --- a/src/blenderbim/blenderbim/bim/module/project/ui.py +++ b/src/blenderbim/blenderbim/bim/module/project/ui.py @@ -264,7 +264,6 @@ class BIM_PT_project(Panel): row = self.layout.row(align=True) row.prop(props, "ifc_file", text="") row.operator("bim.select_ifc_file", icon="FILE_FOLDER", text="") - row.operator("bim.unload_project", text="", icon="CANCEL") class BIM_PT_new_project_wizard(Panel): diff --git a/src/blenderbim/blenderbim/bim/module/type/operator.py b/src/blenderbim/blenderbim/bim/module/type/operator.py index 7842c67155..f3ce40a885 100644 --- a/src/blenderbim/blenderbim/bim/module/type/operator.py +++ b/src/blenderbim/blenderbim/bim/module/type/operator.py @@ -310,7 +310,7 @@ class AddType(bpy.types.Operator, tool.Ifc.Operator): if materials: material = materials[0] # Arbitrarily pick a material else: - material = self.add_default_material() + material = ifcopenshell.api.run("material.add_material", tool.Ifc.get(), name="Unknown") rel = ifcopenshell.api.run( "material.assign_material", ifc_file, products=[element], type="IfcMaterialLayerSet" ) @@ -344,7 +344,7 @@ class AddType(bpy.types.Operator, tool.Ifc.Operator): if materials: material = materials[0] # Arbitrarily pick a material else: - material = self.add_default_material() + material = ifcopenshell.api.run("material.add_material", tool.Ifc.get(), name="Unknown") if template == "PROFILESET": named_profiles = [p for p in ifc_file.by_type("IfcProfileDef") if p.ProfileName] if named_profiles: @@ -495,13 +495,6 @@ class AddType(bpy.types.Operator, tool.Ifc.Operator): props.type_class = props.type_class return {"FINISHED"} - def add_default_material(self): - material = ifcopenshell.api.run("material.add_material", tool.Ifc.get(), name="Unknown") - blender_material = bpy.data.materials.new(material.Name) - tool.Ifc.link(material, blender_material) - blender_material.use_fake_user = True - return material - class RemoveType(bpy.types.Operator, tool.Ifc.Operator): bl_idname = "bim.remove_type"