Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tekla): openings as curves #440

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
using System.Reflection;
using Microsoft.Extensions.DependencyInjection;
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Converters.Common.Registration;
using Speckle.Converters.TeklaShared.ToSpeckle.Helpers;
using Speckle.Converters.TeklaShared.ToSpeckle.Raw;
using Speckle.Converters.TeklaShared.ToSpeckle.TopLevel;
using Speckle.Sdk;
using Speckle.Sdk.Models;
using Tekla.Structures.Datatype;

namespace Speckle.Converters.TeklaShared;
Expand All @@ -29,6 +32,8 @@ public static IServiceCollection AddTeklaConverters(this IServiceCollection serv
ConverterSettingsStore<TeklaConversionSettings>
>();

serviceCollection.AddTransient<ITypedConverter<TSM.BooleanPart, IEnumerable<Base>>, OpeningToSpeckleConverter>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK you do not need to register ITypedConverters explicitly. Some magic (serviceCollection.AddMatchingInterfacesAsTransient(converterAssembly)) can handle them. Did you have a problem before with that?


serviceCollection.AddMatchingInterfacesAsTransient(converterAssembly);

return serviceCollection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ public sealed class DisplayValueExtractor
private readonly IConverterSettingsStore<TeklaConversionSettings> _settingsStore;
private readonly ITypedConverter<TG.Arc, SOG.Arc> _arcConverter;
private readonly ITypedConverter<TSM.Grid, IEnumerable<Base>> _gridConverter;
private readonly ITypedConverter<TSM.BooleanPart, IEnumerable<Base>> _openingConverter;

public DisplayValueExtractor(
ITypedConverter<TSM.Solid, SOG.Mesh> meshConverter,
ITypedConverter<TG.Point, SOG.Point> pointConverter,
ITypedConverter<TG.LineSegment, SOG.Line> lineConverter,
ITypedConverter<TG.Arc, SOG.Arc> arcConverter,
ITypedConverter<TSM.Grid, IEnumerable<Base>> gridConverter,
ITypedConverter<TSM.BooleanPart, IEnumerable<Base>> openingConverter,
IConverterSettingsStore<TeklaConversionSettings> settingsStore
)
{
Expand All @@ -30,6 +32,7 @@ IConverterSettingsStore<TeklaConversionSettings> settingsStore
_lineConverter = lineConverter;
_arcConverter = arcConverter;
_gridConverter = gridConverter;
_openingConverter = openingConverter;
}

public IEnumerable<Base> GetDisplayValue(TSM.ModelObject modelObject)
Expand Down Expand Up @@ -97,6 +100,14 @@ public IEnumerable<Base> GetDisplayValue(TSM.ModelObject modelObject)

break;

case TSM.BooleanPart booleanPart:
var openingConverter = _openingConverter.Convert(booleanPart);
foreach (var line in openingConverter)
{
yield return line;
}
break;

default:
yield break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regardles we can log here instead swallowing. pre-existing problem

}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Models;

namespace Speckle.Converters.TeklaShared.ToSpeckle.Raw;

[NameAndRankValue(nameof(TSM.BooleanPart), NameAndRankValueAttribute.SPECKLE_DEFAULT_RANK)]
public class OpeningToSpeckleConverter : ITypedConverter<TSM.BooleanPart, IEnumerable<Base>>
{
private readonly IConverterSettingsStore<TeklaConversionSettings> _settingsStore;
private readonly ITypedConverter<TG.LineSegment, SOG.Line> _lineConverter;

public OpeningToSpeckleConverter(
IConverterSettingsStore<TeklaConversionSettings> settingsStore,
ITypedConverter<TG.LineSegment, SOG.Line> lineConverter
)
{
_settingsStore = settingsStore;
_lineConverter = lineConverter;
}

private (double minZ, double maxZ) GetParentZBounds(TSM.ModelObject parent)
{
TSM.Solid? solid = null;

// detect if the solid is a part or cut
if (parent is TSM.Part part)
{
solid = part.GetSolid();
}
else if (parent is TSM.BooleanPart boolPart)
{
solid = boolPart.OperativePart?.GetSolid();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have else case somehow. smells a bit


if (solid == null)
{
return (0, 0);
}

double minZ = double.MaxValue;
double maxZ = double.MinValue;

var faceEnum = solid.GetFaceEnumerator();
while (faceEnum.MoveNext())
{
var face = faceEnum.Current;
if (face == null)
{
continue;
}

var loopEnum = face.GetLoopEnumerator();
while (loopEnum.MoveNext())
{
var loop = loopEnum.Current;
if (loop == null)
{
continue;
}

var vertexEnum = loop.GetVertexEnumerator();
while (vertexEnum.MoveNext())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG. 3 nested while loop, how crazy we are ( ͡°( ͡° ͜ʖ( ͡° ͜ʖ ͡°)ʖ ͡°) ͡°)
at least this converter deserves inline comments/notes. hard to understand from first look. also wanna know why we need that while loops, seems like API thing but some explanation around structure would be good

{
var vertex = vertexEnum.Current;
if (vertex == null)
{
continue;
}

minZ = Math.Min(minZ, vertex.Z);
maxZ = Math.Max(maxZ, vertex.Z);
}
}
}

return (minZ, maxZ);
}

private TG.Point ClampPointToZBounds(TG.Point point, double minZ, double maxZ)
{
return new TG.Point(point.X, point.Y, Math.Max(minZ, Math.Min(maxZ, point.Z)));
}

public IEnumerable<Base> Convert(TSM.BooleanPart target)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would put public methods above always, but it is QoL...

{
// skip if this is not a cut operation
// since there are some boolean parts which is not a cut (merge operation for example)
if (target.Type != TSM.BooleanPart.BooleanTypeEnum.BOOLEAN_CUT)
{
yield break;
}

var operativePart = target.OperativePart;
var fatherObject = target.Father;
if (operativePart == null || fatherObject == null)
{
yield break;
}

// when the cut operation is a part cut, it was adding lines for whole part
// therefore we're restricting the Z coordinate to the plane
// get the Z bounds of the parent object
var (minZ, maxZ) = GetParentZBounds(fatherObject);

// Ggt the solid of the operative part
var solid = operativePart.GetSolid();
if (solid == null)
{
yield break;
}

// get the face enumerator to traverse the faces of the solid
var faceEnum = solid.GetFaceEnumerator();
while (faceEnum.MoveNext())
{
var face = faceEnum.Current;
if (face == null)
{
continue;
}

var loopEnum = face.GetLoopEnumerator();
while (loopEnum.MoveNext())
{
var loop = loopEnum.Current;
if (loop == null)
{
continue;
}

var vertices = new List<TG.Point>();
var vertexEnum = loop.GetVertexEnumerator();
while (vertexEnum.MoveNext())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am hesitant with these dudes, but happy to do live PR review together with you on this

{
if (vertexEnum.Current != null)
{
// restrict the Z value within parent bounds
vertices.Add(ClampPointToZBounds(vertexEnum.Current, minZ, maxZ));
}
}

for (int i = 0; i < vertices.Count; i++)
{
var startPoint = vertices[i];
var endPoint = vertices[(i + 1) % vertices.Count];

var lineSegment = new TG.LineSegment(startPoint, endPoint);
var speckleLine = _lineConverter.Convert(lineSegment);
speckleLine["type"] = "Opening";

yield return speckleLine;
}
}
}
}
}
Loading