From 659a29a294b042cefa44accc2bc1a3cce0eb397a Mon Sep 17 00:00:00 2001 From: Jonathon Broughton <760691+jsdbroughton@users.noreply.github.com> Date: Mon, 17 Nov 2025 17:42:52 +0000 Subject: [PATCH] Improves property value conversion and extraction (#1183) * Refactors property extraction to use model units Uses the model's units when extracting properties to ensure consistency and accuracy of converted values. Extracts property sets as a static function to provide re-usability without the class instance. * Refactors Revit category extraction Improves Revit category extraction by utilizing UI units for property conversion, ensuring consistent unit handling. Additionally, refactors the extractor to a static class. * Improves property conversion and handling Introduces robust property conversion and handling logic. Leverages user interface units for length, area, and volume property conversions, ensuring consistency with the Navisworks UI. Enhances property data handling by using dictionaries to represent numerical properties with associated units, providing more context for downstream applications. Adds property name sanitization to ensure compatibility with Speckle's object model. * Removes the unused `Speckle.Converter.Navisworks.Helpers` import from the PropertySetsExtractor class to reduce clutter and improve code maintainability. Relates to CNX-2784 * Standardizes numerical property dictionary creation. Simplifies the creation of numerical property dictionaries by removing the `internalDefinitionName` parameter from the `NumObj` method. This ensures a consistent format for numerical properties across the connector. Relates to CNX-2784 * Refactors property conversion to service Moves property conversion logic into a dedicated service. This improves code organization and testability and allows to reuse logic and manage UI units consistently. * Refactors property conversion for consistency Standardizes property conversion by introducing a dedicated `IPropertyConverter` service. This change ensures consistent handling of property values across different extractors, improving data accuracy and reducing inconsistencies in quantity extraction. It also adds resets of the property converter to ensure clean conversions for each item. Relates to CNX-2784 * Update Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Moves UI Units cache to service Moves the UI Units cache logic from the helpers to a dedicated service. This improves the separation of concerns and makes the code more maintainable and testable. Relates to CNX-2784 * Fixes unit label typos. Corrects minor spelling errors in the unit labels for centimeters, millimeters, micrometers, and kilometers. Relates to CNX-2784 * Ensures correct units are used on send Uses the UI Units Cache service to ensure the correct units are being applied to objects when sending them to Speckle. Relates to CNX-2784 * Enhances the property helper functionality to support additional features. Adjusts the constructor parameters to accommodate new requirements. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Send/NavisworksRootObjectBuilder.cs | 7 +- .../DataExtractors/PropertySetsExtractor.cs | 42 ++++++--- .../RevitBuiltInCategoryExtractor.cs | 24 ++--- .../HierarchicalPropertyHandler.cs | 5 +- .../Helpers/PropertyHelpers.cs | 29 +++++- .../Services/PropertyConversion.cs | 91 +++++++++++++++++++ .../Services/UIUnits.cs | 71 +++++++++++++++ ...ckle.Converters.NavisworksShared.projitems | 2 + 8 files changed, 244 insertions(+), 27 deletions(-) create mode 100644 Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/PropertyConversion.cs create mode 100644 Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/UIUnits.cs diff --git a/Connectors/Navisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs b/Connectors/Navisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs index af4f25547..39e38df80 100644 --- a/Connectors/Navisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs +++ b/Connectors/Navisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; using Speckle.Connector.Navisworks.HostApp; using Speckle.Connector.Navisworks.Services; using Speckle.Connectors.Common.Builders; @@ -27,6 +27,7 @@ public class NavisworksRootObjectBuilder( NavisworksMaterialUnpacker materialUnpacker, NavisworksColorUnpacker colorUnpacker, IElementSelectionService elementSelectionService, + IUiUnitsCache uiUnitsCache, InstanceStoreManager instanceStoreManager ) : IRootObjectBuilder { @@ -255,12 +256,14 @@ public class NavisworksRootObjectBuilder( (string name, string path) = GetElementNameAndPath(convertedBase.applicationId); + var units = uiUnitsCache.Ensure(); + return new NavisworksObject { name = name, displayValue = convertedBase["displayValue"] as List ?? [], properties = convertedBase["properties"] as Dictionary ?? [], - units = converterSettings.Current.Derived.SpeckleUnits, + units = units.ToString(), applicationId = convertedBase.applicationId, ["path"] = path }; diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/PropertySetsExtractor.cs b/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/PropertySetsExtractor.cs index 9b8023f44..485f896e8 100644 --- a/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/PropertySetsExtractor.cs +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/PropertySetsExtractor.cs @@ -1,10 +1,14 @@ -using Speckle.Converter.Navisworks.Settings; +using Speckle.Converter.Navisworks.Services; +using Speckle.Converter.Navisworks.Settings; using Speckle.Converters.Common; using static Speckle.Converter.Navisworks.Helpers.PropertyHelpers; namespace Speckle.Converter.Navisworks.ToSpeckle; -public class PropertySetsExtractor(IConverterSettingsStore settingsStore) +public class PropertySetsExtractor( + IConverterSettingsStore settingsStore, + IPropertyConverter propertyConverter +) { internal Dictionary? GetPropertySets(NAV.ModelItem modelItem) { @@ -18,9 +22,30 @@ public class PropertySetsExtractor(IConverterSettingsStore + /// Extracts property sets from a NAV.ModelItem and adds them to a dictionary, + /// PropertySets are specific to the host application source appended to Navisworks and therefore + /// arbitrary in nature. + /// + /// The NAV.ModelItem from which property sets are extracted. + /// A dictionary containing property sets of the modelItem. private Dictionary ExtractPropertySets(NAV.ModelItem modelItem) { var propertySetDictionary = new Dictionary(); + var modelUnits = GetModelUnits(modelItem); + + propertyConverter.Reset(); foreach (var propertyCategory in modelItem.PropertyCategories) { @@ -33,23 +58,18 @@ public class PropertySetsExtractor(IConverterSettingsStore 0) { - continue; + propertySetDictionary[SanitizePropertyName(propertyCategory.DisplayName)] = propertySet; } - - string categoryName = SanitizePropertyName(propertyCategory.DisplayName); - - propertySetDictionary[categoryName] = propertySet; } return propertySetDictionary; diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/RevitBuiltInCategoryExtractor.cs b/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/RevitBuiltInCategoryExtractor.cs index 5b50dd4e6..9beba6761 100644 --- a/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/RevitBuiltInCategoryExtractor.cs +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/RevitBuiltInCategoryExtractor.cs @@ -1,8 +1,10 @@ -using static Speckle.Converter.Navisworks.Helpers.PropertyHelpers; +using Speckle.Converter.Navisworks.Services; +using Speckle.InterfaceGenerator; namespace Speckle.Converter.Navisworks.ToSpeckle; -public sealed class RevitBuiltInCategoryExtractor +[GenerateAutoInterface] +public class RevitBuiltInCategoryExtractor(IPropertyConverter converter) : IRevitBuiltInCategoryExtractor { private const int ANCESTOR_AND_SELF_COUNT = 4; // It seems like this is the maximum depth found needed in practice private const string REVIT_CAT_GROUP = "LcRevitData_Element"; @@ -13,28 +15,28 @@ public sealed class RevitBuiltInCategoryExtractor /// Attempts to map a Navisworks/Revit display category from the given model item or its ancestors /// to a known Revit built-in category constant (e.g., "OST_Walls"). /// - internal static bool TryGetBuiltInCategory( - NAV.ModelItem item, - out string mapped, - int maxDepth = ANCESTOR_AND_SELF_COUNT - ) + public bool TryGetBuiltInCategory(NAV.ModelItem item, out string mapped, int maxDepth = ANCESTOR_AND_SELF_COUNT) { mapped = string.Empty; - // Look up the category value, starting at this item and walking up to maxDepth ancestors + // Find the category VariantData up the hierarchy var v = FindRevitCategoryInHierarchy(item, maxDepth); - if (v == null) + if (v is null) { return false; } - var name = ConvertPropertyValue(v, "")?.ToString(); + converter.Reset(); + + // Convert using per-object model units and current UI units + var nameObj = converter.ConvertPropertyValue(v, item.Model.Units, item.DisplayName); + var name = nameObj?.ToString(); if (string.IsNullOrWhiteSpace(name)) { return false; } - name = name?.Trim(); + name = name!.Trim(); // Map display name to OST_* built-in category constant var builtInCategory = DisplayNameToRevitBuiltInCategory(name); diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataHandlers/HierarchicalPropertyHandler.cs b/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataHandlers/HierarchicalPropertyHandler.cs index e882cd5f9..810547887 100644 --- a/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataHandlers/HierarchicalPropertyHandler.cs +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/DataHandlers/HierarchicalPropertyHandler.cs @@ -11,7 +11,8 @@ public class HierarchicalPropertyHandler( PropertySetsExtractor propertySetsExtractor, ModelPropertiesExtractor modelPropertiesExtractor, ClassPropertiesExtractor classPropertiesExtractor, - IConverterSettingsStore settingsStore + IConverterSettingsStore settingsStore, + IRevitBuiltInCategoryExtractor revitCategoryExtractor ) : BasePropertyHandler(propertySetsExtractor, modelPropertiesExtractor) { private static string PseudoClassPropertiesKey => "_pseudoClassProperties"; @@ -22,7 +23,7 @@ public class HierarchicalPropertyHandler( var propertyDict = classPropertiesExtractor.GetClassProperties(modelItem) ?? []; // Interop-lite mapping for Revit built-in categories - if (_mapRevit && RevitBuiltInCategoryExtractor.TryGetBuiltInCategory(modelItem, out var builtInCategory)) + if (_mapRevit && revitCategoryExtractor.TryGetBuiltInCategory(modelItem, out var builtInCategory)) { PropertyHelpers.AddPropertyIfNotNullOrEmpty( propertyDict, diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs index 225ae249f..b6fc07ae4 100644 --- a/Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs @@ -1,4 +1,4 @@ -using System.Globalization; +using System.Globalization; using System.Text.RegularExpressions; using Speckle.Objects.Geometry; @@ -8,6 +8,9 @@ public static class PropertyHelpers { private static readonly HashSet s_excludedCategories = ["Geometry", "Metadata"]; + /// + /// Adds a property to an object (either a Base object or a Dictionary) if the value is not null or empty. + /// private static readonly Dictionary> s_typeHandlers = new() { @@ -92,3 +95,27 @@ public static class PropertyHelpers internal static bool ShouldSkipCategory(NAV.PropertyCategory propertyCategory) => s_excludedCategories.Contains(propertyCategory.DisplayName); } + +internal static class UnitLabels +{ + internal static string Linear(NAV.Units u) => + u switch + { + NAV.Units.Kilometers => "Kilometers", + NAV.Units.Meters => "Metres", + NAV.Units.Centimeters => "Centimeters", + NAV.Units.Millimeters => "Millimeters", + NAV.Units.Micrometers => "Micrometers", + NAV.Units.Miles => "Miles", + NAV.Units.Yards => "Yards", + NAV.Units.Feet => "Feet", + NAV.Units.Inches => "Inches", + NAV.Units.Mils => "Mils", + NAV.Units.Microinches => "Microinches", + _ => "Metres" + }; + + internal static string Area(NAV.Units u) => $"Square {Linear(u).ToLower()}"; + + public static string Volume(NAV.Units u) => $"Cubic {Linear(u).ToLower()}"; +} diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/PropertyConversion.cs b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/PropertyConversion.cs new file mode 100644 index 000000000..36497010d --- /dev/null +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/PropertyConversion.cs @@ -0,0 +1,91 @@ +using System.Globalization; +using Speckle.Converter.Navisworks.Helpers; +using Speckle.InterfaceGenerator; + +namespace Speckle.Converter.Navisworks.Services; + +[GenerateAutoInterface] +public class PropertyConverter(IUiUnitsCache uiUnitsCache) : IPropertyConverter +{ + public void Reset() => uiUnitsCache.Reset(); + + public object? ConvertPropertyValue(NAV.VariantData? value, NAV.Units modelUnits, string propDisplayName) => + value == null + ? null + : _handlers.TryGetValue(value.DataType, out var f) + ? f(value, (modelUnits, propDisplayName)) + : value.DataType is NAV.VariantDataType.None or NAV.VariantDataType.Point2D + ? null + : value.ToString(); + + private readonly Dictionary< + NAV.VariantDataType, + Func + > _handlers = + new() + { + { NAV.VariantDataType.Boolean, (v, _) => v.ToBoolean() }, + { NAV.VariantDataType.DisplayString, (v, _) => v.ToDisplayString() }, + { NAV.VariantDataType.IdentifierString, (v, _) => v.ToIdentifierString() }, + { NAV.VariantDataType.Int32, (v, _) => v.ToInt32() }, + { NAV.VariantDataType.Double, (v, _) => v.ToDouble() }, + // Angle as dictionary with units + { NAV.VariantDataType.DoubleAngle, (v, t) => NumObj(t.name, v.ToDoubleAngle(), "Degrees") }, + // Length → dictionary in UI units + { + NAV.VariantDataType.DoubleLength, + (v, t) => + { + var uiUnits = uiUnitsCache.Ensure(); + + var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits); + return NumObj(t.name, v.ToDoubleLength() * k, UnitLabels.Linear(uiUnits)); + } + }, + // Area → dictionary in UI units^2 + { + NAV.VariantDataType.DoubleArea, + (v, t) => + { + var uiUnits = uiUnitsCache.Ensure(); + var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits); + k *= k; + return NumObj(t.name, v.ToDoubleArea() * k, UnitLabels.Area(uiUnits)); + } + }, + // Volume → dictionary in UI units^3 + { + NAV.VariantDataType.DoubleVolume, + (v, t) => + { + var uiUnits = uiUnitsCache.Ensure(); + var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits); + k = k * k * k; + return NumObj(t.name, v.ToDoubleVolume() * k, UnitLabels.Volume(uiUnits)); + } + }, + { NAV.VariantDataType.DateTime, (v, _) => v.ToDateTime().ToString(CultureInfo.InvariantCulture) }, + { NAV.VariantDataType.NamedConstant, (v, _) => v.ToNamedConstant().DisplayName }, + { NAV.VariantDataType.None, (_, _) => null }, + { NAV.VariantDataType.Point2D, (_, _) => null }, + { + NAV.VariantDataType.Point3D, + (v, t) => + { + var uiUnits = uiUnitsCache.Ensure(); + var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits); + var p = v.ToPoint3D(); + + return new Speckle.Objects.Geometry.Point(p.X * k, p.Y * k, p.Z * k, UnitLabels.Linear(uiUnits)); + } + } + }; + + private static Dictionary NumObj(string name, double value, string units) => + new() + { + ["name"] = name, + ["value"] = value, + ["units"] = units + }; +} diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/UIUnits.cs b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/UIUnits.cs new file mode 100644 index 000000000..26183a697 --- /dev/null +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/UIUnits.cs @@ -0,0 +1,71 @@ +using Autodesk.Navisworks.Api.Interop; +using Speckle.InterfaceGenerator; +using static Autodesk.Navisworks.Api.Interop.LcUOption; + +namespace Speckle.Converter.Navisworks.Services; + +[GenerateAutoInterface] +public class UiUnitsCache : IUiUnitsCache +{ + private NAV.Units? _ui; + + public NAV.Units Ensure() + { + if (_ui.HasValue) + { + return _ui.Value; + } + + UiUnitsUtil.TryGetUiLinearUnits(out var ui); + _ui = ui; + return _ui.Value; + } + + public void Reset() => _ui = null; +} + +public static class UiUnitsUtil +{ + // disp_units: 0=linear_format + public static bool TryGetUiLinearUnits(out NAV.Units uiUnits) + { + using var opt = new LcUOptionLock(); + var root = GetRoot(opt); + var disp = root.GetSubOptions("interface").GetSubOptions("disp_units"); + + int code = -1; + + using var v = new NAV.VariantData(); + disp.GetValue(0, v); + var s = v.ToString(); + var colon = s.LastIndexOf(':'); + var open = s.IndexOf('(', colon + 1); + if (colon >= 0 && open > colon && !int.TryParse(s.Substring(colon + 1, open - colon - 1), out code)) + { + code = -1; + } + + uiUnits = code switch + { + 0 => NAV.Units.Kilometers, + 1 => NAV.Units.Meters, + 2 => NAV.Units.Centimeters, + 3 => NAV.Units.Millimeters, + 4 => NAV.Units.Micrometers, + 5 => NAV.Units.Miles, + 6 => NAV.Units.Miles, + 7 => NAV.Units.Yards, + 8 => NAV.Units.Yards, + 9 => NAV.Units.Feet, + 10 => NAV.Units.Feet, + 11 => NAV.Units.Feet, + 12 => NAV.Units.Inches, + 13 => NAV.Units.Inches, + 14 => NAV.Units.Mils, + 15 => NAV.Units.Microinches, + _ => NAV.Units.Meters + }; + + return code >= 0; + } +} diff --git a/Converters/Navisworks/Speckle.Converters.NavisworksShared/Speckle.Converters.NavisworksShared.projitems b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Speckle.Converters.NavisworksShared.projitems index 24e198b16..2523f527d 100644 --- a/Converters/Navisworks/Speckle.Converters.NavisworksShared/Speckle.Converters.NavisworksShared.projitems +++ b/Converters/Navisworks/Speckle.Converters.NavisworksShared/Speckle.Converters.NavisworksShared.projitems @@ -30,6 +30,8 @@ + +