Dxf importer: add support for lwpolyline elevation and small bugfix

Authored by stephen leger (stephen_leger) on Nov 27 2017, 9:06 PM.



Lwpolylines are often used to define terrain curves, and "field 38" is in use to define elevation of the curve so vertices only need x and y coords.

Fix for .is_closed not always a value but @property in some entities, raising AttributeError when set.
Some entities use .flags & 1 to define the closed state so the .is_closed is not "settable"

Diff Detail

Could you include a test-file and the name of the application that saved it?

@Lukas Treyer (cnd). could you check on this?

Patch seems OK, but I don't know this format.


Using try/except here is a bit strange. Why would setting is_closed fail now, where before it didn't?


Edit, I see now this is a property. try/except should say in comment why they are needed.

Campbell Barton (campbellbarton) requested changes to this revision.Dec 6 2017, 2:56 PM
This revision now requires changes to proceed.Dec 6 2017, 2:56 PM

Added comments on why try is needed.

As a side note,
the file is coming from an ADSK product (probably land or something similar) but is near 170Mo so i won't attach.

Thanks for adding elevation support. I don't see any problems with this. Please share the DXF file if possible. How big is it when you zip it? You could share it with http://pasteall.org/blend/ if it doesn't exceed 30MB.

  1. .is_closed is a property from dxfgrabber library
  2. as far as I can tell from reading dxfgrabber's source it is read-only
  3. this means .is_closed = False will always fail, I just couldn't test it yet
  4. instead of try/except we should therefore use straightly the flag and write en_template.flag ^= const.POLYLINE_CLOSED (const.POLYLINE_CLOSED for readability; no comments needed anymore). perhaps we also need to import const at the top.
stephen leger (stephen_leger) marked 2 inline comments as done.Dec 8 2017, 11:19 AM

.is_closed is not always a property !
in dxfgrabber->dxfentities->polylines (line 353) it is not so is_closed will at least work with it.

Added sample dxf for this on dropbox

As a side note, also found a file with a 0 length vector raising on .angle_signed evaluation. (eg: convert line 160)

I don't know exactly how python behaves if you have a getter property and an attribute with the same name. But I assume the getter property will always be called when you want to access that attribute/property. So yes, setting .is_closed might not fail, but I still think it's better to straightly modify flags, since I *assume* we're always calling the property, when we want to know about the .is_closed property.

To make things clear:

.is_closed is always readable.
.is_closed is only writable on polylines, and polylines dosen't rely on flags (dosent even have a flag property).
.is_closed on other entities is not writable (is a getter only) so we rely on flags.

sorry I somehow was caught in Java-thinking and didn't expect more than one class in a file..

so it's only polylines, lwpolylines and splines having .is_closed attribute/property.

why not doing it like this?

if en_template.dxftype == 'POLYLINE':
    en_template.is_closed = False
    # for lwpolylines and splines
    en_template.flags ^= 1

I am not really caring about style here. We can also use try/except if you think this is better.

Only polyline has a writeable .is_closed, other rely on .flags
so if there is a way to know an entity is a polyline, i think we should avoid try except.

Replace try except by explicit check for polyline type.

For some obscure reason i'm unable to add another diff (support for Gradians angle types)
So i do add the diff here.
Original dxf file from ADSK Civil 3D 2009

diff --git a/io_import_dxf/dxfimport/do.py b/io_import_dxf/dxfimport/do.py
index 8954ba5..65575c9 100644
--- a/io_import_dxf/dxfimport/do.py
+++ b/io_import_dxf/dxfimport/do.py
@@ -297,10 +297,18 @@ class Do:
             angdir = self.dwg.header.get('$ANGDIR', 0)
         kappa = 0.5522848
+        # http://help.autodesk.com/view/ACD/2016/ENU/?guid=GUID-A85E8E67-27CD-4C59-BE61-4DC9FADBE74A
+        # TODO: add support for 1 (dms) and 4 (survey)
         if aunits == 0:
+            # Degree
             s = radians(en.start_angle+angbase)
             e = radians(en.end_angle+angbase)
+        elif aunits == 2:
+            # Gradians
+            s = radians(0.9 * (en.start_angle+angbase))
+            e = radians(0.9 * (en.end_angle+angbase))
+            # Radians
             s = en.start_angle+angbase
             e = en.end_angle+angbase

Can this Revision be closed now since it is commited? Thanks.

Sure. I don't think I can do this. No such action available.

Thanks, i'll close it since it is committed.