DXF: Delay insertion and merging of blocks until actually required

[Alan Thomas]

I have a big patch that solves a few different issues with DXF BLOCKs and INSERTs:

Issue 1: Can't insert a block from within another block defined earlier in the BLOCKS table

Suppose the DXF file defines two blocks, block1 and block2. The block1 block contains an INSERT entity that references block2. Because block2 has not been read in when block1's contents are being translated into feature, it doesn't exist from OGR's perspective, so the INSERT is incorrectly omitted.

This patch defers the processing of INSERTs until they actually need to be inlined, either from the Blocks layer, or TranslateINSERT in the Entities layer.

This also allows for more flexibility for merging geometries. Currently blocks are merged by OGRDXFDataSource::ReadBlocksSection, but I'm imagining that, in the future, OGR could implement a flag like DXF_MERGE_COMPLEX_GEOMETRIES allowing the user to obtain DIMENSIONs, LEADERs etc as a single GeometryCollection. This would require OGR to decide whether to merge the block on-the-fly, which is easily possible with this patch.

Issue 2: The name of the block referred to by an INSERT inside a BLOCK is not preserved

This issue is alluded to in #4369. Suppose DXF_INLINE_BLOCKS and DXF_MERGE_BLOCK_GEOMETRIES are both set to FALSE. Each INSERT entity is translated into a POINT with the BlockName, BlockAngle and BlockScale fields set.

If an INSERT entity exists within a block and you query the Blocks layer, you will find a POINT entity with correct BlockAngle and BlockScale, but the BlockName field has been replaced by the name of the block which contains the INSERT entity, instead of the block it refers to!

The patch fixes this by placing the name of the owning block on a "Block" field for all entities when DXF_INLINE_BLOCKS is FALSE, instead of confusingly reusing "BlockName". This change also impacts the DXF writer. This is an unavoidable breaking change -- I'm not sure how that is usually handled.

Issue 3: OCS of INSERT entities is not stored when DXF_INLINE_BLOCKS is false

Without the OCS normal vector, there is not enough information for an application to recreate the block insertion properly.

This patch stores the OCS in a new "BlockOCSNormal" field.

The patch also changes the location of an INSERT entity with DXF_INLINE_BLOCKS set to false to be in the OCS... this is less than ideal for end users, but it is essential to preserve proper read/write round-tripping of DXF files. Alternative suggestions would be welcomed. this is no longer the case, I added the "BlockOCSCoords" field to solve this problem.

Issue 4: INSERT entities are incorrectly merged into a GeometryCollection when DXF_INLINE_BLOCKS is false

Because these POINTs are only meaningful by virtue of the information in their fields, which is lost upon merging, they should not be merged into a GeometryCollection, in the same way as we don't merge text features.

[Alan Thomas]

A question about this patch:

Fields called BlockName, BlockScale, BlockAngle and BlockOCS are now unconditionally created. The end user will only see data in these fields when DXF_INLINE_BLOCKS is false, but we need to create them unconditionally because the data is used internally by the DXF processor, even when DXF_INLINE_BLOCKS is true. Is there a way to achieve the same functionality without needlessly exposing these fields to the end user?

[Alan Thomas]

Patch

[Even Rouault]

Is there a way to achieve the same functionality without needlessly exposing these fields to the end user?

You could possibly create a OGRDXFFeature class extending OGRFeature that would store BlockName, etc... as regular C++ members, which would thus be internal to the DXF driver, and and expose them as regular OGR fields only when that makes sense.

If you don't mind, could you create pull requests on the github mirror at ​https://github.com/OSGeo/gdal ? That way Travis-CI tests will run.

[Alan Thomas]

See ​https://github.com/OSGeo/gdal/pull/251. Let me know if you would like me to put a proper commit message.

I have had a go at implementing OGRDXFFeature as you suggested. Unfortunately, the patch is now getting quite large, but I do not think it is possible to split it into multiple commits because all the changes are so heavily intertwined.

[Even Rouault]

In 40548:

DXF: defer inlining of blocks until actually required (patch by Alan Thomas, fixes #7106, ​https://github.com/OSGeo/gdal/pull/251)

[Even Rouault]

In 40549:

Commit file that should have gone with previous commit (refs #7106)

[Alan Thomas]

Do we have release notes or somewhere where we can highlight the breaking change in this patch?

[Even Rouault]

Replying to atlight:

Do we have release notes or somewhere where we can highlight the breaking change in this patch?

You can prepare an entry in the NEWS file