From ccd7db06f8b71c5194b3c2b47a046c9404d2d215 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sun, 5 Feb 2023 17:09:13 +1100 Subject: [PATCH] Fix #2611. Address multiple situations in the UI where the users could create invalid material data without an indication that it is invalid. --- .../bim/module/material/operator.py | 21 ++-- src/blenderbim/blenderbim/core/material.py | 2 + src/blenderbim/blenderbim/core/tool.py | 1 + src/blenderbim/blenderbim/tool/material.py | 12 ++ src/blenderbim/test/core/test_material.py | 8 ++ src/blenderbim/test/tool/test_material.py | 17 ++- .../test/api/material/test_assign_material.py | 97 +++++++++++++++ .../api/material/test_unassign_material.py | 111 ++++++++++++++++++ 8 files changed, 258 insertions(+), 11 deletions(-) create mode 100644 src/ifcopenshell-python/test/api/material/test_assign_material.py create mode 100644 src/ifcopenshell-python/test/api/material/test_unassign_material.py diff --git a/src/blenderbim/blenderbim/bim/module/material/operator.py b/src/blenderbim/blenderbim/bim/module/material/operator.py index 1f1cf41701..a8a075bc90 100644 --- a/src/blenderbim/blenderbim/bim/module/material/operator.py +++ b/src/blenderbim/blenderbim/bim/module/material/operator.py @@ -242,10 +242,11 @@ class RemoveConstituent(bpy.types.Operator, tool.Ifc.Operator): constituent: bpy.props.IntProperty() def _execute(self, context): - obj = bpy.data.objects.get(self.obj) if self.obj else context.active_object - self.file = IfcStore.get_file() + for inverse in tool.Ifc.get().get_inverse(layer): + if inverse.is_a("IfcMaterialConstituentSet") and len(inverse.MaterialConstituents) == 1: + return ifcopenshell.api.run( - "material.remove_constituent", self.file, **{"constituent": self.file.by_id(self.constituent)} + "material.remove_constituent", tool.Ifc.get(), constituent=tool.Ifc.get().by_id(self.constituent) ) @@ -276,9 +277,10 @@ class RemoveProfile(bpy.types.Operator, tool.Ifc.Operator): profile: bpy.props.IntProperty() def _execute(self, context): - obj = bpy.data.objects.get(self.obj) if self.obj else context.active_object - self.file = IfcStore.get_file() - ifcopenshell.api.run("material.remove_profile", self.file, **{"profile": self.file.by_id(self.profile)}) + for inverse in tool.Ifc.get().get_inverse(layer): + if inverse.is_a("IfcMaterialProfileSet") and len(inverse.MaterialProfiles) == 1: + return + ifcopenshell.api.run("material.remove_profile", tool.Ifc.get(), profile=tool.Ifc.get().by_id(self.profile)) class AddLayer(bpy.types.Operator, tool.Ifc.Operator): @@ -333,9 +335,10 @@ class RemoveLayer(bpy.types.Operator, tool.Ifc.Operator): layer: bpy.props.IntProperty() def _execute(self, context): - obj = bpy.data.objects.get(self.obj) if self.obj else context.active_object - self.file = IfcStore.get_file() - ifcopenshell.api.run("material.remove_layer", self.file, **{"layer": self.file.by_id(self.layer)}) + for inverse in tool.Ifc.get().get_inverse(layer): + if inverse.is_a("IfcMaterialLayerSet") and len(inverse.MaterialLayers) == 1: + return + ifcopenshell.api.run("material.remove_layer", tool.Ifc.get(), layer=tool.Ifc.get().by_id(self.layer)) class AddListItem(bpy.types.Operator, tool.Ifc.Operator): diff --git a/src/blenderbim/blenderbim/core/material.py b/src/blenderbim/blenderbim/core/material.py index e0835f92f8..f22698ee34 100644 --- a/src/blenderbim/blenderbim/core/material.py +++ b/src/blenderbim/blenderbim/core/material.py @@ -44,6 +44,8 @@ def add_material_set(ifc, material, set_type=None): def remove_material(ifc, material_tool, style, material=None): + if material_tool.is_material_used_in_sets(material): + return obj = ifc.get_object(material) ifc.unlink(element=material) ifc.run("material.remove_material", material=material) diff --git a/src/blenderbim/blenderbim/core/tool.py b/src/blenderbim/blenderbim/core/tool.py index a158fb779b..5ddbd13686 100644 --- a/src/blenderbim/blenderbim/core/tool.py +++ b/src/blenderbim/blenderbim/core/tool.py @@ -317,6 +317,7 @@ class Material: def get_name(cls, obj): pass def import_material_definitions(cls, material_type): pass def is_editing_materials(cls): pass + def is_material_used_in_sets(cls, material): pass def select_elements(cls, elements): pass diff --git a/src/blenderbim/blenderbim/tool/material.py b/src/blenderbim/blenderbim/tool/material.py index 617a8592d6..60bf3e32e6 100644 --- a/src/blenderbim/blenderbim/tool/material.py +++ b/src/blenderbim/blenderbim/tool/material.py @@ -89,6 +89,18 @@ class Material(blenderbim.core.tool.Material): def is_editing_materials(cls): return bpy.context.scene.BIMMaterialProperties.is_editing + @classmethod + def is_material_used_in_sets(cls, material): + for inverse in tool.Ifc.get().get_inverse(material): + if inverse.is_a() in [ + "IfcMaterialProfile", + "IfcMaterialLayer", + "IfcMaterialConstituent", + "IfcMaterialList", + ]: + return True + return False + @classmethod def select_elements(cls, elements): for element in elements: diff --git a/src/blenderbim/test/core/test_material.py b/src/blenderbim/test/core/test_material.py index e589c55357..9b6a30c70b 100644 --- a/src/blenderbim/test/core/test_material.py +++ b/src/blenderbim/test/core/test_material.py @@ -94,6 +94,7 @@ class TestAddMaterialSet: class TestRemoveMaterial: def test_removing_a_material(self, ifc, material, style): + material.is_material_used_in_sets("material").should_be_called().will_return(False) ifc.get_object("material").should_be_called().will_return(None) ifc.unlink(element="material").should_be_called() ifc.run("material.remove_material", material="material").should_be_called() @@ -101,6 +102,7 @@ class TestRemoveMaterial: subject.remove_material(ifc, material, style, material="material") def test_removing_a_material_and_reloading_imported_materials(self, ifc, material, style): + material.is_material_used_in_sets("material").should_be_called().will_return(False) ifc.get_object("material").should_be_called().will_return(None) ifc.unlink(element="material").should_be_called() ifc.run("material.remove_material", material="material").should_be_called() @@ -110,6 +112,7 @@ class TestRemoveMaterial: subject.remove_material(ifc, material, style, material="material") def test_removing_a_material_object_if_it_has_no_style(self, ifc, material, style): + material.is_material_used_in_sets("material").should_be_called().will_return(False) ifc.get_object("material").should_be_called().will_return("obj") ifc.unlink(element="material").should_be_called() ifc.run("material.remove_material", material="material").should_be_called() @@ -119,6 +122,7 @@ class TestRemoveMaterial: subject.remove_material(ifc, material, style, material="material") def test_preserving_a_material_object_if_it_is_still_used_as_a_style(self, ifc, material, style): + material.is_material_used_in_sets("material").should_be_called().will_return(False) ifc.get_object("material").should_be_called().will_return("obj") ifc.unlink(element="material").should_be_called() ifc.run("material.remove_material", material="material").should_be_called() @@ -126,6 +130,10 @@ class TestRemoveMaterial: material.is_editing_materials().should_be_called().will_return(False) subject.remove_material(ifc, material, style, material="material") + def test_not_removing_a_material_if_it_is_used_in_a_material_set(self, ifc, material, style): + material.is_material_used_in_sets("material").should_be_called().will_return(True) + subject.remove_material(ifc, material, style, material="material") + class TestRemoveMaterialSet: def test_run(self, ifc, material): diff --git a/src/blenderbim/test/tool/test_material.py b/src/blenderbim/test/tool/test_material.py index 430acbb04e..794669545b 100644 --- a/src/blenderbim/test/tool/test_material.py +++ b/src/blenderbim/test/tool/test_material.py @@ -153,9 +153,22 @@ class TestImportMaterialDefinitions(NewFile): class TestIsEditingMaterials(NewFile): def test_run(self): bpy.context.scene.BIMMaterialProperties.is_editing = False - subject.is_editing_materials() is False + assert subject.is_editing_materials() is False bpy.context.scene.BIMMaterialProperties.is_editing = True - subject.is_editing_materials() is True + assert subject.is_editing_materials() is True + + +class TestIsMaterialUsedInSets(NewFile): + def test_run(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + material_set = ifc.createIfcMaterialLayerSet() + material_set_item = ifc.createIfcMaterialLayer() + material = ifc.createIfcMaterial() + assert subject.is_material_used_in_sets(material) is False + material_set.MaterialLayers = [material_set_item] + material_set_item.Material = material + assert subject.is_material_used_in_sets(material) is True class TestSelectElements(NewFile): diff --git a/src/ifcopenshell-python/test/api/material/test_assign_material.py b/src/ifcopenshell-python/test/api/material/test_assign_material.py new file mode 100644 index 0000000000..9e616cbb95 --- /dev/null +++ b/src/ifcopenshell-python/test/api/material/test_assign_material.py @@ -0,0 +1,97 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2023 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +import test.bootstrap +import ifcopenshell.api +import ifcopenshell.util.element + + +class TestAssignMaterial(test.bootstrap.IFC4): + def test_assign_element_single_material(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterial", material=material) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + assert element.HasAssociations[0].RelatingMaterial == material + + def test_assign_type_single_material(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterial", material=material) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + assert element.HasAssociations[0].RelatingMaterial == material + + def test_assign_type_material_layer_set(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialLayerSet") + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + material_set = element.HasAssociations[0].RelatingMaterial + assert material_set.is_a("IfcMaterialLayerSet") + assert not material_set.MaterialLayers + + def test_assign_type_material_layer_set_and_element_layer_set_usage(self): + element_type = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + ifcopenshell.api.run("type.assign_type", self.file, related_object=element, relating_type=element_type) + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element_type, type="IfcMaterialLayerSet") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialLayerSetUsage") + material_set = element_type.HasAssociations[0].RelatingMaterial + material_usage = element.HasAssociations[0].RelatingMaterial + assert material_usage.is_a("IfcMaterialLayerSetUsage") + assert material_usage.ForLayerSet == material_set + + def test_assign_type_material_profile_set(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialProfileSet") + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + material_set = element.HasAssociations[0].RelatingMaterial + assert material_set.is_a("IfcMaterialProfileSet") + assert not material_set.MaterialProfiles + + def test_assign_type_material_profile_set_and_element_profile_set_usage(self): + element_type = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + ifcopenshell.api.run("type.assign_type", self.file, related_object=element, relating_type=element_type) + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element_type, type="IfcMaterialProfileSet") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialProfileSetUsage") + material_set = element_type.HasAssociations[0].RelatingMaterial + material_usage = element.HasAssociations[0].RelatingMaterial + assert material_usage.is_a("IfcMaterialProfileSetUsage") + assert material_usage.ForProfileSet == material_set + + def test_assign_type_material_constituent_set(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialConstituentSet") + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + material_set = element.HasAssociations[0].RelatingMaterial + assert material_set.is_a("IfcMaterialConstituentSet") + assert not material_set.MaterialConstituents + + def test_assign_element_material_list(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialList", material=material) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + material_list = element.HasAssociations[0].RelatingMaterial + assert material_list.is_a("IfcMaterialList") + assert material_list.Materials[0] == material diff --git a/src/ifcopenshell-python/test/api/material/test_unassign_material.py b/src/ifcopenshell-python/test/api/material/test_unassign_material.py new file mode 100644 index 0000000000..2e2db1946c --- /dev/null +++ b/src/ifcopenshell-python/test/api/material/test_unassign_material.py @@ -0,0 +1,111 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2023 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +import test.bootstrap +import ifcopenshell.api +import ifcopenshell.util.element + + +class TestUnassignMaterial(test.bootstrap.IFC4): + def test_unassign_single_material(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterial", material=material) + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 0 + assert len(self.file.by_type("IfcWall")) == 1 + assert len(self.file.by_type("IfcMaterial")) == 1 + + def test_unassign_single_material_with_multiple_elements(self): + element1 = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + element2 = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element1, type="IfcMaterial", material=material) + ifcopenshell.api.run("material.assign_material", self.file, product=element2, type="IfcMaterial", material=material) + ifcopenshell.api.run("material.unassign_material", self.file, product=element2) + assert element1.HasAssociations + assert not element2.HasAssociations + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + assert len(self.file.by_type("IfcWall")) == 2 + assert len(self.file.by_type("IfcMaterial")) == 1 + + def test_unassign_material_layer_set_from_type(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialLayerSet") + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 0 + assert len(self.file.by_type("IfcWallType")) == 1 + assert len(self.file.by_type("IfcMaterialLayerSet")) == 1 + + def test_unassign_material_layer_set_usage_from_element(self): + element_type = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + ifcopenshell.api.run("type.assign_type", self.file, related_object=element, relating_type=element_type) + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element_type, type="IfcMaterialLayerSet") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialLayerSetUsage") + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + assert element_type.HasAssociations + assert len(self.file.by_type("IfcWallType")) == 1 + assert len(self.file.by_type("IfcWall")) == 1 + assert len(self.file.by_type("IfcMaterialLayerSet")) == 1 + assert len(self.file.by_type("IfcMaterialLayerSetUsage")) == 0 + + def test_unassign_material_profile_set_from_type(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialProfileSet") + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 0 + assert len(self.file.by_type("IfcWallType")) == 1 + assert len(self.file.by_type("IfcMaterialProfileSet")) == 1 + + def test_unassign_material_profile_set_usage_from_element(self): + element_type = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + ifcopenshell.api.run("type.assign_type", self.file, related_object=element, relating_type=element_type) + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element_type, type="IfcMaterialProfileSet") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialProfileSetUsage") + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 1 + assert element_type.HasAssociations + assert len(self.file.by_type("IfcWallType")) == 1 + assert len(self.file.by_type("IfcWall")) == 1 + assert len(self.file.by_type("IfcMaterialProfileSet")) == 1 + assert len(self.file.by_type("IfcMaterialProfileSetUsage")) == 0 + + def test_unassign_material_constituent_set_from_type(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialConstituentSet") + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 0 + assert len(self.file.by_type("IfcWallType")) == 1 + assert len(self.file.by_type("IfcMaterialConstituentSet")) == 1 + + def test_unassign_element_material_list(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + material = ifcopenshell.api.run("material.add_material", self.file, name="CON01") + ifcopenshell.api.run("material.assign_material", self.file, product=element, type="IfcMaterialList", material=material) + ifcopenshell.api.run("material.unassign_material", self.file, product=element) + assert len(self.file.by_type("IfcRelAssociatesMaterial")) == 0 + assert len(self.file.by_type("IfcMaterialList")) == 1 + assert len(self.file.by_type("IfcMaterial")) == 1