DANGER! Potentially fix a bunch of undo bugs by completely rebuilding undo id_map tracking with a much simpler solution.

In the past, we used undo_pre and redo_pre and a bunch of complex commit_link and rollback_link tracking to figure out what IDs and GUIDs were added / removed during transactions and how to handle invalidated objects in the id/guid maps.

I really don't know why I made it that complex.

Now we simply treat all data as bad and rebuild the either map. This means slower undos / redos for large projects, but in theory the maps will never invalidate.

This either fixes a lot of things or breaks a lot of things :) Watch out!

src/blenderbim/blenderbim/bim/__init__.py

@@ -170,9 +170,7 @@ def register():
     for cls in classes:
         bpy.utils.register_class(cls)
     bpy.app.handlers.depsgraph_update_post.append(on_register)
-    bpy.app.handlers.undo_pre.append(handler.undo_pre)
     bpy.app.handlers.undo_post.append(handler.undo_post)
-    bpy.app.handlers.redo_pre.append(handler.redo_pre)
     bpy.app.handlers.redo_post.append(handler.redo_post)
     bpy.app.handlers.load_post.append(handler.load_post)
     bpy.app.handlers.load_post.append(handler.loadIfcStore)

src/blenderbim/blenderbim/bim/handler.py

@@ -54,10 +54,12 @@ def mode_callback(obj, data):
 
 
 def name_callback(obj, data):
-    # TODO Do we still need this, now that we are monitoring the undo redo objects?
     try:
         obj.name
     except:
+        # The object is invalid but somehow still has a callback. Clear all
+        # msgbus subscriptions to prevent useless further triggers.
+        bpy.msgbus.clear_by_owner(obj)
         return  # In case the object RNA is gone during an undo / redo operation
     # Blender names are up to 63 UTF-8 bytes
     if len(bytes(obj.name, "utf-8")) >= 63:
@@ -203,24 +205,13 @@ def loadIfcStore(scene):
     IfcStore.relink_all_objects()
 
 
-@persistent
-def undo_pre(scene):
-    IfcStore.track_undo_redo_stack_object_map()
-
-
 @persistent
 def undo_post(scene):
     if IfcStore.last_transaction != bpy.context.scene.BIMProperties.last_transaction:
         IfcStore.last_transaction = bpy.context.scene.BIMProperties.last_transaction
         IfcStore.undo()
         purge_module_data()
-    IfcStore.track_undo_redo_stack_selected_objects()
+    tool.Ifc.rebuild_element_maps()
-    IfcStore.reload_undo_redo_stack_objects()
-
-
-@persistent
-def redo_pre(scene):
-    IfcStore.track_undo_redo_stack_object_map()
 
 
 @persistent
@@ -229,8 +220,7 @@ def redo_post(scene):
         IfcStore.last_transaction = bpy.context.scene.BIMProperties.last_transaction
         IfcStore.redo()
         purge_module_data()
-    IfcStore.track_undo_redo_stack_selected_objects()
+    tool.Ifc.rebuild_element_maps()
-    IfcStore.reload_undo_redo_stack_objects()
 
 
 def get_application(ifc):

src/blenderbim/blenderbim/bim/ifc.py

@@ -44,9 +44,6 @@ class IfcStore:
     classification_file = None
     library_path = ""
     library_file = None
-    element_listeners = set()
-    undo_redo_stack_objects = set()
-    undo_redo_stack_object_names = {}
     current_transaction = ""
     last_transaction = ""
     history = []
@@ -161,96 +158,6 @@ class IfcStore:
             return
         return obj
 
