specific case of Shape.split( ... keep != Keep.BOTH) raises ValueError: Null TopoDS_Shape object

[jdegenstein]

This works:

with BuildPart() as p:
    Cylinder(25, 50, align=(Align.CENTER, Align.CENTER, Align.MIN))
    with Locations(Plane.isometric.offset(-8)):
        with Locations(Rotation(0, 0, -20)):
            c = Cone(
                40,
                0,
                50,
                mode=Mode.PRIVATE,
                align=(Align.CENTER, Align.CENTER, Align.MIN),
            )

solid = p.part.solid()
conicalface = c.faces().filter_by(GeomType.CONE)[0]
result = solid.split(tool=conicalface, keep=Keep.BOTH)

If the last line is changed to one of the two following options it fails:

result = solid.split(tool=conicalface, keep=Keep.TOP)
result = solid.split(tool=conicalface, keep=Keep.BOTTOM)

I suspect the issue is related to determining the "top" but haven't investigated deeply yet.

[snoyer]

there's a call to tool.thicken(0.1) in split() and

build123d/src/build123d/topology.py

Lines 8916 to 8920 in c0728e0

	solid.MakeOffsetShape()
	try:
	result = Solid(solid.Shape())
	except StdFail_NotDone as err:
	raise RuntimeError("Error applying thicken to given Face") from err

can fail without raising the expected StdFail_NotDone error.

with the following patch, it will raise RuntimeError: Error applying thicken to given Face from thicken() rather than ValueError: Null TopoDS_Shape object from the Solid constructor

diff --git a/src/build123d/topology.py b/src/build123d/topology.py
index be15a1b..ce959e3 100644
--- a/src/build123d/topology.py
+++ b/src/build123d/topology.py
@@ -8914,6 +8914,8 @@ def _thicken(obj: TopoDS_Shape, depth: float):
         RemoveIntEdges=True,
     )
     solid.MakeOffsetShape()
+    if not solid.IsDone():
+        raise RuntimeError("Error applying thicken to given Face")
     try:
         result = Solid(solid.Shape())
     except StdFail_NotDone as err:

(doesn't solve the issue tho)

[GarryBGoode]

I would like to add that this issue affects gggears. Before the refactor, using Keep.BOTH bypassed the offset and bool operations altogether, now it attempts them anyway and runs into the Null TopoDS issue.

@gumyr
Please consider providing an option to bypass the determination of top/bottom entirely due to the possible errors in solid.MakeOffsetShape and the following bool op. Even without the error it would still help performance if these operations can be avoided.