Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix calculation of projection direction in project() #587

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

Windfisch
Copy link

This fixes the fix in 1d3de5c, 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
Copy link
Author

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
Copy link
Author

Windfisch commented Mar 12, 2024

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:

  • project() correctly projects the Rect within the Box onto its outside: image
  • however, the pink projection at the box's bottom is pointing inward to the box.
  • so the extrude(1) won't do anything visible.

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant