section-properties icon indicating copy to clipboard operation
section-properties copied to clipboard

Geometry.from_dxf can only ever import one shape

Open mfeif opened this issue 1 year ago • 16 comments

Describe the bug When importing shapes from a clean/well-formed DXF file, only one shape is ever found.

To Reproduce Steps to reproduce the behaviour:

  1. Import shapes with Geometry.from_dxf()
  2. Notice that only one shape exists

Expected behaviour When working with a clean/well-formed DXF file, .from_dxf() should be able to import multiple shapes for analysis.

Additional context I have a clean DXF file with some closed polylines in it, and all other entities purged/removed. I can import this DXF into shapely. I can also import this file into the cad_to_shapely shim. In both cases, the multiple-shape geometry is preserved.

However, when I do the same via Geometry.from_dxf() only one shape ever comes back. Digging deeper into the code in from_dxf I found this:

my_dxf = c2s.dxf.DxfImporter(dxf_filepath)
my_dxf.process()
my_dxf.cleanup()

polygons = my_dxf.polygons
new_polygons = c2s.utils.find_holes(polygons)
if isinstance(new_polygons, MultiPolygon):
    return CompoundGeometry(new_polygons)
elif isinstance(new_polygons, Polygon):
    return Geometry(new_polygons)
else:
    print(f"No shapely.Polygon objects found in file: {dxf_filepath}")

Looking into find_holes, we find that this behavior is by design:

def find_holes(polygons : List[sg.Polygon]) -> sg.Polygon:
    """
    Construct single polygon from imported CAD goemetry. 
    Assumes there is only one parent shape (the one with the largest gross area.)
    Access external perimeter with *polygon.exterior*
    Access holes perimeter(s, if there are any) with *polygon.interiors*
    Returns:
        Shapely Polygon with holes 
    """
    for p in polygons:
        if p.interiors:
            return p

    #sort by areas, largest first.
    polygons.sort(key=lambda x: x.area, reverse=True)
    parent = polygons.pop(0)

    keepers = []
    for p in polygons:
        if p.within(parent):
            valid = True
            for q in polygons:
                if (p.intersects(q) or p.within(q)) and p is not q:
                    valid = False
           
            if valid: keepers.append(p)


    new = sg.Polygon(parent,holes=keepers)
    return new

So by using this utility function, sectionproperties can never support more than one shape in a DXF. This is not a "bug" in cad_to_shapely, per-se, as its the desired behavior, but by using this utility function, we're throwing out data.

I went through the analogous process by hand, NOT throwing out polygons and manually made a CompoundGeometry object, and it worked:

from cad_to_shapely.dxf import DxfImporter
from shapely.geometry.multipolygon import MultiPolygon

doc = DxfImporter('polylines.dxf')
doc.process()
doc.cleanup()
polys = MultiPolygon(doc.polygons)

g = CompoundGeometry(polys)
g.create_mesh(mesh_sizes=[0.1])

(this showed a little thumbnail of an object in a Jupyter Notebook)

mfeif avatar Sep 19 '22 22:09 mfeif

Hi @mfeif thanks for raising this. Are you able to provide the .dxf file so I can reproduce? You may need to rename the file extension to something else e.g. .txt for it to be able to upload here.

robbievanleeuwen avatar Sep 20 '22 11:09 robbievanleeuwen

Sure thing. Here's a suitable file. This is what it looks like:

dxf

Only the largest (by area) shape is imported via the .from_dxf() constructor, and it returns an instance of Geometry rather than CompoundGeometry.

bug.dxf.txt

mfeif avatar Sep 20 '22 15:09 mfeif

I had a thought; if we go around the .find_holes() function, as I did in my manual example, we will lose the actual hole finding 😄 which may not be the best.

Seems like the detection of intersection and within there could be done while still preserving multiple polygons, though. 🤷

Maybe it's a feature request on that library?

mfeif avatar Sep 20 '22 15:09 mfeif

