STEP Importer Only Imports Solids

jmwright commented 6 years ago

Created based on this conversation on the Google Group.

The issue is that the STEP importer ignores anything in a STEP file that's not a solid. So in the use case discussed on the Google Group, the user needed a face to be imported. That's not currently possible, although FreeCAD clearly supports it. This is the code in question.

This might be a good first issue for someone wanting to get started contributing to CadQuery.

bweissinger commented 5 years ago

@jmwright I took a look at this and it appears that it can be fixed by importing compounds as well as solids.

I also tried importing shells alongside the solids, but that didn't work as well for files with a mix of solids and surfaces. Everything imported OK, but it appeared to be less flexible than compounds.

jmwright commented 5 years ago

@bweissinger Thanks for taking a look at this. Did you do the import using CadQuery? Can you post the steps you took to make it work?

bweissinger commented 5 years ago

I did use CadQuery for the importing. Essentially I modified the current importStep function to cast shells as well.

#Now read and return the shape
try:
    rshape = Part.read(fileName)

    # Extract solids and surfaces
    geometry = []
    for solid in rshape.Solids:
        geometry.append(Shape.cast(solid))

    for shell in rshape.Shell:
        geometry.append(Shape.cast(shell))

    return cadquery.Workplane("XY").newObject(geometry)

However, I might have spoke to soon about compounds vs shells. I think that shells are actually the way to go. Here is a write up of my reasoning and testing.

Reasoning

The current implementation of the code in importStep looks like this:


#Now read and return the shape
try:
    rshape = Part.read(fileName)

    #Make sure that we extract all the solids
    solids = []
    for solid in rshape.Solids:
        solids.append(Shape.cast(solid))

    return cadquery.Workplane("XY").newObject(solids)

except:
    raise ValueError("STEP File Could not be loaded")

If I understand this correctly, rshape should contain the FreeCAD B-REP constructs for the step file. The for loop pulls all of the solids from rshape, and casts them to CadQuery solids. The actual cast call, Shape.cast() (from shapes.py), takes the provided FreeCAD object and pulls the ShapeType (string), then selects the proper shape constructor based on the ShapeType. Available shapes (constructs) are Vertex, Edge, Wire, Face, Shell, Solid, and Compound.

Looking through shapes.py,

Face: A bounded surface that represents part of the boundary of a solid

Shell: The outer boundary of a surface

Solid: A single solid

Compound: A collection of disconnected solids

it seems that if we cast the Shells from the STEP file, we could have all non-solid surfaces as well.

So I decided to test that theory. I was a little worried about geometry being duplicated, but that does not appear to be an issue. I also decided to test compounds as well.

Testing

My dev environment consists of a python 2 virtualenv, with the contents of requirements-dev.txt installed. I am also running a 64bit Linux OS (Arch).

I used the following files for testing:

ExSurface.stp - The provided surface example file from the Google Group conversation
ExSurfaceMulti.stp - The geometry from ExSurface.stp duplicated and moved (in FreeCad), providing two unconnected surfaces.
box.stp - A single solid in the form of a CadQuery box.
boxMulti.stp - The geometry from above, with and additional box seperate from the first.
Micron_DST_Sonar_V2.stp - A model containing a solid and surface.

stpFiles.zip

I ran the following python script to import files and get the number of each geometry actually imported.

 import cadquery

 # Test files
 testFiles = [
              'ExSurface.stp',
              'ExSurfaceMulti.stp',
              'box.stp',
              'boxMulti.stp',
              'Micron_DST_Sonar_V2.stp'
             ]

 # Output files
 logFile = 'log.txt'

 for file in testFiles:

     # Import step file
     result = cadquery.importers \
         .importStep(file)

     # Check the number of each construct imported
     log = 'solids: ' + str(result.solids().size()) + '\n'
     log += 'faces: ' + str(result.faces().size()) + '\n'
     log += 'vertices: ' + str(result.vertices().size()) + '\n'
     log += 'edges: ' + str(result.edges().size()) + '\n'
     log += 'wires: ' + str(result.wires().size()) + '\n'
     log += 'shells: ' + str(result.shells().size()) + '\n'
     log += 'compounds: ' + str(result.compounds().size()) + '\n'

     # Write results to the log file
     f = open(logFile, 'a')
     print >> f, file + "\n" + log + "\n \n"
     f.close()

The code in importers.py was modified for each of the tests.

Solids only

solids = []
for solid in rshape.Solids:
    solids.append(Shape.cast(solid))

return cadquery.Workplane("XY").newObject(solids)

Shells only

shells = []
for shell in rshape.Shells:
    shells.append(Shape.cast(shell))

return cadquery.Workplane("XY").newObject(shells)

Solids + Shells

geometry = []
for solid in rshape.Solids:
    geometry.append(Shape.cast(solid))

for compound in rshape.Compound:
    geometry.append(Shape.cast(shell))

return cadquery.Workplane("XY").newObject(geometry)

Compounds only

compounds = []
for compound in rshape.Compounds:
    compounds.append(Shape.cast(compound))

return cadquery.Workplane("XY").newObject(compounds)

Solids and compounds

geometry = []
    for solid in rshape.Solids:
        geometry.append(Shape.cast(solid))

    for compound in rshape.Compounds:
        geometry.append(Shape.cast(compound))

    return cadquery.Workplane("XY").newObject(geometry)

Solids, Compounds, and Shells

geometry = []
    for solid in rshape.Solids:
        geometry.append(Shape.cast(solid))

    for compound in rshape.Compounds:
        geometry.append(Shape.cast(compound))

    for shell in rshape.Shells:
        geometry.append(Shape.cast(shell))

    return cadquery.Workplane("XY").newObject(geometry)

Results

