Skip to content

Commit

Permalink
Chore(revit): Refactor material cache to ToHost ToSpeckle singletons (#…
Browse files Browse the repository at this point in the history
…262)

* A bit better

* Post conflict errors

* Remove old notes
  • Loading branch information
oguzhankoral authored Sep 25, 2024
1 parent d8f3fa7 commit fa31283
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal sealed class RevitHostObjectBuilder : IHostObjectBuilder, IDisposable
{
private readonly IRootToHostConverter _converter;
private readonly IConverterSettingsStore<RevitConversionSettings> _converterSettings;
private readonly RevitMaterialCacheSingleton _revitMaterialCacheSingleton;
private readonly RevitToHostCacheSingleton _revitToHostCacheSingleton;
private readonly ITransactionManager _transactionManager;
private readonly ILocalToGlobalUnpacker _localToGlobalUnpacker;
private readonly LocalToGlobalConverterUtils _localToGlobalConverterUtils;
Expand All @@ -41,7 +41,7 @@ public RevitHostObjectBuilder(
RevitMaterialBaker materialBaker,
RootObjectUnpacker rootObjectUnpacker,
ILogger<RevitHostObjectBuilder> logger,
RevitMaterialCacheSingleton revitMaterialCacheSingleton
RevitToHostCacheSingleton revitToHostCacheSingleton
)
{
_converter = converter;
Expand All @@ -53,7 +53,7 @@ RevitMaterialCacheSingleton revitMaterialCacheSingleton
_materialBaker = materialBaker;
_rootObjectUnpacker = rootObjectUnpacker;
_logger = logger;
_revitMaterialCacheSingleton = revitMaterialCacheSingleton;
_revitToHostCacheSingleton = revitToHostCacheSingleton;
_activityFactory = activityFactory;
}

Expand Down Expand Up @@ -118,7 +118,7 @@ CancellationToken cancellationToken
var map = _materialBaker.BakeMaterials(unpackedRoot.RenderMaterialProxies, baseGroupName);
foreach (var kvp in map)
{
_revitMaterialCacheSingleton.ObjectIdAndMaterialIndexMap.Add(kvp.Key, kvp.Value);
_revitToHostCacheSingleton.MaterialsByObjectId.Add(kvp.Key, kvp.Value);
}
}

Expand Down Expand Up @@ -149,7 +149,7 @@ CancellationToken cancellationToken
createGroupTransaction.Assimilate();
}

_revitMaterialCacheSingleton.ObjectIdAndMaterialIndexMap.Clear(); // Massive hack!
_revitToHostCacheSingleton.MaterialsByObjectId.Clear(); // Massive hack!

return conversionResults;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class RevitRootObjectBuilder : IRootObjectBuilder<ElementId>
private readonly ISendConversionCache _sendConversionCache;
private readonly ElementUnpacker _elementUnpacker;
private readonly SendCollectionManager _sendCollectionManager;
private readonly RevitMaterialCacheSingleton _revitMaterialCacheSingleton;
private readonly RevitToSpeckleCacheSingleton _revitToSpeckleCacheSingleton;
private readonly ILogger<RevitRootObjectBuilder> _logger;
private readonly ParameterDefinitionHandler _parameterDefinitionHandler;

