CadQuery / cadquery

A python parametric CAD scripting framework based on OCCT
https://cadquery.readthedocs.io
Other
3.07k stars 286 forks source link

step exporter generates incorrect geometry #697

Closed greyltc closed 3 years ago

greyltc commented 3 years ago

This is with cadquery commit gf0308ce and opencascade 7.5.0:

widget = # insert code to make a thin uniform sheet
oddly_shaped_holes = # insert code to make some oddly shaped hole geometry
bunch_of_cylinders = # insert code to make a bunch of non intersecting cylinders on a plane
widget = widget.cut(oddly_shaped_holes)
widget = widget.cut(bunch_of_cylinders)
cadquery.exporters.export(widget, "widget.stl")
cadquery.exporters.export(widget, "widget.step")

widget.stl contains the geometry I expect and the region of interest here looks like this: image but widget.step looks like this: image I made that crescent shape (as shown in the .stl) by using widget = widget.cut(bunch_of_cylinders) to cut a bunch of cylinders out of a thin sheet. Many of the resulting voids in the thin sheet step file (not shown here) are correct, but some, like this one, have somehow managed to invert the boolean operation I asked for and instead of subtracting the cutting tool from the stock, the step file shows the cutting tool shape (a cylinder) added to the stock part.

Is this a bug in opencascade, not cadquery? Is the step file exporter known to be broken? Are there workarounds/tricks I can play to make step files in a more reliable way?

greyltc commented 3 years ago

I should mention that I'm viewing the step file with freecad, and just to make sure that it's not a bug in freecad, I also uploaded the step to https://www.emachineshop.com/free-online-step-file-viewer/ which agrees with freecad's render.

greyltc commented 3 years ago

Could this be a bug in opencascade's fuzzy boolean operations? If I make the cutting tool cylinders an extra 1 mm taller than they need to be and translate them in Z by -0.5mm, the step file is generated correctly.

I have trouble understanding how that could only apply to step file exports though.

jmwright commented 3 years ago

STL export uses meshes, which is a very different process than STEP export. There could be invalid geometry at the locations that the STL exporter smooths over. Have you checked the STL to make sure it doesn't have any errors? Can you produce a minimum working example so that we can test?

greyltc commented 3 years ago

I don't think it's related to the triangle approximation going on in the .stl files. Seems to actually be a problem with the step file export pathway:

fedorkotov commented 3 years ago

I have reproduced the problem

widget = \
    cq.Workplane("XY")\
    .rect(5,5)\
    .extrude(0.5)

oddly_shaped_holes = \
    cq.Workplane("XY")\
    .move(0,1.2)\
    .lineTo(-0.5,1.2)\
    .lineTo(-0.5,1.5)\
    .lineTo(-1.5,1.5)\
    .lineTo(-1.5,0)\
    .mirrorX()\
    .mirrorY()\
    .extrude(2)\
    .edges("|Z")\
    .fillet(0.13)

bunch_of_cylinders = \
    cq.Workplane("XY")\
    .rarray(1,3,4,2)\
    .eachpoint(
        lambda loc: cq.Workplane("XY")\
               .circle(0.15)\
               .extrude(2)\
               .val()\
               .located(loc))
widget = widget.cut(oddly_shaped_holes)
widget = widget.cut(bunch_of_cylinders)
cq.exporters.export(widget, f'widget.stl')
cq.exporters.export(widget, f'widget.step')

Everything is looks ok in cq-editor image

stl looks ok in https://www.viewstl.com/#! image

But in www.emachineshop.com viewer something strange happens with holes image

greyltc commented 3 years ago

I have reproduced the problem

Ha! Thanks! Even before I had a chance to distill it! :+1:

greyltc commented 3 years ago

@fedorkotov

If you change .extrude(2)\ in oddly_shaped_holes and bunch_of_cylinders to .extrude(0.5)\ so that the cutting tools match the thickness of the stock part, I bet you'll see more along the lines of what I see.

If you change widget = widget.cut(bunch_of_cylinders) to widget = widget.cut(bunch_of_cylinders.translate((0,0,-0.5))) the problem will be fixed I think.

fedorkotov commented 3 years ago

Indeed. Your suggestion fixed it :)

This is the result of reducing extrude depth to 0.5 image

