-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add LabelCenterline function #426
base: master
Are you sure you want to change the base?
Add LabelCenterline function #426
Conversation
Rad! |
this is really impressive! The sql style is a bit different from other files (e.g. using |
I'll try to give this a detailed review, as it's something I've been interested in. |
Speed up multipolygon hull calculations somewhat
I converted these to PL/pgSQL functions to avoid the block string, but I see that this also enables some language features we might be able to use to speed up the code. I'll look into it at some point.
Yes, the more circular the feature, the harder it is to determine a sensical centerline. We might try baking in a threshold where circular stuff just defaults to a horizontal line. We could also achieve better results by allowing multiple lines in the output to account for islands and bays, perhaps based on the zoom. Lots of of shapes just really aren't modeled sufficiently by a single line. Ex: Manicouagan Reservoir. |
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.
As all three functions are simply a single SQL statement and returning the result of that statement, they should be SQL, not plpgsql. This has performance advantages by avoiding the time to start up a plpgsql environment every function call, and allowing inlining.
More comments would be good, as it's not clear what's going on in all steps.
Minor code nits:
- these functions should probably all be STRICT
- caps on
as
vsAS
is inconsistent
sql/LabelCenterline.sql
Outdated
CREATE OR REPLACE FUNCTION CountDisconnectedEndpoints(polyline geometry, testline geometry) | ||
RETURNS integer AS $$ | ||
BEGIN | ||
RETURN ST_NPoints(ST_RemoveRepeatedPoints(ST_Points(polyline))) |
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.
An explanation of the equation would be good here.
… using a concave hull
…olygons with lots bulges/arms)
Make simplification parameters ratios to scale to geometries of vastly different scales
Okay, I added a number of enhancements to get nicer results in a reasonably fast timeframe. It now takes just 15 seconds to calculate the basic centerlines for the 395 largest lakes of Michigan, down from 40 seconds. Smoothing is now done proportionally to the size of the feature. Multiple lines over a given length threshold can be included in the output. This allows more accurate labeling at higher zooms. Holes over a given size can be included in the calculation as well. This takes a little longer to calculate, 22 seconds for these lakes. There as six parameters to customize these behaviors. |
Closes #354. The goal is to replace acalcutt/osm-lakelines with a faster SQL function. I've never written anything this complex in SQL before, so any help with efficiency and best practices is appreciated. I'm also new to this project, so I'm not sure about where to hook this in.
The logic:
SFCGAL is not required for this method.
It takes about 40 seconds to calculate lines for the 395 largest lakes of Michigan, which I think is already a big improvement over osm-lakelines. The recursion takes about 75% of this time and can probably be made vastly more efficient.
The results are relatively consistent with osm-lakelines and I think look pretty good, except when there are large islands or bays.