Hi @mfeif, I see that your geometry is not connected. sectionproperties isn't really designed around unconnected geometries as the warping analysis is invalid, see warning here. However you can still perform a geometric analysis and plastic analysis on unconnected geometries.

If you have multiple connected regions, e.g. two rectangles directly adjacent to each other, my understanding is that this should work with CompoundGeometry.from_dxf().

I suspect cad-to-shapely was written with all this in mind. If you want this feature supported it may be worth, as you say, putting in a feature request there?

robbievanleeuwen avatar Sep 21 '22 12:09 robbievanleeuwen

My mistake; that is a made-up DXF file that I made just to exhibit the behavior; it's not realistic.

The actual use case is in the retaining wall industry; the linkages look like this sort of thing: pzc-b_generic_domestic_single

The shapes interlock, but may have a mm or few space between them in actuality. And sometimes they are "connected" as you say (as in the little shapes on the corners of the beam flanges). I made a closer simplified representation that looks like this:

ex

Running it through CompoundGeometry.from_dxf() results in only the beam shape being returned, so no, it doesn't work as you were expecting and I was hoping. I've attached the more appropriate dxf for your experimentation. Cad-to-shapely explicitly only ever returns one polygon, as per the code comments over there. This isn't a bug in section-properties, per-se, but it is not currently working as you (we) expect (returning multiple contiguous shapes). I could ask for a new function over there, as you suggest, but section-properties would still have to change how .from_dxf() works and call that function. Right?

