From 44d83af691ac3fd549778478034e8cb5143c9740 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 2 Dec 2024 16:24:02 +0000 Subject: [PATCH 1/3] Use Sets and Freezing to make conversions faster --- .../Receive/ArcGISHostObjectBuilder.cs | 4 +-- .../HostApp/AutocadGroupBaker.cs | 9 ++--- .../HostApp/AutocadInstanceBaker.cs | 21 +++++------ .../HostApp/AutocadMaterialBaker.cs | 2 +- .../Receive/AutocadHostObjectBuilder.cs | 34 +++++++++--------- .../HostApp/RevitMaterialBaker.cs | 2 +- .../Receive/RevitHostObjectBuilder.cs | 8 ++--- .../HostApp/RhinoColorBaker.cs | 2 +- .../HostApp/RhinoGroupBaker.cs | 4 +-- .../HostApp/RhinoInstanceBaker.cs | 20 +++++------ .../HostApp/RhinoMaterialBaker.cs | 2 +- .../Receive/RhinoHostObjectBuilder.cs | 15 ++++---- .../LocalToGlobalToDirectShapeConverter.cs | 4 +-- .../Extensions/RootObjectBuilderExtensions.cs | 19 ++++++++++ .../Instances/BakeResult.cs | 6 ++-- .../Instances/IInstanceBaker.cs | 2 +- .../Instances/LocalToGlobalMap.cs | 4 +-- .../Instances/LocalToGlobalUnpacker.cs | 35 ++++++++++--------- .../Operations/Receive/RootObjectUnpacker.cs | 25 ++++++------- .../Receive/RootObjectUnpackerResult.cs | 10 +++--- .../Receive/TraversalContextUnpacker.cs | 6 ++-- .../LocalToGlobalConverterUtils.cs | 2 +- 22 files changed, 131 insertions(+), 105 deletions(-) diff --git a/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/ArcGISHostObjectBuilder.cs b/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/ArcGISHostObjectBuilder.cs index 2b98a8300..69cbeaee3 100644 --- a/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/ArcGISHostObjectBuilder.cs +++ b/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/ArcGISHostObjectBuilder.cs @@ -89,7 +89,7 @@ CancellationToken cancellationToken } int count = 0; - List objectsToConvert = GetObjectsToConvert(rootObject); + IReadOnlyCollection objectsToConvert = GetObjectsToConvert(rootObject); Dictionary conversionTracker = new(); // 1. convert everything @@ -242,7 +242,7 @@ await _featureClassUtils return new(bakedObjectIds, results); } - private List GetObjectsToConvert(Base rootObject) + private IReadOnlyCollection GetObjectsToConvert(Base rootObject) { // keep GISlayers in the list, because they are still needed to extract CRS of the commit (code below) List objectsToConvertTc = _traverseFunction.Traverse(rootObject).ToList(); diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupBaker.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupBaker.cs index 9a40b5040..a9c5e801a 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupBaker.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupBaker.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Logging; using Speckle.Connectors.Common.Conversion; using Speckle.Sdk; +using Speckle.Sdk.Dependencies; using Speckle.Sdk.Models.Proxies; namespace Speckle.Connectors.Autocad.HostApp; @@ -29,12 +30,12 @@ public AutocadGroupBaker(AutocadContext autocadContext, ILogger /// // TODO: Oguzhan! Do not report here too! But this is TBD that we don't know the shape of the report yet. - public List CreateGroups( + public IReadOnlyCollection CreateGroups( IEnumerable groupProxies, - Dictionary> applicationIdMap + Dictionary> applicationIdMap ) { - List results = new(); + HashSet results = new(); using var groupCreationTransaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction(); @@ -75,6 +76,6 @@ Dictionary> applicationIdMap groupCreationTransaction.Commit(); - return results; + return results.Freeze(); } } diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs index cf3439f7f..d29e4e47d 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs @@ -12,6 +12,7 @@ using Speckle.Sdk; using Speckle.Sdk.Common; using Speckle.Sdk.Common.Exceptions; +using Speckle.Sdk.Dependencies; using Speckle.Sdk.Models; using Speckle.Sdk.Models.Collections; using Speckle.Sdk.Models.Instances; @@ -22,7 +23,7 @@ namespace Speckle.Connectors.Autocad.HostApp; /// /// Expects to be a scoped dependency receive operation. /// -public class AutocadInstanceBaker : IInstanceBaker> +public class AutocadInstanceBaker : IInstanceBaker> { private readonly AutocadLayerBaker _layerBaker; private readonly AutocadColorBaker _colorBaker; @@ -50,8 +51,8 @@ IConverterSettingsStore converterSettings [SuppressMessage("Maintainability", "CA1506:Avoid excessive class coupling")] public BakeResult BakeInstances( - IReadOnlyCollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents, - Dictionary> applicationIdMap, + ICollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents, + Dictionary> applicationIdMap, string baseLayerName, IProgress onOperationProgressed ) @@ -64,9 +65,9 @@ IProgress onOperationProgressed var definitionIdAndApplicationIdMap = new Dictionary(); using var transaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction(); - var conversionResults = new List(); - var createdObjectIds = new List(); - var consumedObjectIds = new List(); + var conversionResults = new HashSet(); + var createdObjectIds = new HashSet(); + var consumedObjectIds = new HashSet(); var count = 0; foreach (var (collectionPath, instanceOrDefinition) in sortedInstanceComponents) @@ -79,7 +80,7 @@ IProgress onOperationProgressed { // TODO: create definition (block table record) var constituentEntities = definitionProxy - .objects.Select(id => applicationIdMap.TryGetValue(id, out List? value) ? value : null) + .objects.Select(id => applicationIdMap.TryGetValue(id, out IReadOnlyCollection? value) ? value : null) .Where(x => x is not null) .SelectMany(ent => ent!) .ToList(); @@ -109,8 +110,8 @@ IProgress onOperationProgressed definitionIdAndApplicationIdMap[definitionProxy.applicationId] = id; transaction.AddNewlyCreatedDBObject(record, true); var consumedEntitiesHandleValues = constituentEntities.Select(ent => ent.GetSpeckleApplicationId()).ToArray(); - consumedObjectIds.AddRange(consumedEntitiesHandleValues); - createdObjectIds.RemoveAll(newId => consumedEntitiesHandleValues.Contains(newId)); + consumedObjectIds.UnionWith(consumedEntitiesHandleValues); + createdObjectIds.RemoveWhere(newId => consumedEntitiesHandleValues.Contains(newId)); } else if ( instanceOrDefinition is InstanceProxy instanceProxy @@ -167,7 +168,7 @@ instanceOrDefinition is InstanceProxy instanceProxy } transaction.Commit(); - return new(createdObjectIds, consumedObjectIds, conversionResults); + return new(createdObjectIds.Freeze(), consumedObjectIds.Freeze(), conversionResults.Freeze()); } /// diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialBaker.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialBaker.cs index 6afc37ebf..6c305d93a 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialBaker.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialBaker.cs @@ -92,7 +92,7 @@ public void PurgeMaterials(string namePrefix) } public void ParseAndBakeRenderMaterials( - List materialProxies, + IReadOnlyCollection materialProxies, string baseLayerPrefix, IProgress onOperationProgressed ) diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs index b2886a27d..9ecfa4aeb 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs @@ -3,10 +3,12 @@ using Speckle.Connectors.Autocad.HostApp.Extensions; using Speckle.Connectors.Common.Builders; using Speckle.Connectors.Common.Conversion; +using Speckle.Connectors.Common.Extensions; using Speckle.Connectors.Common.Operations; using Speckle.Connectors.Common.Operations.Receive; using Speckle.Converters.Common; using Speckle.Sdk; +using Speckle.Sdk.Dependencies; using Speckle.Sdk.Models; using Speckle.Sdk.Models.Collections; using Speckle.Sdk.Models.Instances; @@ -123,9 +125,9 @@ IProgress onOperationProgressed } // 5 - Convert atomic objects - List results = new(); - List bakedObjectIds = new(); - Dictionary> applicationIdMap = new(); + HashSet results = new(); + HashSet bakedObjectIds = new(); + Dictionary> applicationIdMap = new(); var count = 0; foreach (var (layerPath, atomicObject) in atomicObjectsWithPath) { @@ -133,11 +135,11 @@ IProgress onOperationProgressed onOperationProgressed.Report(new("Converting objects", (double)++count / atomicObjects.Count)); try { - List convertedObjects = ConvertObject(atomicObject, layerPath, baseLayerPrefix); + IReadOnlyCollection convertedObjects = ConvertObject(atomicObject, layerPath, baseLayerPrefix); applicationIdMap[objectId] = convertedObjects; - results.AddRange( + results.UnionWith( convertedObjects.Select(e => new ReceiveConversionResult( Status.SUCCESS, atomicObject, @@ -146,7 +148,7 @@ IProgress onOperationProgressed )) ); - bakedObjectIds.AddRange(convertedObjects.Select(e => e.GetSpeckleApplicationId())); + bakedObjectIds.UnionWith(convertedObjects.Select(e => e.GetSpeckleApplicationId())); } catch (Exception ex) when (!ex.IsFatal()) { @@ -162,19 +164,19 @@ IProgress onOperationProgressed onOperationProgressed ); - bakedObjectIds.RemoveAll(id => consumedObjectIds.Contains(id)); - bakedObjectIds.AddRange(createdInstanceIds); - results.RemoveAll(result => result.ResultId != null && consumedObjectIds.Contains(result.ResultId)); - results.AddRange(instanceConversionResults); + bakedObjectIds.RemoveWhere(id => consumedObjectIds.Contains(id)); + bakedObjectIds.UnionWith(createdInstanceIds); + results.RemoveWhere(result => result.ResultId != null && consumedObjectIds.Contains(result.ResultId)); + results.UnionWith(instanceConversionResults); // 7 - Create groups if (unpackedRoot.GroupProxies != null) { - List groupResults = _groupBaker.CreateGroups( + IReadOnlyCollection groupResults = _groupBaker.CreateGroups( unpackedRoot.GroupProxies, applicationIdMap ); - results.AddRange(groupResults); + results.UnionWith(groupResults); } return new HostObjectBuilderResult(bakedObjectIds, results); @@ -187,10 +189,10 @@ private void PreReceiveDeepClean(string baseLayerPrefix) _materialBaker.PurgeMaterials(baseLayerPrefix); } - private List ConvertObject(Base obj, Collection[] layerPath, string baseLayerNamePrefix) + private IReadOnlyCollection ConvertObject(Base obj, Collection[] layerPath, string baseLayerNamePrefix) { string layerName = _layerBaker.CreateLayerForReceive(layerPath, baseLayerNamePrefix); - var convertedEntities = new List(); + var convertedEntities = new HashSet(); using var tr = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction(); @@ -206,11 +208,11 @@ private List ConvertObject(Base obj, Collection[] layerPath, string base else if (converted is IEnumerable<(object, Base)> fallbackConversionResult) { var bakedFallbackEntities = BakeObjectsAsGroup(fallbackConversionResult, obj, layerName, baseLayerNamePrefix); - convertedEntities.AddRange(bakedFallbackEntities); + convertedEntities.UnionWith(bakedFallbackEntities); } tr.Commit(); - return convertedEntities; + return convertedEntities.Freeze(); } private Entity BakeObject(Entity entity, Base originalObject, string layerName, Base? parentObject = null) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/RevitMaterialBaker.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/RevitMaterialBaker.cs index d883c6c9c..a3b53bfd4 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/RevitMaterialBaker.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/RevitMaterialBaker.cs @@ -108,7 +108,7 @@ public void MapLayersRenderMaterials(RootObjectUnpackerResult unpackedRoot) /// /// public Dictionary BakeMaterials( - List speckleRenderMaterialProxies, + IReadOnlyCollection speckleRenderMaterialProxies, string baseLayerName ) { diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs index 6734d1219..b9492e5c4 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs @@ -34,7 +34,7 @@ internal sealed class RevitHostObjectBuilder : IHostObjectBuilder, IDisposable private readonly RevitMaterialBaker _materialBaker; private readonly ILogger _logger; private readonly ITypedConverter< - (Base atomicObject, List matrix), + (Base atomicObject, IReadOnlyCollection matrix), DirectShape > _localToGlobalDirectShapeConverter; @@ -52,7 +52,7 @@ public RevitHostObjectBuilder( RootObjectUnpacker rootObjectUnpacker, ILogger logger, RevitToHostCacheSingleton revitToHostCacheSingleton, - ITypedConverter<(Base atomicObject, List matrix), DirectShape> localToGlobalDirectShapeConverter + ITypedConverter<(Base atomicObject, IReadOnlyCollection matrix), DirectShape> localToGlobalDirectShapeConverter ) { _converter = converter; @@ -161,7 +161,7 @@ CancellationToken cancellationToken HostObjectBuilderResult builderResult, List<(DirectShape res, string applicationId)> postBakePaintTargets ) BakeObjects( - List localToGlobalMaps, + IReadOnlyCollection localToGlobalMaps, IProgress onOperationProgressed, CancellationToken cancellationToken ) @@ -195,7 +195,7 @@ localToGlobalMap.AtomicObject is ITransformable transformable // and ICurve } localToGlobalMap.AtomicObject = (newTransformable as Base)!; - localToGlobalMap.Matrix = new(); // flush out the list, as we've applied the transforms already + localToGlobalMap.Matrix = new HashSet(); // flush out the list, as we've applied the transforms already } // actual conversion happens here! diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoColorBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoColorBaker.cs index 7c1f38fd8..c931aafb5 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoColorBaker.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoColorBaker.cs @@ -23,7 +23,7 @@ public RhinoColorBaker(ILogger logger) /// Parse Color Proxies and stores in ObjectColorsIdMap the relationship between object ids and colors /// /// - public void ParseColors(List colorProxies) + public void ParseColors(IReadOnlyCollection colorProxies) { foreach (ColorProxy colorProxy in colorProxies) { diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoGroupBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoGroupBaker.cs index fcf52d524..3ed1852f0 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoGroupBaker.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoGroupBaker.cs @@ -25,8 +25,8 @@ ISdkActivityFactory activityFactory } public void BakeGroups( - List groupProxies, - Dictionary> applicationIdMap, + IReadOnlyCollection groupProxies, + Dictionary> applicationIdMap, string baseLayerName ) { diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs index cafc8ba5c..520ccf5ce 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs @@ -16,7 +16,7 @@ namespace Speckle.Connectors.Rhino.HostApp; -public class RhinoInstanceBaker : IInstanceBaker> +public class RhinoInstanceBaker : IInstanceBaker> { private readonly RhinoMaterialBaker _materialBaker; private readonly RhinoLayerBaker _layerBaker; @@ -43,8 +43,8 @@ ILogger logger /// A dict mapping { original application id -> [resulting application ids post conversion] } /// public BakeResult BakeInstances( - IReadOnlyCollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents, - Dictionary> applicationIdMap, + ICollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents, + Dictionary> applicationIdMap, string baseLayerName, IProgress onOperationProgressed ) @@ -59,9 +59,9 @@ IProgress onOperationProgressed var definitionIdAndApplicationIdMap = new Dictionary(); var count = 0; - var conversionResults = new List(); - var createdObjectIds = new List(); - var consumedObjectIds = new List(); + var conversionResults = new HashSet(); + var createdObjectIds = new HashSet(); + var consumedObjectIds = new HashSet(); foreach (var (layerCollection, instanceOrDefinition) in sortedInstanceComponents) { onOperationProgressed.Report(new("Converting blocks", (double)++count / sortedInstanceComponents.Count)); @@ -70,10 +70,10 @@ IProgress onOperationProgressed if (instanceOrDefinition is InstanceDefinitionProxy definitionProxy) { var currentApplicationObjectsIds = definitionProxy - .objects.Select(x => applicationIdMap.TryGetValue(x, out List? value) ? value : null) + .objects.Select(x => applicationIdMap.TryGetValue(x, out IReadOnlyCollection? value) ? value : null) .Where(x => x is not null) .SelectMany(id => id.NotNull()) - .ToList(); + .ToHashSet(); var definitionGeometryList = new List(); var attributes = new List(); @@ -108,8 +108,8 @@ IProgress onOperationProgressed // Rhino deletes original objects on block creation - we should do the same. doc.Objects.Delete(currentApplicationObjectsIds.Select(stringId => new Guid(stringId)), false); - consumedObjectIds.AddRange(currentApplicationObjectsIds); - createdObjectIds.RemoveAll(id => consumedObjectIds.Contains(id)); // in case we've consumed some existing instances + consumedObjectIds.UnionWith(currentApplicationObjectsIds); + createdObjectIds.RemoveWhere(id => consumedObjectIds.Contains(id)); // in case we've consumed some existing instances } if ( diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoMaterialBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoMaterialBaker.cs index b550c4592..f4452b428 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoMaterialBaker.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoMaterialBaker.cs @@ -28,7 +28,7 @@ ILogger logger /// public Dictionary ObjectIdAndMaterialIndexMap { get; } = new(); - public void BakeMaterials(List speckleRenderMaterialProxies, string baseLayerName) + public void BakeMaterials(IReadOnlyCollection speckleRenderMaterialProxies, string baseLayerName) { var doc = _converterSettings.Current.Document; // POC: too much right now to interface around // List conversionResults = new(); // TODO: return this guy diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs index 56ca5de86..d6e71eb01 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs @@ -3,6 +3,7 @@ using Rhino.Geometry; using Speckle.Connectors.Common.Builders; using Speckle.Connectors.Common.Conversion; +using Speckle.Connectors.Common.Extensions; using Speckle.Connectors.Common.Operations; using Speckle.Connectors.Common.Operations.Receive; using Speckle.Connectors.Rhino.HostApp; @@ -120,9 +121,9 @@ CancellationToken cancellationToken } // 5 - Convert atomic objects - List bakedObjectIds = new(); - Dictionary> applicationIdMap = new(); // This map is used in converting blocks in stage 2. keeps track of original app id => resulting new app ids post baking - List conversionResults = new(); + var bakedObjectIds = new HashSet(); + Dictionary> applicationIdMap = new(); // This map is used in converting blocks in stage 2. keeps track of original app id => resulting new app ids post baking + HashSet conversionResults = new(); int count = 0; using (var _ = _activityFactory.Start("Converting objects")) @@ -214,10 +215,10 @@ CancellationToken cancellationToken onOperationProgressed ); - bakedObjectIds.RemoveAll(id => consumedObjectIds.Contains(id)); // remove all objects that have been "consumed" - bakedObjectIds.AddRange(createdInstanceIds); // add instance ids - conversionResults.RemoveAll(result => result.ResultId != null && consumedObjectIds.Contains(result.ResultId)); // remove all conversion results for atomic objects that have been consumed (POC: not that cool, but prevents problems on object highlighting) - conversionResults.AddRange(instanceConversionResults); // add instance conversion results to our list + bakedObjectIds.RemoveWhere(id => consumedObjectIds.Contains(id)); // remove all objects that have been "consumed" + bakedObjectIds.UnionWith(createdInstanceIds); // add instance ids + conversionResults.RemoveWhere(result => result.ResultId != null && consumedObjectIds.Contains(result.ResultId)); // remove all conversion results for atomic objects that have been consumed (POC: not that cool, but prevents problems on object highlighting) + conversionResults.UnionWith(instanceConversionResults); // add instance conversion results to our list } // 7 - Create groups diff --git a/Converters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/LocalToGlobalToDirectShapeConverter.cs b/Converters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/LocalToGlobalToDirectShapeConverter.cs index 840e9e999..216023fe3 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/LocalToGlobalToDirectShapeConverter.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/ToHost/Raw/LocalToGlobalToDirectShapeConverter.cs @@ -12,7 +12,7 @@ namespace Speckle.Converters.RevitShared.ToSpeckle; /// All this is poc that should be burned, once we enable proper block support to revit. /// public class LocalToGlobalToDirectShapeConverter - : ITypedConverter<(Base atomicObject, List matrix), DB.DirectShape> + : ITypedConverter<(Base atomicObject, IReadOnlyCollection matrix), DB.DirectShape> { private readonly IConverterSettingsStore _converterSettings; private readonly ITypedConverter<(Matrix4x4 matrix, string units), DB.Transform> _transformConverter; @@ -26,7 +26,7 @@ public LocalToGlobalToDirectShapeConverter( _transformConverter = transformConverter; } - public DB.DirectShape Convert((Base atomicObject, List matrix) target) + public DB.DirectShape Convert((Base atomicObject, IReadOnlyCollection matrix) target) { // 1- set ds category var category = target.atomicObject["builtinCategory"] as string; diff --git a/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs b/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs index fa630c4c8..704566f72 100644 --- a/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs +++ b/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs @@ -21,3 +21,22 @@ string objectType logger.Log(logLevel, ex, "Conversion of object {objectType} was not successful", objectType); } } + +public static class CollectionExtensions +{ + public static void AddRange(this ICollection collection, IEnumerable items) + { + foreach (var item in items) + { + collection.Add(item); + } + } + +#if NETSTANDARD2_0 + public static HashSet ToHashSet(this IEnumerable items) + { + var set = new HashSet(items); + return set; + } +#endif +} diff --git a/Sdk/Speckle.Connectors.Common/Instances/BakeResult.cs b/Sdk/Speckle.Connectors.Common/Instances/BakeResult.cs index b3866ae7e..447b8cee9 100644 --- a/Sdk/Speckle.Connectors.Common/Instances/BakeResult.cs +++ b/Sdk/Speckle.Connectors.Common/Instances/BakeResult.cs @@ -3,7 +3,7 @@ namespace Speckle.Connectors.Common.Instances; public record BakeResult( - List CreatedInstanceIds, - List ConsumedObjectIds, - List InstanceConversionResults + IReadOnlyCollection CreatedInstanceIds, + IReadOnlyCollection ConsumedObjectIds, + IReadOnlyCollection InstanceConversionResults ); diff --git a/Sdk/Speckle.Connectors.Common/Instances/IInstanceBaker.cs b/Sdk/Speckle.Connectors.Common/Instances/IInstanceBaker.cs index 2ed88c836..a2a8e559f 100644 --- a/Sdk/Speckle.Connectors.Common/Instances/IInstanceBaker.cs +++ b/Sdk/Speckle.Connectors.Common/Instances/IInstanceBaker.cs @@ -15,7 +15,7 @@ public interface IInstanceBaker /// /// BakeResult BakeInstances( - IReadOnlyCollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents, + ICollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents, Dictionary applicationIdMap, string baseLayerName, IProgress onOperationProgressed diff --git a/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalMap.cs b/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalMap.cs index 9e20fc111..24f836ebd 100644 --- a/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalMap.cs +++ b/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalMap.cs @@ -9,7 +9,7 @@ namespace Speckle.Connectors.Common.Instances; public class LocalToGlobalMap { - public LocalToGlobalMap(TraversalContext traversalContext, Base atomicObject, List matrix) + public LocalToGlobalMap(TraversalContext traversalContext, Base atomicObject, IReadOnlyCollection matrix) { TraversalContext = traversalContext; AtomicObject = atomicObject; @@ -18,5 +18,5 @@ public LocalToGlobalMap(TraversalContext traversalContext, Base atomicObject, Li public TraversalContext TraversalContext { get; set; } public Base AtomicObject { get; set; } - public List Matrix { get; set; } + public IReadOnlyCollection Matrix { get; set; } } diff --git a/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalUnpacker.cs b/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalUnpacker.cs index 3a13c6cc7..1451a9a89 100644 --- a/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalUnpacker.cs +++ b/Sdk/Speckle.Connectors.Common/Instances/LocalToGlobalUnpacker.cs @@ -1,5 +1,6 @@ using Speckle.DoubleNumerics; using Speckle.InterfaceGenerator; +using Speckle.Sdk.Dependencies; using Speckle.Sdk.Models; using Speckle.Sdk.Models.GraphTraversal; using Speckle.Sdk.Models.Instances; @@ -14,15 +15,15 @@ namespace Speckle.Connectors.Common.Instances; [GenerateAutoInterface] public class LocalToGlobalUnpacker : ILocalToGlobalUnpacker { - public List Unpack( - List? instanceDefinitionProxies, - List objectsToUnpack + public IReadOnlyCollection Unpack( + IReadOnlyCollection? instanceDefinitionProxies, + IReadOnlyCollection objectsToUnpack ) { - var localToGlobalMaps = new List(); + var localToGlobalMaps = new HashSet(); - var instanceProxies = new List<(TraversalContext tc, InstanceProxy obj)>(); - var atomicObjects = new List<(TraversalContext tc, Base obj)>(); + var instanceProxies = new HashSet<(TraversalContext tc, InstanceProxy obj)>(); + var atomicObjects = new HashSet<(TraversalContext tc, Base obj)>(); // 1. Split up the instances from the non-instances foreach (TraversalContext objectToUnpack in objectsToUnpack) @@ -37,8 +38,8 @@ List objectsToUnpack } } - var objectsAtAbsolute = new List<(TraversalContext tc, Base obj)>(); - var objectsAtRelative = new List<(TraversalContext tc, Base obj)>(); + var objectsAtAbsolute = new HashSet<(TraversalContext tc, Base obj)>(); + var objectsAtRelative = new HashSet<(TraversalContext tc, Base obj)>(); // 2. Split atomic objects that in absolute or relative coordinates. foreach ((TraversalContext tc, Base atomicObject) in atomicObjects) @@ -61,7 +62,7 @@ atomicObject.applicationId is not null // 3. Add atomic objects that on absolute coordinates that doesn't need a transformation. foreach ((TraversalContext tc, Base objectAtAbsolute) in objectsAtAbsolute) { - localToGlobalMaps.Add(new LocalToGlobalMap(tc, objectAtAbsolute, new List())); + localToGlobalMaps.Add(new LocalToGlobalMap(tc, objectAtAbsolute, new HashSet())); } // 4. Return if no logic around instancing. @@ -78,28 +79,28 @@ atomicObject.applicationId is not null instanceProxies, objectAtRelative, objectAtRelative, - new List(), + new HashSet(), localToGlobalMaps ); } - return localToGlobalMaps.Where(ltgm => ltgm.AtomicObject is not InstanceProxy).ToList(); + return localToGlobalMaps.Where(ltgm => ltgm.AtomicObject is not InstanceProxy).Freeze(); } private void UnpackMatrix( - List instanceDefinitionProxies, - List<(TraversalContext tc, InstanceProxy instanceProxy)> instanceProxies, + IReadOnlyCollection instanceDefinitionProxies, + HashSet<(TraversalContext tc, InstanceProxy instanceProxy)> instanceProxies, (TraversalContext tc, Base obj) objectAtRelative, (TraversalContext tc, Base obj) searchForDefinition, - List matrices, - List localToGlobalMaps + HashSet matrices, + HashSet localToGlobalMaps ) { if (searchForDefinition.obj.applicationId is null) { return; } - InstanceDefinitionProxy? definitionProxy = instanceDefinitionProxies.Find(idp => + InstanceDefinitionProxy? definitionProxy = instanceDefinitionProxies.FirstOrDefault(idp => idp.objects.Contains(searchForDefinition.obj.applicationId) ); if (definitionProxy is null) @@ -116,7 +117,7 @@ List localToGlobalMaps var instances = instanceProxies.Where(ic => ic.instanceProxy.definitionId == definitionProxy.applicationId); foreach (var instance in instances) { - List newMatrices = [.. matrices, instance.instanceProxy.transform]; // Do not mutate the list! + HashSet newMatrices = [.. matrices, instance.instanceProxy.transform]; // Do not mutate the list! UnpackMatrix( instanceDefinitionProxies, instanceProxies, diff --git a/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs b/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs index 870d5f392..89db4c51e 100644 --- a/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs +++ b/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs @@ -1,4 +1,5 @@ using Speckle.Objects.Other; +using Speckle.Sdk.Dependencies; using Speckle.Sdk.Models; using Speckle.Sdk.Models.Collections; using Speckle.Sdk.Models.GraphTraversal; @@ -28,26 +29,26 @@ public RootObjectUnpackerResult Unpack(Base root) => TryGetColorProxies(root) ); - public IEnumerable GetObjectsToConvert(Base root) => - _traverseFunction.Traverse(root).Where(obj => obj.Current is not Collection); + public IReadOnlyCollection GetObjectsToConvert(Base root) => + _traverseFunction.Traverse(root).Where(obj => obj.Current is not Collection).Freeze(); - public List? TryGetColorProxies(Base root) => TryGetProxies(root, ProxyKeys.COLOR); + public IReadOnlyCollection? TryGetColorProxies(Base root) => TryGetProxies(root, ProxyKeys.COLOR); - public List? TryGetRenderMaterialProxies(Base root) => + public IReadOnlyCollection? TryGetRenderMaterialProxies(Base root) => TryGetProxies(root, ProxyKeys.RENDER_MATERIAL); - public List? TryGetInstanceDefinitionProxies(Base root) => + public IReadOnlyCollection? TryGetInstanceDefinitionProxies(Base root) => TryGetProxies(root, ProxyKeys.INSTANCE_DEFINITION); - public List? TryGetGroupProxies(Base root) => TryGetProxies(root, ProxyKeys.GROUP); + public IReadOnlyCollection? TryGetGroupProxies(Base root) => TryGetProxies(root, ProxyKeys.GROUP); public ( - List atomicObjects, - List instanceComponents + IReadOnlyCollection atomicObjects, + IReadOnlyCollection instanceComponents ) SplitAtomicObjectsAndInstances(IEnumerable objectsToSplit) { - List atomicObjects = new(); - List instanceComponents = new(); + HashSet atomicObjects = new(); + HashSet instanceComponents = new(); foreach (TraversalContext tc in objectsToSplit) { if (tc.Current is IInstanceComponent) @@ -59,8 +60,8 @@ List instanceComponents atomicObjects.Add(tc); } } - return (atomicObjects, instanceComponents); + return (atomicObjects.Freeze(), instanceComponents.Freeze()); } - private List? TryGetProxies(Base root, string key) => (root[key] as List)?.Cast().ToList(); + private IReadOnlyCollection? TryGetProxies(Base root, string key) => (root[key] as List)?.Cast().ToList(); } diff --git a/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpackerResult.cs b/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpackerResult.cs index 283bb2826..48a860f7f 100644 --- a/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpackerResult.cs +++ b/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpackerResult.cs @@ -6,9 +6,9 @@ namespace Speckle.Connectors.Common.Operations.Receive; public record RootObjectUnpackerResult( - IEnumerable ObjectsToConvert, - List? DefinitionProxies, - List? GroupProxies, - List? RenderMaterialProxies, - List? ColorProxies + IReadOnlyCollection ObjectsToConvert, + IReadOnlyCollection? DefinitionProxies, + IReadOnlyCollection? GroupProxies, + IReadOnlyCollection? RenderMaterialProxies, + IReadOnlyCollection? ColorProxies ); diff --git a/Sdk/Speckle.Connectors.Common/Operations/Receive/TraversalContextUnpacker.cs b/Sdk/Speckle.Connectors.Common/Operations/Receive/TraversalContextUnpacker.cs index bf8b0b179..19ef5d0c5 100644 --- a/Sdk/Speckle.Connectors.Common/Operations/Receive/TraversalContextUnpacker.cs +++ b/Sdk/Speckle.Connectors.Common/Operations/Receive/TraversalContextUnpacker.cs @@ -1,4 +1,4 @@ -using Speckle.Sdk.Models; +using Speckle.Sdk.Models; using Speckle.Sdk.Models.Collections; using Speckle.Sdk.Models.GraphTraversal; using Speckle.Sdk.Models.Instances; @@ -10,11 +10,11 @@ namespace Speckle.Connectors.Common.Operations.Receive; /// public abstract class TraversalContextUnpacker { - public List<(Collection[] path, Base current)> GetAtomicObjectsWithPath( + public IReadOnlyCollection<(Collection[] path, Base current)> GetAtomicObjectsWithPath( IEnumerable atomicObjects ) => atomicObjects.Select(o => (GetCollectionPath(o), o.Current)).ToList(); - public List<(Collection[] path, IInstanceComponent instance)> GetInstanceComponentsWithPath( + public ICollection<(Collection[] path, IInstanceComponent instance)> GetInstanceComponentsWithPath( IEnumerable instanceComponents ) => instanceComponents.Select(o => (GetCollectionPath(o), (o.Current as IInstanceComponent)!)).ToList(); diff --git a/Sdk/Speckle.Converters.Common/LocalToGlobalConverterUtils.cs b/Sdk/Speckle.Converters.Common/LocalToGlobalConverterUtils.cs index d3788fc68..48bd7f1a8 100644 --- a/Sdk/Speckle.Converters.Common/LocalToGlobalConverterUtils.cs +++ b/Sdk/Speckle.Converters.Common/LocalToGlobalConverterUtils.cs @@ -20,7 +20,7 @@ private Vector3 TransformPt(Vector3 vector, Matrix4x4 matrix) } // POC: This could move to converters instead handling all cases like this. - public Base TransformObjects(Base atomicObject, List matrix) + public Base TransformObjects(Base atomicObject, IReadOnlyCollection matrix) { if (matrix.Count == 0) { From 664cb5d9a07cef3393d60e4e7153132649bbe8f2 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 2 Dec 2024 16:24:24 +0000 Subject: [PATCH 2/3] fmt --- .../HostApp/AutocadInstanceBaker.cs | 4 +++- .../Operations/Receive/RevitHostObjectBuilder.cs | 7 +++++-- .../Operations/Receive/RootObjectUnpacker.cs | 9 ++++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs index d29e4e47d..37dc68504 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs @@ -80,7 +80,9 @@ IProgress onOperationProgressed { // TODO: create definition (block table record) var constituentEntities = definitionProxy - .objects.Select(id => applicationIdMap.TryGetValue(id, out IReadOnlyCollection? value) ? value : null) + .objects.Select(id => + applicationIdMap.TryGetValue(id, out IReadOnlyCollection? value) ? value : null + ) .Where(x => x is not null) .SelectMany(ent => ent!) .ToList(); diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs index b9492e5c4..1c6b4fda4 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs @@ -52,7 +52,10 @@ public RevitHostObjectBuilder( RootObjectUnpacker rootObjectUnpacker, ILogger logger, RevitToHostCacheSingleton revitToHostCacheSingleton, - ITypedConverter<(Base atomicObject, IReadOnlyCollection matrix), DirectShape> localToGlobalDirectShapeConverter + ITypedConverter< + (Base atomicObject, IReadOnlyCollection matrix), + DirectShape + > localToGlobalDirectShapeConverter ) { _converter = converter; @@ -161,7 +164,7 @@ CancellationToken cancellationToken HostObjectBuilderResult builderResult, List<(DirectShape res, string applicationId)> postBakePaintTargets ) BakeObjects( - IReadOnlyCollection localToGlobalMaps, + IReadOnlyCollection localToGlobalMaps, IProgress onOperationProgressed, CancellationToken cancellationToken ) diff --git a/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs b/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs index 89db4c51e..be23d2f42 100644 --- a/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs +++ b/Sdk/Speckle.Connectors.Common/Operations/Receive/RootObjectUnpacker.cs @@ -32,7 +32,8 @@ public RootObjectUnpackerResult Unpack(Base root) => public IReadOnlyCollection GetObjectsToConvert(Base root) => _traverseFunction.Traverse(root).Where(obj => obj.Current is not Collection).Freeze(); - public IReadOnlyCollection? TryGetColorProxies(Base root) => TryGetProxies(root, ProxyKeys.COLOR); + public IReadOnlyCollection? TryGetColorProxies(Base root) => + TryGetProxies(root, ProxyKeys.COLOR); public IReadOnlyCollection? TryGetRenderMaterialProxies(Base root) => TryGetProxies(root, ProxyKeys.RENDER_MATERIAL); @@ -40,7 +41,8 @@ public IReadOnlyCollection GetObjectsToConvert(Base root) => public IReadOnlyCollection? TryGetInstanceDefinitionProxies(Base root) => TryGetProxies(root, ProxyKeys.INSTANCE_DEFINITION); - public IReadOnlyCollection? TryGetGroupProxies(Base root) => TryGetProxies(root, ProxyKeys.GROUP); + public IReadOnlyCollection? TryGetGroupProxies(Base root) => + TryGetProxies(root, ProxyKeys.GROUP); public ( IReadOnlyCollection atomicObjects, @@ -63,5 +65,6 @@ IReadOnlyCollection instanceComponents return (atomicObjects.Freeze(), instanceComponents.Freeze()); } - private IReadOnlyCollection? TryGetProxies(Base root, string key) => (root[key] as List)?.Cast().ToList(); + private IReadOnlyCollection? TryGetProxies(Base root, string key) => + (root[key] as List)?.Cast().ToList(); } From 48149997f98f100f5aedde965ad2fd8ab657b636 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 3 Dec 2024 13:16:49 +0000 Subject: [PATCH 3/3] move class to own file --- .../Extensions/CollectionExtensions.cs | 20 +++++++++++++++++++ .../Extensions/RootObjectBuilderExtensions.cs | 19 ------------------ 2 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 Sdk/Speckle.Connectors.Common/Extensions/CollectionExtensions.cs diff --git a/Sdk/Speckle.Connectors.Common/Extensions/CollectionExtensions.cs b/Sdk/Speckle.Connectors.Common/Extensions/CollectionExtensions.cs new file mode 100644 index 000000000..cc4883a8d --- /dev/null +++ b/Sdk/Speckle.Connectors.Common/Extensions/CollectionExtensions.cs @@ -0,0 +1,20 @@ +namespace Speckle.Connectors.Common.Extensions; + +public static class CollectionExtensions +{ + public static void AddRange(this ICollection collection, IEnumerable items) + { + foreach (var item in items) + { + collection.Add(item); + } + } + +#if NETSTANDARD2_0 + public static HashSet ToHashSet(this IEnumerable items) + { + var set = new HashSet(items); + return set; + } +#endif +} diff --git a/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs b/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs index 704566f72..fa630c4c8 100644 --- a/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs +++ b/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs @@ -21,22 +21,3 @@ string objectType logger.Log(logLevel, ex, "Conversion of object {objectType} was not successful", objectType); } } - -public static class CollectionExtensions -{ - public static void AddRange(this ICollection collection, IEnumerable items) - { - foreach (var item in items) - { - collection.Add(item); - } - } - -#if NETSTANDARD2_0 - public static HashSet ToHashSet(this IEnumerable items) - { - var set = new HashSet(items); - return set; - } -#endif -}