diff --git a/docs/assets/sketch_on_custom_plane.png b/docs/assets/sketch_on_custom_plane.png new file mode 100644 index 0000000..447ddac Binary files /dev/null and b/docs/assets/sketch_on_custom_plane.png differ diff --git a/docs/assets/vertical_sketch.png b/docs/assets/vertical_sketch.png new file mode 100644 index 0000000..e64aae0 Binary files /dev/null and b/docs/assets/vertical_sketch.png differ diff --git a/docs/build_sketch.rst b/docs/build_sketch.rst index 00c0324..803dcb7 100644 --- a/docs/build_sketch.rst +++ b/docs/build_sketch.rst @@ -36,6 +36,8 @@ and ``PRIVATE``). .. image:: assets/circle_with_hole.svg :align: center +.. _sketching_on_other_planes: + ************************* Sketching on other Planes ************************* diff --git a/docs/tips.rst b/docs/tips.rst index 28430e6..8cbe506 100644 --- a/docs/tips.rst +++ b/docs/tips.rst @@ -192,4 +192,83 @@ Writing build123d often involves live coding in a REPL or typing in editors with the rest of the CAD GUI taking up screen space. Scripts are usually centred around build123d usage, with usage of other libraries being limited enough that naming conflicts are easily avoided. In this context, it’s entirely reasonable to prioritise developer ergonomics over “correctness” by making build123d’s primitives -available in the global namespace. \ No newline at end of file +available in the global namespace. + +*************************************** +Why doesn't BuildSketch(Plane.XZ) work? +*************************************** + +When creating a sketch not on the default ``Plane.XY`` users may expect that they are drawing directly +on the workplane / coordinate system provided. For example: + +.. code-block:: python + + with BuildSketch(Plane.XZ) as vertical_sketch: + Rectangle(1, 1) + with Locations(vertices().group_by(Axis.X)[-1].sort_by(Axis.Z)[-1]): + Circle(0.2) + +.. image:: assets/vertical_sketch.png + +In this case the circle is not positioned in the top right as one would expect; in-fact, the position +of the circle randomly switches between the bottom and top corner. + +This is because all sketches are created on a local ``Plane.XY`` independent of where they will be +ultimately placed; therefore, the ``sort_by(Axis.Z)`` is sorting two points that have a Z value of +zero as they are located on ``Plane.XY`` and effectively return a random point. + +Why does ``BuildSketch`` work this way? Consider an example where the user wants to work on a +plane not aligned with any Axis, as follows (this is often done when creating a sketch on a ``Face`` +of a 3D part but is simulated here by rotating a ``Plane``): + +.. code-block:: python + + with BuildSketch(Plane.YZ.rotated((123, 45, 6))) as custom_plane: + Rectangle(1, 1, align=Align.MIN) + with Locations(vertices().group_by(Axis.X)[-1].sort_by(Axis.Y)[-1]): + Circle(0.2) + +.. image:: assets/sketch_on_custom_plane.png + +Here one can see both ``sketch_local`` (with the light fill on ``Plane.XY``) and the ``sketch`` +(with the darker fill) placed on the user provided workplane. As the selectors work off global +coordinates, selection of the "top right" of this sketch would be quite challenging and would +likely change if the sketch was ever moved as what could happen if the 3D part changed. For an +example of sketching on a 3D part, see :ref:`sketching_on_other_planes`. + +************************************************************************* +Why is BuildLine not working as expected within the scope of BuildSketch? +************************************************************************* + +As described above, all sketching is done on a local ``Plane.XY``; however, the following +is a common issue: + +.. code-block:: python + + with BuildSketch() as sketch: + with BuildLine(Plane.XZ): + Polyline(...) + make_face() + +Here ``BuildLine`` is within the scope of ``BuildSketch``; therefore, all of the +drawing should be done on ``Plane.XY``; however, the user has specified ``Plane.XZ`` +when creating the ``BuildLine`` instance. Although this isn't absolutely incorrect +it's almost certainly not what the user intended. Here the face created by ``make_face`` will +be reoriented to ``Plane.XY`` as all sketching must be done on that plane. This reorienting +of objects to ``Plane.XY`` allows a user to ``add`` content from other sources to the +sketch without having to manually re-orient the object. + +Unless there is a good reason and the user understands how the ``BuildLine`` object will be +reoriented, all ``BuildLine`` instances within the scope of ``BuildSketch`` should be done +on the default ``Plane.XY``. + +*************************************************************** +Don't Builders inherent workplane/coordinate sytems when nested +*************************************************************** + +Some users expect that nested Builders will inherent the workplane or coordinate system from +their parent Builder - this is not true. When a Builder is instantiated, a workplane is either +provided by the user or it defaults to ``Plane.XY``. Having Builders inherent coordinate systems +from their parents could result in confusion when they are nested as well as change their +behaviour depending on which scope they are in. Inherenting coordinate systems isn't necessarily +incorrect, it was considered for build123d but ultimately the simple static approach was taken. \ No newline at end of file diff --git a/docs/tttt.rst b/docs/tttt.rst index 6a542b4..32c7e68 100644 --- a/docs/tttt.rst +++ b/docs/tttt.rst @@ -1,6 +1,6 @@ -####################### +############################# Too Tall Toby (TTT) Tutorials -####################### +############################# .. image:: assets/ttt.png :align: center diff --git a/src/build123d/operations_generic.py b/src/build123d/operations_generic.py index 8e3637a..5d00e21 100644 --- a/src/build123d/operations_generic.py +++ b/src/build123d/operations_generic.py @@ -532,32 +532,29 @@ def offset( ) -> Union[Curve, Sketch, Part, Compound]: """Generic Operation: offset - Applies to 1, 2, and 3 dimensional objects. + Applies to 1, 2, and 3 dimensional objects. - Offset the given sequence of Edges, Faces, Compound of Faces, or Solids. - The kind parameter controls the shape of the transitions. For Solid - objects, the openings parameter allows selected faces to be open, like - a hollow box with no lid. + Offset the given sequence of Edges, Faces, Compound of Faces, or Solids. + The kind parameter controls the shape of the transitions. For Solid + objects, the openings parameter allows selected faces to be open, like + a hollow box with no lid. - Args: - objects (Union[Edge, Face, Solid, Compound] or Iterable of): objects to offset - amount (float): positive values external, negative internal - openings (list[Face], optional), sequence of faces to open in part. - Defaults to None. - kind (Kind, optional): transition shape. Defaults to Kind.ARC. - side (Side, optional): side to place offset. Defaults to Side.BOTH. - closed (bool, optional): if Side!=BOTH, close the LEFT or RIGHT - offset. Defaults to True. - <<<<<<< Updated upstream - min_edge_length (float, optional): repair degenerate edges generated by offset - by eliminating edges of minimum length in offset wire. Defaults to None. - ======= - >>>>>>> Stashed changes - mode (Mode, optional): combination mode. Defaults to Mode.REPLACE. + Args: + objects (Union[Edge, Face, Solid, Compound] or Iterable of): objects to offset + amount (float): positive values external, negative internal + openings (list[Face], optional), sequence of faces to open in part. + Defaults to None. + kind (Kind, optional): transition shape. Defaults to Kind.ARC. + side (Side, optional): side to place offset. Defaults to Side.BOTH. + closed (bool, optional): if Side!=BOTH, close the LEFT or RIGHT + offset. Defaults to True. + min_edge_length (float, optional): repair degenerate edges generated by offset + by eliminating edges of minimum length in offset wire. Defaults to None. + mode (Mode, optional): combination mode. Defaults to Mode.REPLACE. - Raises: - ValueError: missing objects - ValueError: Invalid object type + Raises: + ValueError: missing objects + ValueError: Invalid object type """ context: Builder = Builder._get_context("offset") diff --git a/src/build123d/topology.py b/src/build123d/topology.py index 0044688..736f01e 100644 --- a/src/build123d/topology.py +++ b/src/build123d/topology.py @@ -5603,7 +5603,9 @@ class Face(Shape): chamfer_builder = BRepFilletAPI_MakeFillet2d(self.wrapped) vertex_edge_map = TopTools_IndexedDataMapOfShapeListOfShape() - TopExp.MapShapesAndAncestors_s(self.wrapped, ta.TopAbs_VERTEX, ta.TopAbs_EDGE, vertex_edge_map) + TopExp.MapShapesAndAncestors_s( + self.wrapped, ta.TopAbs_VERTEX, ta.TopAbs_EDGE, vertex_edge_map + ) for v in vertices: edges = vertex_edge_map.FindFromKey(v.wrapped) @@ -5623,7 +5625,7 @@ class Face(Shape): edge2 = [x for x in edges if x != reference_edge][0] else: edge1, edge2 = edges - + chamfer_builder.AddChamfer( TopoDS.Edge_s(edge1.wrapped), TopoDS.Edge_s(edge2.wrapped),