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

Multiple Partial Overlap Bug #30

Open
Vatvo opened this issue Jun 27, 2024 · 4 comments
Open

Multiple Partial Overlap Bug #30

Vatvo opened this issue Jun 27, 2024 · 4 comments

Comments

@Vatvo
Copy link
Contributor

Vatvo commented Jun 27, 2024

Hello Flo_Curves Devs,

I found a bug with the marking of interior/exterior edges when subtracting complex shapes in a specific way in both v0.7.3 and v0.8.0.

It begins with a base shape created by merging four SimpleBezierPaths into a GraphPath:
Screenshot 2024-06-26 at 1 24 22 PM

And a cutout GraphPath like this:
Screenshot 2024-06-26 at 1 28 40 PM

The cutout path is then subtracted from the base shape using GraphPath::collide() and then GraphPath::set_exterior_by_subtracting(). This produces the incorrect path seen here:
Noncomposite_Incorrect

Here is a recreation of what it should look like if it were correct:
Composite_Correct

The result of this error (when healed using the heal_exterior_gaps() function) looks like this:
Thin_Lock_Broken

When it should look like this:
Thin_Lock_Fixed

The two black and white images were made using the original icons with which this bug was discovered, but I have written a test using the more minimal reproduction seen in the debug images.

The inner loop seen in the top right of the debug image seems to be important. When it is removed, the bug does not occur.
pathloop

Also, if the top of the cutout shape, which intersects that loop, is moved higher (to around y=100) the bug does not occur.
path100

When that point is at its normal position of y=160 it breaks.
path160

I believe the bug is somewhere in the set_exterior_by_subtracting() function, or the function it calls, though I was unable to locate exactly where.

The subtract_multiple_partial_overlap() test I have proposed in pull request #29 creates these shapes, subtracts them, locates the edges of the inner arch, and checks their interior/exterior statuses. If they are correctly labelled as exterior, it passes. I believe that if this test passes, the bug would be fixed.

Also, I’m not sure if this is relevant, but maybe it will be useful information somehow. The edge indicated in the following image is actually two lines that partially overlap:
twolines02

I would think that Flo_Curves should see the overlap and split the edge up appropriately.
splitlines02

I could be wrong about what this is supposed to look like, but maybe this information will be useful.

@Logicalshift
Copy link
Owner

Hi, thanks for the comprehensive report, I was easily able to reproduce the bug. I've got some first impressions for now, I should be able to try to figure out how to fix this this weekend.

I believe you've also identified the cause here: any overlapping lines should have caused a collision so there shouldn't be any of differing lengths. That will always cause incorrect behaviour, but precisely what happens depends on the ray that intersects the lines, hence why altering the shape seems to change the outcome. The longer line is inside the shape at the top, but outside at the bottom, so there are two possible results.

(Partially overlapping curves are the worst for figuring out collisions: apparently even when they are also lines)

There are some other possibilities: rays that run nearly exactly along a linear edge can cause glitches too, and rays that cross two overlapping edges have previously caused problems (v0.8 rewrote the algorithm for that last one so I'm hoping that's actually fully fixed now), and there's always the unexpected wildcard bug in some other part of the code.

I'll figure this out for sure this weekend, but this analysis has been really useful in narrowing down what could be going wrong.

@Logicalshift
Copy link
Owner

I believe I've fixed this as of d425e83 which is on the v0.8 branch.

There were a couple of problems: the main one was to do with overlapping curves. If you had a situation like this, where two overlapping lines are moving in opposite directions and the end points only :

   *------>
       <-------*

flo_curves would not recognise them as overlapping. This was the main reason the intersection wasn't being generated.

Then there was a filter being applied that was supposed to stop the last point of one path edge from intersecting with the first point of the next that also removed the intersection. That only makes sense when computing self-intersections so I've removed it.

Finally fixing the overlapping bug introduced a new bug where very small subdivided curves would be seen as overlapping, which would generate extra collisions. I've fixed that with a check that discards overlaps that are very close to the start and end of the curve (the points should snap together later on anyway)

@Vatvo
Copy link
Contributor Author

Vatvo commented Jun 29, 2024

Looks good. Thanks for the quick response. If it's not too much trouble, could the fix be added to the 0.7 branch too?

@Logicalshift
Copy link
Owner

Yep, no problem. I've copied the fix over into the 0.7 branch and released it with v0.7.3

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