gumyr / build123d

A python CAD programming library
Apache License 2.0
505 stars 85 forks source link

Fix calculation of projection direction in project() #587

Open Windfisch opened 6 months ago

Windfisch commented 6 months ago

This fixes the fix in 1d3de5ca693a345adef5affa320b8c877e79b5f9, which contained the following diff.

The diff changed >0 to <0, which is indeed fixing a problem with the code. However, it also changed to_local_coords to from_local_coords, which is wrong and breaks the code, e.g. when projecting against the XY plane. In this case, some objects are extended away from the projection target, leading to an empty intersection and thus, a crash.

diff --git a/src/build123d/operations_generic.py b/src/build123d/operations_generic.py
index 4589742..6600153 100644
--- a/src/build123d/operations_generic.py
+++ b/src/build123d/operations_generic.py
@@ -766,15 +766,13 @@ def project(
     projected_shapes = []
     obj: Shape
     for obj in face_list + line_list:
-        # obj_to_screen = (workplane.origin - obj.center()).normalized()
         obj_to_screen = (target.center() - obj.center()).normalized()
-        if workplane.to_local_coords(obj_to_screen).Z > 0:
+        if workplane.from_local_coords(obj_to_screen).Z < 0:
             projection_direction = -workplane.z_dir * projection_flip
         else:
             projection_direction = workplane.z_dir * projection_flip
@@ -792,7 +790,7 @@ def project(
     projected_points = []
     for pnt in point_list:
         pnt_to_target = (workplane.origin - pnt).normalized()
-        if workplane.to_local_coords(pnt_to_target).Z > 0:
+        if workplane.from_local_coords(pnt_to_target).Z < 0:
             projection_axis = -Axis(pnt, workplane.z_dir * projection_flip)
         else:
             projection_axis = Axis(pnt, workplane.z_dir * projection_flip)
Windfisch commented 6 months ago

Minimal example where this fixes something that was previously broken:

from build123d import *
box = Location((0,25,0)) * Rot((10,10,10)) * Box(10,25,30)
box2d = project(box.faces(), Plane.XZ)

however, i've seen that this change here breaks another test which uses the builder api, oh boi...

@gumyr any ideas about that one? It strongly seems to me that to_local_coords is the right one, not from_ but hm

Windfisch commented 6 months ago

99 bugs in the code, track 1 down, patch one around, 182 little bugs in the code :/...

So I found and fixed another issue, which fixed that failing test, but now another test is broken...

this time it's

    def test_project_to_sketch2(self):
        with BuildPart() as test2:
            Box(4, 4, 1)
            with BuildSketch(Plane.XY.offset(0.1)) as c:
                Rectangle(1, 1)
            project()
            extrude(amount=1)
            self.assertAlmostEqual(test2.part.volume, 4 * 4 * 1 + 1 * 1 * 1, 5)

and at this point, I'm not sure if the test expectation is correct. Frankly, I don't get it what's supposed to happen here:

Projecting from inside the Box feels ill-defined to me anyway...

I changed the test to project the Rect from outside the box (at height=2, not 0.1).