And this is the result of moving both subtracted features down 0.5 with extrusion depth 2 image

This is the working variant

widget = \
    cq.Workplane("XY")\
    .rect(5,5)\
    .extrude(0.5)

oddly_shaped_holes = \
    cq.Workplane("XY")\
    .workplane(-0.5)\
    .move(0,1.2)\
    .lineTo(-0.5,1.2)\
    .lineTo(-0.5,1.5)\
    .lineTo(-1.5,1.5)\
    .lineTo(-1.5,0)\
    .mirrorX()\
    .mirrorY()\
    .extrude(2)\
    .edges("|Z")\
    .fillet(0.13)

bunch_of_cylinders = \
    cq.Workplane("XY")\
    .workplane(-0.5)\
    .rarray(1,3,4,2)\
    .eachpoint(
        lambda loc: cq.Workplane("XY")\
               .circle(0.15)\
               .extrude(2)\
               .val()\
               .located(loc))
widget = widget.cut(oddly_shaped_holes)
widget = widget.cut(bunch_of_cylinders)
cq.exporters.export(widget, f'widget.stl')
cq.exporters.export(widget, f'widget.step')
fedorkotov commented 3 years ago

@greyltc Please try the same trick on your original model. Will it fix the problem there too?

greyltc commented 3 years ago

Will it fix the problem there too?

Yep, it does. It's like the fuzzy boolean operation feature (see my comment above) is somehow lost in the step export pathway. Edit: I don't know...I'm not really sure that could explain how a boolean subtraction could turn into a boolean addition.

fedorkotov commented 3 years ago

I thought that BREP means all the history of unions, intersections, etc is lost after each operation and only the resulting trimmed surfaces with common edges and vertexes remain. How is it possible that after successfully cutting the holes (as can be seen in cq-viewer or in STL models), STEP export result depends on how exactly the solid was made? Maybe one of the experts here can explain..

marcus7070 commented 3 years ago

I thought that BREP means all the history of unions, intersections, etc is lost after each operation and only the resulting trimmed surfaces with common edges and vertexes remain. How is it possible that after successfully cutting the holes (as can be seen in cq-viewer or in STL models), STEP export result depends on how exactly the solid was made?

OCCT stores that edge as something along the lines of a "trimmed circle". It looks like OOCT is inverting which side of it is trimmed when converting it to a STEP.

Edit: that sounds more confident than I intended. This is just my best guess.

adam-urbanczyk commented 3 years ago

It seems to be STEP related - if I export in the brep format the issue is not there.

greyltc commented 3 years ago

Possibly related: https://tracker.dev.opencascade.org/view.php?id=32132

marcus7070 commented 3 years ago

Two step files attached, one made with CQ-editor export and OCP/OCCT 7.4, the other with 7.5.1. I can see the error in 7.5.1, but not in 7.4.

STEP files renamed to TXT files because github... :expressionless: trouble_7_5_1.step.txt trouble_7_4.step.txt

Made with @fedorkotov's code as written here.

jmwright commented 3 years ago

Looks like that OCCT issue hasn't been triaged yet, and the last comment was the middle of Feb. It looks to me like it's related to this issue.

greyltc commented 3 years ago

To anyone else who is also being bothered by this bug, I have a workaround: The latest freecad appimage release was built with opencascade version 7.4.0. If I save my cadquery-generated geometry as .brep like so:

from OCP.BRepTools import BRepTools
BRepTools.Write_s(widget.toOCC(), "good_geometry.brep")

I then get bugfree .brep files which I can manually import with the appimage version of FreeCAD and reexport as step files that are also bugless (since the problem doesn't exist in occt 7.4).

adam-urbanczyk commented 3 years ago

Also cq.Shape.exportBrep, if you don't want to get your hands dirty with OCP.

greyltc commented 3 years ago

Upstream bug report: https://tracker.dev.opencascade.org/view.php?id=32264

adam-urbanczyk commented 3 years ago

Great, thanks for reporting! They do like draw.exe scripts.

greyltc commented 3 years ago

The opencascade devs have quickly come up with a fix for this issue. corrupted_step_fix.patch.txt I guess this will appear in the upcoming occt 7.6.0 release.

Edit: Actully, looks like they've decided to roll this fix into a 7.5.2 release.