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

handle self-intersecting paths in convert to satin #608

Merged
merged 3 commits into from Mar 21, 2020

Conversation

lexelby
Copy link
Member

@lexelby lexelby commented Feb 1, 2020

This one's bothered me since I wrote Convert to Satin. This version automatically detects paths that loop back on themselves and splits them up. No more telling the user to split up the path, now Ink/Stitch does it for them, splitting as many times as necessary to make it work.

The code isn't brilliant, but it seems to get the job done. It just repeatedly splits the path in half until it finds pieces that don't self-intersect, then converts each to a satin by itself. This means that if you convert a path you might end up with multiple satins, but they're in order so it's really not that big a deal. Maybe in the future we can try to join the satins up, but I don't think it's going to matter all that much to the user.

@kaalleen
Copy link
Collaborator

@kaalleen kaalleen commented Feb 4, 2020

I don't know if this is ready for testing, but since I was at the computer I thought I'd give it a try.
It works great, but if users go a bit too crazy about it, we get an error message:

[...] inkstitch/lib/extensions/convert_to_satin.py", line 167, in get_scores
    angle = abs(math.degrees(math.acos(cos_angle_between)))
ValueError: math domain error

Tested with this file

If the shape is simplified through Ctrl+L everything works fine.

@lexelby
Copy link
Member Author

@lexelby lexelby commented Feb 4, 2020

Yup, definitely ready for testing, same for the other two PRs I made last week.

Nice find, awesome work! I think that problem must have existed since we added Convert To Satin in the first place :)

@lexelby
Copy link
Member Author

@lexelby lexelby commented Feb 4, 2020

Speaking of going crazy, there are some edge cases where this new version still doesn't work, like if a path doubles back on itself sharply at the end. I'm okay with that though, because the user can pretty easily see how to fix it. Garbage in, garbage out. :)

@kaalleen
Copy link
Collaborator

@kaalleen kaalleen commented Feb 4, 2020

Speaking of going crazy, there are some edge cases where this new version still doesn't work, like if a path doubles back on itself sharply at the end. I'm okay with that though, because the user can pretty easily see how to fix it. Garbage in, garbage out. :)

You are reading my mind ;)
I was just going to add a similar comment about this issue right here.
Sharp edges shouldn't be the way you design a satin column and we are giving a proper error message already now. If the user doesn't see it right away, you can still use the troubleshoot extension, which will point directly to the bad rung (... or let's better say bad edge).

@lexelby
Copy link
Member Author

@lexelby lexelby commented Feb 5, 2020

I found a separate error. I tried it on a path and got a "recursion depth exceeded" error. I knew that was a risk, and, well, here we are. :) The path had a really gnarly little tangle at one of the corners, created by using "stroke to path". I cleaned it up and it worked fine. Now to figure out how to automate that cleanup...

@lexelby lexelby force-pushed the lexelby/convert-satin-with-loops branch from 447e99d to d52373a Compare Mar 17, 2020
@lexelby lexelby merged commit c955803 into master Mar 21, 2020
4 checks passed
@lexelby lexelby deleted the lexelby/convert-satin-with-loops branch Mar 21, 2020
kaalleen added a commit that referenced this issue May 16, 2020
## New Features

- New Simulator (#531)
- Import Threadlist (#666)
- Export Threadlist in ZIP file (#664)
- Option to include SVG in ZIP file (#648)
- Break Apart and Retain Holes (#653)
- G-Code: option to alternate z-value (#659)
- New Stitch Plan Extension (#640)
- Optionally enable/disable ties (lock down stitches) (#619)
- Multiple Fill Underlays
- Additional Fonts (#683):
    * Geneva
    * DejaVu

## Improvements

- Better Algorithm for Satin Columns (#607)
- Better Algorithm for Fill Stitches (#606)
- Convert to Satin with Loops (#608)
- Adding '@ #' to all fonts (#680)

## Bug Fixes

- Fix Issue with Troubleshoot Pointer Position (#696)
- Inherit Styles (#673)
- Fix Color Palette Issues (#660)
- Preserve Aspect Ratio (#646)
- and more

## Under the Hood

- Namespaced Attributes (#657)
- Remove stub.py (#629)
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

Successfully merging this pull request may close these issues.

None yet

2 participants