bug fix on is_polygon_in_polygon_xy

[baehrjo]

The is_polygon_in_polygon method was sometimes mistaking returning polygon2 was inside polygon1 while it wasn't. Issue occurs when edge intersection check is required inside is_polygon_in_polygon_xy to 'help decide'. Lines are first created, both for the contour polygon and the polygon to check on. Here is part of the code :
for j in range(len(polygon2)):
line = [polygon2[-j], polygon2[j - 1]]
should be [... , polygon2[-j - 1]] to accurately loop through all the edges of polygon2

What type of change is this?

• Bug fix in a backwards-compatible manner.
• New feature in a backwards-compatible manner.
• Breaking change: bug fix or new feature that involve incompatible API changes.
• Other (e.g. doc update, configuration, etc)

Checklist

• I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
• I ran all tests on my computer and it's all green (i.e. invoke test).
• I ran lint on my computer and there are no errors (i.e. invoke lint).
• I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
• I have added tests that prove my fix is effective or that my feature works.
• I have added necessary documentation (if appropriate)

[tomvanmele]

thanks for identifying the problem and proposing a fix. could you add a test and include one or more boundary cases?

[worbit]

instead of -j-1 i would suggest:
line = [polygon2[j], polygon2[j - 1]]
same for line 384 above:
line = [polygon1[i], polygon1[i - 1]]

[tomvanmele]

@worbit that is indeed much simpler and generates the same loop of edges :)

[tomvanmele]

also here some lint that makes the tests fail...
https://github.com/compas-dev/compas/actions/runs/4502317650/jobs/7941195916?pr=1130

[baehrjo]

Hello,

Looking back at this polygon-in-polygon-fix i noticed it wasn't available yet. Is something missing ? And, my personal curiosity, wondering about the '[Merge branch 'main' into polygon-in-polygon-fix]' thing, is it part of the process before merging back a fix to the main ?

I was about to submit a bunch of new polygon predicates (intersection, containment) that would require polygon-in-polygon to work fine.

Thanks !

[tomvanmele]

hi @baehrjo

sorry for the delay.
this is now superseded by #1153 , correct?
can we close this one then?

[baehrjo]

Hello,

@tomvanmele nope, as that fix was still open here, #1153 only contains new methods, but not that fix as well (i.e. is_polygon_in_polygon is wrong in #1153)

[tomvanmele]

@gonzalocasas this would also be one for LTS