-
-
Notifications
You must be signed in to change notification settings - Fork 926
Fix for 2D polygon area algorithm #2811
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
base: version/7.3.x
Are you sure you want to change the base?
Conversation
Thank you for this PR! :) It looks good to me, although I'll let someone else who's better at maths review that side of things. The only thing I'd personally change are a few unnecessary allocations (& usage of Streams APIs), although none of this is really hot code so it's not too major an issue. |
I've done some testing of this PR, and it definitely seems vastly better than the existing behaviour, thank you for this! I'm approving it based on code changes & my testing- but still would prefer someone more maths-focused to take a look before I merge it :) |
I saw in the limitations that this doesn't handle self-intersecting polygons. We do allow that, so I'm not sure this code is better or worse than the previous one for that case. However, while thinking about it, I came up with what I think is a better idea:
I'm not sure if step 5 is entirely correct given the way we implement What do you think? |
These steps produce the same result as my current algorithm for non-self-crossing polygons, but with significantly less complexity and without the second limitation of my approach. I think it’s a great idea, and with your permission, I’d love to implement it. I’d be happy to add you as a co-author to the commit as well. Regarding the self crossing polygons...
Both approaches behave the same here: the previous algorithm also did not support self-crossing polygons. I assumed this was not required since the original version did not address it either.
I don’t think triangulation is the right approach. For example, in the figure below, the orange points would be counted twice, once per triangle, leading to double counting: ![]()
This seems like the ideal solution to me as well. I’ll try moving forward with that approach. Thanks for the great idea! |
Please feel free to implement it. Add me as co-author if it's easy for you. |
I’ve tried several different approaches to handle self-crossing polygons. Here’s a summary of what I attempted:
![]()
![]()
More bad news... The only actual solution I could find was this one, which in theory is about as costly as just using a bounding box. Given that, I think the best path forward is simply to ignore self-crossing polygons and stick with steps 2–5 from @octylFractal’s algorithm for handling simple polygons. Most known theorems also break down when applied to self-crossing polygons, so I don’t think it’s a major issue. Even WorldEdit’s previous approach didn’t support them, and having a reliable solution for simple polygons is still a solid improvement. I’ll wait for your feedback, but I’m happy to implement either @octylFractal’s algorithm or the “lattice points inside non-lattice polygon” approach. If neither option feels right, I’m afraid I don’t have a better alternative. |
I will look more into it in the next few weeks, when I have time. |
This PR introduces a new algorithm to calculate the area of 2D polygons using discrete points. The previous implementation was designed for concrete points and did not handle certain cases correctly.
Since the new algorithm is relatively complex, I have also prepared a detailed explanation for reference.
Closes #888