-
-
Notifications
You must be signed in to change notification settings - Fork 886
feat: change label placement configuration, add new 'signed area centroid' algorithm, and use new polylabel implementation #2102
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: master
Are you sure you want to change the base?
Conversation
Hi @JaffaKetchup this MR requires your attention please review and let me know your feedback |
Hi @tidu090, we can't promise any timelines (I'm particularly busy atm), but we'll see what we can do. It looks like you've got a formatting issue. Can you run the formatter, push, then I can test it out? |
@JaffaKetchup ran the formatter should pass now, thank you for your attention! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for the changes!
I do like the new calculator, however, I think there still might be a value in keeping the old one.
Therefore, I think it makes sense if you add one/two (see below) new options to the PolygonLabelPlacement
enum. Not sure what's the best name - maybe rename the existing centroid(s) to simpleCentroid...
, and call yours centroid
?
We need to have a look at the defaults anyway (possibly we should be using centroidWithMultiWorld
by default), but that's out of scope. But keeping this name means it avoids a code-breaking change, even if the behaviour does change (and users can revert back).
Additionally, the new algorithm doesn't seem to be suitable for use across the anti-meridian (WithMultiWorld
):
Is this something that could be considered?
Don't worry about making the file messy/duplicating code. I think I might have an idea to replace the enum with a more extensible system in the future, so I can tidy up any duplications after.
Add `PolygonLabelPlacementCalculator` Added polygon label placement example to Polygon demo
@tidu090 I've made some of the changes I suggested. Your new algorithm is in 'centroid.dart'. The changes have resolved a TODO, and made the label placement system much more extensible. The only part remaining is whether this can support multi-worlds. Do you think this is possible with your algorithm? If not, I think I'm OK with this. This will now need a review from another @fleaflet/maintainers since I've made a lot of changes. |
I've made significant changes, so I cannot review this PR anymore. But I approve of the original change/new algorithm - it seems to work well.
@JaffaKetchup thank you for your feedback I will declare a new enum set for user to switch between old & new algo, those changes I will be doing tomorrow. Thank you once again happy to contribute! Btw I have this question if you can't review this PR anymore who will merge it then? |
No need to make a new enum anymore, just need the multi-world implementation if possible. You can copy The other maintainers are a little busy at the moment, but hopefully at some point quite soon we can get through this backlog of PRs and release v8.2. i.e. the other maintainers can review and merge. |
Cool I will work on that |
(Reminder to self: this needs to be added to the CHANGELOG) |
@fleaflet/maintainers I've also changed the polylabel implementation to my own new port (https://github.com/JaffaKetchup/dart_polylabel2). Its similar to the first, but fixes the licensing issue & performance issues (and doesn't rely on 'dart:math's |
This PR updates the logic used to compute the center of a label by replacing the previous average-based approach with a more accurate polygon centroid calculation.
Motivation
Previously, the centroid was calculated by simply averaging all latitudes and longitudes of the polygon points. While this works for simple shapes, it often results in inaccurate label placement for complex or non-convex polygons. The new approach computes the geometric centroid using the signed area formula, which takes the shape of the polygon into account and gives a more accurate center for label placement.
Changes
Reproduction
Attached is a screenshot showing the issue with the previous approach. The following sample coordinates help reproduce the difference:
[ { "lat": -6.148970663625812, "lng": -80.00502973729147 }, { "lat": -6.151999579408217, "lng": -80.00502266917418 }, { "lat": -6.151996562581506, "lng": -80.00344874575943 }, { "lat": -6.148972613882413, "lng": -80.00346259720905 }, { "lat": -6.148970663625812, "lng": -80.00502973729147 } ]
[ { "lat": -6.148979805893571, "lng": -80.00337467617543 }, { "lat": -6.151993401453232, "lng": -80.00337910858363 }, { "lat": -6.151989815157336, "lng": -80.00179839899332 }, { "lat": -6.148963739527827, "lng": -80.00180957676228 }, { "lat": -6.148979805893571, "lng": -80.00337467617543 } ]