Use non-optimal bounding box to determine size of Joint symbol
Currently, build123d uses the optimal bounding box for determining the size a Joint symbol.
Even for not so complex objects this can easily take 0.5 to 1 sec. If you have 5 joints on such an object, building the symbol in build123d easily takes several seconds. As an example, use https://github.com/bernhard-42/bd_animation/blob/main/examples/engine.py for the animation group crank
I'd say Shape.bounding_box needs to get the optimal=True keyword. This would keep the current behaviour, but allow in symbol ,to set optimal to False
Is this still a change you would like to see? Seems like a fairly simple one since Shape.bounding_box depends on BoundBox._from_topo_ds which does have the optimal keyword.
closing with https://github.com/gumyr/build123d/commit/ba348d5d985e5b23a698f99f1ab1150c6be13aa4
@bernhard-42 can you give it a try and let us know if this works as expected from a performance standpoint?
@jdegenstein Thanks for fixing the parameter. It is also needed to tell the symbol methods to use parameter False
diff --git a/src/build123d/joints.py b/src/build123d/joints.py
index bf2b8ca..0c21842 100644
--- a/src/build123d/joints.py
+++ b/src/build123d/joints.py
@@ -69,7 +69,7 @@ class RigidJoint(Joint):
@property
def symbol(self) -> Compound:
"""A CAD symbol (XYZ indicator) as bound to part"""
- size = self.parent.bounding_box().diagonal / 12
+ size = self.parent.bounding_box(optimal=False).diagonal / 12
return Compound.make_triad(axes_scale=size).locate(self.location)
def __init__(
@@ -228,7 +228,7 @@ class RevoluteJoint(Joint):
@property
def symbol(self) -> Compound:
"""A CAD symbol representing the axis of rotation as bound to part"""
- radius = self.parent.bounding_box().diagonal / 30
+ radius = self.parent.bounding_box(optimal=False).diagonal / 30
return Compound(
[
@@ -652,7 +652,7 @@ class BallJoint(Joint):
@property
def symbol(self) -> Compound:
"""A CAD symbol representing joint as bound to part"""
- radius = self.parent.bounding_box().diagonal / 30
+ radius = self.parent.bounding_box(optimal=False).diagonal / 30
circle_x = Edge.make_circle(radius, self.angle_reference)
circle_y = Edge.make_circle(radius, self.angle_reference.rotated((90, 0, 0)))
circle_z = Edge.make_circle(radius, self.angle_reference.rotated((0, 90, 0)))
Without, the symbol still uses the default for optimal which is True
@bernhard-42 Sorry about closing prematurely, I will open another PR to add these changes too
no worries and thank you!
@bernhard-42 I just made the changes you provided above, can you confirm if they are working in the latest dev?
I am away from any keyboard for the next 2 weeks. I can earliest test that end of Sept / beginning of Oct.
I did a quick test of the optimal keyword, and I am seeing results that are actually slower for the optimal=False case. I think we should discuss this further as I am wondering what scenarios optimal=False should be faster.
Not a rush, but just wanted to share what I observed.