inkstitch
This and that
- Don't fail on satin columns with a fill, but stitch the odd fill without complaining
- Ask for an svg file when fill shapes fail
- Fix convert to satin path direction issue for macbooks
- Auto-route satin: ensure rungs
- Satin column: remove duplicate rungs (fixes #1755)
fixes #1733
I've just reverted the "remove duplicate rungs" and added a better fix for #1755. Duplicate rungs shouldn't be a problem -- I specifically had duplicate rungs in mind when I wrote the code in SatinColumn. And as it turns out, duplicate rungs aren't what was causing the problem in #1755.
The problem was that our code in SatinColumn.flattened_rungs was trying to force rungs to intersect, but it didn't always work. I changed SatinColumn.flattened_sections to stop requiring that rungs actually intersect. It just splits the rail based on the nearest point between the rung and the rail. Tada! That let me remove a lot of other code that was trying to make things intersect.
@kaalleen for "auto_satin: ensure rungs", what led you to add that? I'm wondering because I thought auto satin already ensured that there was a rung if necessary. SatinColumn.split() calls SatinColumn._add_rungs_if_necessary, which should detect a satin with no rungs and add a rung. Is that not working?
It did not. I can't find the issue at the moment. I think it happened when there was no rung in the first place and the satin column was split up or so. I should have linked it to the issue in the first place. Sorry. I'll think about it.
I don't like that we need to check for the mac release if the rail has the correct direction. There has been a new shapely release and I was hoping this could fix the issue so that we don't need to check. But the new version causes notarization issues.
@kaalleen for "auto_satin: ensure rungs", what led you to add that? I'm wondering because I thought auto satin already ensured that there was a rung if necessary. SatinColumn.split() calls SatinColumn._add_rungs_if_necessary, which should detect a satin with no rungs and add a rung. Is that not working?
@lexelby I found the file that was causing the issue:
Oh I get it now! I was too caught up in the splitting part, but this is about the satins that don't get split. We're flattening the rails but that breaks satins without rungs because it changes where the points are.
I'm worried that _ensure_rungs() doesn't go far enough. If the user gives us a satin with no rungs and carefully-chosen points, this will throw away their points and give them one new rung at the start of the satin. We're destroying the "rungs" that they had already created, and the resulting satin may not look right.
SatinColumn.split() handles this right: if you give it a satin without rungs, it creates the rungs (SatinColumn._synthesize_rungs()) and includes those as real rungs in the results. We should do that for all satins returned by auto-satin, even if they don't get split.
Looking at it, it actually does add rungs to uncut satins - if they have more nodes than only the start and end. So the solution to the problem would be to add a start rung, when there is only start and end point.
I also updated the strange reversed right rung, instead of doing a distance check, we should simply check if it is a mac or not (since this only happens on macs). I will need to figure out though, if that is the case for all mac computers - or only to specific macOS versions.
Looking at it, it actually does add rungs to uncut satins - if they have more nodes than only the start and end. So the solution to the problem would be to add a start rung, when there is only start and end point.
Oh, sure enough -- but only sometimes. If it has to reverse the satin, then the points get converted to rungs. If auto-route is able to reuse the satin as is, then it does not modify it at all, not even flattening it. So we only need to add a rung if the following are all true:
- The satin does not need to be split.
- The satin has no rungs.
- The satin has only the starting and ending point on each rail -- no points in the middle.
- The satin has to be reversed.
That's a lot that has to happen at the same time. :) I think another way to fix this case would be to have SatinColumn._synthesize_rungs() always add at least one rung, even if the rails have only the start and end point and no middle points. I would like to do it in SatinColumn because it seems weird to me to have that logic about interpolating 0.1px and modifying the d attribute outside of SatinColumn. Also, we already have very similar logic in SatinColumn._add_rungs_if_necessary(), so we could probably share some code between them.
I also updated the strange reversed right rung, instead of doing a distance check, we should simply check if it is a mac or not (since this only happens on macs). I will need to figure out though, if that is the case for all mac computers - or only to specific macOS versions.
I actually like the distance check better than trying to figure out exactly which mac versions are wrong. What if some other platform starts having this problem too? The distance check is general enough that it will automatically fix it. I know it's a little hacky, but I feel like it's not too bad...
Funny, I wrote the above without noticing your latest commit. Looks like you're way ahead of me once again. ;) I like your solution a lot.
I'm going to remove the interpolate in _add_rungs_if_necessary() and instead just have it use the first point. That's easier and makes it more likely that the rung actually intersects. It looks almost identical. We don't really require it to intersect anymore, but it helps rail_indices() be more likely to pick the right paths as the rails.
I actually like the distance check better than trying to figure out exactly which mac versions are wrong. What if some other platform starts having this problem too?
The distance check isn't 100% reliable, so I don't really like it a lot. I pushed a commit to do the following: reverse original path, do a left side offset and reverse the resulting path again. So we can be sure that the path is going into the same direction and we don't need to check for operating systems or check for directions in any other way.