Skip to content

dxf file parsing error, about spline curve #1422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
libaineu2004 opened this issue Jul 14, 2023 · 22 comments · Fixed by #1412
Closed

dxf file parsing error, about spline curve #1422

libaineu2004 opened this issue Jul 14, 2023 · 22 comments · Fixed by #1412
Assignees
Labels
Milestone

Comments

@libaineu2004
Copy link

libaineu2004 commented Jul 14, 2023

I found a problem with parsing errors on dxf's spline curve. Here uploaded 3 dxf files, you can use klayout and AutoCAD for comparison. You can see that there are problems with spline curve analysis, either with more lines or less lines.

klayout version
0.28.8

dxf with spline
https://github.com/libaineu2004/myepolltest/releases/download/dxf/bo.dxf
https://github.com/libaineu2004/myepolltest/releases/download/dxf/potato.dxf
https://github.com/libaineu2004/myepolltest/releases/download/dxf/R.dxf

@klayoutmatthias
Copy link
Collaborator

I do not have AutoCAD for comparison. Could you paste some reference images and describe the expected output?

@klayoutmatthias
Copy link
Collaborator

I used an online viewer, so now your files are on their servers. I guess you don't care ...

I see the difference here:

KLayout:
image

Viewer:
image

So I think this is what you mean.

@klayoutmatthias
Copy link
Collaborator

klayoutmatthias commented Jul 15, 2023

I debugged the problem and I think I fixed it. Reason was an exceptionally high point density in certain segments.

Here are my outputs now:

"R":
image

"potato":
image
NOTE: this test case suffers from this reader limitation currently:

    Invalid SPLINE flag (code 70): 11. Only types 8 (non-rational) and 12 (rational) are supported currently. (line=4580, cell=)

I assume this is the reason for the straight lines sticking out. "11" means "periodic, non-rational SPLINE".

"bo":
image

I could not find an online viewer that displays "R" and "potato", so maybe you can confirm that now that is the correct rendering.

Matthias

@klayoutmatthias klayoutmatthias added this to the 0.28.11 milestone Jul 15, 2023
@klayoutmatthias
Copy link
Collaborator

Ok, found the issue. Potato looks like this now:

image

I guess that fixes it.

@klayoutmatthias klayoutmatthias linked a pull request Jul 15, 2023 that will close this issue
@libaineu2004
Copy link
Author

libaineu2004 commented Jul 15, 2023

online viewer

1、bo.dxf and potato.dxf you found the difference. Congratulations. Those differences are the problem.
2、What is the web address of the online viewer you are visiting? What is the url?

@libaineu2004
Copy link
Author

libaineu2004 commented Jul 15, 2023

klayout version
0.28.8

The klayout software parameters I use are the default values after installation

"R.dxf" is:
S`UTW5$ZUV $PD_NV88(~B4

@klayoutmatthias
Copy link
Collaborator

I tried that online viewer: https://products.groupdocs.app/viewer/total, but also others had difficulties with "R" and "potato".

Anyway, if the "R" with the circular frame is correct it looks like I have fixed the problem.

@klayoutmatthias
Copy link
Collaborator

This one (https://3axis.co/dxf-viewer/dxf-viewer/) shows the "R" like KLayout now:

image

But the potato looks strange:

image

I guess nobody is perfect ...

@libaineu2004
Copy link
Author

Anyway, if the "R" with the circular frame is correct it looks like I have fixed the problem.

autocad is correct. Also, qcad is correct.
https://qcad.org/en/

@klayoutmatthias
Copy link
Collaborator

Thanks for pointing me to qcad ... I tried FreeCAD, but it did not like the splines.

@libaineu2004
Copy link
Author

Have all the problems been solved? Can you tell me what has been changed in the source code?
thank you

@klayoutmatthias
Copy link
Collaborator

klayoutmatthias commented Jul 20, 2023

Code change is this :35e42a8

The PR is linked to this ticket and will be merged soon into master.

@libaineu2004
Copy link
Author

libaineu2004 commented Jul 21, 2023

Code change is this :35e42a8

The PR is linked to this ticket and will be merged soon into master.

I tried using PR's new code and found that the program would crash.
35e42a8

I'm already in the function spline_interpolate. Added:
assert(pe ! = curve_points.end());
image

The assertion will take effect and an error will be reported.

I opened the following three dxf files with the same problem. My programming environment is VS2019 x64.
bo.dxf
potato.dxf
R.dxf

@klayoutmatthias
Copy link
Collaborator

klayoutmatthias commented Jul 21, 2023

I am using exactly these files as unit test cases and they work in my case. They are called issue_1422a.dxf to issue_1422c.dxf. Tests pass for me on VS2017.

I will take a look. VS has iterator assertions which may be useful here. Are you using debug mode?

@libaineu2004
Copy link
Author

libaineu2004 commented Jul 21, 2023

1、yes, debug mode。I missed one change,here:

Wrong:
for (double t = t0 + dt; t < tn + 1e-6; t += dt) {

Correct:
for (double t = t0; t < tn + 1e-6; t += dt) {

Then:
bo.dxf , ok
R.dxf , ok

2、but, potato.dxf not good
image

here, debug model,
j=0, k=2, p=3,
j + k - p = -1, Array out of bounds, crash

微信图片_20230721144746

if (k <= p) or if (k < p)?? which is right?

I use 'if (k <= p)', it can run, no crash, but there's an extra line
微信图片_20230721150008

@klayoutmatthias
Copy link
Collaborator

Please use the wip2 branch tip. The code reference was just for your information, not for reproducing the solution.

wip2 branch works for me on Linux and Windows.

Find the golden data for the tests here: https://github.com/KLayout/klayout/tree/wip2/testdata/dxf (issue_1422a_au.gds.gz to issue_1422c_au.gds.gz).

Matthias

@libaineu2004
Copy link
Author

potato.dxf

Could you just tell me. For these three dxf files, which source files have the source code changed? Besides dbDXFReader.cc, are there any other files that need to be modified?

I look at the 35e42a8, only dbDXFReader. Cc file is modified. But the potato.dxf problem was not solved.

@klayoutmatthias
Copy link
Collaborator

I'm sorry, but 35e42a8 are the only changes made. It should be enough to apply the changes of dbDXFReader.cc - the other modifications are only new tests to cover your case. The potato is reproduced without the sticks on MSVC2017, CentOS7 and Ubuntu 22 for me.

@klayoutmatthias
Copy link
Collaborator

I've put in an assertion and I can reproduce the array out of bounds issue. I will debug it.

@klayoutmatthias
Copy link
Collaborator

I have placed one more patch: 99df15a - this should fix the potato issue.

@libaineu2004
Copy link
Author

I have placed one more patch: 99df15a - this should fix the potato issue.

Yes.
99df15a
The result of my test is correct.
Thank you!

@klayoutmatthias
Copy link
Collaborator

Very good. I'll merge the PR then.

Matthias

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants