diff --git a/CHANGELOG.md b/CHANGELOG.md index 22541dfa7..ffc80fd0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,9 +58,10 @@ - `IndexedPolycurve.GetSubdivisionParameters` now works correctly with `startSetbackDistance` and `endSetbackDistance` parameters. - `Polyline.Frames` now works correctly with `startSetbackDistance` and `endSetbackDistance` parameters. - `Polygon.Frames` now works correctly with `startSetbackDistance` and `endSetbackDistance` parameters. +- `Polyline.TransformAt` returns correct transformations when parameter on domain is provided. +- `IndexedPolycurve` constructor that takes list of `BoundedCurve` now produces `CurveIndices` that share vertices and are withing index range. This means `IndexedPolyline.TransformedPolyline` preserves `CurveIndicies` on new `IndexedPolyline`. - `BoundedCurve.ToPolyline` now works correctly for `EllipticalArc` class. - ### Changed - `GltfExtensions.UseReferencedContentExtension` is now true by default. - `GeometricElement.Intersects` method now supports multiple representations. diff --git a/Elements/src/Geometry/IndexedPolycurve.cs b/Elements/src/Geometry/IndexedPolycurve.cs index 0af0a9603..920afad2a 100644 --- a/Elements/src/Geometry/IndexedPolycurve.cs +++ b/Elements/src/Geometry/IndexedPolycurve.cs @@ -205,7 +205,7 @@ private static (List>, List) CreateVerticesAndCurveIndices(I vertices.Add(curve.Start); last = curve.End; curveIndices.Add(new[] { index, index + 1 }); - index += 2; + index += 1; } else if (curve is Arc) { @@ -213,7 +213,7 @@ private static (List>, List) CreateVerticesAndCurveIndices(I vertices.Add(curve.Mid()); last = curve.End; curveIndices.Add(new[] { index, index + 1, index + 2 }); - index += 3; + index += 2; } else { diff --git a/Elements/src/Geometry/Polyline.cs b/Elements/src/Geometry/Polyline.cs index 0af338570..b7922cdba 100644 --- a/Elements/src/Geometry/Polyline.cs +++ b/Elements/src/Geometry/Polyline.cs @@ -96,9 +96,7 @@ public override Transform TransformAt(double u) throw new Exception($"The parameter {u} is not on the trimmed portion of the basis curve. The parameter must be between {Domain.Min} and {Domain.Max}."); } - var segmentIndex = 0; - var o = PointAtInternal(u, out segmentIndex); - Vector3 x = Vector3.XAxis; // Vector3: Convert to XAxis + var o = PointAtInternal(u, out var segmentIndex); // Check if the provided parameter is equal // to one of the vertices. @@ -119,7 +117,7 @@ public override Transform TransformAt(double u) { var idx = this.Vertices.IndexOf(a); - if (idx == 0 || idx == this.Vertices.Count - 1) + if (!(this is Polygon) && (idx == 0 || idx == this.Vertices.Count - 1)) { return CreateOrthogonalTransform(idx, a, normals[idx]); } @@ -129,26 +127,14 @@ public override Transform TransformAt(double u) } } - var d = this.Length() * u; - var totalLength = 0.0; - var segments = Segments(); - var normal = new Vector3(); - for (var i = 0; i < segments.Length; i++) - { - var s = segments[i]; - var currLength = s.Length(); - if (totalLength <= d && totalLength + currLength >= d) - { - var parameterOnSegment = d - totalLength; - o = s.PointAt(parameterOnSegment); - var previousNormal = normals[i]; - var nextNormal = normals[(i + 1) % this.Vertices.Count]; - normal = ((nextNormal - previousNormal) * parameterOnSegment + previousNormal).Unitized(); - x = s.Direction().Cross(normal); - break; - } - totalLength += currLength; - } + var nextIndex = (segmentIndex + 1) % this.Vertices.Count; + var segment = new Line(Vertices[segmentIndex], Vertices[nextIndex]); + var parameterOnSegment = u - segmentIndex; + var previousNormal = normals[segmentIndex]; + var nextNormal = normals[nextIndex]; + var normal = ((nextNormal - previousNormal) * parameterOnSegment + previousNormal).Unitized(); + var x = segment.Direction().Cross(normal); + return new Transform(o, x, normal, x.Cross(normal)); } diff --git a/Elements/src/Model.cs b/Elements/src/Model.cs index 013a41f6e..f0a6c689a 100644 --- a/Elements/src/Model.cs +++ b/Elements/src/Model.cs @@ -37,9 +37,8 @@ public class Model /// /// Collection of subelements from shared objects or RepresentationInstances (e.g. SolidRepresentation.Profile or RepresentationInstance.Material). - /// /// We do not serialize shared objects to json, but we do include them in other formats like gltf. - /// This collection contains all elements referenced directly by RepresentationInstances, such as Materials and Profiles. + /// This collection contains all elements referenced directly by RepresentationInstances, such as Materials and Profiles. /// These objects affect representation appearance and may be used at glTF creation time. /// [JsonIgnore] @@ -111,14 +110,7 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda return; } - // Some elements compute profiles and transforms - // during UpdateRepresentation. Call UpdateRepresentation - // here to ensure these values are correct in the JSON. - - // TODO: This is really expensive. This should be removed - // when all internal types have been updated to not create elements - // during UpdateRepresentation. This is now possible because - // geometry operations are reactive to changes in their properties. + // Function wrapper code no longer calls UpdateRepresentations, so we need to do it here. if (updateElementRepresentations && element is GeometricElement geo) { geo.UpdateRepresentations(); @@ -136,6 +128,12 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda { if (!this.Elements.ContainsKey(e.Id)) { + // Because function wrapper code doesn't call UpdateRepresentations any more + // we need to call it here for all nested elements while they are added. + if (updateElementRepresentations && e is GeometricElement geoE) + { + geoE.UpdateRepresentations(); + } this.Elements.Add(e.Id, e); } } @@ -272,8 +270,10 @@ public void ToJson(MemoryStream stream, bool indent = false, bool gatherSubEleme /// public string ToJson() { - // The arguments here are meant to match the default arguments of the ToJson(bool, bool) method above. - return ToJson(false, true); + // By default we don't want to update representations because the UpdateRepresentation + // method is called during function adding. Setting this to false makes the behavior + // match our function wrapping code behavior. + return ToJson(false, true, false); } /// diff --git a/Elements/test/IndexedPolyCurveTests.cs b/Elements/test/IndexedPolyCurveTests.cs index 2ddf1a349..f952bb078 100644 --- a/Elements/test/IndexedPolyCurveTests.cs +++ b/Elements/test/IndexedPolyCurveTests.cs @@ -187,6 +187,24 @@ public void Intersects() Assert.Contains(new Vector3(2.5, 7.5), results); } + [Fact] + public void PreservesIndicesTransformed() + { + var pc = CreateTestPolycurve(); + var indices = pc.CurveIndices; + var copy = pc.TransformedPolycurve(new Transform(10, 0, 0)); + var newIndicies = copy.CurveIndices; + Assert.Equal(indices.Count, newIndicies.Count); + for (int i = 0; i < indices.Count; i++) + { + Assert.Equal(indices[i].Count, newIndicies[i].Count); + for (int j = 0; j < indices[i].Count; j++) + { + Assert.Equal(indices[i][j], newIndicies[i][j]); + } + } + } + [Fact] public void GetSubdivisionParameters() { diff --git a/Elements/test/PolylineTests.cs b/Elements/test/PolylineTests.cs index 645c406f1..010221e7e 100644 --- a/Elements/test/PolylineTests.cs +++ b/Elements/test/PolylineTests.cs @@ -775,6 +775,36 @@ public void PolylineForceAngleComplianceWithZeroAngle() Assert.True(CheckPolylineAngles(angles, normalizedPathEndYAxisReferenceVector)); } + [Fact] + public void PolylineTransformAt() + { + var polyline = new Polyline((0, 0), (5, 0), (5, 10), (-5, 10)); + + var transform = polyline.TransformAt(0); + Assert.Equal((0, 0), transform.Origin); + Assert.Equal(Vector3.XAxis.Negate(), transform.ZAxis); + + transform = polyline.TransformAt(0.5); + Assert.Equal((2.5, 0), transform.Origin); + Assert.Equal(Vector3.XAxis.Negate(), transform.ZAxis); + + transform = polyline.TransformAt(1); + Assert.Equal((5, 0), transform.Origin); + Assert.Equal((Vector3.XAxis + Vector3.YAxis).Unitized().Negate(), transform.ZAxis); + + transform = polyline.TransformAt(1.25); + Assert.Equal((5, 2.5), transform.Origin); + Assert.Equal(Vector3.YAxis.Negate(), transform.ZAxis); + + transform = polyline.TransformAt(2); + Assert.Equal((5, 10), transform.Origin); + Assert.Equal((Vector3.YAxis - Vector3.XAxis).Unitized().Negate(), transform.ZAxis); + + transform = polyline.TransformAt(2.75); + Assert.Equal((-2.5, 10), transform.Origin); + Assert.Equal(Vector3.XAxis, transform.ZAxis); + } + [Fact] public void Frames() {