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

Fix Intersector#overlapConvexPolygons #5048

Closed
wants to merge 2 commits into from
Closed

Conversation

schosin
Copy link

@schosin schosin commented Jan 4, 2018

In some rare cases it's possible that Intersector#overlapConvexPolygons calculates an incorrect MTV. Currently the direction of the MTV is negated depending on the number of points on either side of the checked edge.

The correct way is to negate the direction of the MTV depending on the direction of the centers of both polygons.

@schosin schosin changed the title added failing IntersectorTest#testOverlapConvexPolygons Fix Intersector#overlapConvexPolygons Jan 4, 2018
@mgsx-dev mgsx-dev added math core affecting all platforms labels Sep 4, 2020
@obigu
Copy link
Contributor

obigu commented Sep 4, 2020

I think your fix doesn't work for other cases. Imagine a right triangle (1 angle is 90º) with 2 equal sides resting on one cathetus (short side). The center point of that triangle will be in the middle of the hypotenuse. Imagine now a small rectangle with the same center than the triangle. If I understand your fix correctly in that case MTV will resolve to 0,0 (which is not correct). Now, imagine we move the rectangle diagonally up-right or down-left the MTV will invert but it actually should keep being positive x and y. I think it's a complex geometry problem with complex corner cases.

@kdenzel
Copy link
Contributor

kdenzel commented Sep 10, 2020

So i had also issues with the function overlapConvexPolygons and i decided to understand the SAT Algorithm and how to handle the minimum translation Vector. I am making a game and completely did a rewrite on this function. If anyone is interested in seeing it here is the link. If you want, you can clone the repository and play with the Testclasses i made. Try changing my custom Intersector class with this of libgdx and you will see the issues with it. I made a Testclass where you can drag 2 triangles to overlap each other and when they collide, the minimum translation vector did return wrong results because the orientation of the x and y coordinates returned by the direction vector were wrong.

@obigu
Copy link
Contributor

obigu commented Sep 11, 2020

So i had also issues with the function overlapConvexPolygons and i decided to understand the SAT Algorithm and how to handle the minimum translation Vector. I am making a game and completely did a rewrite on this function. If anyone is interested in seeing it here is the link. If you want, you can clone the repository and play with the Testclasses i made. Try changing my custom Intersector class with this of libgdx and you will see the issues with it. I made a Testclass where you can drag 2 triangles to overlap each other and when they collide, the minimum translation vector did return wrong results because the orientation of the x and y coordinates returned by the direction vector were wrong.

On a first look it looks a good. I think we can use your SAT algorithm implementation as a base to replace the current buggy implementation. I think there's a some room for improvement (such as using Vector2 for some operations) but I'd rather discuss them on a PR. Would you like to create a PR with your implementation including some non visual/interactive tests that also cover corner cases (such as containment)?

kdenzel pushed a commit to kdenzel/libgdx that referenced this pull request Sep 11, 2020
…apConvexPolygons in Intersector should now work as expected.

-Added graphical test.
@MrStahlfelge
Copy link
Member

Now that we have #5048, can we close this one?

@obigu
Copy link
Contributor

obigu commented Sep 12, 2020 via email

@obigu obigu closed this Sep 13, 2020
obigu pushed a commit that referenced this pull request Sep 17, 2020
…ow should return the right minimum translation vector values. (#6185)

Fixes Intersector.overlapConvexPolygons (#5048)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core affecting all platforms math
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants