chore(geom): make Rectangle implementations consistent #22
Conversation
skaldarnar
commented
Feb 14, 2021
•
skaldarnar
commented
- chore((Rectangleic, Rectanglefc, Rectangledc): align immutable interface classes
- feat(Rectanglei): add missing API variants
- style(Rectanglei): sort implementations according to interface
- refactor(Rectanglei): use more forwarding instead of duplicate implementation
- chore((Rectangleic, Rectanglefc, Rectangledc): align immutable interface classes - feat(Rectanglei): add missing API variants - chore(Rectanglei): sort implementations according to interface - refactor(Rectanglei): use more forwarding instead of duplicate implementations
| long dx = java.lang.Math.max(java.lang.Math.abs(2 * px - cx2) - getSizeX(), 0) / 2; | ||
| long dy = java.lang.Math.max(java.lang.Math.abs(2 * py - cy2) - getSizeY(), 0) / 2; |
There was a problem hiding this comment.
why are we using java.lang.Man.max I would use the one offered under joml.
| /** | ||
| * Check if this rectangle contains the given <code>point</code>. | ||
| * | ||
| * @param point | ||
| * the point to test | ||
| * @param point the point to test | ||
| * @return <code>true</code> iff this rectangle contains the point; <code>false</code> otherwise | ||
| */ | ||
| boolean containsPoint(Vector2ic point); |
There was a problem hiding this comment.
I'm kind of hesitant about this function. might be a bit better to only accepting the floating point variations. did we decided if the edge of a Rectanglei is inclusive. to an extent the intersects is in a similar situation. is a rectangle intersecting if its only the edge? wonder if it would be better to just leave it up to the person using this rather then proving some concrete method.
There was a problem hiding this comment.
That's exactly the reasoning why this moved out of JOML and into our org. I think we should define the behavior to be used for Terasology so that we don't have any confusion about it throughout the code base.
There was a problem hiding this comment.
This is used in BrowserWidget, DistrictOverlay and MinimapIconOverlay.
There was a problem hiding this comment.
Thinking about properties that should hold:
assertTrue(rect.contains(rect)); // rectangle should contain itselfNow I'm wondering which parts of the rectangle are inclusive and which are exclusive. I don't think we have to think about the edges on it's own, but only about the min and max corner:
assertTrue(rect.contains(rect.getMin(new Vector2i()));
assertTrue(rect.contains(rect.getMax(new Vector2i()));
// currently both hold If both are true, then there's an overlap/intersection between two rectangles right next to each other. If only one is, you can "tile" touching rectangles without intersection.
Rectanglei rectA = new Rectanglei(0, 0, 1, 1);
Rectanglei rectB = new Rectanglei(0, 1, 1, 2);
assertTrue(rectA.intersectsRectangle(rectB));
assertEquals(new Rectanglei(0, 1, 1, 1), rectA.intersection(rectB, new Rectanglei());Given that one of the corners is exclusive, this would mean that a rectangle created from a point does not contain the point:
assertFalse(new Rectanglei(point).contains(point));There was a problem hiding this comment.
Started #23 for writing tests on Rectanglei to verify whatever our contract is going to be. Also gives an insight on how the current implementation behaves...
