search

gumyr / build123d

A python CAD programming library
Apache License 2.0
382 stars 72 forks source link

Fix JernArc not always co-planar with input plane #644

Closed jdegenstein closed 1 week ago

jdegenstein commented 1 week ago

This PR fixes https://github.com/gumyr/build123d/issues/635 in which I determined that the localization of the tangent vector was resulting in an incorrect tangent direction. Ultimately it was discovered that this problem went to a fairly low level within build123d and is related to how affine transformations in OCCT do not affect gp_Vec, gp_Pnt, and gp_Dir in the same ways. In particular a tangent direction is analogous to a gp_Dir that should not be affected by translation or scaling, and only by the rotations. The existing Vector.transform method was extended to include an affine transformation option for gp_Dir. JernArc was reworked to depend on this newly created functionality in Vector.transform.

I also added additional tests to cover the newly added features and more tests for JernArc itself.

I am looking for input on whether this change makes architectural sense and is not overly simplistic and so would lead to problems in the future.

gumyr commented 1 week ago

I find the vec_dir parameter confusing. How about naming it is_direction: Bool = False?

There are many ways to fix this like introducing a Direction(Vector) class or adding a Matrix.is_direction_transformation() method that would zero out the translation and scaling elements but this solves the immediate problem so let's go with this (with the naming change if you agree).

jdegenstein commented 1 week ago

Thanks @gumyr for your comments! I agree with the is_direction naming change and implemented it in this PR. I agree that if this problem keeps coming up then larger solutions like class Direction(Vector) might become the better approach.