fix(dui3): blocks correctly handled in acad

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/DependencyInjection/AutocadConnectorModule.cs

@@ -37,9 +37,10 @@ public void Load(SpeckleContainerBuilder builder)
     builder.AddSingleton(new AutocadDocumentManager()); // TODO: Dependent to TransactionContext, can be moved to AutocadContext
     builder.AddSingleton<DocumentModelStore, AutocadDocumentStore>();
     builder.AddSingleton<AutocadContext>();
-    builder.AddSingleton<AutocadLayerManager>();
     builder.AddSingleton<AutocadIdleManager>();
 
+    builder.AddScoped<AutocadLayerManager>();
+
     // Operations
     builder.AddScoped<SendOperation<AutocadRootObject>>();
     builder.AddSingleton(DefaultTraversal.CreateTraversalFunc());

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceObjectManager.cs

@@ -18,11 +18,17 @@ namespace Speckle.Connectors.Autocad.HostApp;
 /// </summary>
 public class AutocadInstanceObjectManager : IInstanceObjectsManager<AutocadRootObject, List<Entity>>
 {
+  private readonly AutocadLayerManager _autocadLayerManager;
   private Dictionary<string, InstanceProxy> InstanceProxies { get; set; } = new();
   private Dictionary<string, List<InstanceProxy>> InstanceProxiesByDefinitionId { get; set; } = new();
   private Dictionary<string, InstanceDefinitionProxy> DefinitionProxies { get; set; } = new();
   private Dictionary<string, AutocadRootObject> FlatAtomicObjects { get; set; } = new();
 
+  public AutocadInstanceObjectManager(AutocadLayerManager autocadLayerManager)
+  {
+    _autocadLayerManager = autocadLayerManager;
+  }
+
   public UnpackResult<AutocadRootObject> UnpackSelection(IEnumerable<AutocadRootObject> objects)
   {
     using var transaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();
@@ -146,7 +152,16 @@ public BakeResult BakeInstances(
 
           var record = new BlockTableRecord();
           var objectIds = new ObjectIdCollection();
-          record.Name = baseLayerName + definitionProxy.applicationId;
+          record.Name = baseLayerName;
+          if (definitionProxy["name"] is string name)
+          {
+            record.Name += name;
+          }
+          else
+          {
+            record.Name += definitionProxy.applicationId;
+          }
+
           foreach (var entity in constituentEntities)
           {
             // record.AppendEntity(entity);
@@ -175,8 +190,13 @@ instanceOrDefinition is InstanceProxy instanceProxy
           var modelSpaceBlockTableRecord = Application.DocumentManager.CurrentDocument.Database.GetModelSpace(
             OpenMode.ForWrite
           );
+          _autocadLayerManager.CreateLayerForReceive(path[0]);
+          var blockRef = new BlockReference(insertionPoint, definitionId)
+          {
+            BlockTransform = matrix3d,
+            Layer = path[0],
+          };
 
-          var blockRef = new BlockReference(insertionPoint, definitionId) { BlockTransform = matrix3d }; // TODO: Bake on correct layer
           modelSpaceBlockTableRecord.AppendEntity(blockRef);
 
           if (instanceProxy.applicationId != null)
@@ -200,17 +220,17 @@ instanceOrDefinition is InstanceProxy instanceProxy
     return new(createdObjectIds, consumedObjectIds, conversionResults);
   }
 
+  /// <summary>
+  /// Cleans up any previously created instances.
+  /// POC: This function will not be able to delete block definitions if the user creates a new one composed out of received definitions.
+  /// </summary>
+  /// <param name="namePrefix"></param>
   public void PurgeInstances(string namePrefix)
   {
     using var transaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();
-
-    // Step 1: purge instances and instance definitions
     var instanceDefinitionsToDelete = new Dictionary<string, BlockTableRecord>();
-    using var modelSpaceRecord = Application.DocumentManager.CurrentDocument.Database.GetModelSpace(OpenMode.ForRead);
-    using var blockTable = (BlockTable)
-      transaction.GetObject(Application.DocumentManager.CurrentDocument.Database.BlockTableId, OpenMode.ForWrite);
 
-    // Recurses through a given block table record and purges inner instances as required.
+    // Helper function that recurses through a given block table record's constituent objects and purges inner instances as required.
     void TraverseAndClean(BlockTableRecord btr)
     {
       foreach (var objectId in btr)
@@ -221,24 +241,27 @@ void TraverseAndClean(BlockTableRecord btr)
           continue;
         }
         var definition = (BlockTableRecord)transaction.GetObject(obj.BlockTableRecord, OpenMode.ForRead);
-        // POC: this is tightly coupled with a naming convention for definitions in the Instance object manager
-        if (definition.Name.Contains(namePrefix))
+        if (obj.IsErased)
         {
-          obj.UpgradeOpen();
-          obj.Erase();
           TraverseAndClean(definition);
-          instanceDefinitionsToDelete[obj.BlockTableRecord.ToString()] = definition;
+          continue;
         }
+
+        obj.UpgradeOpen();
+        obj.Erase();
+        TraverseAndClean(definition);
+        instanceDefinitionsToDelete[obj.BlockTableRecord.ToString()] = definition;
       }
     }
 
-    TraverseAndClean(modelSpaceRecord);
+    using var blockTable = (BlockTable)
+      transaction.GetObject(Application.DocumentManager.CurrentDocument.Database.BlockTableId, OpenMode.ForRead);
 
-    // cleanup potentially orphaned definitions (if user deletes an instance reference, we don't reach composing definitions anymore - as such, we need to go through each btr too)
+    // deep clean definitions
     foreach (var btrId in blockTable)
     {
       var btr = (BlockTableRecord)transaction.GetObject(btrId, OpenMode.ForRead);
-      if (btr.Name.Contains(namePrefix))
+      if (btr.Name.Contains(namePrefix)) // POC: this is tightly coupled with a naming convention for definitions in the instance object manager
       {
         TraverseAndClean(btr);
         instanceDefinitionsToDelete[btr.Name] = btr;

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadLayerManager.cs

@@ -1,56 +1,53 @@
 using Autodesk.AutoCAD.DatabaseServices;
 using Autodesk.AutoCAD.EditorInput;
 using Autodesk.AutoCAD.LayerManager;
+using Speckle.Core.Models;
+using Speckle.Core.Models.GraphTraversal;
 
 namespace Speckle.Connectors.Autocad.HostApp;
 
+/// <summary>
+/// Expects to be a scoped dependency for a given operation and helps with layer creation and cleanup.
+/// </summary>
 public class AutocadLayerManager
 {
   private readonly AutocadContext _autocadContext;
   private readonly string _layerFilterName = "Speckle";
 
   // POC: Will be addressed to move it into AutocadContext!
-  private Document Doc => Autodesk.AutoCAD.ApplicationServices.Core.Application.DocumentManager.MdiActiveDocument;
+  private Document Doc => Application.DocumentManager.MdiActiveDocument;
+  private readonly HashSet<string> _uniqueLayerNames = new();
 
   public AutocadLayerManager(AutocadContext autocadContext)
   {
     _autocadContext = autocadContext;
   }
 
-  /// <summary>
-  /// Constructs layer name with prefix and valid characters.
-  /// </summary>
-  /// <param name="baseLayerPrefix"> Prefix to add layer name.</param>
-  /// <param name="path"> list of entries to concat with hyphen.</param>
-  /// <returns>Full layer name with provided prefix and path.</returns>
-  public string GetFullLayerName(string baseLayerPrefix, string path)
-  {
-    var layerFullName = baseLayerPrefix + string.Join("-", path);
-    return _autocadContext.RemoveInvalidChars(layerFullName);
-  }
-
   /// <summary>
   /// Will create a layer with the provided name, or, if it finds an existing one, will "purge" all objects from it.
   /// This ensures we're creating the new objects we've just received rather than overlaying them.
   /// </summary>
   /// <param name="layerName">Name to search layer for purge and create.</param>
-  public void CreateLayerOrPurge(string layerName)
+  public void CreateLayerForReceive(string layerName)
   {
-    // POC: Will be addressed to move it into AutocadContext!
-    Document doc = Application.DocumentManager.MdiActiveDocument;
-    doc.LockDocument();
-    using Transaction transaction = doc.TransactionManager.StartTransaction();
+    if (!_uniqueLayerNames.Add(layerName))
+    {
+      return;
+    }
+
+    Doc.LockDocument();
+    using Transaction transaction = Doc.TransactionManager.StartTransaction();
 
     LayerTable? layerTable =
-      transaction.TransactionManager.GetObject(doc.Database.LayerTableId, OpenMode.ForRead) as LayerTable;
+      transaction.TransactionManager.GetObject(Doc.Database.LayerTableId, OpenMode.ForRead) as LayerTable;
     LayerTableRecord layerTableRecord = new() { Name = layerName };
 
     bool hasLayer = layerTable != null && layerTable.Has(layerName);
     if (hasLayer)
     {
       TypedValue[] tvs = { new((int)DxfCode.LayerName, layerName) };
       SelectionFilter selectionFilter = new(tvs);
-      SelectionSet selectionResult = doc.Editor.SelectAll(selectionFilter).Value;
+      SelectionSet selectionResult = Doc.Editor.SelectAll(selectionFilter).Value;
       if (selectionResult == null)
       {
         return;
@@ -69,7 +66,38 @@ public void CreateLayerOrPurge(string layerName)
     transaction.Commit();
   }
 
-  // POC: Consider to extract somehow in factory or service!
+  public void DeleteAllLayersByPrefix(string prefix)
+  {
+    Doc.LockDocument();
+    using Transaction transaction = Doc.TransactionManager.StartTransaction();
+
+    var layerTable = (LayerTable)transaction.TransactionManager.GetObject(Doc.Database.LayerTableId, OpenMode.ForRead);
+    foreach (var layerId in layerTable)
+    {
+      var layer = (LayerTableRecord)transaction.GetObject(layerId, OpenMode.ForRead);
+      var layerName = layer.Name;
+      if (layer.Name.Contains(prefix))
+      {
+        // Delete objects from this layer
+        TypedValue[] tvs = { new((int)DxfCode.LayerName, layerName) };
+        SelectionFilter selectionFilter = new(tvs);
+        SelectionSet selectionResult = Doc.Editor.SelectAll(selectionFilter).Value;
+        if (selectionResult == null)
+        {
+          return;
+        }
+        foreach (SelectedObject selectedObject in selectionResult)
+        {
+          transaction.GetObject(selectedObject.ObjectId, OpenMode.ForWrite).Erase();
+        }
+        // Delete layer
+        layer.UpgradeOpen();
+        layer.Erase();
+      }
+    }
+    transaction.Commit();
+  }
+
   /// <summary>
   /// Creates a layer filter for the just received model, grouped under a top level filter "Speckle". Note: manual close and open of the layer properties panel required (it's an acad thing).
   /// This comes in handy to quickly access the layers created for this specific model.
@@ -114,4 +142,19 @@ public void CreateLayerFilter(string projectName, string modelName)
     groupFilter.NestedFilters.Add(layerFilter);
     Doc.Database.LayerFilters = layerFilterTree;
   }
+
+  /// <summary>
+  /// Gets a valid layer name for a given context.
+  /// </summary>
+  /// <param name="context"></param>
+  /// <param name="baseLayerPrefix"></param>
+  /// <returns></returns>
+  public string GetLayerPath(TraversalContext context, string baseLayerPrefix)
+  {
+    string[] collectionBasedPath = context.GetAscendantOfType<Collection>().Select(c => c.name).Reverse().ToArray();
+    string[] path = collectionBasedPath.Length != 0 ? collectionBasedPath : context.GetPropertyPath().ToArray();
+
+    var name = baseLayerPrefix + string.Join("-", path);
+    return _autocadContext.RemoveInvalidChars(name);
+  }
 }

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs

@@ -21,7 +21,8 @@ public class AutocadHostObjectBuilder : IHostObjectBuilder
   private readonly AutocadLayerManager _autocadLayerManager;
   private readonly IRootToHostConverter _converter;
   private readonly GraphTraversal _traversalFunction;
-  private readonly HashSet<string> _uniqueLayerNames = new();
+
+  // private readonly HashSet<string> _uniqueLayerNames = new();
   private readonly IInstanceObjectsManager<AutocadRootObject, List<Entity>> _instanceObjectsManager;
 
   public AutocadHostObjectBuilder(
@@ -55,6 +56,7 @@ CancellationToken cancellationToken
     string baseLayerPrefix = $"SPK-{projectName}-{modelName}-";
 
     PreReceiveDeepClean(baseLayerPrefix);
+
     List<ReceiveConversionResult> results = new();
     List<string> bakedObjectIds = new();
 
@@ -79,7 +81,7 @@ CancellationToken cancellationToken
 
     foreach (TraversalContext tc in objectGraph)
     {
-      var layerName = GetLayerPath(tc, baseLayerPrefix);
+      var layerName = _autocadLayerManager.GetLayerPath(tc, baseLayerPrefix);
       if (tc.Current is IInstanceComponent instanceComponent)
       {
         instanceComponents.Add((new string[] { layerName }, instanceComponent));
@@ -143,21 +145,8 @@ CancellationToken cancellationToken
 
   private void PreReceiveDeepClean(string baseLayerPrefix)
   {
+    _autocadLayerManager.DeleteAllLayersByPrefix(baseLayerPrefix);
     _instanceObjectsManager.PurgeInstances(baseLayerPrefix);
-
-    using var transaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();
-    var layerTable = (LayerTable)
-      transaction.GetObject(Application.DocumentManager.CurrentDocument.Database.LayerTableId, OpenMode.ForRead);
-
-    foreach (var layerId in layerTable)
-    {
-      var layer = (LayerTableRecord)transaction.GetObject(layerId, OpenMode.ForRead);
-      if (layer.Name.Contains(baseLayerPrefix))
-      {
-        _autocadLayerManager.CreateLayerOrPurge(layer.Name);
-      }
-    }
-    transaction.Commit();
   }
 
   private IEnumerable<Entity> ConvertObject(Base obj, string layerName)
@@ -166,13 +155,8 @@ private IEnumerable<Entity> ConvertObject(Base obj, string layerName)
       Application.DocumentManager.MdiActiveDocument
     );
 
-    if (_uniqueLayerNames.Add(layerName))
-    {
-      _autocadLayerManager.CreateLayerOrPurge(layerName);
-    }
+    _autocadLayerManager.CreateLayerForReceive(layerName);
 
-    //POC: this transaction used to be called in the converter, We've moved it here to unify converter implementation
-    //POC: Is this transaction 100% needed? we are already inside a transaction?
     object converted;
     using (var tr = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction())
     {
@@ -194,12 +178,4 @@ private IEnumerable<Entity> ConvertObject(Base obj, string layerName)
       yield return conversionResult;
     }
   }
-
-  private string GetLayerPath(TraversalContext context, string baseLayerPrefix)
-  {
-    string[] collectionBasedPath = context.GetAscendantOfType<Collection>().Select(c => c.name).ToArray();
-    string[] path = collectionBasedPath.Length != 0 ? collectionBasedPath : context.GetPropertyPath().ToArray();
-
-    return _autocadLayerManager.GetFullLayerName(baseLayerPrefix, string.Join("-", path)); //TODO: reverse path?
-  }
 }