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

Function splitPath(intersections) in PathItem.Boolean.js distorts path #609

Closed
iconexperience opened this issue Jan 4, 2015 · 11 comments
Closed

Comments

@iconexperience
Copy link
Contributor

If there are linear curves in a path, the function PathItem.Boolean.splitPath(intersections) may distort the path by setting handles of non-linear curves to zero. Below is an example that illustrates the problem. Both parts should be identical, but the second path, which is split at the "corner", is distorted, because one handle of the quadratic curve is set to zero:

http://sketch.paperjs.org/#S/lVbfT9swEP5XTp2EElGStmgPo4Np2xMS0pDYW4uQca/E4NqZ7RRNE//7zk7qJmlgww9N4py/7+67H+mfkWIbHJ2Nbp7Q8WI0HnG98s95DkIJByVzBTC1Ai61QhBuqbbM1NvnoPAZruk2SedL5feyjd7iT52EF1ool0wnkzHQTxpNflVsZZgT/HtlesYzb/xxko5hv3fq92ZtACnUAMdp22RdSfn7BiVyhyty1JkK6d3e+RltBssQV/R/ljnDlJXMYdJghmMkBzdIu7BmTySDcmgsgQutgDlwBcKSpDMKzXIEel0LNKv5orUl0kVNw33sdjG9zR7QXWnOPNRX50m9r+ntPJDaUjZJIKzwEOTeI9buBdt1pWqHoh04clbBWhtY4UYr60zggbIypbYUjNEbSmCJJnu0EFJ56XCTfdNaIlO0GZCTwrnSnuX5g3BFdZ9xvclLf+rR1lcyzO+lvs83zJJnuTU8907nA4gfrqafTlOo1ArNnlsKjsriUh1GkbTVtin8WSqg5ZV1BEr54kjCTrIJrem4frtbvlaYucGHDSpng1h+O7IYtKR/MEoidLAg0ZKQPY89BkmXLlgmUT24Yk4Wn0HS5fi4g7Bz0tb2B+cXwie5bZ3nYg1JgBtighOYpju47K6gtpT4o3IZheALJ9TqEN4Fveqfu1SvHnsHw/+BvtS3L1H9trad7LYiHUPokTGUBrfUIV7nC0oFXU9OOkJ7JKl5H4v07RWDXyEPmmd3JTM0+si+5aiXq6GDo6Mdc3YXPIHz8+ZoeDyEbp+I6LX4vbJwkJ8P2LYlA5TUoL1z3r9uaaSHbnRKupevJpBWGPO3+oUsg1EmbOwRinJxe5jcXSJoKoe5PiB9Uyw9vXcHItdKbMUKE1dPwmYeHkhIY0nhlvSlscwLGvJCgUPrgDOLg0Vaf68CV3bX7E1f0Sca0nS+pjwJXdmOmMP52TM5auI4nQ6l8OtLE2/0xcfE5DP7bemDImxok1dC2q0zIrqAKfXLO8lmr+HxGDYz7ip0Y/gopRTQ0DtK03SgCAcDfJszujZcXHHUdD6+dSmTS5et3U6mQqnvEzNchG82VW8Yl5UtkgamTbQbHMGleRx5/6To9yudof9h91TXT6X/e2NHZ4vbl78=

This behaviour is caused by lines 204-205 in PathItem.Boolean.js (https://github.com/paperjs/paper.js/blob/master/src/path/PathItem.Boolean.js#L204-L205):

                segment._handleOut.set(0, 0);
                segment._handleIn.set(0, 0);

I am not sure if I understand the code correctly, but maybe the lines should be changed to the following. This at least fixes the problem for me:

                if (i < linearSegments.length - 1) segment._handleOut.set(0, 0);
                if (i > 0) segment._handleIn.set(0, 0);

In my opinion this issue is responsible for many artefacts that we see with the current boolean implementation.

@lehni lehni self-assigned this Jan 4, 2015
@lehni
Copy link
Member

lehni commented Jan 4, 2015

@iconexperience do you have a boolean operation example where this problem is also occurring? Would be good to build a unit test for it...

@lehni
Copy link
Member

lehni commented Jan 4, 2015

I think @hkrish has already fixed this in 1c9d1de, but it's in the boolean-operations branch that has a couple of other issues that I still need to resolve. I will merge this one into master after some testing.

@iconexperience
Copy link
Contributor Author

@lehni
I will send an example within the next hours.

@lehni
Copy link
Member

lehni commented Jan 4, 2015

Great, thanks! I've cherry-picked @hkrish's commit, and cleaned it up a bit. Works like a charm! Closing this.

@lehni lehni closed this as completed Jan 4, 2015
@lehni
Copy link
Member

lehni commented Jan 4, 2015

The example actually doesn't result in a circle right now, due to #449. But the issue outlined here with spitPath() is now resolved.

lehni added a commit that referenced this issue Jan 4, 2015
@iconexperience
Copy link
Contributor Author

Great, we are getting closer to a working boolean! How much is left in the Boolean branch that awaits merging into the Master branch? Or in other words: should I test against both branches during the next days?

@lehni
Copy link
Member

lehni commented Jan 4, 2015

It's down to this one commit by @hkrish that I don't fully understand and that seems to produce some errors in boolean-test: 6c6ad76
I will look into this with him. But we still need to resolve #449 and #450 in order to get to fully functioning boolean operations, and that part will be rather tricky.

@iconexperience
Copy link
Contributor Author

Thanks, I will ignore these two issues and focus on other glitches and potential improvements during the next days.

If you come to the point that only issues 449 and 450 are missing for a complete implementation of the boolean functionality, you should be aware that you have done some superhuman work already.

@lehni
Copy link
Member

lehni commented Jan 4, 2015

I just managed to squash the nasty bugs from 6c6ad76 here: 5f3df1f, 72bd150 and already merged boolean-operations into master, closing #610 as well. Seems all pretty solid now!

@lehni
Copy link
Member

lehni commented Jan 4, 2015

Issue #541 also still needs fixing.

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

No branches or pull requests

2 participants