From 6e81a465fb48961fc5666c96ad84b57b0d124bff Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Wed, 14 Aug 2024 11:58:07 +0100 Subject: [PATCH 01/17] chore: cleans up display value extractor removes transform references and checks for groups, as objects are ungrouped by that point --- .../Helpers/DisplayValueExtractor.cs | 89 ++----------------- 1 file changed, 8 insertions(+), 81 deletions(-) diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs index 15d839e36..5025a90c2 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs @@ -19,27 +19,9 @@ ILogger logger _logger = logger; } - public List GetDisplayValue( - DB.Element element, - DB.Options? options = null, - // POC: should this be part of the context? - DB.Transform? transform = null - ) + public List GetDisplayValue(DB.Element element, DB.Options? options = null) { - var displayMeshes = new List(); - - // test if the element is a group first - if (element is DB.Group g) - { - foreach (var id in g.GetMemberIds()) - { - var groupMeshes = GetDisplayValue(element.Document.GetElement(id), options); - displayMeshes.AddRange(groupMeshes); - } - return displayMeshes; - } - - var (solids, meshes) = GetSolidsAndMeshesFromElement(element, options, transform); + var (solids, meshes) = GetSolidsAndMeshesFromElement(element, options); var meshesByMaterial = GetMeshesByMaterial(meshes, solids); @@ -82,11 +64,7 @@ ILogger logger return meshesByMaterial; } - private (List, List) GetSolidsAndMeshesFromElement( - DB.Element element, - DB.Options? options, - DB.Transform? transform = null - ) + private (List, List) GetSolidsAndMeshesFromElement(DB.Element element, DB.Options? options) { //options = ViewSpecificOptions ?? options ?? new Options() { DetailLevel = DetailLevelSetting }; options ??= new DB.Options { DetailLevel = DB.ViewDetailLevel.Fine }; @@ -109,7 +87,7 @@ ILogger logger if (geom != null) { // retrieves all meshes and solids from a geometry element - SortGeometry(element, solids, meshes, geom, transform?.Inverse); + SortGeometry(element, solids, meshes, geom); } return (solids, meshes); @@ -125,26 +103,15 @@ ILogger logger /// /// This remark also leads me to think that a family instance will not have top-level solids and geom instances. /// We are logging cases where this is not true. + /// + /// Note: this is basically a geometry unpacker for all types of geometry /// /// /// /// /// - /// - private void SortGeometry( - DB.Element element, - List solids, - List meshes, - DB.GeometryElement geom, - DB.Transform? inverseTransform = null - ) + private void SortGeometry(DB.Element element, List solids, List meshes, DB.GeometryElement geom) { - var topLevelSolidsCount = 0; - var topLevelMeshesCount = 0; - var topLevelGeomElementCount = 0; - var topLevelGeomInstanceCount = 0; - bool hasSymbolGeometry = false; - foreach (DB.GeometryObject geomObj in geom) { // POC: switch could possibly become factory and IIndex<,> pattern and move conversions to @@ -162,12 +129,6 @@ private void SortGeometry( continue; } - if (inverseTransform != null) - { - topLevelSolidsCount++; - solid = DB.SolidUtils.CreateTransformed(solid, inverseTransform); - } - solids.Add(solid); break; case DB.Mesh mesh: @@ -175,52 +136,18 @@ private void SortGeometry( { continue; } - - if (inverseTransform != null) - { - topLevelMeshesCount++; - mesh = mesh.get_Transformed(inverseTransform); - } - meshes.Add(mesh); break; case DB.GeometryInstance instance: // element transforms should not be carried down into nested geometryInstances. // Nested geomInstances should have their geom retreived with GetInstanceGeom, not GetSymbolGeom - if (inverseTransform != null) - { - topLevelGeomInstanceCount++; - SortGeometry(element, solids, meshes, instance.GetSymbolGeometry()); - if (meshes.Count > 0 || solids.Count > 0) - { - hasSymbolGeometry = true; - } - } - else - { - SortGeometry(element, solids, meshes, instance.GetInstanceGeometry()); - } + SortGeometry(element, solids, meshes, instance.GetInstanceGeometry()); break; case DB.GeometryElement geometryElement: - if (inverseTransform != null) - { - topLevelGeomElementCount++; - } SortGeometry(element, solids, meshes, geometryElement); break; } } - - if (inverseTransform != null) - { - LogInstanceMeshRetrievalWarnings( - element, - topLevelSolidsCount, - topLevelMeshesCount, - topLevelGeomElementCount, - hasSymbolGeometry - ); - } } // POC: should be hoovered up with the new reporting, logging, exception philosophy From 7741b4c4b765f24ef010c53e792868355ac28012 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Wed, 14 Aug 2024 11:58:46 +0100 Subject: [PATCH 02/17] feat: generates render material proxy maps in revit send --- .../RevitConnectorModule.cs | 1 + .../HostApp/SendMaterialManager.cs | 43 +++++++++++++++++++ .../Operations/Send/RevitRootObjectBuilder.cs | 12 +++++- .../Speckle.Connectors.RevitShared.projitems | 1 + .../Helpers/IRevitConversionContextStack.cs | 6 ++- .../Helpers/RevitConversionContextStack.cs | 2 + .../MeshByMaterialDictionaryToSpeckle.cs | 23 +++++++--- .../Raw/MaterialConversionToSpeckle.cs | 3 +- 8 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs index c428ce8ea..0abda792e 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs @@ -68,6 +68,7 @@ public void Load(SpeckleContainerBuilder builder) builder.AddScoped>(); builder.AddScoped(); builder.AddScoped(); + builder.AddScoped(); builder.AddScoped, RevitRootObjectBuilder>(); builder.AddSingleton(); diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs new file mode 100644 index 000000000..c124e167b --- /dev/null +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs @@ -0,0 +1,43 @@ +using Speckle.Objects; +using Speckle.Objects.Geometry; +using Speckle.Objects.Other; +using Speckle.Sdk.Models; + +namespace Speckle.Connectors.Revit.HostApp; + +public class SendMaterialManager +{ + public Dictionary RenderMaterialProxies { get; } = new(); + + public void AddObjectToRenderMaterialMap(Base obj) + { + if (obj is not IDisplayValue> displayable) + { + return; + } + + foreach (var mesh in displayable.displayValue) + { + var renderMaterial = mesh["renderMaterial"] as RenderMaterial; + if (renderMaterial == null) + { + continue; + } + + var renderMaterialId = renderMaterial.applicationId ?? renderMaterial.GetId(); + var objectId = mesh.applicationId ?? mesh.GetId(); + + if (!RenderMaterialProxies.TryGetValue(renderMaterialId, out RenderMaterialProxy? proxy)) + { + RenderMaterialProxies[renderMaterialId] = new RenderMaterialProxy() + { + applicationId = renderMaterialId, + value = renderMaterial, + objects = [objectId] + }; + continue; + } + proxy.objects.Add(objectId); + } + } +} diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index dae373caa..2a0dd27a6 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -25,6 +25,7 @@ public class RevitRootObjectBuilder : IRootObjectBuilder private readonly ISyncToThread _syncToThread; private readonly SendSelectionUnpacker _sendSelectionUnpacker; private readonly SendCollectionManager _sendCollectionManager; + private readonly SendMaterialManager _sendMaterialManager; public RevitRootObjectBuilder( IRootToSpeckleConverter converter, @@ -32,7 +33,8 @@ public RevitRootObjectBuilder( ISendConversionCache sendConversionCache, ISyncToThread syncToThread, SendSelectionUnpacker sendSelectionUnpacker, - SendCollectionManager sendCollectionManager + SendCollectionManager sendCollectionManager, + SendMaterialManager sendMaterialManager ) { _converter = converter; @@ -41,6 +43,7 @@ SendCollectionManager sendCollectionManager _syncToThread = syncToThread; _sendSelectionUnpacker = sendSelectionUnpacker; _sendCollectionManager = sendCollectionManager; + _sendMaterialManager = sendMaterialManager; // Note, this class is instantiated per unit of work (aka per send operation), so we can safely initialize what we need in here. _collectionCache = new Dictionary(); @@ -109,7 +112,8 @@ public Task Build( var collection = _sendCollectionManager.GetAndCreateObjectHostCollection(revitElement, _rootObject); collection.elements.Add(converted); - // TODO: extract material into proxies here? + _sendMaterialManager.AddObjectToRenderMaterialMap(converted); // TODO: extract material into proxies here? + results.Add(new(Status.SUCCESS, applicationId, revitElement.GetType().Name, converted)); } catch (Exception ex) when (!ex.IsFatal()) @@ -121,6 +125,10 @@ public Task Build( onOperationProgressed?.Invoke("Converting", (double)++countProgress / revitElements.Count); } + var materialProxies = _sendMaterialManager.RenderMaterialProxies.Values.ToList(); + var nextLevel = _contextStack.RenderMaterialProxies.Values.ToList(); + _rootObject["renderMaterialProxies"] = nextLevel; + // POC: Log would be nice, or can be removed. Debug.WriteLine( $"Cache hit count {cacheHitCount} out of {objects.Count} ({(double)cacheHitCount / objects.Count})" diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems b/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems index 8a34384cc..4b39daa53 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems @@ -39,6 +39,7 @@ + diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs index 1cf5d4b80..fbb654783 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs @@ -1,5 +1,6 @@ using Autodesk.Revit.DB; using Speckle.Converters.Common; +using Speckle.Objects.Other; namespace Speckle.Converters.RevitShared.Helpers; @@ -10,4 +11,7 @@ namespace Speckle.Converters.RevitShared.Helpers; )] // POC: so this should *probably* be Document and NOT UI.UIDocument, the former is Conversion centric // and the latter is more for connector -public interface IRevitConversionContextStack : IConversionContextStack { } +public interface IRevitConversionContextStack : IConversionContextStack +{ + public Dictionary RenderMaterialProxies { get; } +} diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs index ce43a8890..1ce360c40 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs @@ -1,5 +1,6 @@ using Autodesk.Revit.DB; using Speckle.Converters.Common; +using Speckle.Objects.Other; namespace Speckle.Converters.RevitShared.Helpers; @@ -13,6 +14,7 @@ namespace Speckle.Converters.RevitShared.Helpers; public class RevitConversionContextStack : ConversionContextStack, IRevitConversionContextStack { public const double TOLERANCE = 0.0164042; // 5mm in ft + public Dictionary RenderMaterialProxies { get; } = new(); public RevitConversionContextStack(RevitContext context, IHostToSpeckleUnitConverter unitConverter) : base( diff --git a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs index e4db3acde..e1d5b3253 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs @@ -38,11 +38,12 @@ public MeshByMaterialDictionaryToSpeckle( public List Convert(Dictionary> target) { var result = new List(target.Keys.Count); + var materialProxyMap = _contextStack.RenderMaterialProxies; - foreach (var meshData in target) + foreach (var keyValuePair in target) { - DB.ElementId materialId = meshData.Key; - List meshes = meshData.Value; + DB.ElementId materialId = keyValuePair.Key; + List meshes = keyValuePair.Value; // We compute the final size of the arrays to prevent unnecessary resizing. (int verticesSize, int facesSize) = GetVertexAndFaceListSize(meshes); @@ -51,13 +52,25 @@ public MeshByMaterialDictionaryToSpeckle( var speckleMesh = new SOG.Mesh( new List(verticesSize), new List(facesSize), - units: _contextStack.Current.SpeckleUnits + units: _contextStack.Current.SpeckleUnits, + applicationId: Guid.NewGuid().ToString() // NOTE: as we are composing meshes out of multiple ones for the same material, we need to generate our own application id. c'est la vie. ); var doc = _contextStack.Current.Document; if (doc.GetElement(materialId) is DB.Material material) { - speckleMesh["renderMaterial"] = _materialConverter.Convert(material); + var speckleMaterial = _materialConverter.Convert(material); // TODO: get out + if (!materialProxyMap.TryGetValue(materialId.ToString()!, out RenderMaterialProxy? renderMaterialProxy)) + { + renderMaterialProxy = new RenderMaterialProxy() + { + value = speckleMaterial, + applicationId = materialId.ToString()!, + objects = [] + }; + materialProxyMap[materialId.ToString()!] = renderMaterialProxy; + } + renderMaterialProxy.objects.Add(speckleMesh.applicationId!); } // Append the revit mesh data to the speckle mesh diff --git a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/MaterialConversionToSpeckle.cs b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/MaterialConversionToSpeckle.cs index 23a320be6..a52da874a 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/MaterialConversionToSpeckle.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/MaterialConversionToSpeckle.cs @@ -13,7 +13,8 @@ public RenderMaterial Convert(DB.Material target) => { name = target.Name, opacity = 1 - target.Transparency / 100d, - diffuse = System.Drawing.Color.FromArgb(target.Color.Red, target.Color.Green, target.Color.Blue).ToArgb() + diffuse = System.Drawing.Color.FromArgb(target.Color.Red, target.Color.Green, target.Color.Blue).ToArgb(), + applicationId = target.Id.ToString() //metalness = revitMaterial.Shininess / 128d, //Looks like these are not valid conversions //roughness = 1 - (revitMaterial.Smoothness / 100d) }; From 52e2f6a23a0e81d401f60087fea4adf7aa43ad58 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Wed, 14 Aug 2024 12:00:40 +0100 Subject: [PATCH 03/17] chore: removes unused code --- .../RevitConnectorModule.cs | 1 - .../HostApp/SendMaterialManager.cs | 43 ------------------- .../Operations/Send/RevitRootObjectBuilder.cs | 16 ++----- .../Speckle.Connectors.RevitShared.projitems | 1 - 4 files changed, 3 insertions(+), 58 deletions(-) delete mode 100644 Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs index 0abda792e..c428ce8ea 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs @@ -68,7 +68,6 @@ public void Load(SpeckleContainerBuilder builder) builder.AddScoped>(); builder.AddScoped(); builder.AddScoped(); - builder.AddScoped(); builder.AddScoped, RevitRootObjectBuilder>(); builder.AddSingleton(); diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs deleted file mode 100644 index c124e167b..000000000 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendMaterialManager.cs +++ /dev/null @@ -1,43 +0,0 @@ -using Speckle.Objects; -using Speckle.Objects.Geometry; -using Speckle.Objects.Other; -using Speckle.Sdk.Models; - -namespace Speckle.Connectors.Revit.HostApp; - -public class SendMaterialManager -{ - public Dictionary RenderMaterialProxies { get; } = new(); - - public void AddObjectToRenderMaterialMap(Base obj) - { - if (obj is not IDisplayValue> displayable) - { - return; - } - - foreach (var mesh in displayable.displayValue) - { - var renderMaterial = mesh["renderMaterial"] as RenderMaterial; - if (renderMaterial == null) - { - continue; - } - - var renderMaterialId = renderMaterial.applicationId ?? renderMaterial.GetId(); - var objectId = mesh.applicationId ?? mesh.GetId(); - - if (!RenderMaterialProxies.TryGetValue(renderMaterialId, out RenderMaterialProxy? proxy)) - { - RenderMaterialProxies[renderMaterialId] = new RenderMaterialProxy() - { - applicationId = renderMaterialId, - value = renderMaterial, - objects = [objectId] - }; - continue; - } - proxy.objects.Add(objectId); - } - } -} diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index 2a0dd27a6..8c056b048 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -19,13 +19,11 @@ public class RevitRootObjectBuilder : IRootObjectBuilder // POC: SendSelection and RevitConversionContextStack should be interfaces, former needs interfaces private readonly IRootToSpeckleConverter _converter; private readonly IRevitConversionContextStack _contextStack; - private readonly Dictionary _collectionCache; private readonly Collection _rootObject; private readonly ISendConversionCache _sendConversionCache; private readonly ISyncToThread _syncToThread; private readonly SendSelectionUnpacker _sendSelectionUnpacker; private readonly SendCollectionManager _sendCollectionManager; - private readonly SendMaterialManager _sendMaterialManager; public RevitRootObjectBuilder( IRootToSpeckleConverter converter, @@ -33,8 +31,7 @@ public RevitRootObjectBuilder( ISendConversionCache sendConversionCache, ISyncToThread syncToThread, SendSelectionUnpacker sendSelectionUnpacker, - SendCollectionManager sendCollectionManager, - SendMaterialManager sendMaterialManager + SendCollectionManager sendCollectionManager ) { _converter = converter; @@ -43,10 +40,7 @@ SendMaterialManager sendMaterialManager _syncToThread = syncToThread; _sendSelectionUnpacker = sendSelectionUnpacker; _sendCollectionManager = sendCollectionManager; - _sendMaterialManager = sendMaterialManager; - // Note, this class is instantiated per unit of work (aka per send operation), so we can safely initialize what we need in here. - _collectionCache = new Dictionary(); _rootObject = new Collection() { name = _contextStack.Current.Document.PathName.Split('\\').Last().Split('.').First() @@ -112,8 +106,6 @@ public Task Build( var collection = _sendCollectionManager.GetAndCreateObjectHostCollection(revitElement, _rootObject); collection.elements.Add(converted); - _sendMaterialManager.AddObjectToRenderMaterialMap(converted); // TODO: extract material into proxies here? - results.Add(new(Status.SUCCESS, applicationId, revitElement.GetType().Name, converted)); } catch (Exception ex) when (!ex.IsFatal()) @@ -124,10 +116,8 @@ public Task Build( onOperationProgressed?.Invoke("Converting", (double)++countProgress / revitElements.Count); } - - var materialProxies = _sendMaterialManager.RenderMaterialProxies.Values.ToList(); - var nextLevel = _contextStack.RenderMaterialProxies.Values.ToList(); - _rootObject["renderMaterialProxies"] = nextLevel; + var materialProxies = _contextStack.RenderMaterialProxies.Values.ToList(); + _rootObject["renderMaterialProxies"] = materialProxies; // POC: Log would be nice, or can be removed. Debug.WriteLine( diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems b/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems index 4b39daa53..8a34384cc 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems @@ -39,7 +39,6 @@ - From 13fcf8900fff515f1e7b3208594634fdb45e62dd Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Wed, 14 Aug 2024 22:31:08 +0100 Subject: [PATCH 04/17] feat(revit): material proxies wip --- .../HostApp/SendSelectionUnpacker.cs | 5 +- .../Operations/Send/RevitRootObjectBuilder.cs | 25 +++++--- .../Plugin/RevitIdleManager.cs | 11 +++- .../RevitConverterModule.cs | 5 +- .../Helpers/DisplayValueExtractor.cs | 12 +++- .../Helpers/IRevitConversionContextStack.cs | 3 +- .../Helpers/RevitConversionContextStack.cs | 58 ++++++++++++++++++- .../MeshByMaterialDictionaryToSpeckle.cs | 25 +++++--- 8 files changed, 114 insertions(+), 30 deletions(-) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs index 440da5a2b..12b570273 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs @@ -76,8 +76,9 @@ private List UnpackElements(IEnumerable elements) unpackedElements.Add(element); } } - - return unpackedElements; + // Why filtering for duplicates? Well, well, well... it's related to the comment above on groups: if a group + // contains a nested family, GetMemberIds() will return... duplicates of the exploded family components. + return unpackedElements.GroupBy(el => el.Id).Select(g => g.First()).ToList(); // no disinctBy in here sadly. } private List PackCurtainWallElements(List elements) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index 8c056b048..62e8d3779 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -7,6 +7,7 @@ using Speckle.Connectors.Utils.Conversion; using Speckle.Connectors.Utils.Operations; using Speckle.Converters.Common; +using Speckle.Converters.RevitShared.Extensions; using Speckle.Converters.RevitShared.Helpers; using Speckle.Sdk; using Speckle.Sdk.Models; @@ -18,7 +19,7 @@ public class RevitRootObjectBuilder : IRootObjectBuilder { // POC: SendSelection and RevitConversionContextStack should be interfaces, former needs interfaces private readonly IRootToSpeckleConverter _converter; - private readonly IRevitConversionContextStack _contextStack; + private readonly IRevitConversionContextStack _conversionContextStack; private readonly Collection _rootObject; private readonly ISendConversionCache _sendConversionCache; private readonly ISyncToThread _syncToThread; @@ -27,7 +28,7 @@ public class RevitRootObjectBuilder : IRootObjectBuilder public RevitRootObjectBuilder( IRootToSpeckleConverter converter, - IRevitConversionContextStack contextStack, + IRevitConversionContextStack conversionContextStack, ISendConversionCache sendConversionCache, ISyncToThread syncToThread, SendSelectionUnpacker sendSelectionUnpacker, @@ -35,7 +36,7 @@ SendCollectionManager sendCollectionManager ) { _converter = converter; - _contextStack = contextStack; + _conversionContextStack = conversionContextStack; _sendConversionCache = sendConversionCache; _syncToThread = syncToThread; _sendSelectionUnpacker = sendSelectionUnpacker; @@ -43,7 +44,7 @@ SendCollectionManager sendCollectionManager _rootObject = new Collection() { - name = _contextStack.Current.Document.PathName.Split('\\').Last().Split('.').First() + name = _conversionContextStack.Current.Document.PathName.Split('\\').Last().Split('.').First() }; } @@ -55,7 +56,7 @@ public Task Build( ) => _syncToThread.RunOnThread(() => { - var doc = _contextStack.Current.Document; + var doc = _conversionContextStack.Current.Document; if (doc.IsFamilyDocument) { @@ -67,7 +68,7 @@ public Task Build( // Convert ids to actual revit elements foreach (var id in objects) { - var el = _contextStack.Current.Document.GetElement(id); + var el = _conversionContextStack.Current.Document.GetElement(id); if (el != null) { revitElements.Add(el); @@ -85,7 +86,7 @@ public Task Build( var countProgress = 0; // because for(int i = 0; ...) loops are so last year var cacheHitCount = 0; List results = new(revitElements.Count); - + List succesfullyConvertedElementIds = new(); foreach (Element revitElement in atomicObjects) { ct.ThrowIfCancellationRequested(); @@ -106,17 +107,23 @@ public Task Build( var collection = _sendCollectionManager.GetAndCreateObjectHostCollection(revitElement, _rootObject); collection.elements.Add(converted); + succesfullyConvertedElementIds.Add(revitElement.Id.ToString()); results.Add(new(Status.SUCCESS, applicationId, revitElement.GetType().Name, converted)); } catch (Exception ex) when (!ex.IsFatal()) { results.Add(new(Status.ERROR, applicationId, revitElement.GetType().Name, null, ex)); - // POC: add logging } onOperationProgressed?.Invoke("Converting", (double)++countProgress / revitElements.Count); } - var materialProxies = _contextStack.RenderMaterialProxies.Values.ToList(); + + // TODO: Stacked and curtain walls do not work as expected, we'll need to either: + // - unpack them, and introduce "subcomponent" proxies later + // - process their ids separately to get their subcomponents ids + var materialProxies = _conversionContextStack.RenderMaterialProxyCache.GetRenderMaterialProxyListForObjects( + succesfullyConvertedElementIds.Distinct().ToList() + ); _rootObject["renderMaterialProxies"] = materialProxies; // POC: Log would be nice, or can be removed. diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitIdleManager.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitIdleManager.cs index 632313d42..b4531ddf4 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitIdleManager.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitIdleManager.cs @@ -3,6 +3,7 @@ using Speckle.Connectors.DUI.Bridge; using Speckle.Converters.RevitShared.Helpers; using Speckle.InterfaceGenerator; +using Speckle.Sdk; using Speckle.Sdk.Common; namespace Speckle.Connectors.Revit.Plugin; @@ -24,7 +25,15 @@ public void SubscribeToIdle(string id, Action action) => action, () => { - _uiApplication.Idling += RevitAppOnIdle; + try + { + _uiApplication.Idling += RevitAppOnIdle; + } + catch (Exception e) when (!e.IsFatal()) + { + // TODO: wrap this guy in the top level exception handler (?) + // This happens very rarely, see previous report [CNX-125: Autodesk.Revit.Exceptions.InvalidOperationException: Can not subscribe to an event during execution of that event!](https://linear.app/speckle/issue/CNX-125/autodeskrevitexceptionsinvalidoperationexception-can-not-subscribe-to) + } } ); diff --git a/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs b/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs index 4f9a385be..b0c3b9cc6 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs @@ -5,8 +5,6 @@ using Speckle.Converters.RevitShared.Helpers; using Speckle.Converters.RevitShared.Services; -// using Speckle.Converters.RevitShared.ToSpeckle; - namespace Speckle.Converters.RevitShared.DependencyInjection; public class RevitConverterModule : ISpeckleModule @@ -24,6 +22,8 @@ public void Load(SpeckleContainerBuilder builder) builder.AddScoped(); builder.AddSingleton(new RevitContext()); + builder.AddSingleton(new RenderMaterialProxyCacheSingleton()); + // POC: do we need ToSpeckleScalingService as is, do we need to interface it out? builder.AddScoped(); builder.AddScoped(); @@ -39,7 +39,6 @@ public void Load(SpeckleContainerBuilder builder) builder.AddScoped(); builder.AddScoped(); builder.AddScoped(); - // builder.AddScoped(); builder.AddScoped(); builder.AddScoped(); diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs index 5025a90c2..3a6c9ce1e 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs @@ -7,11 +7,17 @@ namespace Speckle.Converters.RevitShared.Helpers; // POC: needs breaking down https://spockle.atlassian.net/browse/CNX-9354 public sealed class DisplayValueExtractor { - private readonly ITypedConverter>, List> _meshByMaterialConverter; + private readonly ITypedConverter< + (Dictionary> target, DB.ElementId parentElementId), + List + > _meshByMaterialConverter; private readonly ILogger _logger; public DisplayValueExtractor( - ITypedConverter>, List> meshByMaterialConverter, + ITypedConverter< + (Dictionary> target, DB.ElementId parentElementId), + List + > meshByMaterialConverter, ILogger logger ) { @@ -25,7 +31,7 @@ ILogger logger var meshesByMaterial = GetMeshesByMaterial(meshes, solids); - return _meshByMaterialConverter.Convert(meshesByMaterial); + return _meshByMaterialConverter.Convert((meshesByMaterial, element.Id)); } private static Dictionary> GetMeshesByMaterial( diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs index fbb654783..a2decff16 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs @@ -1,6 +1,5 @@ using Autodesk.Revit.DB; using Speckle.Converters.Common; -using Speckle.Objects.Other; namespace Speckle.Converters.RevitShared.Helpers; @@ -13,5 +12,5 @@ namespace Speckle.Converters.RevitShared.Helpers; // and the latter is more for connector public interface IRevitConversionContextStack : IConversionContextStack { - public Dictionary RenderMaterialProxies { get; } + public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } } diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs index 1ce360c40..41da4c259 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs @@ -13,10 +13,14 @@ namespace Speckle.Converters.RevitShared.Helpers; // and the latter is more for connector public class RevitConversionContextStack : ConversionContextStack, IRevitConversionContextStack { + public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } public const double TOLERANCE = 0.0164042; // 5mm in ft - public Dictionary RenderMaterialProxies { get; } = new(); - public RevitConversionContextStack(RevitContext context, IHostToSpeckleUnitConverter unitConverter) + public RevitConversionContextStack( + RevitContext context, + IHostToSpeckleUnitConverter unitConverter, + RenderMaterialProxyCacheSingleton renderMaterialProxyCache + ) : base( // POC: we probably should not get here without a valid document // so should this perpetuate or do we assume this is valid? @@ -26,5 +30,53 @@ public RevitConversionContextStack(RevitContext context, IHostToSpeckleUnitConve ?? throw new SpeckleConversionException("Active UI document could not be determined"), context.UIApplication.ActiveUIDocument.Document.GetUnits().GetFormatOptions(SpecTypeId.Length).GetUnitTypeId(), unitConverter - ) { } + ) + { + RenderMaterialProxyCache = renderMaterialProxyCache; + } +} + +/// +/// singleton; should persist across units of work +/// TODO: description +/// TODO: move to appropriate location +/// +public class RenderMaterialProxyCacheSingleton +{ + /// + /// map(object id, ( map (materialId, proxy) ) ) + /// a per object map of material proxies. not the best way??? + /// + public Dictionary> ObjectRenderMaterialProxiesMap { get; } = new(); + + public List GetRenderMaterialProxyListForObjects(List elementIds) + { + // TODO: + // merge all render material proxies by their material id + // return that + var proxiesToMerge = ObjectRenderMaterialProxiesMap + .Where(kvp => elementIds.Contains(kvp.Key)) + .Select(kvp => kvp.Value); // TODO + var tlist = ObjectRenderMaterialProxiesMap.Values.ToList(); + + var mergeTarget = new Dictionary(); + foreach (var dictionary in proxiesToMerge) + { + foreach (var kvp in dictionary) + { + if (!mergeTarget.TryGetValue(kvp.Key, out RenderMaterialProxy? value)) + { + value = kvp.Value; + mergeTarget[kvp.Key] = value; + continue; + } + value.objects.AddRange(kvp.Value.objects); + } + } + foreach (var renderMaterialProxy in mergeTarget.Values) + { + renderMaterialProxy.objects = renderMaterialProxy.objects.Distinct().ToList(); + } + return mergeTarget.Values.ToList(); + } } diff --git a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs index e1d5b3253..7d304a0fd 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/Raw/Geometry/MeshByMaterialDictionaryToSpeckle.cs @@ -5,7 +5,7 @@ namespace Speckle.Converters.RevitShared.ToSpeckle; public class MeshByMaterialDictionaryToSpeckle - : ITypedConverter>, List> + : ITypedConverter<(Dictionary> target, DB.ElementId parentElementId), List> { private readonly IRevitConversionContextStack _contextStack; private readonly ITypedConverter _xyzToPointConverter; @@ -25,7 +25,7 @@ public MeshByMaterialDictionaryToSpeckle( /// /// Converts a dictionary of Revit meshes, where key is MaterialId, into a list of Speckle meshes. /// - /// A dictionary with DB.ElementId keys and List of DB.Mesh values. + /// A tuple consisting of (1) a dictionary with DB.ElementId keys and List of DB.Mesh values and (2) the root element id (the one generating all the meshes). /// /// Returns a list of objects where each mesh represents one unique material in the input dictionary. /// @@ -34,13 +34,22 @@ public MeshByMaterialDictionaryToSpeckle( /// These meshes are created with an initial capacity based on the size of the vertex and face arrays to avoid unnecessary resizing. /// Also note that, for each unique material, the method tries to retrieve the related DB.Material from the current document and convert it. If the conversion is successful, /// the material is added to the corresponding Speckle mesh. If the conversion fails, the operation simply continues without the material. + /// TODO: update description /// - public List Convert(Dictionary> target) + public List Convert((Dictionary> target, DB.ElementId parentElementId) args) { - var result = new List(target.Keys.Count); - var materialProxyMap = _contextStack.RenderMaterialProxies; + var result = new List(args.target.Keys.Count); + var objectRenderMaterialProxiesMap = _contextStack.RenderMaterialProxyCache.ObjectRenderMaterialProxiesMap; - foreach (var keyValuePair in target) + var materialProxyMap = new Dictionary(); + objectRenderMaterialProxiesMap[args.parentElementId.ToString()!] = materialProxyMap; + + if (args.target.Count == 0) + { + return new(); + } + + foreach (var keyValuePair in args.target) { DB.ElementId materialId = keyValuePair.Key; List meshes = keyValuePair.Value; @@ -57,9 +66,11 @@ public MeshByMaterialDictionaryToSpeckle( ); var doc = _contextStack.Current.Document; + if (doc.GetElement(materialId) is DB.Material material) { - var speckleMaterial = _materialConverter.Convert(material); // TODO: get out + var speckleMaterial = _materialConverter.Convert(material); + if (!materialProxyMap.TryGetValue(materialId.ToString()!, out RenderMaterialProxy? renderMaterialProxy)) { renderMaterialProxy = new RenderMaterialProxy() From 408f651ff3844d134de64aa19f52ab57bfb3d800 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Thu, 15 Aug 2024 10:45:42 +0100 Subject: [PATCH 05/17] wip --- .../Operations/Send/RevitRootObjectBuilder.cs | 1 - .../Helpers/RevitConversionContextStack.cs | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index 62e8d3779..83ecc0394 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -7,7 +7,6 @@ using Speckle.Connectors.Utils.Conversion; using Speckle.Connectors.Utils.Operations; using Speckle.Converters.Common; -using Speckle.Converters.RevitShared.Extensions; using Speckle.Converters.RevitShared.Helpers; using Speckle.Sdk; using Speckle.Sdk.Models; diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs index 41da4c259..329d336fe 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs @@ -49,15 +49,18 @@ public class RenderMaterialProxyCacheSingleton /// public Dictionary> ObjectRenderMaterialProxiesMap { get; } = new(); + /// + /// Returns the material proxy list for the given objects. + /// + /// + /// public List GetRenderMaterialProxyListForObjects(List elementIds) { - // TODO: // merge all render material proxies by their material id // return that var proxiesToMerge = ObjectRenderMaterialProxiesMap .Where(kvp => elementIds.Contains(kvp.Key)) - .Select(kvp => kvp.Value); // TODO - var tlist = ObjectRenderMaterialProxiesMap.Values.ToList(); + .Select(kvp => kvp.Value); var mergeTarget = new Dictionary(); foreach (var dictionary in proxiesToMerge) From 9d3328699a71d66e3ab403ac4c1498254fef4a5d Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Thu, 15 Aug 2024 14:01:04 +0100 Subject: [PATCH 06/17] removes unused converted cache --- .../Helpers/ToSpeckleConvertedObjectsCache.cs | 25 ------------------- 1 file changed, 25 deletions(-) delete mode 100644 Converters/Revit/Speckle.Converters.RevitShared/Helpers/ToSpeckleConvertedObjectsCache.cs diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/ToSpeckleConvertedObjectsCache.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/ToSpeckleConvertedObjectsCache.cs deleted file mode 100644 index 51746ea97..000000000 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/ToSpeckleConvertedObjectsCache.cs +++ /dev/null @@ -1,25 +0,0 @@ -using Speckle.Sdk.Models; - -namespace Speckle.Converters.RevitShared.Helpers; - -// POC: review the cache? should this be a common class? -// Does caching work this way everywhere, i.e, string key and base value? -public sealed class ToSpeckleConvertedObjectsCache -{ - private readonly Dictionary _uniqueIdToConvertedBaseDict = new(); - - public void AddConvertedBase(string revitUniqueId, Base b) - { - _uniqueIdToConvertedBaseDict.Add(revitUniqueId, b); - } - - public bool ContainsBaseConvertedFromId(string revitUniqueId) - { - return _uniqueIdToConvertedBaseDict.ContainsKey(revitUniqueId); - } - - public bool TryGetConvertedBase(string revitUniqueId, out Base? value) - { - return _uniqueIdToConvertedBaseDict.TryGetValue(revitUniqueId, out value); - } -} From 13cacaed4bfb8f1e8dddedf577cb670f992496c6 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Fri, 16 Aug 2024 11:41:10 +0100 Subject: [PATCH 07/17] wip --- .../RevitConnectorModule.cs | 2 +- ...electionUnpacker.cs => ElementUnpacker.cs} | 47 +++++++++++++++++-- .../Operations/Send/RevitRootObjectBuilder.cs | 20 ++++---- .../Speckle.Connectors.RevitShared.projitems | 2 +- .../Helpers/IRevitConversionContextStack.cs | 1 + .../Helpers/RevitConversionContextStack.cs | 9 ++++ .../RevitRootToSpeckleConverter.cs | 10 +++- .../Speckle.Converters.RevitShared.projitems | 3 +- .../WallTopLevelConverterToSpeckle.cs | 3 +- 9 files changed, 74 insertions(+), 23 deletions(-) rename Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/{SendSelectionUnpacker.cs => ElementUnpacker.cs} (72%) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs index c428ce8ea..56e61f76b 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/DependencyInjection/RevitConnectorModule.cs @@ -66,7 +66,7 @@ public void Load(SpeckleContainerBuilder builder) // send operation and dependencies builder.AddScoped>(); - builder.AddScoped(); + builder.AddScoped(); builder.AddScoped(); builder.AddScoped, RevitRootObjectBuilder>(); builder.AddSingleton(); diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs similarity index 72% rename from Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs rename to Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs index 12b570273..6275f7de3 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/SendSelectionUnpacker.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs @@ -22,16 +22,16 @@ namespace Speckle.Connectors.Revit.HostApp; /// /// /// -public class SendSelectionUnpacker +public class ElementUnpacker { private readonly IRevitConversionContextStack _contextStack; - public SendSelectionUnpacker(IRevitConversionContextStack contextStack) + public ElementUnpacker(IRevitConversionContextStack contextStack) { _contextStack = contextStack; } - public IEnumerable UnpackSelection(IEnumerable selectionElements) + public IEnumerable UnpackSelectionForConversion(IEnumerable selectionElements) { // Note: steps kept separate on purpose. // Step 1: unpack groups @@ -43,20 +43,20 @@ public IEnumerable UnpackSelection(IEnumerable selectionElemen return PackCurtainWallElements(atomicObjects); } - // This needs some yield refactoring private List UnpackElements(IEnumerable elements) { var unpackedElements = new List(); // note: could be a hashset/map so we prevent duplicates (?) foreach (var element in elements) { + // UNPACK: Groups if (element is Group g) { // POC: this might screw up generating hosting rel generation here, because nested families in groups get flattened out by GetMemberIds(). - // in other words, if a group contains nested families, .GetMemberIds() will return all "exploded" families. var groupElements = g.GetMemberIds().Select(_contextStack.Current.Document.GetElement); unpackedElements.AddRange(UnpackElements(groupElements)); } + // UNPACK: Family instances (as they potentially have nested families inside) else if (element is FamilyInstance familyInstance) { var familyElements = familyInstance @@ -89,4 +89,41 @@ private List PackCurtainWallElements(List elements) ); return elements; } + + public List GetElementsAndSubelementIdsFromAtomicObjects(List elements) + { + var ids = new HashSet(); + foreach (var element in elements) + { + switch (element) + { + case Wall wall: + if (wall.CurtainGrid is { } grid) + { + foreach (var mullionId in grid.GetMullionIds()) + { + ids.Add(mullionId.ToString()); + } + foreach (var panelId in grid.GetPanelIds()) + { + ids.Add(panelId.ToString()); + } + } + else if (wall.IsStackedWall) + { + foreach (var stackedWallId in wall.GetStackedWallMemberIds()) + { + ids.Add(stackedWallId.ToString()); + } + } + break; + default: + break; + } + + ids.Add(element.Id.ToString()); + } + + return ids.ToList(); + } } diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index 83ecc0394..da1c80389 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -22,7 +22,7 @@ public class RevitRootObjectBuilder : IRootObjectBuilder private readonly Collection _rootObject; private readonly ISendConversionCache _sendConversionCache; private readonly ISyncToThread _syncToThread; - private readonly SendSelectionUnpacker _sendSelectionUnpacker; + private readonly ElementUnpacker _elementUnpacker; private readonly SendCollectionManager _sendCollectionManager; public RevitRootObjectBuilder( @@ -30,7 +30,7 @@ public RevitRootObjectBuilder( IRevitConversionContextStack conversionContextStack, ISendConversionCache sendConversionCache, ISyncToThread syncToThread, - SendSelectionUnpacker sendSelectionUnpacker, + ElementUnpacker elementUnpacker, SendCollectionManager sendCollectionManager ) { @@ -38,7 +38,7 @@ SendCollectionManager sendCollectionManager _conversionContextStack = conversionContextStack; _sendConversionCache = sendConversionCache; _syncToThread = syncToThread; - _sendSelectionUnpacker = sendSelectionUnpacker; + _elementUnpacker = elementUnpacker; _sendCollectionManager = sendCollectionManager; _rootObject = new Collection() @@ -80,12 +80,13 @@ public Task Build( } // Unpack groups (& other complex data structures) - var atomicObjects = _sendSelectionUnpacker.UnpackSelection(revitElements).ToList(); + var atomicObjects = _elementUnpacker.UnpackSelectionForConversion(revitElements).ToList(); - var countProgress = 0; // because for(int i = 0; ...) loops are so last year + var countProgress = 0; var cacheHitCount = 0; List results = new(revitElements.Count); - List succesfullyConvertedElementIds = new(); + List succesfullyConvertedElementIds = new(); // TODO: should be part of the context stack, imho + foreach (Element revitElement in atomicObjects) { ct.ThrowIfCancellationRequested(); @@ -119,10 +120,9 @@ public Task Build( // TODO: Stacked and curtain walls do not work as expected, we'll need to either: // - unpack them, and introduce "subcomponent" proxies later - // - process their ids separately to get their subcomponents ids - var materialProxies = _conversionContextStack.RenderMaterialProxyCache.GetRenderMaterialProxyListForObjects( - succesfullyConvertedElementIds.Distinct().ToList() - ); + // - process their ids separately to get their subcomponents + var ids = _elementUnpacker.GetElementsAndSubelementIdsFromAtomicObjects(atomicObjects); + var materialProxies = _conversionContextStack.RenderMaterialProxyCache.GetRenderMaterialProxyListForObjects(ids); _rootObject["renderMaterialProxies"] = materialProxies; // POC: Log would be nice, or can be removed. diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems b/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems index 8a34384cc..a8d9472b0 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Speckle.Connectors.RevitShared.projitems @@ -39,7 +39,7 @@ - + diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs index a2decff16..ab0e1c0b5 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs @@ -13,4 +13,5 @@ namespace Speckle.Converters.RevitShared.Helpers; public interface IRevitConversionContextStack : IConversionContextStack { public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } + public List ConvertedElementsAndSubelementIds { get; } } diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs index 329d336fe..49fe608a5 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs @@ -13,7 +13,16 @@ namespace Speckle.Converters.RevitShared.Helpers; // and the latter is more for connector public class RevitConversionContextStack : ConversionContextStack, IRevitConversionContextStack { + /// + /// Persistent cache (across conversions) for all generated render material proxies. Note this cache stores a list of render material proxies per element id. + /// public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } + + /// + /// Keeps track of all converted elements and any resulting subelement ids. For example, if sending a curtain wall, this will contain the original wall id, as well as the ids of all the mullions/panels. Stacked wall component ids will also be stored in here. + /// + public List ConvertedElementsAndSubelementIds { get; } = new(); + public const double TOLERANCE = 0.0164042; // 5mm in ft public RevitConversionContextStack( diff --git a/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs b/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs index 0d9e7363b..5dfb43648 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs @@ -10,14 +10,17 @@ public class RevitRootToSpeckleConverter : IRootToSpeckleConverter { private readonly IConverterResolver _toSpeckle; private readonly ParameterValueExtractor _parameterValueExtractor; + private readonly IRevitConversionContextStack _contextStack; public RevitRootToSpeckleConverter( IConverterResolver toSpeckle, - ParameterValueExtractor parameterValueExtractor + ParameterValueExtractor parameterValueExtractor, + IRevitConversionContextStack contextStack ) { _toSpeckle = toSpeckle; _parameterValueExtractor = parameterValueExtractor; + _contextStack = contextStack; } // POC: our assumption here is target is valid for conversion @@ -37,12 +40,15 @@ public Base Convert(object target) // POC : where should logic common to most objects go? // shouldn't target ALWAYS be DB.Element? + // Dim thinks so, FWIW if (target is DB.Element element) { // POC: is this the right place? result.applicationId = element.UniqueId; - _parameterValueExtractor.RemoveUniqueId(element.UniqueId); + + // Add all converted element ids to the context stack. The assumption here is that everything will pass through the converter, like mullions and panels do in the Wall converter. + _contextStack.ConvertedElementsAndSubelementIds.Add(element.Id); } return result; diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems b/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems index b1954f59e..8429f5543 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems +++ b/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems @@ -104,9 +104,8 @@ - - \ No newline at end of file + diff --git a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/TopLevel/WallTopLevelConverterToSpeckle.cs b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/TopLevel/WallTopLevelConverterToSpeckle.cs index f636f03c6..13b1319e2 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/TopLevel/WallTopLevelConverterToSpeckle.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/ToSpeckle/TopLevel/WallTopLevelConverterToSpeckle.cs @@ -140,8 +140,7 @@ private void AssignDisplayValue(DB.Wall target, RevitWall speckleWall) } else { - // POC: I have no why previously we were setting the display value, and then unsetting it. - // Probably curtain walls need a special case/etc.? + // Curtain walls have a bunch of subelements (mullions and co) that have their own display values. speckleWall.displayValue = new List(); } } From 5acd5a76909a5e75788f5d1645facd3d1617cfd9 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Fri, 16 Aug 2024 12:15:20 +0100 Subject: [PATCH 08/17] chore: cleanup part 1 --- .../HostApp/ElementUnpacker.cs | 28 ++++++++----------- .../Operations/Send/RevitRootObjectBuilder.cs | 12 +++----- .../Helpers/IRevitConversionContextStack.cs | 1 - .../Helpers/RevitConversionContextStack.cs | 16 ++++------- .../RevitRootToSpeckleConverter.cs | 3 -- 5 files changed, 21 insertions(+), 39 deletions(-) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs index 6275f7de3..d8dcf1112 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/ElementUnpacker.cs @@ -5,22 +5,6 @@ namespace Speckle.Connectors.Revit.HostApp; /// /// Class that unpacks a given set of selection elements into atomic objects. -/// POC: it's a fast solution to a lot of complex problems we don't want to answer now, but should in the near future. -/// What this does: -/// -/// -/// Groups: -/// explodes them into sub constituent objects, recursively. -/// -/// -/// Nested families: -/// explodes them into sub constituent objects, recursively. -/// -/// -/// Curtain walls: -/// If parent wall is part of selection, does not select individual elements out. Otherwise, selects individual elements (Panels, Mullions) as atomic objects. -/// -/// /// public class ElementUnpacker { @@ -31,6 +15,12 @@ public ElementUnpacker(IRevitConversionContextStack contextStack) _contextStack = contextStack; } + /// + /// Unpacks a random set of revit objects into atomic objects. It currently unpacks groups recurisvely, nested families into atomic family instances. + /// This method will also "pack" curtain walls if necessary (ie, if mullions or panels are selected without their parent curtain wall, they are sent independently; if the parent curtain wall is selected, they will be removed out as the curtain wall will include all its children). + /// + /// + /// public IEnumerable UnpackSelectionForConversion(IEnumerable selectionElements) { // Note: steps kept separate on purpose. @@ -90,6 +80,12 @@ private List PackCurtainWallElements(List elements) return elements; } + /// + /// Given a set of atomic elements, it will return a list of all their ids as well as their subelements. This currently handles curtain walls and stacked walls. + /// This might not be an exhaustive list of valid objects with "subelements" in revit, and will need revisiting. + /// + /// + /// public List GetElementsAndSubelementIdsFromAtomicObjects(List elements) { var ids = new HashSet(); diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index da1c80389..f9081a710 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -85,7 +85,6 @@ public Task Build( var countProgress = 0; var cacheHitCount = 0; List results = new(revitElements.Count); - List succesfullyConvertedElementIds = new(); // TODO: should be part of the context stack, imho foreach (Element revitElement in atomicObjects) { @@ -107,7 +106,6 @@ public Task Build( var collection = _sendCollectionManager.GetAndCreateObjectHostCollection(revitElement, _rootObject); collection.elements.Add(converted); - succesfullyConvertedElementIds.Add(revitElement.Id.ToString()); results.Add(new(Status.SUCCESS, applicationId, revitElement.GetType().Name, converted)); } catch (Exception ex) when (!ex.IsFatal()) @@ -118,14 +116,12 @@ public Task Build( onOperationProgressed?.Invoke("Converting", (double)++countProgress / revitElements.Count); } - // TODO: Stacked and curtain walls do not work as expected, we'll need to either: - // - unpack them, and introduce "subcomponent" proxies later - // - process their ids separately to get their subcomponents - var ids = _elementUnpacker.GetElementsAndSubelementIdsFromAtomicObjects(atomicObjects); - var materialProxies = _conversionContextStack.RenderMaterialProxyCache.GetRenderMaterialProxyListForObjects(ids); + var idsAndSubElementIds = _elementUnpacker.GetElementsAndSubelementIdsFromAtomicObjects(atomicObjects); + var materialProxies = _conversionContextStack.RenderMaterialProxyCache.GetRenderMaterialProxyListForObjects( + idsAndSubElementIds + ); _rootObject["renderMaterialProxies"] = materialProxies; - // POC: Log would be nice, or can be removed. Debug.WriteLine( $"Cache hit count {cacheHitCount} out of {objects.Count} ({(double)cacheHitCount / objects.Count})" ); diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs index ab0e1c0b5..a2decff16 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs @@ -13,5 +13,4 @@ namespace Speckle.Converters.RevitShared.Helpers; public interface IRevitConversionContextStack : IConversionContextStack { public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } - public List ConvertedElementsAndSubelementIds { get; } } diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs index 49fe608a5..4cb1b1d3d 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs @@ -18,11 +18,6 @@ public class RevitConversionContextStack : ConversionContextStack public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } - /// - /// Keeps track of all converted elements and any resulting subelement ids. For example, if sending a curtain wall, this will contain the original wall id, as well as the ids of all the mullions/panels. Stacked wall component ids will also be stored in here. - /// - public List ConvertedElementsAndSubelementIds { get; } = new(); - public const double TOLERANCE = 0.0164042; // 5mm in ft public RevitConversionContextStack( @@ -46,9 +41,10 @@ RenderMaterialProxyCacheSingleton renderMaterialProxyCache } /// -/// singleton; should persist across units of work -/// TODO: description -/// TODO: move to appropriate location +/// Persistent cache (across conversions) for all generated render material proxies. This cache stores a list of render material proxies per element id, and provides a method to get out the merged render material proxy list from a set of object ids for setting on the root commit object post conversion phase. +/// +/// Why is this needed? Because two reasons: send caching bypasses converter and revit conversions typically generate multiple display values per element. Ask dim for more and he might start crying. +/// /// public class RenderMaterialProxyCacheSingleton { @@ -59,14 +55,12 @@ public class RenderMaterialProxyCacheSingleton public Dictionary> ObjectRenderMaterialProxiesMap { get; } = new(); /// - /// Returns the material proxy list for the given objects. + /// Returns the merged material proxy list for the given object ids. Use this to get post conversion a correct list of material proxies for setting on the root commit object. /// /// /// public List GetRenderMaterialProxyListForObjects(List elementIds) { - // merge all render material proxies by their material id - // return that var proxiesToMerge = ObjectRenderMaterialProxiesMap .Where(kvp => elementIds.Contains(kvp.Key)) .Select(kvp => kvp.Value); diff --git a/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs b/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs index 5dfb43648..ac539cd04 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/RevitRootToSpeckleConverter.cs @@ -46,9 +46,6 @@ public Base Convert(object target) // POC: is this the right place? result.applicationId = element.UniqueId; _parameterValueExtractor.RemoveUniqueId(element.UniqueId); - - // Add all converted element ids to the context stack. The assumption here is that everything will pass through the converter, like mullions and panels do in the Wall converter. - _contextStack.ConvertedElementsAndSubelementIds.Add(element.Id); } return result; From f1dba34f41a6929ae195ad1ed866ed77893ddc02 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Fri, 16 Aug 2024 12:18:21 +0100 Subject: [PATCH 09/17] chore: finished cleanup (moved revit render material proxy cache out) --- .../RevitConverterModule.cs | 2 +- .../Helpers/IRevitConversionContextStack.cs | 2 +- .../Helpers/RevitConversionContextStack.cs | 52 +------------------ .../RevitRenderMaterialProxyCacheSingleton.cs | 50 ++++++++++++++++++ .../Speckle.Converters.RevitShared.projitems | 1 + 5 files changed, 55 insertions(+), 52 deletions(-) create mode 100644 Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitRenderMaterialProxyCacheSingleton.cs diff --git a/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs b/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs index b0c3b9cc6..2aff5593e 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared.DependencyInjection/RevitConverterModule.cs @@ -22,7 +22,7 @@ public void Load(SpeckleContainerBuilder builder) builder.AddScoped(); builder.AddSingleton(new RevitContext()); - builder.AddSingleton(new RenderMaterialProxyCacheSingleton()); + builder.AddSingleton(new RevitRenderMaterialProxyCacheSingleton()); // POC: do we need ToSpeckleScalingService as is, do we need to interface it out? builder.AddScoped(); diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs index a2decff16..742db810d 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/IRevitConversionContextStack.cs @@ -12,5 +12,5 @@ namespace Speckle.Converters.RevitShared.Helpers; // and the latter is more for connector public interface IRevitConversionContextStack : IConversionContextStack { - public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } + public RevitRenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } } diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs index 4cb1b1d3d..1dfd5d89a 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitConversionContextStack.cs @@ -1,6 +1,5 @@ using Autodesk.Revit.DB; using Speckle.Converters.Common; -using Speckle.Objects.Other; namespace Speckle.Converters.RevitShared.Helpers; @@ -16,14 +15,14 @@ public class RevitConversionContextStack : ConversionContextStack /// Persistent cache (across conversions) for all generated render material proxies. Note this cache stores a list of render material proxies per element id. /// - public RenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } + public RevitRenderMaterialProxyCacheSingleton RenderMaterialProxyCache { get; } public const double TOLERANCE = 0.0164042; // 5mm in ft public RevitConversionContextStack( RevitContext context, IHostToSpeckleUnitConverter unitConverter, - RenderMaterialProxyCacheSingleton renderMaterialProxyCache + RevitRenderMaterialProxyCacheSingleton renderMaterialProxyCache ) : base( // POC: we probably should not get here without a valid document @@ -39,50 +38,3 @@ RenderMaterialProxyCacheSingleton renderMaterialProxyCache RenderMaterialProxyCache = renderMaterialProxyCache; } } - -/// -/// Persistent cache (across conversions) for all generated render material proxies. This cache stores a list of render material proxies per element id, and provides a method to get out the merged render material proxy list from a set of object ids for setting on the root commit object post conversion phase. -/// -/// Why is this needed? Because two reasons: send caching bypasses converter and revit conversions typically generate multiple display values per element. Ask dim for more and he might start crying. -/// -/// -public class RenderMaterialProxyCacheSingleton -{ - /// - /// map(object id, ( map (materialId, proxy) ) ) - /// a per object map of material proxies. not the best way??? - /// - public Dictionary> ObjectRenderMaterialProxiesMap { get; } = new(); - - /// - /// Returns the merged material proxy list for the given object ids. Use this to get post conversion a correct list of material proxies for setting on the root commit object. - /// - /// - /// - public List GetRenderMaterialProxyListForObjects(List elementIds) - { - var proxiesToMerge = ObjectRenderMaterialProxiesMap - .Where(kvp => elementIds.Contains(kvp.Key)) - .Select(kvp => kvp.Value); - - var mergeTarget = new Dictionary(); - foreach (var dictionary in proxiesToMerge) - { - foreach (var kvp in dictionary) - { - if (!mergeTarget.TryGetValue(kvp.Key, out RenderMaterialProxy? value)) - { - value = kvp.Value; - mergeTarget[kvp.Key] = value; - continue; - } - value.objects.AddRange(kvp.Value.objects); - } - } - foreach (var renderMaterialProxy in mergeTarget.Values) - { - renderMaterialProxy.objects = renderMaterialProxy.objects.Distinct().ToList(); - } - return mergeTarget.Values.ToList(); - } -} diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitRenderMaterialProxyCacheSingleton.cs b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitRenderMaterialProxyCacheSingleton.cs new file mode 100644 index 000000000..5a4f3959a --- /dev/null +++ b/Converters/Revit/Speckle.Converters.RevitShared/Helpers/RevitRenderMaterialProxyCacheSingleton.cs @@ -0,0 +1,50 @@ +using Speckle.Objects.Other; + +namespace Speckle.Converters.RevitShared.Helpers; + +/// +/// Persistent cache (across conversions) for all generated render material proxies. This cache stores a list of render material proxies per element id, and provides a method to get out the merged render material proxy list from a set of object ids for setting on the root commit object post conversion phase. +/// +/// Why is this needed? Because two reasons: send caching bypasses converter and revit conversions typically generate multiple display values per element. Ask dim for more and he might start crying. +/// +/// +public class RevitRenderMaterialProxyCacheSingleton +{ + /// + /// map(object id, ( map (materialId, proxy) ) ) + /// a per object map of material proxies. not the best way??? + /// + public Dictionary> ObjectRenderMaterialProxiesMap { get; } = new(); + + /// + /// Returns the merged material proxy list for the given object ids. Use this to get post conversion a correct list of material proxies for setting on the root commit object. + /// + /// + /// + public List GetRenderMaterialProxyListForObjects(List elementIds) + { + var proxiesToMerge = ObjectRenderMaterialProxiesMap + .Where(kvp => elementIds.Contains(kvp.Key)) + .Select(kvp => kvp.Value); + + var mergeTarget = new Dictionary(); + foreach (var dictionary in proxiesToMerge) + { + foreach (var kvp in dictionary) + { + if (!mergeTarget.TryGetValue(kvp.Key, out RenderMaterialProxy? value)) + { + value = kvp.Value; + mergeTarget[kvp.Key] = value; + continue; + } + value.objects.AddRange(kvp.Value.objects); + } + } + foreach (var renderMaterialProxy in mergeTarget.Values) + { + renderMaterialProxy.objects = renderMaterialProxy.objects.Distinct().ToList(); + } + return mergeTarget.Values.ToList(); + } +} diff --git a/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems b/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems index 8429f5543..f28f650b0 100644 --- a/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems +++ b/Converters/Revit/Speckle.Converters.RevitShared/Speckle.Converters.RevitShared.projitems @@ -12,6 +12,7 @@ + From 61cde3c13375561d761b0509f5ff631cb3945374 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Fri, 16 Aug 2024 15:39:23 +0100 Subject: [PATCH 10/17] fix(revit): invalid progress reporting on send --- .../Operations/Send/RevitRootObjectBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs index f9081a710..a37af27cb 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs @@ -113,7 +113,7 @@ public Task Build( results.Add(new(Status.ERROR, applicationId, revitElement.GetType().Name, null, ex)); } - onOperationProgressed?.Invoke("Converting", (double)++countProgress / revitElements.Count); + onOperationProgressed?.Invoke("Converting", (double)++countProgress / atomicObjects.Count); } var idsAndSubElementIds = _elementUnpacker.GetElementsAndSubelementIdsFromAtomicObjects(atomicObjects); From 76de6ae46d2a5a5e4bfe1f7288bc499e8bdb3f1f Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Fri, 16 Aug 2024 15:51:56 +0100 Subject: [PATCH 11/17] feat(rhino): adds rhino support for revit proxy materials --- .../Receive/RhinoHostObjectBuilder.cs | 122 +++++++----------- .../ToHost/ConverterWithFallback.cs | 8 +- 2 files changed, 54 insertions(+), 76 deletions(-) diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs index 66c23e958..495d7df99 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs @@ -168,31 +168,37 @@ private HostObjectBuilderResult BakeObjects( onOperationProgressed?.Invoke("Converting objects", (double)++count / atomicObjects.Count); try { - // Convert and bake an object: // 1: create layer int layerIndex = _layerManager.GetAndCreateLayerFromPath(path, baseLayerName, out bool _); // 2: convert var result = _converter.Convert(obj); - var objectId = obj.applicationId ?? obj.id; // POC: assuming objects have app ids for this to work? - - // 3: colors and materials - int? matIndex = _materialManager.ObjectIdAndMaterialIndexMap.TryGetValue(objectId, out int mIndex) - ? mIndex - : null; - Color? objColor = _colorManager.ObjectColorsIdMap.TryGetValue(objectId, out Color color) ? color : null; + // 3: bake + var conversionIds = new List(); + if (result is GeometryBase geometryBase) + { + var guid = BakeObject(geometryBase, obj, layerIndex); + conversionIds.Add(guid.ToString()); + } + else if (result is IEnumerable<(object, Base)> fallbackConversionResult) + { + var guids = BakeObjectsAsGroup(fallbackConversionResult, obj, layerIndex, baseLayerName); + conversionIds.AddRange(guids.Select(id => id.ToString())); + } - // 4: actually bake - var conversionIds = HandleConversionResult(result, obj, layerIndex, matIndex, objColor).ToList(); - foreach (var r in conversionIds) + if (conversionIds.Count == 0) { - conversionResults.Add(new(Status.SUCCESS, obj, r, result.GetType().ToString())); - bakedObjectIds.Add(r); + throw new SpeckleConversionException($"Failed to convert object."); } + // 4: log + var id = conversionIds[0]; + conversionResults.Add(new(Status.SUCCESS, obj, id, result.GetType().ToString())); + bakedObjectIds.Add(id); + // 5: populate app id map - applicationIdMap[objectId] = conversionIds; + applicationIdMap[obj.applicationId ?? obj.id] = conversionIds; } catch (Exception ex) when (!ex.IsFatal()) { @@ -259,82 +265,52 @@ private void PreReceiveDeepClean(string baseLayerName) _groupManager.PurgeGroups(baseLayerName); } - private IReadOnlyList HandleConversionResult( - object conversionResult, - Base originalObject, - int layerIndex, - int? materialIndex = null, - Color? color = null - ) + private Guid BakeObject(GeometryBase obj, Base originalObject, int layerIndex) { - var doc = _contextStack.Current.Document; - List newObjectIds = new(); - switch (conversionResult) - { - case IEnumerable list: - { - Group group = BakeObjectsAsGroup(originalObject.id, list, layerIndex, materialIndex, color); - newObjectIds.Add(group.Id.ToString()); - break; - } - case GeometryBase newObject: - { - ObjectAttributes atts = new() { LayerIndex = layerIndex }; - if (materialIndex is int index) - { - atts.MaterialIndex = index; - atts.MaterialSource = ObjectMaterialSource.MaterialFromObject; - } + ObjectAttributes atts = new() { LayerIndex = layerIndex }; + var objectId = originalObject.applicationId ?? originalObject.id; - if (color is Color objColor) - { - atts.ObjectColor = objColor; - atts.ColorSource = ObjectColorSource.ColorFromObject; - } + if (_materialManager.ObjectIdAndMaterialIndexMap.TryGetValue(objectId, out int mIndex)) + { + atts.MaterialIndex = mIndex; + atts.MaterialSource = ObjectMaterialSource.MaterialFromObject; + } - Guid newObjectGuid = doc.Objects.Add(newObject, atts); - newObjectIds.Add(newObjectGuid.ToString()); - break; - } - default: - throw new SpeckleConversionException( - $"Unexpected result from conversion: Expected {nameof(GeometryBase)} but instead got {conversionResult.GetType().Name}" - ); + if (_colorManager.ObjectColorsIdMap.TryGetValue(objectId, out Color color)) + { + atts.ObjectColor = color; + atts.ColorSource = ObjectColorSource.ColorFromObject; } - return newObjectIds; + return _contextStack.Current.Document.Objects.Add(obj, atts); } - private Group BakeObjectsAsGroup( - string groupName, - IEnumerable list, + private List BakeObjectsAsGroup( + IEnumerable<(object, Base)> fallbackConversionResult, + Base originatingObject, int layerIndex, - int? materialIndex = null, - Color? color = null + string baseLayerName ) { - var doc = _contextStack.Current.Document; List objectIds = new(); - foreach (GeometryBase obj in list) + foreach (var (conversionResult, originalBaseObject) in fallbackConversionResult) { - ObjectAttributes atts = new() { LayerIndex = layerIndex }; - if (materialIndex is int index) - { - atts.MaterialIndex = index; - atts.MaterialSource = ObjectMaterialSource.MaterialFromObject; - } - - if (color is Color objColor) + if (conversionResult is not GeometryBase geometryBase) { - atts.ObjectColor = objColor; - atts.ColorSource = ObjectColorSource.ColorFromObject; + // TODO: throw? + continue; } - objectIds.Add(doc.Objects.Add(obj, atts)); + var id = BakeObject(geometryBase, originalBaseObject, layerIndex); + objectIds.Add(id); } - int groupIndex = _contextStack.Current.Document.Groups.Add(groupName, objectIds); + var groupIndex = _contextStack.Current.Document.Groups.Add( + $@"{originatingObject.speckle_type.Split('.').Last()} - {originatingObject.applicationId ?? originatingObject.id} ({baseLayerName})", + objectIds + ); var group = _contextStack.Current.Document.Groups.FindIndex(groupIndex); - return group; + objectIds.Insert(0, group.Id); + return objectIds; } } diff --git a/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs b/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs index 3d348e8ad..5a80a255c 100644 --- a/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs +++ b/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs @@ -43,7 +43,7 @@ public object Convert(Base target) // Direct conversion if a converter is found if (_baseConverter.TryGetConverter(type, out IToHostTopLevelConverter? result)) { - return result.Convert(target); + return result.Convert(target); // 1-1 mapping } // Fallback to display value if it exists. @@ -54,7 +54,7 @@ public object Convert(Base target) { throw new NotSupportedException($"No display value found for {type}"); } - return FallbackToDisplayValue(displayValue); + return FallbackToDisplayValue(displayValue); // 1-many mapping } throw new NotSupportedException($"No conversion found for {type}"); @@ -63,7 +63,9 @@ public object Convert(Base target) private object FallbackToDisplayValue(IReadOnlyList displayValue) { var tempDisplayableObject = new DisplayableObject(displayValue); + var conversionResult = _baseConverter.Convert(tempDisplayableObject); + var res = conversionResult is IEnumerable result ? result.Zip(displayValue, (a, b) => (a, b)) : []; - return _baseConverter.Convert(tempDisplayableObject); + return res; } } From 35934a0cfb419b1732ec90ce6c47cecb0e165e19 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Sat, 17 Aug 2024 13:32:18 +0100 Subject: [PATCH 12/17] fix(acad): checks group names for invalid chars --- .../HostApp/AutocadGroupManager.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupManager.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupManager.cs index b3f4d9358..4148fcdc1 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupManager.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupManager.cs @@ -11,6 +11,13 @@ namespace Speckle.Connectors.Autocad.HostApp; /// public class AutocadGroupManager { + private readonly AutocadContext _autocadContext; + + public AutocadGroupManager(AutocadContext autocadContext) + { + _autocadContext = autocadContext; + } + /// /// Unpacks a selection of atomic objects into their groups /// @@ -86,11 +93,12 @@ Dictionary> applicationIdMap ids.Add(entity.ObjectId); } - var newGroup = new Group(gp.name, true); // NOTE: this constructor sets both the description (as it says) but also the name at the same time + var groupName = _autocadContext.RemoveInvalidChars(gp.name); + var newGroup = new Group(groupName, true); // NOTE: this constructor sets both the description (as it says) but also the name at the same time newGroup.Append(ids); groupDictionary.UpgradeOpen(); - groupDictionary.SetAt(gp.name, newGroup); + groupDictionary.SetAt(groupName, newGroup); groupCreationTransaction.AddNewlyCreatedDBObject(newGroup, true); } From 59db1c819d474beba7e7806cb0e5f2523b285b4c Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Sat, 17 Aug 2024 13:32:36 +0100 Subject: [PATCH 13/17] fix(acad): checks material name for invalid chars --- .../HostApp/AutocadMaterialManager.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialManager.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialManager.cs index 4a2dc38ed..b66611389 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialManager.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialManager.cs @@ -128,7 +128,10 @@ Transaction tr // POC: Currently we're relying on the render material name for identification if it's coming from speckle and from which model; could we do something else? // POC: we should assume render materials all have application ids? string renderMaterialId = renderMaterial.applicationId ?? renderMaterial.id; - string matName = $"{renderMaterial.name}-({renderMaterialId})-{baseLayerPrefix}"; + string matName = _autocadContext.RemoveInvalidChars( + $"{renderMaterial.name}-({renderMaterialId})-{baseLayerPrefix}" + ); + MaterialMap map = new(); MaterialOpacityComponent opacity = new(renderMaterial.opacity, map); var systemDiffuse = System.Drawing.Color.FromArgb(renderMaterial.diffuse); From 7a3899c08291e9f5f08729992807bba05e20f02f Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Sat, 17 Aug 2024 13:34:09 +0100 Subject: [PATCH 14/17] feat(acad): adds support for baking fallback objects properly including materials! --- .../Receive/AutocadHostObjectBuilder.cs | 99 +++++++++++++------ .../ToHost/ConverterWithFallback.cs | 2 +- 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs b/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs index 1572b0a12..77003946f 100644 --- a/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs +++ b/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs @@ -203,47 +203,82 @@ private void PreReceiveDeepClean(string baseLayerPrefix) private IEnumerable ConvertObject(Base obj, Collection[] layerPath, string baseLayerNamePrefix) { string layerName = _autocadLayerManager.CreateLayerForReceive(layerPath, baseLayerNamePrefix); + var convertedEntities = new List(); - using (var tr = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction()) - { - var converted = _converter.Convert(obj); + using var tr = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction(); - IEnumerable flattened = Utilities.FlattenToHostConversionResult(converted).Cast(); + // 1: convert + var converted = _converter.Convert(obj); - // get color and material if any - string objId = obj.applicationId ?? obj.id; - AutocadColor? objColor = _colorManager.ObjectColorsIdMap.TryGetValue(objId, out AutocadColor? color) - ? color - : null; - ObjectId objMaterial = _materialManager.ObjectMaterialsIdMap.TryGetValue(objId, out ObjectId matId) - ? matId - : ObjectId.Null; + // 2: handle result + if (converted is Entity entity) + { + var bakedEntity = BakeObject(entity, obj, layerName); + convertedEntities.Add(bakedEntity); + } + else if (converted is IEnumerable<(object, Base)> fallbackConversionResult) + { + var bakedFallbackEntities = BakeObjectsAsGroup(fallbackConversionResult, obj, layerName, baseLayerNamePrefix); + convertedEntities.AddRange(bakedFallbackEntities); + } - foreach (Entity? conversionResult in flattened) - { - if (conversionResult == null) - { - // POC: This needed to be double checked why we check null and continue - continue; - } + tr.Commit(); + return convertedEntities; + } - // set color and material - // POC: if these are displayvalue meshes, we will need to check for their ids somehow - if (objColor is not null) - { - conversionResult.Color = objColor; - } + private Entity BakeObject(Entity entity, Base originalObject, string layerName) + { + var objId = originalObject.applicationId ?? originalObject.id; + if (_colorManager.ObjectColorsIdMap.TryGetValue(objId, out AutocadColor? color)) + { + entity.Color = color; + } - if (objMaterial != ObjectId.Null) - { - conversionResult.MaterialId = objMaterial; - } + if (_materialManager.ObjectMaterialsIdMap.TryGetValue(objId, out ObjectId matId)) + { + entity.MaterialId = matId; + } - conversionResult.AppendToDb(layerName); - yield return conversionResult; + entity.AppendToDb(layerName); + return entity; + } + + private List BakeObjectsAsGroup( + IEnumerable<(object, Base)> fallbackConversionResult, + Base originatingObject, + string layerName, + string baseLayerName + ) + { + var ids = new ObjectIdCollection(); + var entities = new List(); + foreach (var (conversionResult, originalBaseObject) in fallbackConversionResult) + { + if (conversionResult is not Entity entity) + { + // TODO: throw? + continue; } - tr.Commit(); + BakeObject(entity, originalBaseObject, layerName); + ids.Add(entity.ObjectId); + entities.Add(entity); } + + var tr = Application.DocumentManager.CurrentDocument.Database.TransactionManager.TopTransaction; + var groupDictionary = (DBDictionary) + tr.GetObject(Application.DocumentManager.CurrentDocument.Database.GroupDictionaryId, OpenMode.ForWrite); + + var groupName = _autocadContext.RemoveInvalidChars( + $@"{originatingObject.speckle_type.Split('.').Last()} - {originatingObject.applicationId ?? originatingObject.id} ({baseLayerName})" + ); + + var newGroup = new Group(groupName, true); + newGroup.Append(ids); + groupDictionary.UpgradeOpen(); + groupDictionary.SetAt(groupName, newGroup); + tr.AddNewlyCreatedDBObject(newGroup, true); + + return entities; } } diff --git a/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs b/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs index 5a80a255c..75722c999 100644 --- a/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs +++ b/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs @@ -54,7 +54,7 @@ public object Convert(Base target) { throw new NotSupportedException($"No display value found for {type}"); } - return FallbackToDisplayValue(displayValue); // 1-many mapping + return FallbackToDisplayValue(displayValue); // 1 - many mapping } throw new NotSupportedException($"No conversion found for {type}"); From 19d6ff1d45d670085520d24f103d4ad802fdc1d0 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Mon, 19 Aug 2024 09:58:42 +0100 Subject: [PATCH 15/17] wip: arcgis support for multiple fallback values --- .../Operations/Receive/HostObjectBuilder.cs | 148 ++++++++++-------- .../Utils/INonNativeFeaturesUtils.cs | 2 +- .../Utils/NonNativeFeaturesUtils.cs | 3 +- 3 files changed, 88 insertions(+), 65 deletions(-) diff --git a/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs b/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs index 14ce47c54..76e824f34 100644 --- a/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs +++ b/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs @@ -88,7 +88,7 @@ CancellationToken cancellationToken int allCount = objectsToConvert.Count; int count = 0; - Dictionary conversionTracker = new(); + Dictionary> conversionTracker = new(); // 1. convert everything List results = new(objectsToConvert.Count); @@ -104,25 +104,45 @@ CancellationToken cancellationToken if (IsGISType(obj)) { string nestedLayerPath = $"{string.Join("\\", path)}"; - string datasetId = await QueuedTask.Run(() => (string)_converter.Convert(obj)).ConfigureAwait(false); - conversionTracker[objectToConvert.TraversalContext] = new ObjectConversionTracker( - obj, - nestedLayerPath, - datasetId - ); + string datasetId = await QueuedTask.Run(() => (string)_converter.Convert(obj)).ConfigureAwait(false); // NOTE: convert call + conversionTracker[objectToConvert.TraversalContext] = + [ + new ObjectConversionTracker(obj, nestedLayerPath, datasetId) + ]; } else { obj = _localToGlobalConverterUtils.TransformObjects(objectToConvert.AtomicObject, objectToConvert.Matrix); string nestedLayerPath = $"{string.Join("\\", path)}\\{obj.speckle_type.Split(".")[^1]}"; - Geometry converted = await QueuedTask.Run(() => (Geometry)_converter.Convert(obj)).ConfigureAwait(false); - - conversionTracker[objectToConvert.TraversalContext] = new ObjectConversionTracker( - obj, - nestedLayerPath, - converted - ); + var converted = await QueuedTask.Run(() => _converter.Convert(obj)).ConfigureAwait(false); // NOTE: convert call + if (converted is Geometry geometry) + { + conversionTracker[objectToConvert.TraversalContext] = + [ + new ObjectConversionTracker(obj, nestedLayerPath, geometry) + ]; + } + else if (converted is IEnumerable<(object, Base)> fallbackConversionResult) + { + var filtered = new List(); + foreach (var (conversionResult, originalBaseObject) in fallbackConversionResult) + { + if (conversionResult is not Geometry fallbackGeometry) + { + // TODO: throw? + continue; + } + filtered.Add(new ObjectConversionTracker(obj, nestedLayerPath, fallbackGeometry)); + } + + conversionTracker[objectToConvert.TraversalContext] = filtered; + } + // conversionTracker[objectToConvert.TraversalContext] = new ObjectConversionTracker( + // obj, + // nestedLayerPath, + // converted + // ); } } catch (Exception ex) when (!ex.IsFatal()) // DO NOT CATCH SPECIFIC STUFF, conversion errors should be recoverable @@ -152,63 +172,65 @@ await QueuedTask foreach (var item in conversionTracker) { cancellationToken.ThrowIfCancellationRequested(); - var trackerItem = conversionTracker[item.Key]; // updated tracker object - - // BAKE OBJECTS HERE - if (trackerItem.Exception != null) - { - results.Add(new(Status.ERROR, trackerItem.Base, null, null, trackerItem.Exception)); - } - else if (trackerItem.DatasetId == null) - { - results.Add( - new( - Status.ERROR, - trackerItem.Base, - null, - null, - new ArgumentException($"Unknown error: Dataset not created for {trackerItem.Base.speckle_type}") - ) - ); - } - else if (bakedMapMembers.TryGetValue(trackerItem.DatasetId, out MapMember? value)) - { - // if the layer already created, just add more features to report, and more color categories - // add layer and layer URI to tracker - trackerItem.AddConvertedMapMember(value); - trackerItem.AddLayerURI(value.URI); - conversionTracker[item.Key] = trackerItem; // not necessary atm, but needed if we use conversionTracker further - // only add a report item - AddResultsFromTracker(trackerItem, results); - - // add color category - await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); - } - else + var trackerItems = conversionTracker[item.Key]; // updated tracker object + foreach (var trackerItem in trackerItems) { - // no layer yet, create and add layer to Map - MapMember mapMember = await AddDatasetsToMap(trackerItem, createdLayerGroups, projectName, modelName) - .ConfigureAwait(false); + // BAKE OBJECTS HERE + if (trackerItem.Exception != null) + { + results.Add(new(Status.ERROR, trackerItem.Base, null, null, trackerItem.Exception)); + } + else if (trackerItem.DatasetId == null) + { + results.Add( + new( + Status.ERROR, + trackerItem.Base, + null, + null, + new ArgumentException($"Unknown error: Dataset not created for {trackerItem.Base.speckle_type}") + ) + ); + } + else if (bakedMapMembers.TryGetValue(trackerItem.DatasetId, out MapMember? value)) + { + // if the layer already created, just add more features to report, and more color categories + // add layer and layer URI to tracker + trackerItem.AddConvertedMapMember(value); + trackerItem.AddLayerURI(value.URI); + // only add a report item + AddResultsFromTracker(trackerItem, results); + + // add color category + await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); + } + else + { + // no layer yet, create and add layer to Map + MapMember mapMember = await AddDatasetsToMap(trackerItem, createdLayerGroups, projectName, modelName) + .ConfigureAwait(false); + + // add layer and layer URI to tracker + trackerItem.AddConvertedMapMember(mapMember); + trackerItem.AddLayerURI(mapMember.URI); - // add layer and layer URI to tracker - trackerItem.AddConvertedMapMember(mapMember); - trackerItem.AddLayerURI(mapMember.URI); - conversionTracker[item.Key] = trackerItem; // not necessary atm, but needed if we use conversionTracker further + // add layer URI to bakedIds + bakedObjectIds.Add(trackerItem.MappedLayerURI == null ? "" : trackerItem.MappedLayerURI); - // add layer URI to bakedIds - bakedObjectIds.Add(trackerItem.MappedLayerURI == null ? "" : trackerItem.MappedLayerURI); + // mark dataset as already created + bakedMapMembers[trackerItem.DatasetId] = mapMember; - // mark dataset as already created - bakedMapMembers[trackerItem.DatasetId] = mapMember; + // add report item + AddResultsFromTracker(trackerItem, results); - // add report item - AddResultsFromTracker(trackerItem, results); + // add color category + await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); + } - // add color category - await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); + onOperationProgressed?.Invoke("Adding to Map", (double)++bakeCount / conversionTracker.Count); } - onOperationProgressed?.Invoke("Adding to Map", (double)++bakeCount / conversionTracker.Count); } + bakedObjectIds.AddRange(createdLayerGroups.Values.Select(x => x.URI)); // TODO: validated a correct set regarding bakedobject ids diff --git a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs index d07582677..282b41014 100644 --- a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs +++ b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs @@ -5,7 +5,7 @@ namespace Speckle.Converters.ArcGIS3.Utils; public interface INonNativeFeaturesUtils { public void WriteGeometriesToDatasets( - Dictionary conversionTracker, + Dictionary> conversionTracker, Action? onOperationProgressed ); } diff --git a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs index 5ae7996c2..8ab164309 100644 --- a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs +++ b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs @@ -29,7 +29,7 @@ IArcGISFieldUtils fieldUtils public void WriteGeometriesToDatasets( // Dictionary conversionTracker - Dictionary conversionTracker, + Dictionary> conversionTracker, Action? onOperationProgressed ) { @@ -44,6 +44,7 @@ public void WriteGeometriesToDatasets( { TraversalContext context = item.Key; var trackerItem = item.Value; + ACG.Geometry? geom = trackerItem.HostAppGeom; string? datasetId = trackerItem.DatasetId; if (geom != null && datasetId == null) // only non-native geomerties, not written into a dataset yet From 8cece657211aaa83808724303c5c5d6bcea747a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?O=C4=9Fuzhan=20Koral?= <45078678+oguzhankoral@users.noreply.github.com> Date: Mon, 19 Aug 2024 14:07:38 +0300 Subject: [PATCH 16/17] Fix build errors (#166) --- .../Utils/NonNativeFeaturesUtils.cs | 113 ++++++++++-------- 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs index 8ab164309..876d94622 100644 --- a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs +++ b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs @@ -38,71 +38,75 @@ public void WriteGeometriesToDatasets( // 1. Sort features into groups by path and geom type Dictionary> geometryGroups = new(); - foreach (var item in conversionTracker) + foreach (var conversionTrackers in conversionTracker) { - try - { - TraversalContext context = item.Key; - var trackerItem = item.Value; + TraversalContext context = conversionTrackers.Key; - ACG.Geometry? geom = trackerItem.HostAppGeom; - string? datasetId = trackerItem.DatasetId; - if (geom != null && datasetId == null) // only non-native geomerties, not written into a dataset yet + foreach (var trackerItem in conversionTrackers.Value) + { + try { - // add dictionnary item if doesn't exist yet - // Key must be unique per parent and speckleType - // Adding Offsets/rotation to Unique key, so the modified CAD geometry doesn't overwrite non-modified one - // or, same commit received with different Offsets are saved to separate datasets + ACG.Geometry? geom = trackerItem.HostAppGeom; + string? datasetId = trackerItem.DatasetId; + if (geom != null && datasetId == null) // only non-native geomerties, not written into a dataset yet + { + // add dictionnary item if doesn't exist yet + // Key must be unique per parent and speckleType + // Adding Offsets/rotation to Unique key, so the modified CAD geometry doesn't overwrite non-modified one + // or, same commit received with different Offsets are saved to separate datasets - // Also, keep char limit for dataset name under 128: https://pro.arcgis.com/en/pro-app/latest/help/data/geodatabases/manage-saphana/enterprise-geodatabase-limits.htm + // Also, keep char limit for dataset name under 128: https://pro.arcgis.com/en/pro-app/latest/help/data/geodatabases/manage-saphana/enterprise-geodatabase-limits.htm - string speckleType = trackerItem.Base.speckle_type.Split(".")[^1]; - //speckleType = speckleType.Substring(0, Math.Min(10, speckleType.Length - 1)); - speckleType = speckleType.Length > 10 ? speckleType[..9] : speckleType; - string? parentId = context.Parent?.Current.id; + string speckleType = trackerItem.Base.speckle_type.Split(".")[^1]; + //speckleType = speckleType.Substring(0, Math.Min(10, speckleType.Length - 1)); + speckleType = speckleType.Length > 10 ? speckleType[..9] : speckleType; + string? parentId = context.Parent?.Current.id; - CRSoffsetRotation activeSR = _contextStack.Current.Document.ActiveCRSoffsetRotation; - string xOffset = Convert.ToString(activeSR.LonOffset).Replace(".", "_"); - xOffset = xOffset.Length > 15 ? xOffset[..14] : xOffset; + CRSoffsetRotation activeSR = _contextStack.Current.Document.ActiveCRSoffsetRotation; + string xOffset = Convert.ToString(activeSR.LonOffset).Replace(".", "_"); + xOffset = xOffset.Length > 15 ? xOffset[..14] : xOffset; - string yOffset = Convert.ToString(activeSR.LatOffset).Replace(".", "_"); - yOffset = yOffset.Length > 15 ? yOffset[..14] : yOffset; + string yOffset = Convert.ToString(activeSR.LatOffset).Replace(".", "_"); + yOffset = yOffset.Length > 15 ? yOffset[..14] : yOffset; - string trueNorth = Convert.ToString(activeSR.TrueNorthRadians).Replace(".", "_"); - trueNorth = trueNorth.Length > 10 ? trueNorth[..9] : trueNorth; + string trueNorth = Convert.ToString(activeSR.TrueNorthRadians).Replace(".", "_"); + trueNorth = trueNorth.Length > 10 ? trueNorth[..9] : trueNorth; - // text: 36 symbols, speckleTYPE: 10, sr: 10, offsets: 40, id: 32 = 128 - string uniqueKey = - $"speckle_{speckleType}_SR_{activeSR.SpatialReference.Name[..Math.Min(15, activeSR.SpatialReference.Name.Length - 1)]}_X_{xOffset}_Y_{yOffset}_North_{trueNorth}_speckleID_{parentId}"; + // text: 36 symbols, speckleTYPE: 10, sr: 10, offsets: 40, id: 32 = 128 + string uniqueKey = + $"speckle_{speckleType}_SR_{activeSR.SpatialReference.Name[..Math.Min(15, activeSR.SpatialReference.Name.Length - 1)]}_X_{xOffset}_Y_{yOffset}_North_{trueNorth}_speckleID_{parentId}"; - if (!geometryGroups.TryGetValue(uniqueKey, out _)) + if (!geometryGroups.TryGetValue(uniqueKey, out _)) + { + geometryGroups[uniqueKey] = new(); + } + + geometryGroups[uniqueKey].Add((trackerItem.Base, geom)); + + // record changes in conversion tracker + trackerItem.AddDatasetId(uniqueKey); + trackerItem.AddDatasetRow(geometryGroups[uniqueKey].Count - 1); + conversionTracker[context] = [trackerItem]; + } + else if (geom == null && datasetId != null) // GIS layers, already written to a dataset { - geometryGroups[uniqueKey] = new(); + continue; + } + else + { + throw new ArgumentException($"Unexpected geometry and datasetId values: {geom}, {datasetId}"); } - geometryGroups[uniqueKey].Add((trackerItem.Base, geom)); - - // record changes in conversion tracker - trackerItem.AddDatasetId(uniqueKey); - trackerItem.AddDatasetRow(geometryGroups[uniqueKey].Count - 1); - conversionTracker[item.Key] = trackerItem; + //var trackerItem = item.Value; } - else if (geom == null && datasetId != null) // GIS layers, already written to a dataset + catch (Exception ex) when (!ex.IsFatal()) { - continue; + // POC: report, etc. + // var trackerItem = item.Value; + trackerItem.AddException(ex); + conversionTracker[context] = [trackerItem]; + Debug.WriteLine($"conversion error happened. {ex.Message}"); } - else - { - throw new ArgumentException($"Unexpected geometry and datasetId values: {geom}, {datasetId}"); - } - } - catch (Exception ex) when (!ex.IsFatal()) - { - // POC: report, etc. - var trackerItem = item.Value; - trackerItem.AddException(ex); - conversionTracker[item.Key] = trackerItem; - Debug.WriteLine($"conversion error happened. {ex.Message}"); } } @@ -121,11 +125,14 @@ public void WriteGeometriesToDatasets( // only record in conversionTracker: foreach (var conversionItem in conversionTracker) { - if (conversionItem.Value.DatasetId == uniqueKey) + foreach (var item2 in conversionItem.Value) { - var trackerItem = conversionItem.Value; - trackerItem.AddException(ex); - conversionTracker[conversionItem.Key] = trackerItem; + if (item2.DatasetId == uniqueKey) + { + var trackerItem = conversionItem.Value; + item2.AddException(ex); + conversionTracker[conversionItem.Key] = trackerItem; + } } } } From b76636cf922cc2aa6b53b6194d38c552a19649ed Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Mon, 19 Aug 2024 13:05:21 +0100 Subject: [PATCH 17/17] fix: arcgis --- .../Operations/Receive/HostObjectBuilder.cs | 148 ++++++++---------- .../ToHost/Raw/MeshListToHostConverter.cs | 1 + .../Utils/INonNativeFeaturesUtils.cs | 2 +- .../Utils/NonNativeFeaturesUtils.cs | 114 +++++++------- .../ToHost/ConverterWithFallback.cs | 10 +- 5 files changed, 126 insertions(+), 149 deletions(-) diff --git a/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs b/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs index 76e824f34..14ce47c54 100644 --- a/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs +++ b/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/HostObjectBuilder.cs @@ -88,7 +88,7 @@ CancellationToken cancellationToken int allCount = objectsToConvert.Count; int count = 0; - Dictionary> conversionTracker = new(); + Dictionary conversionTracker = new(); // 1. convert everything List results = new(objectsToConvert.Count); @@ -104,45 +104,25 @@ CancellationToken cancellationToken if (IsGISType(obj)) { string nestedLayerPath = $"{string.Join("\\", path)}"; - string datasetId = await QueuedTask.Run(() => (string)_converter.Convert(obj)).ConfigureAwait(false); // NOTE: convert call - conversionTracker[objectToConvert.TraversalContext] = - [ - new ObjectConversionTracker(obj, nestedLayerPath, datasetId) - ]; + string datasetId = await QueuedTask.Run(() => (string)_converter.Convert(obj)).ConfigureAwait(false); + conversionTracker[objectToConvert.TraversalContext] = new ObjectConversionTracker( + obj, + nestedLayerPath, + datasetId + ); } else { obj = _localToGlobalConverterUtils.TransformObjects(objectToConvert.AtomicObject, objectToConvert.Matrix); string nestedLayerPath = $"{string.Join("\\", path)}\\{obj.speckle_type.Split(".")[^1]}"; - var converted = await QueuedTask.Run(() => _converter.Convert(obj)).ConfigureAwait(false); // NOTE: convert call - if (converted is Geometry geometry) - { - conversionTracker[objectToConvert.TraversalContext] = - [ - new ObjectConversionTracker(obj, nestedLayerPath, geometry) - ]; - } - else if (converted is IEnumerable<(object, Base)> fallbackConversionResult) - { - var filtered = new List(); - foreach (var (conversionResult, originalBaseObject) in fallbackConversionResult) - { - if (conversionResult is not Geometry fallbackGeometry) - { - // TODO: throw? - continue; - } - filtered.Add(new ObjectConversionTracker(obj, nestedLayerPath, fallbackGeometry)); - } - - conversionTracker[objectToConvert.TraversalContext] = filtered; - } - // conversionTracker[objectToConvert.TraversalContext] = new ObjectConversionTracker( - // obj, - // nestedLayerPath, - // converted - // ); + Geometry converted = await QueuedTask.Run(() => (Geometry)_converter.Convert(obj)).ConfigureAwait(false); + + conversionTracker[objectToConvert.TraversalContext] = new ObjectConversionTracker( + obj, + nestedLayerPath, + converted + ); } } catch (Exception ex) when (!ex.IsFatal()) // DO NOT CATCH SPECIFIC STUFF, conversion errors should be recoverable @@ -172,65 +152,63 @@ await QueuedTask foreach (var item in conversionTracker) { cancellationToken.ThrowIfCancellationRequested(); - var trackerItems = conversionTracker[item.Key]; // updated tracker object - foreach (var trackerItem in trackerItems) - { - // BAKE OBJECTS HERE - if (trackerItem.Exception != null) - { - results.Add(new(Status.ERROR, trackerItem.Base, null, null, trackerItem.Exception)); - } - else if (trackerItem.DatasetId == null) - { - results.Add( - new( - Status.ERROR, - trackerItem.Base, - null, - null, - new ArgumentException($"Unknown error: Dataset not created for {trackerItem.Base.speckle_type}") - ) - ); - } - else if (bakedMapMembers.TryGetValue(trackerItem.DatasetId, out MapMember? value)) - { - // if the layer already created, just add more features to report, and more color categories - // add layer and layer URI to tracker - trackerItem.AddConvertedMapMember(value); - trackerItem.AddLayerURI(value.URI); - // only add a report item - AddResultsFromTracker(trackerItem, results); - - // add color category - await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); - } - else - { - // no layer yet, create and add layer to Map - MapMember mapMember = await AddDatasetsToMap(trackerItem, createdLayerGroups, projectName, modelName) - .ConfigureAwait(false); + var trackerItem = conversionTracker[item.Key]; // updated tracker object - // add layer and layer URI to tracker - trackerItem.AddConvertedMapMember(mapMember); - trackerItem.AddLayerURI(mapMember.URI); + // BAKE OBJECTS HERE + if (trackerItem.Exception != null) + { + results.Add(new(Status.ERROR, trackerItem.Base, null, null, trackerItem.Exception)); + } + else if (trackerItem.DatasetId == null) + { + results.Add( + new( + Status.ERROR, + trackerItem.Base, + null, + null, + new ArgumentException($"Unknown error: Dataset not created for {trackerItem.Base.speckle_type}") + ) + ); + } + else if (bakedMapMembers.TryGetValue(trackerItem.DatasetId, out MapMember? value)) + { + // if the layer already created, just add more features to report, and more color categories + // add layer and layer URI to tracker + trackerItem.AddConvertedMapMember(value); + trackerItem.AddLayerURI(value.URI); + conversionTracker[item.Key] = trackerItem; // not necessary atm, but needed if we use conversionTracker further + // only add a report item + AddResultsFromTracker(trackerItem, results); + + // add color category + await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); + } + else + { + // no layer yet, create and add layer to Map + MapMember mapMember = await AddDatasetsToMap(trackerItem, createdLayerGroups, projectName, modelName) + .ConfigureAwait(false); - // add layer URI to bakedIds - bakedObjectIds.Add(trackerItem.MappedLayerURI == null ? "" : trackerItem.MappedLayerURI); + // add layer and layer URI to tracker + trackerItem.AddConvertedMapMember(mapMember); + trackerItem.AddLayerURI(mapMember.URI); + conversionTracker[item.Key] = trackerItem; // not necessary atm, but needed if we use conversionTracker further - // mark dataset as already created - bakedMapMembers[trackerItem.DatasetId] = mapMember; + // add layer URI to bakedIds + bakedObjectIds.Add(trackerItem.MappedLayerURI == null ? "" : trackerItem.MappedLayerURI); - // add report item - AddResultsFromTracker(trackerItem, results); + // mark dataset as already created + bakedMapMembers[trackerItem.DatasetId] = mapMember; - // add color category - await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); - } + // add report item + AddResultsFromTracker(trackerItem, results); - onOperationProgressed?.Invoke("Adding to Map", (double)++bakeCount / conversionTracker.Count); + // add color category + await _colorManager.SetOrEditLayerRenderer(item.Key, trackerItem).ConfigureAwait(false); } + onOperationProgressed?.Invoke("Adding to Map", (double)++bakeCount / conversionTracker.Count); } - bakedObjectIds.AddRange(createdLayerGroups.Values.Select(x => x.URI)); // TODO: validated a correct set regarding bakedobject ids diff --git a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/ToHost/Raw/MeshListToHostConverter.cs b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/ToHost/Raw/MeshListToHostConverter.cs index 2f67de74c..7d5b982fa 100644 --- a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/ToHost/Raw/MeshListToHostConverter.cs +++ b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/ToHost/Raw/MeshListToHostConverter.cs @@ -26,6 +26,7 @@ public ACG.Multipatch Convert(List target) } ACG.MultipatchBuilderEx multipatchPart = new(_contextStack.Current.Document.ActiveCRSoffsetRotation.SpatialReference); + foreach (SOG.Mesh part in target) { part.TriangulateMesh(); diff --git a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs index 282b41014..d07582677 100644 --- a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs +++ b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/INonNativeFeaturesUtils.cs @@ -5,7 +5,7 @@ namespace Speckle.Converters.ArcGIS3.Utils; public interface INonNativeFeaturesUtils { public void WriteGeometriesToDatasets( - Dictionary> conversionTracker, + Dictionary conversionTracker, Action? onOperationProgressed ); } diff --git a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs index 876d94622..5ae7996c2 100644 --- a/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs +++ b/Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/NonNativeFeaturesUtils.cs @@ -29,7 +29,7 @@ IArcGISFieldUtils fieldUtils public void WriteGeometriesToDatasets( // Dictionary conversionTracker - Dictionary> conversionTracker, + Dictionary conversionTracker, Action? onOperationProgressed ) { @@ -38,76 +38,71 @@ public void WriteGeometriesToDatasets( // 1. Sort features into groups by path and geom type Dictionary> geometryGroups = new(); - foreach (var conversionTrackers in conversionTracker) + foreach (var item in conversionTracker) { - TraversalContext context = conversionTrackers.Key; - - foreach (var trackerItem in conversionTrackers.Value) + try { - try + TraversalContext context = item.Key; + var trackerItem = item.Value; + ACG.Geometry? geom = trackerItem.HostAppGeom; + string? datasetId = trackerItem.DatasetId; + if (geom != null && datasetId == null) // only non-native geomerties, not written into a dataset yet { - ACG.Geometry? geom = trackerItem.HostAppGeom; - string? datasetId = trackerItem.DatasetId; - if (geom != null && datasetId == null) // only non-native geomerties, not written into a dataset yet - { - // add dictionnary item if doesn't exist yet - // Key must be unique per parent and speckleType - // Adding Offsets/rotation to Unique key, so the modified CAD geometry doesn't overwrite non-modified one - // or, same commit received with different Offsets are saved to separate datasets - - // Also, keep char limit for dataset name under 128: https://pro.arcgis.com/en/pro-app/latest/help/data/geodatabases/manage-saphana/enterprise-geodatabase-limits.htm + // add dictionnary item if doesn't exist yet + // Key must be unique per parent and speckleType + // Adding Offsets/rotation to Unique key, so the modified CAD geometry doesn't overwrite non-modified one + // or, same commit received with different Offsets are saved to separate datasets - string speckleType = trackerItem.Base.speckle_type.Split(".")[^1]; - //speckleType = speckleType.Substring(0, Math.Min(10, speckleType.Length - 1)); - speckleType = speckleType.Length > 10 ? speckleType[..9] : speckleType; - string? parentId = context.Parent?.Current.id; + // Also, keep char limit for dataset name under 128: https://pro.arcgis.com/en/pro-app/latest/help/data/geodatabases/manage-saphana/enterprise-geodatabase-limits.htm - CRSoffsetRotation activeSR = _contextStack.Current.Document.ActiveCRSoffsetRotation; - string xOffset = Convert.ToString(activeSR.LonOffset).Replace(".", "_"); - xOffset = xOffset.Length > 15 ? xOffset[..14] : xOffset; + string speckleType = trackerItem.Base.speckle_type.Split(".")[^1]; + //speckleType = speckleType.Substring(0, Math.Min(10, speckleType.Length - 1)); + speckleType = speckleType.Length > 10 ? speckleType[..9] : speckleType; + string? parentId = context.Parent?.Current.id; - string yOffset = Convert.ToString(activeSR.LatOffset).Replace(".", "_"); - yOffset = yOffset.Length > 15 ? yOffset[..14] : yOffset; + CRSoffsetRotation activeSR = _contextStack.Current.Document.ActiveCRSoffsetRotation; + string xOffset = Convert.ToString(activeSR.LonOffset).Replace(".", "_"); + xOffset = xOffset.Length > 15 ? xOffset[..14] : xOffset; - string trueNorth = Convert.ToString(activeSR.TrueNorthRadians).Replace(".", "_"); - trueNorth = trueNorth.Length > 10 ? trueNorth[..9] : trueNorth; + string yOffset = Convert.ToString(activeSR.LatOffset).Replace(".", "_"); + yOffset = yOffset.Length > 15 ? yOffset[..14] : yOffset; - // text: 36 symbols, speckleTYPE: 10, sr: 10, offsets: 40, id: 32 = 128 - string uniqueKey = - $"speckle_{speckleType}_SR_{activeSR.SpatialReference.Name[..Math.Min(15, activeSR.SpatialReference.Name.Length - 1)]}_X_{xOffset}_Y_{yOffset}_North_{trueNorth}_speckleID_{parentId}"; + string trueNorth = Convert.ToString(activeSR.TrueNorthRadians).Replace(".", "_"); + trueNorth = trueNorth.Length > 10 ? trueNorth[..9] : trueNorth; - if (!geometryGroups.TryGetValue(uniqueKey, out _)) - { - geometryGroups[uniqueKey] = new(); - } + // text: 36 symbols, speckleTYPE: 10, sr: 10, offsets: 40, id: 32 = 128 + string uniqueKey = + $"speckle_{speckleType}_SR_{activeSR.SpatialReference.Name[..Math.Min(15, activeSR.SpatialReference.Name.Length - 1)]}_X_{xOffset}_Y_{yOffset}_North_{trueNorth}_speckleID_{parentId}"; - geometryGroups[uniqueKey].Add((trackerItem.Base, geom)); - - // record changes in conversion tracker - trackerItem.AddDatasetId(uniqueKey); - trackerItem.AddDatasetRow(geometryGroups[uniqueKey].Count - 1); - conversionTracker[context] = [trackerItem]; - } - else if (geom == null && datasetId != null) // GIS layers, already written to a dataset + if (!geometryGroups.TryGetValue(uniqueKey, out _)) { - continue; - } - else - { - throw new ArgumentException($"Unexpected geometry and datasetId values: {geom}, {datasetId}"); + geometryGroups[uniqueKey] = new(); } - //var trackerItem = item.Value; + geometryGroups[uniqueKey].Add((trackerItem.Base, geom)); + + // record changes in conversion tracker + trackerItem.AddDatasetId(uniqueKey); + trackerItem.AddDatasetRow(geometryGroups[uniqueKey].Count - 1); + conversionTracker[item.Key] = trackerItem; + } + else if (geom == null && datasetId != null) // GIS layers, already written to a dataset + { + continue; } - catch (Exception ex) when (!ex.IsFatal()) + else { - // POC: report, etc. - // var trackerItem = item.Value; - trackerItem.AddException(ex); - conversionTracker[context] = [trackerItem]; - Debug.WriteLine($"conversion error happened. {ex.Message}"); + throw new ArgumentException($"Unexpected geometry and datasetId values: {geom}, {datasetId}"); } } + catch (Exception ex) when (!ex.IsFatal()) + { + // POC: report, etc. + var trackerItem = item.Value; + trackerItem.AddException(ex); + conversionTracker[item.Key] = trackerItem; + Debug.WriteLine($"conversion error happened. {ex.Message}"); + } } // 2. for each group create a Dataset and add geometries there as Features @@ -125,14 +120,11 @@ public void WriteGeometriesToDatasets( // only record in conversionTracker: foreach (var conversionItem in conversionTracker) { - foreach (var item2 in conversionItem.Value) + if (conversionItem.Value.DatasetId == uniqueKey) { - if (item2.DatasetId == uniqueKey) - { - var trackerItem = conversionItem.Value; - item2.AddException(ex); - conversionTracker[conversionItem.Key] = trackerItem; - } + var trackerItem = conversionItem.Value; + trackerItem.AddException(ex); + conversionTracker[conversionItem.Key] = trackerItem; } } } diff --git a/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs b/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs index 75722c999..5b1d4c93e 100644 --- a/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs +++ b/Sdk/Speckle.Converters.Common.DependencyInjection/ToHost/ConverterWithFallback.cs @@ -64,8 +64,14 @@ private object FallbackToDisplayValue(IReadOnlyList displayValue) { var tempDisplayableObject = new DisplayableObject(displayValue); var conversionResult = _baseConverter.Convert(tempDisplayableObject); - var res = conversionResult is IEnumerable result ? result.Zip(displayValue, (a, b) => (a, b)) : []; - return res; + // if the host app returns a list of objects as the result of the fallback conversion, we zip them together with the original base display value objects that generated them. + if (conversionResult is IEnumerable result) + { + return result.Zip(displayValue, (a, b) => (a, b)); + } + + // if not, and the host app "merges" together somehow multiple display values into one entity, we return that. + return conversionResult; } }