Skip to content
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

Fails for the Union of these paths #3

Open
photopea opened this issue Sep 25, 2024 · 7 comments
Open

Fails for the Union of these paths #3

photopea opened this issue Sep 25, 2024 · 7 comments

Comments

@photopea
Copy link

The library throws an error when computing the Union of these two paths:

M 572.559987828135,1413.760003894567 L 523.119988933206,1413.760003894567 L 572.559987828135,1413.760003894567
M 570.879987865686,1389.280004441738 L 523.119988933206,1389.280004441738 L 570.879987865686,1389.280004441738

It probably fails for all paths of this structure (a line "there and back").

@photopea
Copy link
Author

Have you tested your library on some well-known dataset of vector paths?

@r-flash
Copy link
Owner

r-flash commented Sep 25, 2024

Thank you for reporting! No, I'm not aware of such dataset. Do you know any? I'm building my own dataset in the visual-tests directory.

@r-flash
Copy link
Owner

r-flash commented Oct 7, 2024

@photopea, I just replicated the error and added your example to the tests (did not push it t othe repository yet). Now, the question is how should this case be handled? So far, I am discarding all lines that do not enclose any area. If I fixed this issue the most straightforward way, all lines in your example would be discarded too. Would that be an acceptable result for you?

@photopea
Copy link
Author

photopea commented Oct 7, 2024

This case is very stange and I can not prevent my users from entering such a shape. I think whatever way you decide to handle it, is ok.

You can look at how paper.js handles it: http://paperjs.org/examples/boolean-operations/ . It seems like it keeps both paths untouched.

@r-flash
Copy link
Owner

r-flash commented Oct 27, 2024

Started discarding path segments that go there and back, as in this case. New builds are available at https://github.com/r-flash/PathBool.js/tree/master/dist .

@photopea
Copy link
Author

I just started to use your latest release of PathBool.js

Whenever I start using your library at www.photopea.com , within a few hours, someone reports a bug, and I see a case which your library cannot process :(

Have you thought about testing your library on some prepared samples? Maybe there is a dataset somewhere online. You can also make a generator of random paths and test your library on them. Or find a database of SVG files and extract paths from them for testing your library.

@r-flash
Copy link
Owner

r-flash commented Nov 1, 2024

I'm sorry to hear that, but I'm afraid that this project is simply too new and untested to be used in production like this (hence the disclaimer in README). That said, I really appreciate you testing it out and submitting the bug reports!

The library is currently tested on a limited set of samples which cover some corner cases (see the visual-tests folder). I'm not aware of any existing dataset. What I can do though is to test on some publicly available icon sets, for example. That would at least ensure that the program is not crashing. Generating random paths is also a good idea. That would, however, not ensure the correctness of outputs. (I tried generating the ground truth with Inkscape, but surprisingly it fails on most of the corner cases.)

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

No branches or pull requests

2 participants