KLayout / klayout

KLayout Main Sources
http://www.klayout.org
GNU General Public License v3.0
804 stars 205 forks source link

dxf file parsing error, about spline curve #1422

Closed libaineu2004 closed 1 year ago

libaineu2004 commented 1 year ago

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 commented 1 year ago

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

klayoutmatthias commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

image

I guess that fixes it.

libaineu2004 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

libaineu2004 commented 1 year ago

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

klayoutmatthias commented 1 year ago

Code change is this :https://github.com/KLayout/klayout/pull/1423/commits/35e42a81178aa3fe071a7f656a4c026eeb596679

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

libaineu2004 commented 1 year ago

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. https://github.com/KLayout/klayout/commit/35e42a81178aa3fe071a7f656a4c026eeb596679

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/KLayout/klayout/commit/35e42a81178aa3fe071a7f656a4c026eeb596679, only dbDXFReader. Cc file is modified. But the potato.dxf problem was not solved.

klayoutmatthias commented 1 year ago

I'm sorry, but https://github.com/KLayout/klayout/pull/1423/commits/35e42a81178aa3fe071a7f656a4c026eeb596679 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 commented 1 year ago

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

klayoutmatthias commented 1 year ago

I have placed one more patch: https://github.com/KLayout/klayout/pull/1423/commits/99df15a5fff7fd6ac0f64b781d34cb2b2a4bc706 - this should fix the potato issue.

libaineu2004 commented 1 year ago

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

Yes. https://github.com/KLayout/klayout/commit/99df15a5fff7fd6ac0f64b781d34cb2b2a4bc706 The result of my test is correct. Thank you!

klayoutmatthias commented 1 year ago

Very good. I'll merge the PR then.

Matthias