8000
Skip to content

Fix DXF polygon loading by correctly joining oriented segments#264

Open
tokoro10g wants to merge 1 commit intoreilleya:stagingfrom
tokoro10g:fix/load-dxf-polygon
Open

Fix DXF polygon loading by correctly joining oriented segments#264
tokoro10g wants to merge 1 commit intoreilleya:stagingfrom
tokoro10g:fix/load-dxf-polygon

Conversation

@tokoro10g
Copy link
Copy Markdown
@tokoro10g tokoro10g commented Mar 25, 2026 •
edited
Loading

This PR fixes the polygon loading algorithm.

For example, if we have

A = [p1, p2]
B = [p3, p2]

it tries to join the segments like the following in l.90:

C = flip(A) + B = [p2, p1, p3, p2]

which is obviously invalid. It should perform C = A + flip(B) instead.

@tokoro10g
Fix DXF polygon loading by correctly joining oriented segments
72b8e03 
Update the polygon editor's segment-joining loop to properly handle all four endpoint-to-endpoint proximity cases. By allowing reversal of both the primary and comparison chunks, segments can be joined into a continuous path regardless of their initial direction.
@reilleya
Copy link
Copy Markdown
Owner
reilleya commented Mar 31, 2026

Hi! Thanks for the contribution. Do you have an example DXF that doesn't load? Took me a moment to think through the existing code, but I believe it is right. In your example, line 90 actually does

C = A + flip(B) = [p1, p2, p2, p3]

because

join = [chunkId, compChunkID, True]

and this block of code reverses the second chunk in the array (not the first) before concatenating them:

addChunk = chunks[join[1]]
if join[2]:
    addChunk.reverse()
chunks[join[0]] += addChunk

Totally possible I'm missing something, though!

@tokoro10g
Copy link
Copy Markdown
Author
tokoro10g commented Apr 3, 2026

Thank you for this awesome project!

Sorry, I was looking at the wrong line, l.93 had an error.

A = [p1, p2]
B = [p1, p3]

It was implemented like

C = A + flip(B) = [p1, p2, p3, p1]

This cannot be solved by simply swapping chunkId and compChunkID in l.93 because the joining order gets also swapped:

B + flip(A) = [p1, p3, p2, p1]

so I decided to add another flag for flipping.

@reilleya
Copy link
Copy Markdown
Owner
reilleya commented Apr 12, 2026

Huh, yeah, looks like line 93 is incorrect! Thanks. Before I merge the change, do you have an example of a DXF that doesn't load without this change? I don't think I've ever run into a DXF that the software couldn't load (as long as it only includes elements the software knows about) so I'm kind of confused about how this was broken. Maybe it is very rare compared to the other cases?

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tokoro10g @reilleya
0