- Fixes Issue #5048. The function Intersector.overlapConvexPolygons now should return the right minimum translation vector values. #6185
Conversation
…apConvexPolygons in Intersector should now work as expected. -Added graphical test.
There was a problem hiding this comment.
It looks good to me. It could be done in a more object oriented style (by defining a Projection class or using Vector2 for example) but it follows the previous implementation style and it's easy to read. I agree the interactive test is very powerful even if some discrete unit tests would also be nice (not necessary though).
| shapeRenderer.setColor(Color.RED); | ||
| shapeRenderer.line(mtv.normal.x, mtv.normal.y, mtv.normal.x * 10, mtv.normal.y * 10); | ||
| shapeRenderer.set(ShapeRenderer.ShapeType.Filled); | ||
| shapeRenderer.circle((mtv.normal.x), (mtv.normal.y), 0.5f); |
There was a problem hiding this comment.
The test is great, I don't understand what the circle represents. Wouldn't it be more useful (instead of having that circle) to display the MTV depth with a green line over the current red line. Something like:
shapeRenderer.line(0, 0, mtv.depth * mtv.normal.x , mtv.depth * mtv.normal.y);
I would also make the red line start at (0,0).
There was a problem hiding this comment.
Hello @obigu thanks for your reply. I made the circle to better visualize the direction the line is pointing at. Without the point you would only see a line but it is harder to predict where the start of this line is. Maybe it would be better to draw the red line also in the oposite direction then the dot will become the origin of the line.
Yes you are right, i can translate the position of the line to 0,0. Be careful, the line represents the minimum projecton axis where the points min and max are projected from the shapes and also the direction. Showing the green line as depth is a good idea. At the moment i just show the axis with a length of 10. I will fix that.
(by defining a Projection class or using Vector2 for example)
Wouldn't it be a performance lose if we would always create new Instances in this method?
There was a problem hiding this comment.
Thanks. It's a bit better now but why do you substract mtv.normal.x and mtv.normal.y to all the lines (red, grey and green) second point coordinates?. The idea of the green line is to visually displays the MTV depth, now it goes negative when it's small.
There was a problem hiding this comment.
Oh yes my bad. I forgot it is a unit vector and not a coordinate in space. Tried translating it (the line) to point 0,0 which of course doesn't make any sense because the vector is just pointing in a direction. Did not see it because i used a larger scale but now i saw it reducing the scale. Thanks for reviewing and mentioning it.
-renamed parameters in this function back to the original name -Improved the visualisation of the test class
|
Thanks for reviewing the code. What also comes into my mind is the java doc. The statement that the polygons must be in counter clockwise order is no longer true i guess. I would go as far as saying this was never the case. Let me explain. The SAT checks every axis on each shape and it shouldn't matter in which direction you check first. Wether you check the first axis last or the last axis first. I also tested it with the visual test class and the results stay the same because the direction of the unit vector "mtv.normal" depends on the points projected on the one dimensional axis of the shapes. Also this Link shows, that i am not the only one who thinks that the direction of the vertices doesn't matter. |
You're right, the polygons don't need to be defined counter-clockwise. Do you mind updating the docs to remove that statement from all |
Hello everyone, this should fix #5048
Sorry for not adding a non graphical test but the magic happens in the SAT Algorithm. If you are familiar with the algorithm the placement and the orientation of the minimum translation vector happens on the one dimensional axis.
The magic happens when we project the axis of the max and min points of the polygon and based on the scalar value (dot product) of this values i calculate the direction of the minimum translation vector. You can see this with the red line in the graphical Test.
TL;DR i added no functional Test because of math magic. It is hard to predict the assert value so a graphical test with debug output says more than 1mio asserts.
Also you see in the code and in the graphical test i also handle edge cases like when one polygon contains another.