ExSurface.stp

	Shells Only	Solids and Shells	Compounds Only	Compounds and Solids	Compounds, Solids, and Shells
solids	0	0	0	0	0
faces	1	1	1	1	1
vertices	4	4	6	6	6
edges	4	4	5	5	5
wires	1	1	2	2	2
shells	1	1	1	1	1
compounds	0	0	1	1	1

Casting only solids, nothing was imported, as expected. Casting shells did in fact bring in the surface. Compounds shows an extraneous edge, and I am not exactly sure where that is coming from.

ExSurfaceMulti.stp

	Shells Only	Solids and Shells	Compounds Only	Compounds and Solids	Compounds, Solids, and Shells
solids	0	0	0	0	0
faces	2	2	2	2	2
vertices	8	8	8	8	8
edges	8	8	8	8	8
wires	2	2	2	2	2
shells	2	2	2	2	2
compounds	0	0	1	1	1

For multiple surfaces, we can see that shells and compounds both imported the correct geometry. If we use compounds, a single compound is also shown as part of the file. Interestingly though, the extra edge from the previous file is not found in this one. FreeCAD may not have imported it during the creation of this file.

box.stp

	Solids Only	Shells Only	Solids and Shells	Compounds and Solids	Compounds, Solids, and Shells
solids	1	0	1	1	1
faces	6	6	6	6	6
vertices	8	8	8	8	8
edges	12	12	12	12	12
wires	6	6	6	6	6
shells	1	1	1	1	1
compounds	0	0	0	0	0

Solids of course works here. If we use Solids and Shells, it appears that no geometry is duplicated. Compounds returns nothing for a single solid.

boxMulti.stp

	Solids Only	Shells Only	Solids and Shells	Compounds Only	Compounds and Solids	Compounds, Solids, and Shells
solids	2	0	2	1	3	3
faces	12	12	12	12	12	12
vertices	16	16	16	16	16	16
edges	24	24	24	24	24	24
wires	12	12	12	12	12	12
shells	2	2	2	2	2	2
compounds	0	0	0	1	1	1

Interestingly, using Compounds and Solids, CadQuery reports having 3 solids, instead of 2, but the same amount of edges, faces, etc.

Micron_DST_Sonar_V2.stp

	Solids Only	Shells Only	Solids and Shells	Compounds Only	Compounds and Solids	Compounds, Solids, and Shells
solids	1	0	1	1	1	1
faces	250	251	251	250	250	251
vertices	357	358	358	357	357	358
edges	575	576	576	575	575	576
wires	280	282	282	280	280	282
shells	1	2	2	1	1	2
compounds	0	0	0	0	0	0

It looks like using Solids and Shells will import all solids and non-solids.

Conclusion

I think that casting Shells, in addition to solids, will cover the importing of non-solid surfaces. That being said, CadQuery has no surface modelling/editing features, and no way to export projects containing a mix of the two, correct? Could the current implementation of only importing solids be there on purpose?

jmwright commented 5 years ago

@bweissinger Wow, that's great! Very thorough.

That being said, CadQuery has no surface modelling/editing features, and no way to export projects containing a mix of the two, correct? Could the current implementation of only importing solids be there on purpose?

I don't think it's because we weren't interested in handling surfaces, it just wasn't a priority during implementation (which was done entirely by Dave back in those days). With your changes, does the test suite still run successfully? If so, would you be interested in putting a PR together for this? Even though it's less than ideal, I'm thinking that better support for surfaces can be implemented over time. This could be the first step in that direction.

As far as the extra edge with compounds in some situations, I've run into the same sort of thing. There's still a long-standing issue that's probably related that I never got fixed.

dcowden commented 5 years ago

Yep, Jeremy is right. Solids were just the initial focus, there was no intentional bias against surfaces.

On Mon, Nov 26, 2018, 8:17 PM Jeremy Wright <notifications@github.com wrote:

@bweissinger https://github.com/bweissinger Wow, that's great! Very thorough.

That being said, CadQuery has no surface modelling/editing features, and no way to export projects containing a mix of the two, correct? Could the current implementation of only importing solids be there on purpose?

I don't think it's because we weren't interested in handling surfaces, it just wasn't a priority during implementation (which was done entirely by Dave back in those days). With your changes, does the test suite still run successfully? If so, would you be interested in putting a PR together for this? Even though it's less than ideal, I'm thinking that better support for surfaces can be implemented over time. This could be the first step in that direction.

As far as the extra edge with compounds in some situations, I've run into the same sort of thing. There's still a long-standing issue that's probably related that I never got fixed.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dcowden/cadquery/issues/295#issuecomment-441861050, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPOA7-4TXEC6fhYG97bXXLR1NV3vcVfks5uzJKqgaJpZM4XD8hQ .

bweissinger commented 5 years ago

Ok, awesome. I thought that might be the case, just wanted to make sure. The tests are good, and I could certainly do a PR.

I just realized it needs added to importStepFromURL() as well. Looking at that function, the importing of the saved temp file

rshape = Part.read(tempFile.name)

        #Make sure that we extract all the solids
        solids = []
        for solid in rshape.Solids:
            solids.append(Shape.cast(solid))

        return cadquery.Workplane("XY").newObject(solids)

is a duplicate of the importStep() function. Any problem if I just call importStep() to load the saved file?

return importStep(tempFile.name)

jmwright commented 5 years ago

is a duplicate of the importStep() function. Any problem if I just call importStep() to load the saved file?

I'm not sure why this functionality is duplicated. Probably an oversight on our part. I think what you're proposing is fine.

jmwright commented 5 years ago

I've posted to the original issue on the Google Group letting that user know you've fixed this. Thanks again for the contribution!

dcowden / cadquery