hypar-io / Elements

The smallest useful BIM.
https://www.hypar.io
MIT License
349 stars 74 forks source link

Arc: fix interactions negative domain in GetSubdivisionParameters, Complement and ToPolyline. #998

Closed DmytroMuravskyi closed 1 year ago

DmytroMuravskyi commented 1 year ago

BACKGROUND:

DESCRIPTION:

TESTING:

FUTURE WORK:

REQUIRED:


This change is Reviewable

andrewheumann commented 1 year ago

I think this solution is the wrong approach — it leave's a curve's tangent vector in disagreement with its parameter space.

var arc1 = new Arc(transform, 2, 0, Math.PI / 2.0);
var arc2 = new Arc(transform, 2, Math.PI / 2.0, 0);
Assert.Equal(arc1.PointAtNormalized(0.2), arc2.PointAtNormalized(0.8));
Assert.True(arc1.TransformAtNormalized(0.2).ZAxis.Dot(arc2.TransformAtNormalized(0.8).ZAxis) < -0.9999);

This will fail with this change.

I have implemented the earlier proposed approach here, which satisfies this expectation. https://github.com/hypar-io/Elements/pull/999

DmytroMuravskyi commented 1 year ago

Few details I noticed about your implementation that might be by design or already known:

wynged commented 1 year ago

@andrewheumann want to make sure you see these comments from Dmytro above.