Expand All @@ -39,15 +39,15 @@ public RevitRootObjectBuilder(
SendCollectionManager sendCollectionManager,
ILogger<RevitRootObjectBuilder> logger,
ParameterDefinitionHandler parameterDefinitionHandler,
RevitMaterialCacheSingleton revitMaterialCacheSingleton
RevitToSpeckleCacheSingleton revitToSpeckleCacheSingleton
)
{
_converter = converter;
_converterSettings = converterSettings;
_sendConversionCache = sendConversionCache;
_elementUnpacker = elementUnpacker;
_sendCollectionManager = sendCollectionManager;
_revitMaterialCacheSingleton = revitMaterialCacheSingleton;
_revitToSpeckleCacheSingleton = revitToSpeckleCacheSingleton;
_logger = logger;
_parameterDefinitionHandler = parameterDefinitionHandler;

Expand Down Expand Up @@ -135,7 +135,7 @@ public async Task<RootObjectBuilderResult> Build(
}

var idsAndSubElementIds = _elementUnpacker.GetElementsAndSubelementIdsFromAtomicObjects(atomicObjects);
var materialProxies = _revitMaterialCacheSingleton.GetRenderMaterialProxyListForObjects(idsAndSubElementIds);
var materialProxies = _revitToSpeckleCacheSingleton.GetRenderMaterialProxyListForObjects(idsAndSubElementIds);
_rootObject[ProxyKeys.RENDER_MATERIAL] = materialProxies;
// NOTE: these are currently not used anywhere, so we could even skip them (?).
_rootObject[ProxyKeys.PARAMETER_DEFINITIONS] = _parameterDefinitionHandler.Definitions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public void Load(SpeckleContainerBuilder builder)
builder.AddScoped<IRootToHostConverter, RevitRootToHostConverter>();
builder.AddSingleton(new RevitContext());

builder.AddSingleton(new RevitMaterialCacheSingleton());
builder.AddSingleton(new RevitToHostCacheSingleton());
builder.AddSingleton(new RevitToSpeckleCacheSingleton());

// POC: do we need ToSpeckleScalingService as is, do we need to interface it out?
builder.AddScoped<ScalingServiceToSpeckle>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace Speckle.Converters.RevitShared.Helpers;

public class RevitToHostCacheSingleton
{
/// <summary>
/// POC: Not sure is there a way to create it on "RevitHostObjectBuilder" with a scope instead singleton. For now we fill this dictionary and clear it on "RevitHostObjectBuilder".
/// Map extracted by revit material baker to be able to use it in converter.
/// This is needed because we cannot set materials for meshes in connector.
/// They needed to be set while creating "TessellatedFace".
/// </summary>
public Dictionary<string, DB.ElementId> MaterialsByObjectId { get; } = new();
}
Original file line number Diff line number Diff line change
@@ -1,37 +1,34 @@
using Speckle.Objects.Other;
using Speckle.Objects.Other;
using Speckle.Objects.Other.Revit;

namespace Speckle.Converters.RevitShared.Helpers;

/// <summary>
/// <para>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.</para>
/// <para>
/// 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.
/// Lifetime of this singleton should be per document.
/// </para>
/// <para>
/// 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.
/// </para>
/// TODO: this dude needs to be split into single responsability (render materials and material quantities), and removed from the context - as it's not needed for it to be there. It can be DI'ed as appropriate (see ParameterDefinitionHandler)
/// </summary>
public class RevitMaterialCacheSingleton
public class RevitToSpeckleCacheSingleton
{
/// <summary>
/// map(object id, ( map (materialId, proxy) ) )
/// a per object map of material proxies. not the best way???
/// (DB.Material id, RevitMaterial). This can be generated from converting render materials or material quantities.
/// </summary>
public Dictionary<string, Dictionary<string, RenderMaterialProxy>> ObjectRenderMaterialProxiesMap { get; } = new();
public Dictionary<string, RevitMaterial> RevitMaterialCache { get; } = new();

/// <summary>
/// POC: The map we mutate PER RECEIVE operation, this smells a LOT! Once we have better conversion context stack that we can manage our data between connector - converter, this property must go away!
/// (DB.Material id, RenderMaterial). This can be generated from converting render materials or material quantities.
/// </summary>
public Dictionary<string, DB.ElementId> ObjectIdAndMaterialIndexMap { get; } = new();
public Dictionary<string, RenderMaterial> SpeckleRenderMaterialCache { get; } = new();

/// <summary>
/// map (DB.Material id, RevitMaterial). This can be generated from converting render materials or material quantities.
/// </summary>
public Dictionary<string, RevitMaterial> ConvertedRevitMaterialMap { get; } = new();

/// <summary>
/// map (DB.Material id, RenderMaterial). This can be generated from converting render materials or material quantities.
/// map(object id, ( map (materialId, proxy) ) )
/// a per object map of material proxies. not the best way???
/// </summary>
public Dictionary<string, RenderMaterial> ConvertedRenderMaterialMap { get; } = new();
public Dictionary<string, Dictionary<string, RenderMaterialProxy>> ObjectRenderMaterialProxiesMap { get; } = new();

/// <summary>
/// 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ public static IServiceCollection AddRevitConverters(this IServiceCollection serv
serviceCollection.AddScoped<IRootToHostConverter, RevitRootToHostConverter>();
serviceCollection.AddSingleton(new RevitContext());

serviceCollection.AddSingleton(new RevitMaterialCacheSingleton());
serviceCollection.AddSingleton(new RevitToHostCacheSingleton());
serviceCollection.AddSingleton(new RevitToSpeckleCacheSingleton());

// POC: do we need ToSpeckleScalingService as is, do we need to interface it out?
serviceCollection.AddScoped<ScalingServiceToSpeckle>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
<Compile Include="$(MSBuildThisFileDirectory)Helpers\IRevitCategories.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Helpers\ISlopeArrowExtractor.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Helpers\ParameterValueSetter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Helpers\RevitMaterialCacheSingleton.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Helpers\RevitToHostCacheSingleton.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Helpers\RevitToSpeckleCacheSingleton.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Helpers\SlopeArrowExtractor.cs" />
<Compile Include="$(MSBuildThisFileDirectory)IReferencePointConverter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Extensions\CategoryExtensions.cs" />
Expand Down Expand Up @@ -69,7 +70,8 @@
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\Geometry\PolylineToSpeckleConverter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\Geometry\VectorToSpeckleConverter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\Geometry\XyzConversionToPoint.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\MaterialConversionToSpeckle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\MaterialAsRevitMaterialConversionToSpeckle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\MaterialAsSpeckleMaterialConversionToSpeckle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\MaterialQuantitiesToSpeckle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\TopLevel\GridTopLevelConverterToSpeckle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\TopLevel\LevelTopLevelConverterToSpeckle.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ namespace Speckle.Converters.RevitShared.ToHost.TopLevel;
public class MeshConverterToHost : ITypedConverter<SOG.Mesh, List<DB.GeometryObject>>
{
private readonly ITypedConverter<SOG.Point, DB.XYZ> _pointConverter;
private readonly RevitMaterialCacheSingleton _revitMaterialCacheSingleton;
private readonly RevitToHostCacheSingleton _revitToHostCacheSingleton;

public MeshConverterToHost(
ITypedConverter<SOG.Point, XYZ> pointConverter,
RevitMaterialCacheSingleton revitMaterialCacheSingleton
RevitToHostCacheSingleton revitToHostCacheSingleton
)
{
_pointConverter = pointConverter;
_revitMaterialCacheSingleton = revitMaterialCacheSingleton;
_revitToHostCacheSingleton = revitToHostCacheSingleton;
}

public List<DB.GeometryObject> Convert(SOG.Mesh mesh)
Expand All @@ -39,10 +39,7 @@ RevitMaterialCacheSingleton revitMaterialCacheSingleton
ElementId materialId = ElementId.InvalidElementId;

if (
_revitMaterialCacheSingleton.ObjectIdAndMaterialIndexMap.TryGetValue(
mesh.applicationId ?? mesh.id,
out var mappedElementId
)
_revitToHostCacheSingleton.MaterialsByObjectId.TryGetValue(mesh.applicationId ?? mesh.id, out var mappedElementId)
)
{
materialId = mappedElementId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Speckle.Converters.RevitShared.Helpers;
using Speckle.Converters.RevitShared.Settings;
using Speckle.Objects.Other;
using Speckle.Objects.Other.Revit;
using Speckle.Sdk.Common;

namespace Speckle.Converters.RevitShared.ToSpeckle;
Expand All @@ -12,21 +11,21 @@ public class MeshByMaterialDictionaryToSpeckle
: ITypedConverter<(Dictionary<DB.ElementId, List<DB.Mesh>> target, DB.ElementId parentElementId), List<SOG.Mesh>>
{
private readonly IConverterSettingsStore<RevitConversionSettings> _converterSettings;
private readonly ITypedConverter<DB.Material, (RevitMaterial, RenderMaterial)> _materialConverter;
private readonly ITypedConverter<DB.Material, RenderMaterial> _speckleRenderMaterialConverter;
private readonly ITypedConverter<List<DB.Mesh>, SOG.Mesh> _meshListConverter;
private readonly RevitMaterialCacheSingleton _revitMaterialCacheSingleton;
private readonly RevitToSpeckleCacheSingleton _revitToSpeckleCacheSingleton;

public MeshByMaterialDictionaryToSpeckle(
ITypedConverter<DB.Material, (RevitMaterial, RenderMaterial)> materialConverter,
ITypedConverter<List<DB.Mesh>, SOG.Mesh> meshListConverter,
IConverterSettingsStore<RevitConversionSettings> converterSettings,
RevitMaterialCacheSingleton revitMaterialCacheSingleton
RevitToSpeckleCacheSingleton revitToSpeckleCacheSingleton,
ITypedConverter<DB.Material, RenderMaterial> speckleRenderMaterialConverter
)
{
_materialConverter = materialConverter;
_meshListConverter = meshListConverter;
_converterSettings = converterSettings;
_revitMaterialCacheSingleton = revitMaterialCacheSingleton;
_revitToSpeckleCacheSingleton = revitToSpeckleCacheSingleton;
_speckleRenderMaterialConverter = speckleRenderMaterialConverter;
}

/// <summary>
Expand All @@ -46,7 +45,7 @@ RevitMaterialCacheSingleton revitMaterialCacheSingleton
public List<SOG.Mesh> Convert((Dictionary<DB.ElementId, List<DB.Mesh>> target, DB.ElementId parentElementId) args)
{
var result = new List<SOG.Mesh>(args.target.Keys.Count);
var objectRenderMaterialProxiesMap = _revitMaterialCacheSingleton.ObjectRenderMaterialProxiesMap;
var objectRenderMaterialProxiesMap = _revitToSpeckleCacheSingleton.ObjectRenderMaterialProxiesMap;

var materialProxyMap = new Dictionary<string, RenderMaterialProxy>();
objectRenderMaterialProxiesMap[args.parentElementId.ToString().NotNull()] = materialProxyMap;
Expand All @@ -69,7 +68,7 @@ RevitMaterialCacheSingleton revitMaterialCacheSingleton
// get the render material if any
if (_converterSettings.Current.Document.GetElement(materialId) is DB.Material material)
{
(RevitMaterial _, RenderMaterial convertedRenderMaterial) = _materialConverter.Convert(material);
RenderMaterial convertedRenderMaterial = _speckleRenderMaterialConverter.Convert(material);

if (!materialProxyMap.TryGetValue(materialIdString, out RenderMaterialProxy? renderMaterialProxy))
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using Speckle.Converters.Common.Objects;
using Speckle.Converters.RevitShared.Helpers;
using Speckle.Objects.Other.Revit;

namespace Speckle.Converters.RevitShared.ToSpeckle;

public class MaterialAsRevitMaterialConversionToSpeckle : ITypedConverter<DB.Material, RevitMaterial>
{
private readonly RevitToSpeckleCacheSingleton _revitToSpeckleCacheSingleton;

public MaterialAsRevitMaterialConversionToSpeckle(RevitToSpeckleCacheSingleton revitToSpeckleCacheSingleton)
{
_revitToSpeckleCacheSingleton = revitToSpeckleCacheSingleton;
}

public RevitMaterial Convert(DB.Material target)
{
RevitMaterial material;
if (
_revitToSpeckleCacheSingleton.RevitMaterialCache.TryGetValue(target.UniqueId, out RevitMaterial? cachedMaterial)
)
{
material = cachedMaterial;
}
else
{
material = ConvertToRevitMaterial(target);
_revitToSpeckleCacheSingleton.RevitMaterialCache.Add(target.UniqueId, material);
}

return material;
}

private RevitMaterial ConvertToRevitMaterial(DB.Material target)
{
// POC: we need to validate these properties on the RevitMaterial class, ie are they used?
RevitMaterial material =
new(
target.Name,
target.MaterialCategory,
target.MaterialClass,
target.Shininess,
target.Smoothness,
target.Transparency
)
{
applicationId = target.UniqueId
};

// POC: I'm removing material parameter assigning here as we're exploding a bit the whole space with too many parameters.
// We can bring this back if needed/requested by our end users.
// _parameterObjectAssigner.AssignParametersToBase(target, material);
return material;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
using Speckle.Converters.Common.Objects;
using Speckle.Converters.RevitShared.Helpers;
using Speckle.Objects.Other;

namespace Speckle.Converters.RevitShared.ToSpeckle;

public class MaterialAsSpeckleMaterialConversionToSpeckle : ITypedConverter<DB.Material, RenderMaterial>
{
private readonly RevitToSpeckleCacheSingleton _revitToSpeckleCacheSingleton;

public MaterialAsSpeckleMaterialConversionToSpeckle(RevitToSpeckleCacheSingleton revitToSpeckleCacheSingleton)
{
_revitToSpeckleCacheSingleton = revitToSpeckleCacheSingleton;
}

public RenderMaterial Convert(DB.Material target)
{
RenderMaterial renderMaterial;
if (
_revitToSpeckleCacheSingleton.SpeckleRenderMaterialCache.TryGetValue(
target.UniqueId,
out RenderMaterial? cachedRenderMaterial
)
)
{
renderMaterial = cachedRenderMaterial;
}
else
{
renderMaterial = ConvertToRenderMaterial(target);
_revitToSpeckleCacheSingleton.SpeckleRenderMaterialCache.Add(target.UniqueId, renderMaterial);
}

return renderMaterial;
}

private RenderMaterial ConvertToRenderMaterial(DB.Material target)
{
RenderMaterial renderMaterial =
new()
{
name = target.Name,
opacity = 1 - target.Transparency / 100d,
diffuse = System.Drawing.Color.FromArgb(target.Color.Red, target.Color.Green, target.Color.Blue).ToArgb(),
applicationId = target.UniqueId
};

return renderMaterial;
}
}
Loading

0 comments on commit fa31283

Please sign in to comment.