-    @staticmethod
-    def add_element_listener(callback):
-        IfcStore.element_listeners.add(callback)
-
-    @staticmethod
-    def track_undo_redo_stack_object_map():
-        """Keeps track of currently mapped object names, typically during undo and redo
-
-        When any Blender object is stored outside a Blender PointerProperty, such as
-        in a regular Python list, there is the likely probability that the object
-        will be invalidated when undo or redo occurs. Object invalidation seems to
-        occur whenever an object is affected during an operation.
-
-        For example, if an operator deletes a modifier on o1, then o1 will be invalidated.
-        """
-        for key, value in IfcStore.id_map.items():
-            try:
-                IfcStore.undo_redo_stack_object_names[key] = value.name
-            except:
-                continue
-
-    @staticmethod
-    def track_undo_redo_stack_selected_objects():
-        """Keeps track of selected object names, typically during undo and redo
-
-        When any Blender object is stored outside a Blender PointerProperty, such as
-        in a regular Python list, there is the likely probability that the object
-        will be invalidated when undo or redo occurs. Object invalidation seems to
-        occur for selected objects either pre/post undo/redo event, including
-        selected objects for consecutive undo/redos, and all children. This is
-        important because selected objects are often deleted from the scene.
-
-        So if I first select o1, then o2, then o3, then press undo, o3 will be
-        invalidated. If instead I press undo twice, o3 and o2 will be invalidated.
-        """
-        if bpy.context.active_object:
-            objects = set([o.name for o in bpy.context.selected_objects + [bpy.context.active_object]])
-            objects.update([o.name for o in bpy.context.active_object.children])
-        else:
-            objects = set([o.name for o in bpy.context.selected_objects])
-        for obj in bpy.context.selected_objects:
-            objects.update([o.name for o in obj.children])
-        IfcStore.undo_redo_stack_objects |= objects
-
-    @staticmethod
-    def reload_undo_redo_stack_objects():
-        """Reloads any invalidated objects after undo or redo
-
-        After an undo or redo operation, objects may have been invalidated in
-        our id_map and guid_map. Invalidated objects are typically those that
-        have been manipulated or deleted. This checks the cache of mapped and
-        selected objects prior to the operation and ensures that if the object
-        is invalidated, they are reloaded based on the object name that was
-        tracked prior to the undo / redo.
-        """
-        file = IfcStore.get_file()
-        if not file:
-            return
-
-        # First, reload objects that were selected or active
-        for name in IfcStore.undo_redo_stack_objects:
-            obj = bpy.data.objects.get(name)
-            if not obj:
-                continue
-            if not obj.BIMObjectProperties.ifc_definition_id:
-                continue
-            element = file.by_id(obj.BIMObjectProperties.ifc_definition_id)
-            data = {"id": element.id(), "obj": obj.name}
-            if hasattr(element, "GlobalId"):
-                data["guid"] = element.GlobalId
-            IfcStore.commit_link_element(data)
-
-        # Scan for any straggling invalidated objects which were indirectly affected and reload them too.
-        for key, value in IfcStore.id_map.items():
-            try:
-                value.name
-            except:
-                # TODO not so sure about this obj_name check
-                obj_name = IfcStore.undo_redo_stack_object_names.get(key, None)
-                if not obj_name:
-                    continue
-                obj = bpy.data.objects.get(obj_name)
-                if not obj or not obj.BIMObjectProperties.ifc_definition_id:
-                    continue
-                element = file.by_id(obj.BIMObjectProperties.ifc_definition_id)
-                data = {"id": element.id(), "obj": obj.name}
-                if hasattr(element, "GlobalId"):
-                    data["guid"] = element.GlobalId
-                IfcStore.commit_link_element(data)
-
     @staticmethod
     def relink_all_objects():
         if not IfcStore.get_file():
@@ -309,9 +216,6 @@ class IfcStore:
             blenderbim.bim.handler.subscribe_to(obj, "mode", blenderbim.bim.handler.mode_callback)
             blenderbim.bim.handler.subscribe_to(obj, "active_material_index", blenderbim.bim.handler.active_material_index_callback)
 
-        for listener in IfcStore.element_listeners:
-            listener(element, obj)
-
         if IfcStore.history:
             data = {"id": element.id(), "guid": getattr(element, "GlobalId", None), "obj": obj.name}
             IfcStore.history[-1]["operations"].append(
@@ -437,8 +341,6 @@ class IfcStore:
 
     @staticmethod
     def begin_transaction(operator):
-        IfcStore.undo_redo_stack_objects = set()
-        IfcStore.undo_redo_stack_object_names = {}
         IfcStore.current_transaction = str(uuid.uuid4())
         operator.transaction_key = IfcStore.current_transaction
 

src/blenderbim/blenderbim/tool/ifc.py

@@ -21,6 +21,8 @@ import bpy
 import numpy as np
 import ifcopenshell.api
 import blenderbim.core.tool
+import blenderbim.bim.handler
+import blenderbim.tool as tool
 from blenderbim.bim.ifc import IfcStore
 
 
@@ -88,6 +90,54 @@ class Ifc(blenderbim.core.tool.Ifc):
     def get_object(cls, element):
         return IfcStore.get_element(element.id())
 
+    @classmethod
+    def rebuild_element_maps(cls):
+        """Rebuilds the id_map and guid_map
+
+        When any Blender object is stored outside a Blender PointerProperty,
+        such as in a regular Python list, there is the likely probability that
+        the object will be invalidated when undo or redo occurs. Object
+        invalidation seems to occur whenever an object is affected during an
+        operation, or selected, or has a related modifier, and so on ... to
+        cover all bases, this completely rebuilds the element maps.
+        """
+
+        IfcStore.id_map = {}
+        IfcStore.guid_map = {}
+
+        if not cls.get():
+            return
+
+        for obj in bpy.data.objects:
+            bpy.msgbus.clear_by_owner(obj)
+
+            element = cls.get_entity(obj)
+            if not element:
+                continue
+            IfcStore.id_map[element.id()] = obj
+            global_id = getattr(element, "GlobalId", None)
+            if global_id:
+                IfcStore.guid_map[global_id] = obj
+
+            blenderbim.bim.handler.subscribe_to(obj, "name", blenderbim.bim.handler.name_callback)
+            blenderbim.bim.handler.subscribe_to(obj, "mode", blenderbim.bim.handler.mode_callback)
+            blenderbim.bim.handler.subscribe_to(
+                obj, "active_material_index", blenderbim.bim.handler.active_material_index_callback
+            )
+
+        for obj in bpy.data.materials:
+            bpy.msgbus.clear_by_owner(obj)
+
+            material = cls.get_entity(obj)
+            style = tool.Style.get_style(obj)
+            if material:
+                IfcStore.id_map[material.id()] = obj
+            if style:
+                IfcStore.id_map[style.id()] = obj
+
+            blenderbim.bim.handler.subscribe_to(obj, "name", blenderbim.bim.handler.name_callback)
+            blenderbim.bim.handler.subscribe_to(obj, "diffuse_color", blenderbim.bim.handler.color_callback)
+
     @classmethod
     def link(cls, element, obj):
         IfcStore.link_element(element, obj)