ricosjp / truck

Truck is a Rust CAD Kernel.
Apache License 2.0
1.02k stars 53 forks source link

Fix `rsweep` handling of partial negative angles. #62

Closed lynaghk closed 6 months ago

lynaghk commented 7 months ago

I believe the current rsweep code is incorrect when using negative angles less than a full rotation.

This example code:

let vertex = builder::vertex(Point3::new(1.0, 0.0, 0.0));
let circle = builder::rsweep(&vertex, Point3::origin(), Vector3::unit_z(), Rad(7.0));
let circle = translated(&circle, 2.0 * Vector3::unit_x());
let disk = builder::try_attach_plane(&vec![circle]).unwrap();
let solid = builder::rsweep(&disk, Point3::origin(), Vector3::unit_y(), -Rad(1.0));

render_to("foo.obj", solid);

yields a full torus even though the sweep angle is only -1 radian:

Screen Shot 2024-04-06 at 11 55 40 AM

This commit changes the conditional so that:

In both cases negative input angles are handled by inverting the rotation axis.

After this commit, the code above yields a non-inverted object:

Screen Shot 2024-04-06 at 11 55 11 AM

I wasn't sure the best way to add a test, but if you tell me how you'd like to do it I can add an additional commit to this PR. My best idea is to do the partial sweep with negative angle and check that the resulting solid's mesh is closed, but I don't know if meshing is perhaps too expensive to add in the test suite and another approach would be preferred.

CLAassistant commented 7 months ago

CLA assistant check
All committers have signed the CLA.

ytanimura commented 6 months ago

Thank you for your merge request and sorry for the late reply.

I am surprised, there is indeed a bug. Thank you for finding it! We will implement some tests and merge this request together.