-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
align extruded solids according to the PartBuilder #217
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev #217 +/- ##
==========================================
+ Coverage 83.65% 83.66% +0.01%
==========================================
Files 19 19
Lines 5464 5468 +4
==========================================
+ Hits 4571 4575 +4
Misses 893 893
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
a34bf3a
to
b980b24
Compare
b980b24
to
c1b47d3
Compare
In this case the repositioning makes sense but it quickly gets complex for the user to understand. Ironically, I recently removed the last of this repositioning logic from the operations. See the comment in the docs here: https://build123d.readthedocs.io/en/latest/operations.html
I guess I should expand this warning to workplanes as well. Here are a couple ways to do the same: import build123d as bd
with bd.BuildPart(bd.Plane.XZ) as bp:
with bd.BuildSketch(bd.Plane.XZ):
rect = bd.Rectangle(50, 50)
bd.extrude(amount=5)
with bd.BuildSketch(bd.Plane.XZ):
rect2 = bd.Rectangle(40, 40)
bd.extrude(amount=15)
algebra = bd.Plane.XZ * bd.extrude(bd.Rectangle(50, 50), 5) + bd.Plane.XZ * bd.extrude(
bd.Rectangle(40, 40), 15
) One extra line in the builder solution or do the whole thing in one line with algebra. |
Personally, i find it much more confusing to have an operation behave so differently when it is called with the explicit argument. How does it get complex with repositioning? and what were the reasons to remove the repositioning logic? |
Here was the last Issue on this: #215 The rule that operations are only a function of their parameters and not workplane or location contexts seems like the least complex for users to understand. I think the problem behind this specific issue is that the |
I currently believe that using an object outside of the "scope" it is created in should not be promoted as a typical use pattern. By "using" I mean applying an operation from the object (such as extrude). |
On the contrary, I am totally fine with 2D object being unaligned. What i would have expected:
context would consist of:
In this sense, i would expect the following to be valid: with bd.BuildPart(bd.Plane.XZ) as part:
# 3d-objects are being captured and oriented in the XZ plane in this context
circle = bd.Circle(2) # creates a 2D primitive -> not captured
bd.extrude(circle, amount=10) # extrudes the 2D primitive to a 3d Primitive, which is captured and aligned Lifting the limitations in I would even go further and say that contexts can be nested and there is no "inertial system", so that in nested context the inner orientation is relative to the outer one.
The cone in outer_part would point in the Y direction, while in inner_part it points towards the Z direction. |
Although these design decisions are valid, they aren't what I've decided to implement for build123d. Everyone has a different perspective of what makes sense to them; for example most users just expect to extrude along the Face normal even though that isn't really a very stable reference to decide on an extrusion direction (the normal can be flipped or even change across the Face if it's non-planar). The current rule that operations only depend on their inputs and not the workplane/locations around them is my attempt to make a simple rule that applies everywhere and most users seem to find that it makes sense. It seems as though you'd be more comfortable with the algebra api where you have full control of everything which is great, there is no "best" api. |
this addresses the following example:
before:
after:
with the change, calls to extrude align the solids according to the workplanes in the context