(And I am looking for geometric analysis (finding the centroid and so on) not warping; so that's cool by me.)

ex.dxf.txt

mfeif avatar Sep 21 '22 17:09 mfeif

Great, thanks for investigating this one @mfeif! @aegis1980 wondering if this is something that could be supported in cad-to-shapely?

robbievanleeuwen avatar Sep 22 '22 07:09 robbievanleeuwen

@mfeif and @robbievanleeuwen From what I can tell this arose from the need to batch analyzing Geometry sections. Is this correct? This is a good discussion, and I'm happy to help with this. I'm going to offer a challenge to the described expected behavior of Geometry.from_dxf. I do think that this could rather be a utility feature/function.

The definition of Geometry is https://github.com/robbievanleeuwen/section-properties/blob/acb77bb089cce907801aa94715b63370e6120db9/sectionproperties/pre/geometry.py#L29. In my opinion, the from_dxf method is a classmethod of Geometry (the decorator should be there, but it is missing), and in that respect, it is behaving as expected. Data in the dxf is interpreted according to the definition of Geometry. For multiple closed polygons in a dxf file, the 1st is selected (I'm actually not sure of this logic). There may be a more elegant way to make this selection, but only one Geometry should be produced.

This perspective may be too rigid and maybe it is appropriate for a classmethod to return an array of objects, but I thought I would offer it up. Let me know your thoughts.

normanrichardson avatar Sep 22 '22 18:09 normanrichardson

Fair point, but I am using CompoundGeometry.from_dxf() 😊

mfeif avatar Sep 22 '22 18:09 mfeif

When I wrote cad-to-shapely, I put 'finding lots of shapes with holes' in the too hard box. Info is not implicit in 'dumb' 2d geometry so find_holes really a utility function. Finding the holes when there is a single main closed section is easy, since holes have to be smaller (in area) than the parent, but the downside is the issue raised here. Importing hatches, regions and/or blocks (in cad to shapely, @mfeif Thanks for that) might be a way to go - not excited about implementing myself! feel free to do so!! Or recursively calling find_holes maybe, for those too lazy to so hatches/regions/blocks.

aegis1980 avatar Sep 22 '22 20:09 aegis1980

@normanrichardson with you on the classmethod aspect and agree that Geometry.from_dxf() should return a Geometry object with a single region, while CompoundGeometry.from_dxf() should return a CompoundGeometry object with multiple regions (not necessarily contiguous). Unfortunately, this is not currently the behaviour.

A current workaround for users, although tedious, would be to export each region as a separate dxf files. Import these as multiple Geometry objects and then use the + operator to combine these geometries into a CompoundGeometry object.

In terms of implementing the desired CompoundGeometry.from_dxf() behaviour, I don't have any experience with any of the dxf libraries and unfortunately don't have the time to learn it at the moment. I have put the help wanted and good first issue labels on this issue and would welcome anyone interested in tackling this, noting that it would probably be a joint PR for both cad-to-shapely and sectionproperties.

robbievanleeuwen avatar Sep 23 '22 03:09 robbievanleeuwen

Importing hatches, regions and/or blocks (in cad to shapely, @mfeif Thanks for that) might be a way to go - not excited about implementing myself! feel free to do so!! Or recursively calling find_holes maybe, for those too lazy to so hatches/regions/blocks.

I'm not following about the hatches and so on.

I think using the logic in .find_holes iteratively, preserving multiple geometries (except those that are holes) makes closer sense. I might even be able to tackle that since you've already done the hard part of figuring out how to do it.

mfeif avatar Sep 23 '22 17:09 mfeif

Hey folks,

We encountered similar problems when trying to write a test to discover if a CompoundGeometry is disjoint. As @aegis1980 pointed out, it's hard!

After several days of trying things out, the only solution I could come up with was to create a network of shapely Polygons based on their connectivity to each other. Similarily, I think we had to solve a similar problem when dealing with overlapping geometries with a hole going through both geometries.

I think we could use cad-to-shapely to import all geometries and perhaps by-pass the hole detection part. Does the hole detection assume that if a geometry is completely contained within another geometry it is a hole? To make this whole thing more complicated, is there not also the possibility of nested materials within each other which may or may not have a hole in them?

I think that @mfeif is correct that it would be good if .from_dxf() could work with multiple geometries. I think perhaps the best approach is to bypass the hole detection in cad-to-shapely and bring all geometries into a single CompoundGeometry. From there the user can assign materials and do additive/subtractive modelling of the geometries as required to achieve their desired result.

connorferster avatar Oct 21 '22 16:10 connorferster

I agree this seems like the best approach.

Do you see the best way forward as this?

  • An option in cad-to-shapely to avoid hole detection, e.g. an optional parameter hole_detection: bool = True that is True by default but can be set to False if required
  • Modified code in sectionproperties to implement the above

@connorferster are you able to make a PR to cad-to-shapely to implement?

robbievanleeuwen avatar Oct 24 '22 05:10 robbievanleeuwen

I added in dev branch of cad_to_shapely utils.py a function filter_polygons that'll return a list of inferred "topographical surfaces" from closed curves (shapely polygons with/without holes/both). This ought to address various quandaries bought up in this thread & others. Aim is to depreciate find_holes.

Got some bits and bobs to sort out, then will try and tie in with section-properties from_dxf/load_dxf and make pull request.

@robbievanleeuwen At moment section-properties load_dxf calls c2s.utils.find_holes. This island-finding stuff is kind of in hinterland "upstream" of section-properties and "downstream" of cad-to-shapely (why it in c2s.utils module)

Also some complications arise when you consider if user wants to import multiple sections from a single dxf (list of Geometry objects; I recall someone wanting to do that), or a single compound section (CompoundGeometry). I am going to aim at the latter in pull request on the grounds that if you want to import and analyse multiple sections user can sort it out 'upstream' in CAD package (multiple DXF files), or programmatically-by-yourself (somehow!)

aegis1980 avatar Nov 01 '22 06:11 aegis1980

Love your work @aegis1980!

robbievanleeuwen avatar Nov 01 '22 07:11 robbievanleeuwen

A CompoundGeometry for a multi-section DXF would work great! A user can access the individual geometries by indexing into the .geoms attribute.

I have a student right now who is doing this exact use case for his minor project so the fix will be welcome!

Many thanks!

On Nov 1, 2022, at 00:06, Robbie van Leeuwen @.***> wrote:

 Love your work @aegis1980!

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

connorferster avatar Nov 01 '22 07:11 connorferster