From 5e9288982529dfec9256ab884cdbe1a9bdff287f Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Fri, 8 Nov 2024 17:54:41 +0000 Subject: [PATCH 1/2] Rhino stability fixes (#375) * fix: (wip) ensures layer creation is happening where it should (InvokeAndWait) * fix: ensures polylines with null domains still convert ok * feat: comments and makes non-public function private * feat: wraps up fixes makes layer purging quiet, and removes stale comments --- .../HostApp/RhinoInstanceBaker.cs | 5 +- .../HostApp/RhinoLayerBaker.cs | 52 ++++++++++++++++-- .../Operations/Receive/DisableRedrawScope.cs | 1 + .../Receive/RhinoHostObjectBuilder.cs | 53 ++++++++++--------- .../ToHost/Raw/PolylineToHostConverter.cs | 6 ++- 5 files changed, 83 insertions(+), 34 deletions(-) diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs index d8af6ecf8..8cc716e9f 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs @@ -118,10 +118,7 @@ instanceOrDefinition is InstanceProxy instanceProxy ) { var transform = MatrixToTransform(instanceProxy.transform, instanceProxy.units); - - // POC: having layer creation during instance bake means no render materials!! - int layerIndex = _layerBaker.GetAndCreateLayerFromPath(layerCollection, baseLayerName); - + int layerIndex = _layerBaker.GetLayerIndex(layerCollection, baseLayerName); string instanceProxyId = instanceProxy.applicationId ?? instanceProxy.id; ObjectAttributes atts = new() { LayerIndex = layerIndex }; diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs index 0f956b9ed..95aa94fd0 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoLayerBaker.cs @@ -1,6 +1,7 @@ using Rhino; using Rhino.DocObjects; using Speckle.Connectors.Common.Operations.Receive; +using Speckle.Sdk; using Speckle.Sdk.Models.Collections; using Layer = Rhino.DocObjects.Layer; @@ -25,18 +26,48 @@ public RhinoLayerBaker(RhinoMaterialBaker materialBaker, RhinoColorBaker colorBa /// Creates the base layer and adds it to the cache. /// /// - public void CreateBaseLayer(string baseLayerName) + private void CreateBaseLayer(string baseLayerName) { var index = RhinoDoc.ActiveDoc.Layers.Add(new Layer { Name = baseLayerName }); // POC: too much effort right now to wrap around the interfaced layers and doc - _hostLayerCache.Add(baseLayerName, index); + _hostLayerCache[baseLayerName] = index; } /// - /// For receive: Use this method to construct layers in the host app when receiving. It progressively caches layers while creating them, so a second call to get the same layer will be fast. + /// Creates all layers needed for receiving data. /// - public int GetAndCreateLayerFromPath(Collection[] collectionPath, string baseLayerName) + /// Collections of paths + /// Name of the base layer + /// Make sure this is executing on the main thread, using e.g RhinoApp.InvokeAndWait. + public void CreateAllLayersForReceive(IEnumerable paths, string baseLayerName) { - var layerPath = collectionPath.Select(o => string.IsNullOrWhiteSpace(o.name) ? "unnamed" : o.name); + CreateBaseLayer(baseLayerName); + var uniquePaths = new Dictionary(); + foreach (var path in paths) + { + var names = path.Select(o => string.IsNullOrWhiteSpace(o.name) ? "unnamed" : o.name); + var key = string.Join(",", names!); + uniquePaths[key] = path; + } + + foreach (var uniquePath in uniquePaths) + { + var layerIndex = CreateLayerFromPath(uniquePath.Value, baseLayerName); + } + } + + /// + /// Retrieves the index of a layer based on the given collection path and base layer name. + /// + /// The array containing the collection path to the layer. + /// The name of the base layer. + /// The index of the layer in the cache. + /// Thrown when the layer is not found in the cache. This can happen if you didn't call previously + public int GetLayerIndex(Collection[] collectionPath, string baseLayerName) + { + var layerPath = collectionPath + .Select(o => string.IsNullOrWhiteSpace(o.name) ? "unnamed" : o.name) + .Prepend(baseLayerName); + var layerFullName = string.Join(Layer.PathSeparator, layerPath); if (_hostLayerCache.TryGetValue(layerFullName, out int existingLayerIndex)) @@ -44,6 +75,17 @@ public int GetAndCreateLayerFromPath(Collection[] collectionPath, string baseLay return existingLayerIndex; } + throw new SpeckleException("Did not find a layer in the cache."); + } + + /// + /// Creates a layer based on the given collection path and adds it to the Rhino document. + /// + /// An array of Collection objects representing the path to create the layer. + /// The base layer name to start creating the new layer. + /// The index of the last created layer. + private int CreateLayerFromPath(Collection[] collectionPath, string baseLayerName) + { var currentLayerName = baseLayerName; var currentDocument = RhinoDoc.ActiveDoc; // POC: too much effort right now to wrap around the interfaced layers Layer? previousLayer = currentDocument.Layers.FindName(currentLayerName); diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/DisableRedrawScope.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/DisableRedrawScope.cs index b46437bd1..3a3e68440 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/DisableRedrawScope.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/DisableRedrawScope.cs @@ -21,5 +21,6 @@ public DisableRedrawScope(ViewTable viewTable, bool returnToStatus = true) public void Dispose() { _viewTable.RedrawEnabled = _returnToStatus; + _viewTable.Redraw(); } } diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs index 80f44c35a..939051fea 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs @@ -54,7 +54,9 @@ ISdkActivityFactory activityFactory _activityFactory = activityFactory; } +#pragma warning disable CA1506 public async Task Build( +#pragma warning restore CA1506 Base rootObject, string projectName, string modelName, @@ -68,9 +70,9 @@ CancellationToken cancellationToken // 0 - Clean then Rock n Roll! PreReceiveDeepClean(baseLayerName); - _layerBaker.CreateBaseLayer(baseLayerName); // 1 - Unpack objects and proxies from root commit object + var unpackedRoot = _rootObjectUnpacker.Unpack(rootObject); // 2 - Split atomic objects and instance components with their path @@ -107,11 +109,11 @@ CancellationToken cancellationToken onOperationProgressed.Report(new("Baking layers (redraw disabled)", null)); using (var _ = _activityFactory.Start("Pre baking layers")) { - using var layerNoDraw = new DisableRedrawScope(_converterSettings.Current.Document.Views); - foreach (var (path, _) in atomicObjectsWithPath) + RhinoApp.InvokeAndWait(() => { - _layerBaker.GetAndCreateLayerFromPath(path, baseLayerName); - } + using var layerNoDraw = new DisableRedrawScope(_converterSettings.Current.Document.Views); + _layerBaker.CreateAllLayersForReceive(atomicObjectsWithPath.Select(t => t.path), baseLayerName); + }); } // 5 - Convert atomic objects @@ -130,7 +132,7 @@ CancellationToken cancellationToken try { // 0: get pre-created layer from cache in layer baker - int layerIndex = _layerBaker.GetAndCreateLayerFromPath(path, baseLayerName); + int layerIndex = _layerBaker.GetLayerIndex(path, baseLayerName); // 1: create object attributes for baking string name = obj["name"] as string ?? ""; @@ -217,7 +219,6 @@ CancellationToken cancellationToken } _converterSettings.Current.Document.Views.Redraw(); - return new HostObjectBuilderResult(bakedObjectIds, conversionResults); } @@ -230,31 +231,35 @@ private void PreReceiveDeepClean(string baseLayerName) RhinoMath.UnsetIntIndex ); - _instanceBaker.PurgeInstances(baseLayerName); - _materialBaker.PurgeMaterials(baseLayerName); - - var doc = _converterSettings.Current.Document; - // Cleans up any previously received objects - if (rootLayerIndex != RhinoMath.UnsetIntIndex) + RhinoApp.InvokeAndWait(() => { - var documentLayer = doc.Layers[rootLayerIndex]; - var childLayers = documentLayer.GetChildren(); - if (childLayers != null) + _instanceBaker.PurgeInstances(baseLayerName); + _materialBaker.PurgeMaterials(baseLayerName); + + var doc = _converterSettings.Current.Document; + // Cleans up any previously received objects + if (rootLayerIndex != RhinoMath.UnsetIntIndex) { - using var layerNoDraw = new DisableRedrawScope(doc.Views); - foreach (var layer in childLayers) + var documentLayer = doc.Layers[rootLayerIndex]; + var childLayers = documentLayer.GetChildren(); + if (childLayers != null) { - var purgeSuccess = doc.Layers.Purge(layer.Index, true); - if (!purgeSuccess) + using var layerNoDraw = new DisableRedrawScope(doc.Views); + foreach (var layer in childLayers) { - Console.WriteLine($"Failed to purge layer: {layer}"); + var purgeSuccess = doc.Layers.Purge(layer.Index, true); + if (!purgeSuccess) + { + Console.WriteLine($"Failed to purge layer: {layer}"); + } } } + doc.Layers.Purge(documentLayer.Index, true); } - } - // Cleans up any previously received group - _groupBaker.PurgeGroups(baseLayerName); + // Cleans up any previously received group + _groupBaker.PurgeGroups(baseLayerName); + }); } /// diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/Raw/PolylineToHostConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/Raw/PolylineToHostConverter.cs index 50b906790..87f69b0f0 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/Raw/PolylineToHostConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/Raw/PolylineToHostConverter.cs @@ -55,7 +55,11 @@ public RG.Polyline Convert(SOG.Polyline target) RG.PolylineCurve ITypedConverter.Convert(SOG.Polyline target) { var poly = Convert(target).ToPolylineCurve(); - poly.Domain = _intervalConverter.Convert(target.domain); + if (target.domain is not null) // note it can be null + { + poly.Domain = _intervalConverter.Convert(target.domain); + } + return poly; } } From 34457ead0a0f2edac2467ade724fb1841b342e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?O=C4=9Fuzhan=20Koral?= <45078678+oguzhankoral@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:06:07 +0000 Subject: [PATCH 2/2] remove useless code (#377) --- .../Bindings/RevitSendBinding.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitSendBinding.cs b/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitSendBinding.cs index c2cf62291..c8362e69c 100644 --- a/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitSendBinding.cs +++ b/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitSendBinding.cs @@ -133,18 +133,6 @@ public async Task Send(string modelCardId) ); List elements = await RefreshElementsOnSender(modelCard.NotNull()).ConfigureAwait(false); - - if (modelCard is SenderModelCard senderModel) - { - foreach (Element element in elements) - { - senderModel.SendFilter.NotNull().IdMap.NotNull()[element.Id.ToString()] = element.UniqueId; - } - await Commands - .SetFilterObjectIds(modelCardId, senderModel.SendFilter.NotNull().IdMap.NotNull()) - .ConfigureAwait(false); - } - List elementIds = elements.Select(el => el.Id).ToList(); if (elementIds.Count == 0)