Skip to content

DXF: write extended data of polylines and inserts with attributes before their sub-entities - #1336

Open
SolidRockProgrammer wants to merge 2 commits into
DomCR:masterfrom
Datgel:fix/polyline-xdata-before-vertices-upstream
Open

SolidRockProgrammer wants to merge 2 commits into
DomCR:masterfrom
Datgel:fix/polyline-xdata-before-vertices-upstream

Conversation

@SolidRockProgrammer

Copy link
Copy Markdown

Description

DxfSectionWriterBase.writeEntity writes an entity's extended data at the end, after the entity-specific writer has returned. For a heavy polyline (Polyline2D, Polyline3D, PolyfaceMesh, PolygonMesh) and for an Insert with attributes, that writer has already written the VERTEX / ATTRIB entities and the SEQEND. So the owner's 1001 groups end up after the SEQEND's group 8, and a DXF reader attaches them to the SEQEND, the entity they follow. ACadSharp's own DxfReader does this. The owner loses its xdata. DWG output is not affected.

Example of what was written (abridged):

  0
POLYLINE
...
210
...
  0
VERTEX
...
  0
SEQEND
  5
...
  8
0
1001
MY_APP        <- read as the SEQEND's xdata

writeInsert already had a //WARNING: Write extended data before attributes comment for this case.

Fix: writePolyline writes the polyline's extended data after group 210, before the vertices. writeInsert writes it before the attributes, in place of the warning comment. writeEntity skips its trailing writeExtendedData for those two cases.

The check names the four heavy polyline types rather than IPolyline, on purpose. LwPolyline implements IPolyline too, but it has no sub-entities and still needs its xdata written at the end. My first version used IPolyline and dropped every LWPOLYLINE's xdata. No existing test covered that, so the suite stayed green. The new EntityWithoutSubEntitiesKeepsItsXData test covers it now.

Tasks done in this PR

  • Something awesome.
  • An evil bug have been defeated.
  • Code cleanup and maintenance has been done.

Tests: ACadSharp.Tests/IO/SubEntityXDataTests.cs (8 cases):

  • Polyline2D with xdata, AC1015 and AC1032: the 1001 group comes before the first VERTEX in the written text, and after DxfReader.Read the xdata is on the polyline, not on its vertices or SEQEND.
  • Insert with an attribute, AC1015 and AC1032: the same checks against ATTRIB.
  • Polyline3D: the same.
  • An Insert without attributes, an LwPolyline and a Line keep their xdata.

Before the fix, 4 of the original 5 cases fail, e.g. "the owner's xdata (line 1044) is written after its first VERTEX (line 932)". With the check widened to IPolyline, the LWPOLYLINE case fails ("LWPOLYLINE lost its SUBENTITY_XDATA_TEST xdata"). On this branch, SubEntityXDataTests and the existing XDataTests pass on net9.0. The full suite was run on the same change applied to v3.8.0, and nothing new failed. WriterSingleObjectTests was not run, because some of its cases already fail on v3.8.0 itself.

Minimal reproduction:

CadDocument doc = new CadDocument();
doc.AppIds.Add(new AppId("MY_APP"));
Polyline2D pline = new Polyline2D(new[] { new Vertex2D(new XY(0, 0)), new Vertex2D(new XY(10, 0)) }, false);
ExtendedData data = new ExtendedData();
data.Records.Add(new ExtendedDataString("value"));
pline.ExtendedData.Add("MY_APP", data);
doc.Entities.Add(pline);

using MemoryStream stream = new MemoryStream();
DxfWriter.Write(stream, doc, false, new DxfWriterConfiguration { CloseStream = false });
CadDocument read = DxfReader.Read(new MemoryStream(stream.ToArray()));
Polyline2D back = read.Entities.OfType<Polyline2D>().Single();
// before: back.ExtendedData has no MY_APP; back.Vertices.Seqend.ExtendedData has it

Related Issues / Pull Requests

  • None found (searched issues for xdata / SEQEND / extended data with attributes).

Notes for reviewer

  • Two commits: the fix, then the follow-up that narrows the check from IPolyline to the heavy polyline types. Feel free to squash.
  • This was written with AI assistance (Claude) and reviewed and tested by me before submitting. We found it when a polyline's xdata in a DXF we exported came back on the SEQEND.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant